From f0c7d0b7e7acc17b6693751e848d3b8fc7a23d11 Mon Sep 17 00:00:00 2001 From: weishu Date: Tue, 28 Jul 2026 11:39:53 +0800 Subject: [PATCH] fix(codex): preserve reasoning effort across mode switches --- cli/src/codex/codexLocalLauncher.test.ts | 50 ++++++++++++++++++++- cli/src/codex/codexLocalLauncher.ts | 20 ++++++++- cli/src/codex/runCodex.test.ts | 27 ++++++++++- cli/src/codex/runCodex.ts | 26 ++++++----- cli/src/codex/utils/appServerConfig.test.ts | 5 +-- cli/src/codex/utils/appServerConfig.ts | 2 +- 6 files changed, 110 insertions(+), 20 deletions(-) diff --git a/cli/src/codex/codexLocalLauncher.test.ts b/cli/src/codex/codexLocalLauncher.test.ts index c6dafbfd..48c649a7 100644 --- a/cli/src/codex/codexLocalLauncher.test.ts +++ b/cli/src/codex/codexLocalLauncher.test.ts @@ -80,6 +80,8 @@ function createSessionStub( let localLaunchFailure: { message: string; exitReason: 'switch' | 'exit' } | null = null; let sessionId: string | null = null; let transcriptPath: string | null = initialTranscriptPath; + let modelReasoningEffort: string | null = null; + const modelReasoningEffortUpdates: Array = []; const transcriptPathCallbacks: Array<(path: string) => void> = []; return { @@ -102,7 +104,11 @@ function createSessionStub( } }, getPermissionMode: () => permissionMode, - getModelReasoningEffort: () => null, + getModelReasoningEffort: () => modelReasoningEffort, + setModelReasoningEffort: (effort: string | null) => { + modelReasoningEffort = effort; + modelReasoningEffortUpdates.push(effort); + }, onSessionFound: (value: string) => { sessionId = value; }, @@ -145,7 +151,9 @@ function createSessionStub( userMessages, agentMessages, getUserActivityCount: () => userActivityCount, - getLocalLaunchFailure: () => localLaunchFailure + getLocalLaunchFailure: () => localLaunchFailure, + getModelReasoningEffort: () => modelReasoningEffort, + getModelReasoningEffortUpdates: () => modelReasoningEffortUpdates }; } @@ -361,6 +369,44 @@ describe('codexLocalLauncher', () => { }); }); + it('tracks explicit and default reasoning effort from local turn context', async () => { + const transcriptPath = await writeTranscriptMeta('codex-turn-context.jsonl', 'codex-thread-effort'); + const { session, getModelReasoningEffort, getModelReasoningEffortUpdates } = createSessionStub('default'); + let releaseRunBarrier: (() => void) | undefined; + harness.runBarrier = new Promise((resolve) => { + releaseRunBarrier = resolve; + }); + + const launcherPromise = codexLocalLauncher(session as never); + await wait(50); + harness.sessionHookHandlers[0]?.('codex-thread-effort', { + transcript_path: transcriptPath + }); + await wait(100); + + await appendFile(transcriptPath, [ + JSON.stringify({ + type: 'turn_context', + payload: { effort: 'max' } + }), + JSON.stringify({ + type: 'event_msg', + payload: { type: 'token_count', info: {} } + }), + JSON.stringify({ + type: 'turn_context', + payload: { model: 'gpt-5.4' } + }) + ].join('\n') + '\n'); + await wait(700); + + releaseRunBarrier?.(); + await launcherPromise; + + expect(getModelReasoningEffortUpdates()).toEqual(['max', null]); + expect(getModelReasoningEffort()).toBeNull(); + }); + it('renders nested Code Mode plans and commands without their covered exec wrapper', async () => { const transcriptPath = join(tempDir, 'codex-hook-transcript.jsonl'); const { session, agentMessages } = createSessionStub('default'); diff --git a/cli/src/codex/codexLocalLauncher.ts b/cli/src/codex/codexLocalLauncher.ts index ae37bb90..9b66e231 100644 --- a/cli/src/codex/codexLocalLauncher.ts +++ b/cli/src/codex/codexLocalLauncher.ts @@ -5,7 +5,7 @@ import { codexLocal } from './codexLocal'; import type { ReasoningEffort } from './appServerTypes'; import { CodexSession } from './session'; import { createCodexSessionScanner, type CodexSessionScanner } from './utils/codexSessionScanner'; -import { convertCodexEvent, type CodexMessage } from './utils/codexEventConverter'; +import { convertCodexEvent, type CodexMessage, type CodexSessionEvent } from './utils/codexEventConverter'; import { buildHapiMcpBridge } from './utils/buildHapiMcpBridge'; import { parseCodexCliOverrides, stripCodexCliOverrides } from './utils/codexCliOverrides'; import { buildCodexPermissionModeCliArgs } from './utils/permissionModeConfig'; @@ -22,6 +22,20 @@ type PendingExecWrapper = { turnId?: string; }; +function extractTurnContextReasoningEffort(event: CodexSessionEvent): ReasoningEffort | null | undefined { + if (event.type !== 'turn_context') { + return undefined; + } + if (!event.payload || typeof event.payload !== 'object') { + return null; + } + const effort = (event.payload as Record).effort; + if (typeof effort !== 'string' || !effort.trim()) { + return null; + } + return effort.trim().toLowerCase(); +} + export async function codexLocalLauncher(session: CodexSession): Promise<'switch' | 'exit'> { const resumeSessionId = session.sessionId; let primarySessionId = resumeSessionId; @@ -172,6 +186,10 @@ export async function codexLocalLauncher(session: CodexSession): Promise<'switch session.onSessionFound(sessionId); }, onEvent: (event) => { + const observedReasoningEffort = extractTurnContextReasoningEffort(event); + if (observedReasoningEffort !== undefined) { + session.setModelReasoningEffort(observedReasoningEffort); + } const converted = convertCodexEvent(event); if (converted?.sessionId) { if (!isPrimarySessionId(converted.sessionId)) { diff --git a/cli/src/codex/runCodex.test.ts b/cli/src/codex/runCodex.test.ts index 5fc99c80..6449f27c 100644 --- a/cli/src/codex/runCodex.test.ts +++ b/cli/src/codex/runCodex.test.ts @@ -211,6 +211,13 @@ describe('runCodex', () => { expect(mockCodexSession.setServiceTier).not.toHaveBeenCalled() }) + it('does not collapse inherited Codex reasoning effort into explicit default on startup', async () => { + await runCodexImpl({ workingDirectory: '/tmp/project' }) + + expect(mockCodexSession.setModelReasoningEffort).not.toHaveBeenCalled() + expect(harness.loopArgs[0]?.modelReasoningEffort).toBeUndefined() + }) + it('uses lazy bootstrap for a fresh terminal launch', async () => { await runCodexImpl({ workingDirectory: '/tmp/project' }) @@ -256,7 +263,23 @@ describe('runCodex', () => { await handler?.({ modelReasoningEffort: 'max' }) await handler?.({ modelReasoningEffort: ' EXTREME ' }) - expect(mockCodexSession.setModelReasoningEffort).toHaveBeenNthCalledWith(2, 'max') - expect(mockCodexSession.setModelReasoningEffort).toHaveBeenNthCalledWith(3, 'extreme') + expect(mockCodexSession.setModelReasoningEffort).toHaveBeenNthCalledWith(1, 'max') + expect(mockCodexSession.setModelReasoningEffort).toHaveBeenNthCalledWith(2, 'extreme') + }) + + it('still persists an explicit reasoning effort reset as null', async () => { + await runCodexImpl({ workingDirectory: '/tmp/project' }) + + const registration = harness.session.rpcHandlerManager.registerHandler.mock.calls.find( + ([method]) => method === RPC_METHODS.SetSessionConfig + ) + const handler = registration?.[1] as ((payload: unknown) => Promise) | undefined + const result = await handler?.({ modelReasoningEffort: null }) + + expect(mockCodexSession.setModelReasoningEffort).toHaveBeenCalledTimes(1) + expect(mockCodexSession.setModelReasoningEffort).toHaveBeenCalledWith(null) + expect(result).toEqual({ + applied: expect.objectContaining({ modelReasoningEffort: null }) + }) }) }) diff --git a/cli/src/codex/runCodex.ts b/cli/src/codex/runCodex.ts index 4534336c..ef5dd386 100644 --- a/cli/src/codex/runCodex.ts +++ b/cli/src/codex/runCodex.ts @@ -88,7 +88,9 @@ export async function runCodex(opts: { ?? (persistedPermissionMode && isPermissionModeAllowedForFlavor(persistedPermissionMode, 'codex') ? persistedPermissionMode as PermissionMode : undefined) ?? 'default'; let currentModel = opts.model; - let currentModelReasoningEffort: ReasoningEffort | undefined = opts.modelReasoningEffort; + // Three states matter here: `undefined` inherits Codex configuration, + // `null` explicitly clears a HAPI override, and a string explicitly sets it. + let currentModelReasoningEffort: ReasoningEffort | null | undefined = opts.modelReasoningEffort; let currentCollaborationMode: EnhancedMode['collaborationMode'] = opts.collaborationMode ?? 'default'; let currentProactiveMultiAgent: boolean | undefined; // Service tier (Fast mode), stored representation: `'fast'` and @@ -118,7 +120,11 @@ export async function runCodex(opts: { if (options?.syncModel !== false) { sessionInstance.setModel(currentModel ?? null); } - sessionInstance.setModelReasoningEffort(currentModelReasoningEffort ?? null); + // Do not collapse inherited Codex config into an explicit default. + // Explicit clears remain `null` and must still be synchronized. + if (currentModelReasoningEffort !== undefined) { + sessionInstance.setModelReasoningEffort(currentModelReasoningEffort); + } // Preserve the third state: only sync when the user/persisted session // has an explicit tier. `undefined` means "omit" so the keepalive does // not overwrite the account-default or persisted Fast tier with null. @@ -149,7 +155,7 @@ export async function runCodex(opts: { currentModel = updates.model ?? undefined; } if (updates.modelReasoningEffort !== undefined) { - currentModelReasoningEffort = updates.modelReasoningEffort ?? undefined; + currentModelReasoningEffort = updates.modelReasoningEffort; } if (updates.collaborationMode !== undefined) { currentCollaborationMode = updates.collaborationMode; @@ -174,7 +180,7 @@ export async function runCodex(opts: { } const sessionModelReasoningEffort = sessionWrapperRef.current?.getModelReasoningEffort(); if (sessionModelReasoningEffort !== undefined) { - currentModelReasoningEffort = (sessionModelReasoningEffort ?? undefined) as ReasoningEffort | undefined; + currentModelReasoningEffort = sessionModelReasoningEffort as ReasoningEffort | null; } const sessionCollaborationMode = sessionWrapperRef.current?.getCollaborationMode(); if (sessionCollaborationMode) { @@ -199,7 +205,7 @@ export async function runCodex(opts: { permissionMode: currentPermissionMode, collaborationMode: currentCollaborationMode, model: currentModel, - modelReasoningEffort: currentModelReasoningEffort, + modelReasoningEffort: currentModelReasoningEffort ?? undefined, serviceTier: currentServiceTier, proactiveMultiAgent: currentProactiveMultiAgent }); @@ -219,7 +225,7 @@ export async function runCodex(opts: { messageQueue.pushIsolateAndClear(goalCommand, { permissionMode: currentPermissionMode ?? 'default', model: currentModel, - modelReasoningEffort: currentModelReasoningEffort, + modelReasoningEffort: currentModelReasoningEffort ?? undefined, collaborationMode: currentCollaborationMode, serviceTier: currentServiceTier }, localId); @@ -258,7 +264,7 @@ export async function runCodex(opts: { const enhancedMode: EnhancedMode = { permissionMode: messagePermissionMode ?? 'default', model: currentModel, - modelReasoningEffort: currentModelReasoningEffort, + modelReasoningEffort: currentModelReasoningEffort ?? undefined, collaborationMode: currentCollaborationMode, proactiveMultiAgent: currentProactiveMultiAgent, serviceTier: currentServiceTier @@ -273,7 +279,7 @@ export async function runCodex(opts: { const enhancedMode: EnhancedMode = { permissionMode: currentPermissionMode ?? 'default', model: currentModel, - modelReasoningEffort: currentModelReasoningEffort, + modelReasoningEffort: currentModelReasoningEffort ?? undefined, collaborationMode: currentCollaborationMode, proactiveMultiAgent: currentProactiveMultiAgent, serviceTier: currentServiceTier @@ -367,7 +373,7 @@ export async function runCodex(opts: { } if (config.modelReasoningEffort !== undefined) { - currentModelReasoningEffort = parseReasoningEffortValue(config.modelReasoningEffort); + currentModelReasoningEffort = parseReasoningEffortValue(config.modelReasoningEffort) ?? null; } if (config.collaborationMode !== undefined) { @@ -413,7 +419,7 @@ export async function runCodex(opts: { startedBy, permissionMode: currentPermissionMode, model: currentModel, - modelReasoningEffort: currentModelReasoningEffort, + modelReasoningEffort: currentModelReasoningEffort ?? undefined, collaborationMode: currentCollaborationMode, resumeSessionId: opts.resumeSessionId, sourceSessionId: codexSourceSessionId, diff --git a/cli/src/codex/utils/appServerConfig.test.ts b/cli/src/codex/utils/appServerConfig.test.ts index 09ddef2c..b30b34a2 100644 --- a/cli/src/codex/utils/appServerConfig.test.ts +++ b/cli/src/codex/utils/appServerConfig.test.ts @@ -401,10 +401,10 @@ describe('appServerConfig', () => { mode: 'plan', settings: { model: 'o3', - reasoning_effort: null, developer_instructions: withCollaborationInstructions(`${codexSystemPrompt}\n\nOnly respond in Chinese.`) } }); + expect(params.collaborationMode?.settings).not.toHaveProperty('reasoning_effort'); }); it('injects spawn_agent argument rules into collaboration mode instructions', () => { @@ -463,7 +463,6 @@ describe('appServerConfig', () => { mode: 'default', settings: { model: 'o3', - reasoning_effort: null, developer_instructions: withCollaborationInstructions(codexSystemPrompt) } }); @@ -484,7 +483,6 @@ describe('appServerConfig', () => { mode: 'default', settings: { model: 'o3', - reasoning_effort: null, developer_instructions: withCollaborationInstructions(codexSystemPrompt) } }); @@ -504,7 +502,6 @@ describe('appServerConfig', () => { mode: 'default', settings: { model: 'gpt-5', - reasoning_effort: null, developer_instructions: withCollaborationInstructions(codexSystemPrompt) } }); diff --git a/cli/src/codex/utils/appServerConfig.ts b/cli/src/codex/utils/appServerConfig.ts index bd2591b9..28efcf8d 100644 --- a/cli/src/codex/utils/appServerConfig.ts +++ b/cli/src/codex/utils/appServerConfig.ts @@ -280,7 +280,7 @@ export function buildTurnStartParams(args: { mode: collaborationMode, settings: { model, - reasoning_effort: modelReasoningEffort ?? null, + ...(modelReasoningEffort !== undefined ? { reasoning_effort: modelReasoningEffort } : {}), developer_instructions: appendCollaborationInstructions(developerInstructions, args.mode?.proactiveMultiAgent) } };