diff --git a/src/main/codex/codex-structured-session-options.test.ts b/src/main/codex/codex-structured-session-options.test.ts index e3a0e2f2ca7..4917391d3ee 100644 --- a/src/main/codex/codex-structured-session-options.test.ts +++ b/src/main/codex/codex-structured-session-options.test.ts @@ -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, + reported: Record + ): 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({ diff --git a/src/main/codex/codex-structured-session-options.ts b/src/main/codex/codex-structured-session-options.ts index cca31866a70..a6985f0d143 100644 --- a/src/main/codex/codex-structured-session-options.ts +++ b/src/main/codex/codex-structured-session-options.ts @@ -78,6 +78,9 @@ function modelOption(value: unknown): AgentSessionModelOption | null { export async function readCodexStructuredSessionOptions(input: { connection: Pick 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 { 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 { - 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 }) } diff --git a/src/shared/structured-agent-session-options.test.ts b/src/shared/structured-agent-session-options.test.ts index ec2444bd696..a7ebd1a42cf 100644 --- a/src/shared/structured-agent-session-options.test.ts +++ b/src/shared/structured-agent-session-options.test.ts @@ -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'),