mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(claude): refuse a structured model the provider does not list (#19946)
* fix(claude): refuse a structured model the provider does not list setClaudeStructuredOption applied a model to a live Claude structured session with no check that the provider lists it, while pre-flighting `effort` against the same catalog a few lines above. Measured on Claude Code 2.1.260: set_model resolves for an unlisted id, list_models never gains a row for it, and every later turn returns is_error with empty modelUsage and zero tokens — a session that looks alive and produces nothing. Nothing undoes the write, so the refusal has to precede it. Two paths reach it: restore replays a stored pick the provider may since have retired, which needs no user error at all, and any caller can send an arbitrary id mid-session. An absent, failed or empty list deliberately refuses nothing, mirroring the null rule the effort guard already applies: no catalog identifies no model, and a CLI predating list_models would otherwise have every model refused under it — silently, since restore swallows the rejection into restoreSkippedOptions. * refactor(claude): keep the model pre-flight's permissive case in the authority claudeCatalogAdmitsModel now answers the question outright instead of handing back a nullable id set the caller had to interpret. The rule that an unidentified catalog refuses nothing lives inside the function, so a second caller cannot get it wrong by omission — and getting it wrong is silent, because restore swallows the rejection into restoreSkippedOptions. The refusal message names the model the user asked for, since it reaches them as the chat error row. --------- Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
co-authored by
Merge Sim
parent
ef6eeab26e
commit
a05e2139d6
@@ -0,0 +1,130 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { AgentSessionOptionRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-option-error'
|
||||
import {
|
||||
restoreClaudeStructuredSessionOptions,
|
||||
setClaudeStructuredOption
|
||||
} from './claude-structured-options'
|
||||
import type { ClaudeSession } from './claude-structured-session-state'
|
||||
|
||||
/** Verbatim row shapes from Claude Code 2.1.260's list_models response. */
|
||||
const DEFAULT_ROW = { value: 'default', resolvedModel: 'claude-opus-5', displayName: 'Default' }
|
||||
const SONNET = { value: 'sonnet', resolvedModel: 'claude-sonnet-5', displayName: 'Sonnet' }
|
||||
const HAIKU = {
|
||||
value: 'haiku',
|
||||
resolvedModel: 'claude-haiku-4-5-20251001',
|
||||
displayName: 'Haiku'
|
||||
}
|
||||
|
||||
function sessionWith(catalog: readonly Record<string, unknown>[] | 'unavailable') {
|
||||
const calls: string[] = []
|
||||
return {
|
||||
session: {
|
||||
options: new Map<string, string>(),
|
||||
reportedOptions: {} as { model?: string; effort?: string },
|
||||
optionMutationSequence: 0,
|
||||
reportedModelMutation: 0,
|
||||
confirmedOptions: new Set<string>(),
|
||||
restoreSkippedOptions: new Set<string>(),
|
||||
connection: {
|
||||
supportedModels: async () => {
|
||||
calls.push('list_models')
|
||||
if (catalog === 'unavailable') {
|
||||
throw new Error('this CLI predates list_models')
|
||||
}
|
||||
return [...catalog]
|
||||
},
|
||||
setModel: async (model: string) => {
|
||||
calls.push(`set_model:${model}`)
|
||||
}
|
||||
}
|
||||
} as unknown as ClaudeSession,
|
||||
calls
|
||||
}
|
||||
}
|
||||
|
||||
describe('Claude model pre-flight against the catalog the CLI listed', () => {
|
||||
it('refuses a model the provider does not list', async () => {
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW, SONNET, HAIKU])
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'not-a-real-model-xyz' }, undefined)
|
||||
).rejects.toBeInstanceOf(AgentSessionOptionRejectedError)
|
||||
// Measured on Claude Code 2.1.260: set_model resolves for an unlisted id and
|
||||
// every later turn returns is_error with zero tokens. Nothing undoes the
|
||||
// write, so the refusal has to land before it.
|
||||
expect(calls).toEqual(['list_models'])
|
||||
expect(session.options.has('model')).toBe(false)
|
||||
})
|
||||
|
||||
it('refuses an unlisted model replayed by restore, and skips it', async () => {
|
||||
// Needs no user error: a model valid when it was persisted can be retired.
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW, SONNET])
|
||||
session.options.set('model', 'claude-opus-4-retired')
|
||||
|
||||
await restoreClaudeStructuredSessionOptions(session, undefined)
|
||||
|
||||
expect(calls).toEqual(['list_models'])
|
||||
expect(session.options.has('model')).toBe(false)
|
||||
expect([...session.restoreSkippedOptions]).toEqual(['model'])
|
||||
})
|
||||
|
||||
it('applies a model the provider lists', async () => {
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW, SONNET, HAIKU])
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'haiku' }, undefined)
|
||||
).resolves.toEqual({ model: 'haiku' })
|
||||
expect(calls).toEqual(['list_models', 'set_model:haiku'])
|
||||
})
|
||||
|
||||
it('applies a resolved model id the catalog carries only under its alias', async () => {
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW, SONNET])
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'claude-sonnet-5' }, undefined)
|
||||
).resolves.toEqual({ model: 'claude-sonnet-5' })
|
||||
expect(calls).toEqual(['list_models', 'set_model:claude-sonnet-5'])
|
||||
})
|
||||
|
||||
it('refuses nothing when list_models is unavailable', async () => {
|
||||
// A CLI predating list_models would otherwise have every model refused, and
|
||||
// restore swallows the rejection, so the user's pick would vanish silently.
|
||||
const { session, calls } = sessionWith('unavailable')
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'sonnet' }, undefined)
|
||||
).resolves.toEqual({ model: 'sonnet' })
|
||||
expect(calls).toEqual(['list_models', 'set_model:sonnet'])
|
||||
})
|
||||
|
||||
it('refuses nothing when the listed catalog is empty', async () => {
|
||||
// An empty answer identifies no model, so it is not evidence against one.
|
||||
const { session, calls } = sessionWith([])
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'sonnet' }, undefined)
|
||||
).resolves.toEqual({ model: 'sonnet' })
|
||||
expect(calls).toEqual(['list_models', 'set_model:sonnet'])
|
||||
})
|
||||
|
||||
it('refuses nothing when the catalog carries only the synthetic default row', async () => {
|
||||
// listedModels drops that row, leaving a list that identifies no model.
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW])
|
||||
|
||||
await expect(
|
||||
setClaudeStructuredOption(session, { key: 'model', value: 'sonnet' }, undefined)
|
||||
).resolves.toEqual({ model: 'sonnet' })
|
||||
expect(calls).toEqual(['list_models', 'set_model:sonnet'])
|
||||
})
|
||||
|
||||
it('leaves a restored model the provider lists in place', async () => {
|
||||
const { session, calls } = sessionWith([DEFAULT_ROW, SONNET])
|
||||
session.options.set('model', 'sonnet')
|
||||
|
||||
await restoreClaudeStructuredSessionOptions(session, undefined)
|
||||
|
||||
expect(calls).toEqual(['list_models', 'set_model:sonnet'])
|
||||
expect(session.options.get('model')).toBe('sonnet')
|
||||
expect([...session.restoreSkippedOptions]).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -6,7 +6,12 @@ import { ClaudeSlashCommandCatalog } from './claude-slash-command-catalog'
|
||||
|
||||
function sessionFor(setModel: ClaudeSession['connection']['setModel']): ClaudeSession {
|
||||
return {
|
||||
connection: { setModel } as ClaudeSession['connection'],
|
||||
// An empty catalog identifies no model, so the pre-flight refuses nothing and
|
||||
// this stays a test about fencing.
|
||||
connection: {
|
||||
setModel,
|
||||
supportedModels: async (): Promise<unknown[]> => []
|
||||
} as ClaudeSession['connection'],
|
||||
providerSessionId: 'provider-session',
|
||||
claudeConfigDir: '/accounts/claude',
|
||||
leafUuid: null,
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
isAgentSessionOptionRejectedError
|
||||
} from '../native-chat/agent-session-wire/structured-agent-session-option-error'
|
||||
import {
|
||||
claudeCatalogAdmitsModel,
|
||||
readClaudeCurrentModel,
|
||||
readClaudeModelEffortLevels,
|
||||
readClaudeSettingsEffort
|
||||
@@ -66,6 +67,13 @@ export async function setClaudeStructuredOption(
|
||||
)
|
||||
}
|
||||
}
|
||||
// set_model resolves for a model the provider never lists and the session then
|
||||
// fails every turn with zero tokens, so the acceptance proves nothing and only
|
||||
// the catalog does. Restore replays a pick the provider may since have retired,
|
||||
// which reaches here with no user error at all.
|
||||
if (input.key === 'model' && !(await claudeCatalogAdmitsModel(session, input.value, timeoutMs))) {
|
||||
throw new AgentSessionOptionRejectedError(`claude does not list a model named ${input.value}`)
|
||||
}
|
||||
const modelWasConfirmed = readClaudeCurrentModel(session).confirmed
|
||||
const mutationSequence = ++session.optionMutationSequence
|
||||
// Only a model write can stale the model report — an effort or permission-mode
|
||||
|
||||
@@ -80,8 +80,11 @@ describe('ClaudeStructuredSessionAdapter turns and controls', () => {
|
||||
await expect(
|
||||
adapter.setOption({ sessionId: 'session-1', key: 'model', value: 'sonnet', fence: 7 })
|
||||
).resolves.toEqual({ model: 'sonnet' })
|
||||
expect(claude.connections[0].calls.slice(-2)).toEqual([
|
||||
// The model write pre-flights the catalog first; this CLI lists nothing, which
|
||||
// identifies no model and so refuses none.
|
||||
expect(claude.connections[0].calls.slice(-3)).toEqual([
|
||||
{ subtype: 'interrupt', params: {} },
|
||||
{ subtype: 'list_models' },
|
||||
{ subtype: 'set_model', params: { model: 'sonnet' } }
|
||||
])
|
||||
|
||||
|
||||
@@ -149,6 +149,28 @@ export async function readClaudeModelEffortLevels(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the catalog admits the model, matched by alias or resolved id so a pick
|
||||
* stored as either one is found. The permissive case lives here rather than at the
|
||||
* call site: every caller must treat an unidentified catalog the same way, and one
|
||||
* that forgot would refuse every model on a CLI that cannot answer.
|
||||
*/
|
||||
export async function claudeCatalogAdmitsModel(
|
||||
session: ClaudeSession,
|
||||
modelId: string,
|
||||
timeoutMs: number | undefined
|
||||
): Promise<boolean> {
|
||||
const catalog = await session.connection.supportedModels({ timeoutMs }).catch(() => null)
|
||||
const models = listedModels(catalog ? { models: catalog } : null)
|
||||
// An empty list identifies no model, so it is not evidence against one — a live
|
||||
// CLI predating `list_models` would otherwise have every model refused under it.
|
||||
// Do not turn this into a refusal.
|
||||
return (
|
||||
models.length === 0 ||
|
||||
models.some((model) => model.id === modelId || model.resolvedModel === modelId)
|
||||
)
|
||||
}
|
||||
|
||||
export async function readClaudeStructuredSessionOptions(
|
||||
session: ClaudeSession,
|
||||
timeoutMs: number | undefined
|
||||
|
||||
Reference in New Issue
Block a user