mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 00:02:29 +00:00
Report Codex thread confirmations so an unlisted model keeps its name
Withholding unlisted ids regressed Codex. On main the reader fabricated a row for an unlisted `current.model`, so the pill named it; dropping that fabrication left the name to provenance, and Codex reported none — `readCodexStructuredSessionOptions` emitted no `confirmed` ids, so `result.current.confirmed ?? []` recorded even a model the thread was genuinely running as `dispatched`. Probed against origin/main: a resumed thread on `gpt-unlisted` rendered `gpt-unlisted` there and `Model` here. That is the bug this branch fixes for Claude, recreated for Codex. `input.current` cannot answer it alone: it merges two sources. `session.options` holds a pick restored from a previous session or a write of ours the thread has not echoed, while `session.reportedOptions` has one writer — the opened or resumed thread — and is the agent's own state. Only `readLiveCodexSessionOptions` knows which answered, so it now says, and the reader passes it on. `confirmed` is an existing optional wire field whose absence already means "unconfirmed", so a host that predates this sends nothing and old clients read exactly what they read now. A model the reader substitutes because nothing was current is never confirmed: that id is our choice, not a report.
This commit is contained in:
@@ -4,6 +4,7 @@ import { CodexAcquisitionWindow } from './codex-structured-acquisition-window'
|
||||
import {
|
||||
applyCodexStructuredSessionOption,
|
||||
readCodexStructuredSessionOptions,
|
||||
readLiveCodexSessionOptions,
|
||||
reportedCodexThreadOptions,
|
||||
restoredCodexSessionOptions
|
||||
} from './codex-structured-session-options'
|
||||
@@ -142,6 +143,51 @@ describe('structured Codex session options', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('confirms a model the thread reported, but not a pick we restored', async () => {
|
||||
// The display rule names an unlisted model only when the agent reported it, so the
|
||||
// reader has to say which of the two answered. `session.options` is a restored pick
|
||||
// or an unechoed write of ours; `reportedOptions` is the opened thread's own state.
|
||||
const request = vi.fn(async () => ({
|
||||
data: [{ model: 'gpt-live', displayName: 'GPT Live', isDefault: true }],
|
||||
nextCursor: null
|
||||
}))
|
||||
const session = (
|
||||
options: Map<string, string>,
|
||||
reported: Record<string, string>
|
||||
): CodexSession =>
|
||||
({ connection: { request }, options, reportedOptions: reported }) as unknown as CodexSession
|
||||
|
||||
await expect(
|
||||
readLiveCodexSessionOptions(session(new Map(), { model: 'gpt-unlisted' }), undefined)
|
||||
).resolves.toMatchObject({ current: { model: 'gpt-unlisted', confirmed: ['model'] } })
|
||||
|
||||
const restored = await readLiveCodexSessionOptions(
|
||||
session(new Map([['model', 'gpt-stale']]), {}),
|
||||
undefined
|
||||
)
|
||||
expect(restored.current).toEqual({ model: 'gpt-stale' })
|
||||
expect(restored.current.confirmed).toBeUndefined()
|
||||
})
|
||||
|
||||
it('does not confirm a model it substituted rather than read', async () => {
|
||||
// With nothing current the reader picks the default; that is our choice, not a report.
|
||||
const request = vi.fn(async () => ({
|
||||
data: [{ model: 'gpt-live', displayName: 'GPT Live', isDefault: true }],
|
||||
nextCursor: null
|
||||
}))
|
||||
|
||||
await expect(
|
||||
readCodexStructuredSessionOptions({
|
||||
connection: { request } as never,
|
||||
current: {},
|
||||
confirmed: ['model']
|
||||
})
|
||||
).resolves.toEqual({
|
||||
models: [{ id: 'gpt-live', label: 'GPT Live', isDefault: true, efforts: [] }],
|
||||
current: { model: 'gpt-live' }
|
||||
})
|
||||
})
|
||||
|
||||
it('hydrates current values from thread start or resume', () => {
|
||||
expect(
|
||||
reportedCodexThreadOptions({
|
||||
|
||||
@@ -78,6 +78,9 @@ function modelOption(value: unknown): AgentSessionModelOption | null {
|
||||
export async function readCodexStructuredSessionOptions(input: {
|
||||
connection: Pick<CodexAppServerConnection, 'request'>
|
||||
current: { model?: string; effort?: string }
|
||||
/** Ids in `current` that came from the thread's own state rather than from a value we
|
||||
* wrote. Only the caller knows which answered, so it must say. */
|
||||
confirmed?: readonly string[]
|
||||
timeoutMs?: number
|
||||
}): Promise<AgentSessionOptionsResult> {
|
||||
const models: AgentSessionModelOption[] = []
|
||||
@@ -108,9 +111,15 @@ export async function readCodexStructuredSessionOptions(input: {
|
||||
if (!model) {
|
||||
throw new Error('codex app-server returned no available models')
|
||||
}
|
||||
// Never pass on a confirmation for a model this function substituted rather than read.
|
||||
const confirmed = input.confirmed?.filter((id) => id !== 'model' || model === input.current.model)
|
||||
return {
|
||||
models,
|
||||
current: { model, ...(input.current.effort ? { effort: input.current.effort } : {}) }
|
||||
current: {
|
||||
model,
|
||||
...(input.current.effort ? { effort: input.current.effort } : {}),
|
||||
...(confirmed?.length ? { confirmed } : {})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -127,11 +136,21 @@ export function readLiveCodexSessionOptions(
|
||||
session: CodexSession,
|
||||
timeoutMs: number | undefined
|
||||
): Promise<AgentSessionOptionsResult> {
|
||||
const model = session.options.get('model') ?? session.reportedOptions.model
|
||||
const effort = session.options.get('effort') ?? session.reportedOptions.effort
|
||||
const pendingModel = session.options.get('model')
|
||||
const pendingEffort = session.options.get('effort')
|
||||
const model = pendingModel ?? session.reportedOptions.model
|
||||
const effort = pendingEffort ?? session.reportedOptions.effort
|
||||
// Why: `session.options` holds a restored pick or a write of ours the thread has not
|
||||
// echoed — neither is the agent speaking. A value that fell through to the opened
|
||||
// thread's own state is, which is what lets an unlisted model still be named.
|
||||
const confirmed = [
|
||||
...(!pendingModel && model ? ['model'] : []),
|
||||
...(!pendingEffort && effort ? ['effort'] : [])
|
||||
]
|
||||
return readCodexStructuredSessionOptions({
|
||||
connection: session.connection,
|
||||
current: { ...(model ? { model } : {}), ...(effort ? { effort } : {}) },
|
||||
...(confirmed.length > 0 ? { confirmed } : {}),
|
||||
timeoutMs
|
||||
})
|
||||
}
|
||||
|
||||
@@ -97,10 +97,8 @@ describe('structured agent session options', () => {
|
||||
{ models: [], current: { model: 'gpt-5.9-secret' } }
|
||||
)
|
||||
|
||||
// Codex's reader emits no `confirmed` ids, so even a model the thread genuinely runs
|
||||
// records as `dispatched`, not `reported` — which is why the name stays withheld here
|
||||
// while the equivalent Claude case is named. Pinned so that when the reader starts
|
||||
// confirming, this reddens and the name can be turned on deliberately.
|
||||
// Unconfirmed: a pick restored from a previous session, which the thread has not
|
||||
// echoed. An empty list must not promote it to a name it was never owed.
|
||||
expect(state.record.model).toEqual({ value: 'gpt-5.9-secret', source: 'dispatched' })
|
||||
const snapshot = structuredAgentSessionOptionSnapshot(state)
|
||||
expect(snapshot.map((descriptor) => descriptor.id)).toEqual(['model'])
|
||||
@@ -109,6 +107,21 @@ describe('structured agent session options', () => {
|
||||
expect(model.kind.type === 'select' ? model.kind.choices : null).toEqual([])
|
||||
})
|
||||
|
||||
it('names the model an empty-listing provider reported, without offering it', () => {
|
||||
// The regression this rule exists to prevent: a thread whose `model/list` came back
|
||||
// empty still runs a model and says so, so the pill must name it rather than blank.
|
||||
const state = applyStructuredAgentSessionOptions(
|
||||
createStructuredAgentSessionOptionState('codex'),
|
||||
CODEX_SESSION_OPTION_CATALOG,
|
||||
{ models: [], current: { model: 'gpt-5.9-secret', confirmed: ['model'] } }
|
||||
)
|
||||
|
||||
const model = structuredAgentSessionOptionSnapshot(state)[0]
|
||||
expect(model).toMatchObject({ valueSource: 'reported' })
|
||||
expect(model.kind.type === 'select' ? model.kind.currentValue : null).toBe('gpt-5.9-secret')
|
||||
expect(model.kind.type === 'select' ? model.kind.choices : null).toEqual([])
|
||||
})
|
||||
|
||||
it('projects live options as directly settable descriptors', () => {
|
||||
const state = applyStructuredAgentSessionOptions(
|
||||
createStructuredAgentSessionOptionState('codex'),
|
||||
|
||||
Reference in New Issue
Block a user