mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
revert(native-chat): drop the speculative Fable model-switch detections (#18215)
Both changes shipped in #18055 were written against strings never observed in a real session, and neither fixed a reported problem. Guessing at agent output we have not seen is how the picker got a row that silently no-ops. Fable consent detection is removed outright. It watched the session for "Fable N uses usage credits and needs a one-time consent" and answered `interaction-required`. No consent prompt appeared in any validation run — the test account had already consented — so the matched wording was never confirmed. With the detector gone nothing produces `interaction-required`, so the outcome leaves the union and its unreachable handler goes with it. A real consent prompt now reports the switch as unverified, which is the honest failure mode for output we cannot recognize. The weekly usage scope goes back to exact `display_name === 'fable'`. It had been widened to `/^fable\b/` against a hypothetical rename of Anthropic's own usage window; the API still reports "Fable", so the match was insurance against a scenario with no evidence behind it. Tests covering the removed behavior are deleted rather than rewritten, including the two pre-existing `interaction-required` cases that asserted the terminal is revealed. The disabled-row filter from #18055 is deliberately untouched. Claude-Session: https://claude.ai/code/session_01SJy4XGrdre6YaU1wYNKak4 Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
co-authored by
Merge Sim
parent
6c66487fca
commit
616fa751da
@@ -144,51 +144,6 @@ describe('fetchClaudeRateLimits', () => {
|
||||
expect(fetchViaPty).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('maps a scoped Fable window whose display name carries a point release', async () => {
|
||||
const configDir = '/Users/test/.claude'
|
||||
const authPreparation: ClaudeRuntimeAuthPreparation = {
|
||||
configDir,
|
||||
envPatch: { CLAUDE_CONFIG_DIR: configDir },
|
||||
stripAuthEnv: false,
|
||||
provenance: 'managed:account-1'
|
||||
}
|
||||
vi.mocked(readActiveClaudeKeychainCredentialsStrict).mockResolvedValueOnce(
|
||||
JSON.stringify({ claudeAiOauth: { accessToken: 'oauth-token' } })
|
||||
)
|
||||
netFetchMock.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
five_hour: { utilization: 36 },
|
||||
seven_day: { utilization: 73 },
|
||||
// Discriminating: a passing scope match must beat this fallback.
|
||||
fable_weekly: { utilization: 12 },
|
||||
limits: [
|
||||
{
|
||||
kind: 'weekly_scoped',
|
||||
percent: 64,
|
||||
resets_at: '2026-07-17T20:00:00.099908+00:00',
|
||||
is_active: true,
|
||||
scope: { model: { display_name: 'Fable 5.1' } }
|
||||
}
|
||||
]
|
||||
}),
|
||||
{ status: 200 }
|
||||
)
|
||||
)
|
||||
|
||||
await expect(
|
||||
fetchClaudeRateLimits({ authPreparation, allowUsagePanelSupplement: true })
|
||||
).resolves.toMatchObject({
|
||||
provider: 'claude',
|
||||
status: 'ok',
|
||||
fableWeekly: {
|
||||
usedPercent: 64,
|
||||
resetsAt: Date.parse('2026-07-17T20:00:00.099908+00:00')
|
||||
}
|
||||
})
|
||||
expect(fetchViaPty).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('surfaces inactive scoped Fable usage over the legacy OAuth fallback', async () => {
|
||||
const configDir = '/Users/test/.claude'
|
||||
const authPreparation: ClaudeRuntimeAuthPreparation = {
|
||||
|
||||
@@ -32,17 +32,13 @@ async function ensureProxyFromEnvironment(): Promise<void> {
|
||||
}).catch(() => {})
|
||||
}
|
||||
|
||||
// Why: the scope name carries the shipped version once a point release exists
|
||||
// ("Fable 5.1"), so exact equality would drop the window.
|
||||
const FABLE_SCOPE_RE = /^fable\b/
|
||||
|
||||
function mapFableWeeklyWindow(data: OAuthUsageResponse): RateLimitWindow | null {
|
||||
const scoped = Array.isArray(data.limits)
|
||||
? data.limits.find(
|
||||
(limit) =>
|
||||
limit?.kind === 'weekly_scoped' &&
|
||||
Number.isFinite(limit.percent) &&
|
||||
FABLE_SCOPE_RE.test(limit.scope?.model?.display_name?.trim().toLowerCase() ?? '')
|
||||
limit.scope?.model?.display_name?.trim().toLowerCase() === 'fable'
|
||||
)
|
||||
: undefined
|
||||
return (
|
||||
|
||||
@@ -27,10 +27,10 @@ const mocks = vi.hoisted(() => ({
|
||||
sessionOptionsSnapshot?: SessionOptionDescriptor[]
|
||||
attachDisabled?: boolean
|
||||
} | null,
|
||||
modelSwitchOutcome: 'applied' as 'applied' | 'rejected' | 'interaction-required' | 'unknown',
|
||||
modelSwitchOutcome: 'applied' as 'applied' | 'rejected' | 'unknown',
|
||||
confirmationObserver: null as {
|
||||
ready: Promise<void>
|
||||
result: Promise<'applied' | 'rejected' | 'interaction-required' | 'unknown'>
|
||||
result: Promise<'applied' | 'rejected' | 'unknown'>
|
||||
arm: ReturnType<typeof vi.fn>
|
||||
startDetection: ReturnType<typeof vi.fn>
|
||||
dispose: ReturnType<typeof vi.fn>
|
||||
@@ -731,33 +731,6 @@ describe('NativeChatComposer', () => {
|
||||
expect(onSwitchToTerminal).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reveals Claude interaction only when the model switch needs user input', async () => {
|
||||
mocks.sendHandle.settleAfterMs = 0
|
||||
mocks.modelSwitchOutcome = 'interaction-required'
|
||||
const onSwitchToTerminal = vi.fn()
|
||||
render(
|
||||
<NativeChatComposer
|
||||
terminalTabId="tab-1"
|
||||
paneKey="tab-1:leaf-1"
|
||||
targetPtyId="pty-1"
|
||||
agent="claude"
|
||||
onSwitchToTerminal={onSwitchToTerminal}
|
||||
/>
|
||||
)
|
||||
|
||||
await act(async () => {
|
||||
await mocks.fieldProps?.sessionOptionsSurface?.setOption('model', 'fable')
|
||||
})
|
||||
|
||||
expect(mocks.sendNativeChatMessageVerified).toHaveBeenCalledWith(
|
||||
{},
|
||||
'pty-1',
|
||||
'/model fable',
|
||||
expect.any(AbortSignal)
|
||||
)
|
||||
expect(onSwitchToTerminal).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('types the Codex picker command and switches to the terminal', async () => {
|
||||
mocks.sendHandle.settleAfterMs = 0
|
||||
const onSwitchToTerminal = vi.fn()
|
||||
|
||||
@@ -138,69 +138,6 @@ describe('Claude model switch confirmation detection', () => {
|
||||
await expect(observer.result).resolves.toBe('rejected')
|
||||
})
|
||||
|
||||
it('requests interaction for Fable one-time usage-credit consent', async () => {
|
||||
const dataObserver = { current: (_data: string): void => {} }
|
||||
const observer = createClaudeModelSwitchConfirmationObserver({
|
||||
ptyId: 'pty-1',
|
||||
settings: {},
|
||||
expectedModelLabel: 'Fable 5',
|
||||
subscribeToData: (watcher) => {
|
||||
dataObserver.current = watcher
|
||||
return vi.fn(() => {})
|
||||
},
|
||||
timeoutMs: 100
|
||||
})
|
||||
|
||||
await observer.ready
|
||||
observer.arm()
|
||||
dataObserver.current('Fable 5 uses usage credits and needs a one-time consent — ')
|
||||
dataObserver.current('pick Fable from /model in an interactive session to set it up')
|
||||
|
||||
await expect(observer.result).resolves.toBe('interaction-required')
|
||||
})
|
||||
|
||||
it('requests interaction for a Fable point-release consent prompt', async () => {
|
||||
const dataObserver = { current: (_data: string): void => {} }
|
||||
const observer = createClaudeModelSwitchConfirmationObserver({
|
||||
ptyId: 'pty-1',
|
||||
settings: {},
|
||||
expectedModelLabel: 'Fable 5.1',
|
||||
subscribeToData: (watcher) => {
|
||||
dataObserver.current = watcher
|
||||
return vi.fn(() => {})
|
||||
},
|
||||
timeoutMs: 100
|
||||
})
|
||||
|
||||
await observer.ready
|
||||
observer.arm()
|
||||
// Only the versioned consent line: the generic "pick Fable from /model"
|
||||
// sentence must not be what carries this case.
|
||||
dataObserver.current('Fable 5.1 uses usage credits and needs a one-time consent')
|
||||
|
||||
await expect(observer.result).resolves.toBe('interaction-required')
|
||||
})
|
||||
|
||||
it('requests interaction for a versioned Fable switch prompt', async () => {
|
||||
const dataObserver = { current: (_data: string): void => {} }
|
||||
const observer = createClaudeModelSwitchConfirmationObserver({
|
||||
ptyId: 'pty-1',
|
||||
settings: {},
|
||||
expectedModelLabel: 'Fable 5.1',
|
||||
subscribeToData: (watcher) => {
|
||||
dataObserver.current = watcher
|
||||
return vi.fn(() => {})
|
||||
},
|
||||
timeoutMs: 100
|
||||
})
|
||||
|
||||
await observer.ready
|
||||
observer.arm()
|
||||
dataObserver.current('Switch to \u001b[1mFable 5.1\u001b[0m? This model uses usage credits.')
|
||||
|
||||
await expect(observer.result).resolves.toBe('interaction-required')
|
||||
})
|
||||
|
||||
it('reports unknown when the PTY observer cannot be established', async () => {
|
||||
const observer = createClaudeModelSwitchConfirmationObserver({
|
||||
ptyId: 'pty-1',
|
||||
|
||||
@@ -10,7 +10,7 @@ const MAX_OBSERVED_BYTES = 64 * 1024
|
||||
|
||||
type SubscribeToData = (watcher: (data: string) => void) => Promise<() => void> | (() => void)
|
||||
|
||||
export type ClaudeModelSwitchOutcome = 'applied' | 'rejected' | 'interaction-required' | 'unknown'
|
||||
export type ClaudeModelSwitchOutcome = 'applied' | 'rejected' | 'unknown'
|
||||
|
||||
export type ClaudeModelSwitchConfirmationObserver = {
|
||||
ready: Promise<void>
|
||||
@@ -62,22 +62,6 @@ function hasClaudeModelSwitchRejection(buffer: string): boolean {
|
||||
return compactTerminalText(buffer).includes('keptmodelas')
|
||||
}
|
||||
|
||||
// Why: compactTerminalText only strips whitespace, so a point release keeps its
|
||||
// dot ("fable5.1uses..."). The version is optional because the CLI's own label
|
||||
// for the newest Fable carries no number at all.
|
||||
const FABLE_VERSION = String.raw`fable(?:\d+(?:\.\d+)*)?`
|
||||
const FABLE_CONSENT_RE = new RegExp(`${FABLE_VERSION}usesusagecreditsandneedsaone-timeconsent`)
|
||||
const FABLE_SWITCH_PROMPT_RE = new RegExp(`switchto${FABLE_VERSION}\\?`)
|
||||
|
||||
function hasClaudeModelSwitchInteraction(buffer: string): boolean {
|
||||
const text = compactTerminalText(buffer)
|
||||
return (
|
||||
FABLE_CONSENT_RE.test(text) ||
|
||||
text.includes('pickfablefrom/modelinaninteractivesessiontosetitup') ||
|
||||
(FABLE_SWITCH_PROMPT_RE.test(text) && text.includes('usagecredits'))
|
||||
)
|
||||
}
|
||||
|
||||
function subscribeToClaudeModelSwitchData(args: {
|
||||
ptyId: string
|
||||
settings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null | undefined
|
||||
@@ -156,10 +140,6 @@ export function createClaudeModelSwitchConfirmationObserver(args: {
|
||||
finish('rejected')
|
||||
return
|
||||
}
|
||||
if (hasClaudeModelSwitchInteraction(observed)) {
|
||||
finish('interaction-required')
|
||||
return
|
||||
}
|
||||
if (!confirmationSubmitted && hasClaudeModelSwitchConfirmation(observed)) {
|
||||
confirmationSubmitted = true
|
||||
try {
|
||||
|
||||
@@ -159,28 +159,6 @@ describe('native chat PTY session options', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('reveals the terminal only when Claude actually requires model-switch interaction', async () => {
|
||||
seedNativeChatAppliedSessionOptions('pty-1', 'claude', { model: 'sonnet' })
|
||||
const dispatch = vi.fn().mockResolvedValue({ outcome: 'interaction-required' })
|
||||
const onAgentPicker = vi.fn()
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
scopeKey: 'pty-1',
|
||||
mode: 'live',
|
||||
dispatchCommand: dispatch,
|
||||
onAgentPicker
|
||||
})!
|
||||
|
||||
const result = await surface.setOption('model', 'haiku')
|
||||
|
||||
expect(dispatch).toHaveBeenCalledWith('/model haiku', {
|
||||
detectAgentInteraction: 'claude-model-switch-confirmation',
|
||||
expectedChoiceLabel: 'Haiku'
|
||||
})
|
||||
expect(onAgentPicker).toHaveBeenCalledOnce()
|
||||
expect(result.snapshot[0]).toMatchObject({ valueSource: 'unknown' })
|
||||
})
|
||||
|
||||
it('keeps the prior model and persistence when Claude rejects the switch', async () => {
|
||||
seedNativeChatAppliedSessionOptions('pty-1', 'claude', {
|
||||
model: 'fable',
|
||||
|
||||
@@ -171,12 +171,6 @@ function applyDispatchOutcome(
|
||||
ctx.publish()
|
||||
throw new Error('Could not verify the model change; open the terminal to check.')
|
||||
}
|
||||
if (dispatchResult?.outcome === 'interaction-required') {
|
||||
ctx.clearModelTruth()
|
||||
const snapshot = ctx.publish()
|
||||
ctx.onAgentPicker?.()
|
||||
return { snapshot }
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user