From b337411d2dddf50db8fb29a85ea52d5d894b6248 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 27 May 2026 19:31:50 +0100 Subject: [PATCH] fix(codex): rotate relay generation on fresh thread fallback --- .../codex/src/app-server/attempt-startup.ts | 6 +- .../run-attempt.native-hook-relay.test.ts | 53 ++++++++ .../codex/src/app-server/run-attempt.ts | 94 ++++--------- .../codex/src/app-server/thread-lifecycle.ts | 128 ++++-------------- 4 files changed, 112 insertions(+), 169 deletions(-) diff --git a/extensions/codex/src/app-server/attempt-startup.ts b/extensions/codex/src/app-server/attempt-startup.ts index 8c8a02441e1..e1bca31d124 100644 --- a/extensions/codex/src/app-server/attempt-startup.ts +++ b/extensions/codex/src/app-server/attempt-startup.ts @@ -84,8 +84,9 @@ export async function startCodexAttemptThread(params: { effectiveWorkspace: string; dynamicTools: CodexDynamicToolSpec[]; developerInstructions: string | undefined; - finalConfigPatch: Parameters[0]["finalConfigPatch"]; - nativeHookRelayGeneration: string | undefined; + finalConfigPatch?: Parameters[0]["finalConfigPatch"]; + buildFinalConfigPatch?: Parameters[0]["buildFinalConfigPatch"]; + nativeHookRelayGeneration?: string; bundleMcpThreadConfig: CodexBundleMcpThreadConfig; nativeToolSurfaceEnabled: boolean; sandboxExecServerEnabled: boolean; @@ -254,6 +255,7 @@ export async function startCodexAttemptThread(params: { developerInstructions: params.developerInstructions, config: threadConfig, finalConfigPatch: params.finalConfigPatch, + buildFinalConfigPatch: params.buildFinalConfigPatch, nativeHookRelayGeneration: params.nativeHookRelayGeneration, nativeCodeModeEnabled: params.nativeToolSurfaceEnabled, nativeCodeModeOnlyEnabled: params.appServer.codeModeOnly, diff --git a/extensions/codex/src/app-server/run-attempt.native-hook-relay.test.ts b/extensions/codex/src/app-server/run-attempt.native-hook-relay.test.ts index ee5f5e9b2f0..48065e72861 100644 --- a/extensions/codex/src/app-server/run-attempt.native-hook-relay.test.ts +++ b/extensions/codex/src/app-server/run-attempt.native-hook-relay.test.ts @@ -585,6 +585,59 @@ describe("runCodexAppServerAttempt native hook relay", () => { testing.flushPendingCodexNativeHookRelayUnregistersForTests(); }); + it("rotates native hook relay generations when resume fails over to a fresh thread", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const workspaceDir = path.join(tempDir, "workspace"); + await writeCodexAppServerBinding(sessionFile, { + threadId: "thread-existing", + cwd: workspaceDir, + model: "gpt-5.4-codex", + modelProvider: "openai", + nativeHookRelayGeneration: "generation-from-failed-resume", + }); + const harness = createStartedThreadHarness(async (method) => { + if (method === "thread/resume") { + throw new Error("resume failed"); + } + return undefined; + }); + + const run = runCodexAppServerAttempt(createParams(sessionFile, workspaceDir), { + nativeHookRelay: { + enabled: true, + events: ["pre_tool_use"], + }, + }); + await harness.waitForMethod("turn/start"); + + const startRequest = harness.requests.find((request) => request.method === "thread/start"); + const relayId = extractRelayIdFromThreadRequest(startRequest?.params); + const currentGeneration = extractGenerationFromThreadRequest(startRequest?.params); + expect(currentGeneration).not.toBe("generation-from-failed-resume"); + await expect( + invokeNativeHookRelay({ + provider: "codex", + relayId, + generation: "generation-from-failed-resume", + event: "pre_tool_use", + requireGeneration: true, + rawPayload: { + hook_event_name: "PreToolUse", + tool_name: "Bash", + tool_use_id: "failed-resume-stale-tool", + tool_input: { command: "pwd" }, + }, + }), + ).rejects.toThrow("native hook relay bridge stale registration"); + + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await run; + expect((await readCodexAppServerBinding(sessionFile))?.nativeHookRelayGeneration).toBe( + currentGeneration, + ); + testing.flushPendingCodexNativeHookRelayUnregistersForTests(); + }); + it("builds deterministic opaque Codex native hook relay ids", () => { const relayId = testing.buildCodexNativeHookRelayId({ agentId: "dev-codex", diff --git a/extensions/codex/src/app-server/run-attempt.ts b/extensions/codex/src/app-server/run-attempt.ts index 5e5f1a582c8..0b2613c687a 100644 --- a/extensions/codex/src/app-server/run-attempt.ts +++ b/extensions/codex/src/app-server/run-attempt.ts @@ -113,7 +113,6 @@ import { isCodexAppServerApprovalPolicyAllowedByRequirements, isCodexSandboxExecServerEnabled, readCodexPluginConfig, - resolveCodexPluginsPolicy, resolveCodexComputerUseConfig, resolveCodexAppServerRuntimeOptions, shouldAutoApproveCodexAppServerApprovals, @@ -127,7 +126,6 @@ import { import { buildDynamicTools, createCodexDynamicToolBuildStageTracker, - disableCodexPluginThreadConfig, filterCodexDynamicToolsForAllowlist, formatCodexDynamicToolBuildStageSummary, includeForcedCodexDynamicToolAllow, @@ -181,11 +179,6 @@ import { } from "./native-hook-relay.js"; import { registerCodexNativeSubagentMonitor } from "./native-subagent-monitor.js"; import { describeCodexNotificationCorrelation } from "./notification-correlation.js"; -import { buildCodexPluginAppCacheKey } from "./plugin-app-cache-key.js"; -import { - buildCodexPluginThreadConfigInputFingerprint, - shouldBuildCodexPluginThreadConfig, -} from "./plugin-thread-config.js"; import { isCodexAppServerProfilerEnabled } from "./profiler-flag.js"; import { assertCodexTurnStartResponse, @@ -214,7 +207,6 @@ import { buildTurnCollaborationMode, buildTurnStartParams, codexDynamicToolsFingerprint, - resolveCodexNativeHookRelayBindingReuse, type CodexAppServerThreadLifecycleBinding, type CodexContextEngineThreadBootstrapProjection, } from "./thread-lifecycle.js"; @@ -802,58 +794,16 @@ export async function runCodexAppServerAttempt( timeoutMs: params.timeoutMs, timeoutFloorMs: options.startupTimeoutFloorMs, }); - const nativeHookRelayPluginThreadConfigRequired = - !nativeToolSurfaceEnabled || shouldBuildCodexPluginThreadConfig(pluginConfig); - const nativeHookRelayPluginThreadConfigPluginConfig = nativeToolSurfaceEnabled - ? pluginConfig - : disableCodexPluginThreadConfig(pluginConfig); - const nativeHookRelayPluginAppCacheKey = nativeHookRelayPluginThreadConfigRequired - ? buildCodexPluginAppCacheKey({ - appServer, - agentDir, - authProfileId: startupAuthProfileId, - accountId: startupAuthAccountCacheKey, - envApiKeyFingerprint: startupEnvApiKeyCacheKey, - }) - : undefined; - const nativeHookRelayResolvedPluginPolicy = nativeHookRelayPluginThreadConfigRequired - ? resolveCodexPluginsPolicy(nativeHookRelayPluginThreadConfigPluginConfig) - : undefined; - const nativeHookRelayBindingReuse = resolveCodexNativeHookRelayBindingReuse({ - binding: startupBinding, - attemptParams: buildActiveRunAttemptParams(), - agentId: sessionAgentId, - dynamicTools: toolBridge.specs, - nativeCodeModeEnabled: nativeToolSurfaceEnabled, - userMcpServersEnabled: nativeToolSurfaceEnabled, - mcpServersFingerprint: bundleMcpThreadConfig.fingerprint, - mcpServersFingerprintEvaluated: bundleMcpThreadConfig.evaluated, - environmentSelection: undefined, - contextEngineProjection, - pluginThreadConfig: nativeHookRelayPluginThreadConfigRequired - ? { - enabled: true, - inputFingerprint: buildCodexPluginThreadConfigInputFingerprint({ - pluginConfig: nativeHookRelayPluginThreadConfigPluginConfig, - appCacheKey: nativeHookRelayPluginAppCacheKey!, - }), - enabledPluginConfigKeys: nativeHookRelayResolvedPluginPolicy?.pluginPolicies - .filter((plugin) => plugin.enabled) - .map((plugin) => plugin.configKey) - .toSorted(), - } - : undefined, - }); - try { - emitCodexAppServerEvent(params, { - stream: "codex_app_server.lifecycle", - data: { phase: "startup" }, - }); + const buildNativeHookRelayFinalConfigPatch = ( + decision: { action: "resume"; binding: CodexAppServerThreadBinding } | { action: "start" }, + ) => { + nativeHookRelay?.unregister(); nativeHookRelay = createCodexNativeHookRelay({ options: options.nativeHookRelay, - generation: nativeHookRelayBindingReuse.generation, + generation: + decision.action === "resume" ? decision.binding.nativeHookRelayGeneration : undefined, generationMismatchGraceMs: - nativeHookRelayBindingReuse.canReuseBinding && !nativeHookRelayBindingReuse.generation + decision.action === "resume" && !decision.binding.nativeHookRelayGeneration ? CODEX_NATIVE_HOOK_RELAY_TTL_GRACE_MS : undefined, events: nativeHookRelayEvents, @@ -868,15 +818,24 @@ export async function runCodexAppServerAttempt( turnStartTimeoutMs: params.timeoutMs, signal: runAbortController.signal, }); - const nativeHookRelayConfig = nativeHookRelay - ? buildCodexNativeHookRelayConfig({ - relay: nativeHookRelay, - events: nativeHookRelayEvents, - hookTimeoutSec: options.nativeHookRelay?.hookTimeoutSec, - }) - : options.nativeHookRelay?.enabled === false - ? buildCodexNativeHookRelayDisabledConfig() - : undefined; + return { + configPatch: nativeHookRelay + ? buildCodexNativeHookRelayConfig({ + relay: nativeHookRelay, + events: nativeHookRelayEvents, + hookTimeoutSec: options.nativeHookRelay?.hookTimeoutSec, + }) + : options.nativeHookRelay?.enabled === false + ? buildCodexNativeHookRelayDisabledConfig() + : undefined, + nativeHookRelayGeneration: nativeHookRelay?.generation, + }; + }; + try { + emitCodexAppServerEvent(params, { + stream: "codex_app_server.lifecycle", + data: { phase: "startup" }, + }); const startupResult = await startCodexAttemptThread({ attemptClientFactory, appServer, @@ -892,8 +851,7 @@ export async function runCodexAppServerAttempt( effectiveWorkspace, dynamicTools: toolBridge.specs, developerInstructions: promptBuild.developerInstructions, - finalConfigPatch: nativeHookRelayConfig, - nativeHookRelayGeneration: nativeHookRelay?.generation, + buildFinalConfigPatch: buildNativeHookRelayFinalConfigPatch, bundleMcpThreadConfig, nativeToolSurfaceEnabled, sandboxExecServerEnabled, diff --git a/extensions/codex/src/app-server/thread-lifecycle.ts b/extensions/codex/src/app-server/thread-lifecycle.ts index cf4fa6051bb..3c4b6c90ba7 100644 --- a/extensions/codex/src/app-server/thread-lifecycle.ts +++ b/extensions/codex/src/app-server/thread-lifecycle.ts @@ -56,6 +56,15 @@ export type CodexAppServerThreadLifecycleBinding = CodexAppServerThreadBinding & lifecycle: CodexAppServerThreadLifecycle; }; +export type CodexThreadFinalConfigPatchDecision = + | { action: "resume"; binding: CodexAppServerThreadBinding } + | { action: "start" }; + +export type CodexThreadFinalConfigPatchResult = { + configPatch?: JsonObject; + nativeHookRelayGeneration?: string; +}; + export type CodexContextEngineThreadBootstrapProjection = { mode: "thread_bootstrap"; epoch: string; @@ -69,11 +78,6 @@ export type CodexPluginThreadConfigProvider = { build: () => Promise; }; -export type CodexNativeHookRelayBindingReuseDecision = { - canReuseBinding: boolean; - generation?: string; -}; - export const CODEX_NATIVE_PERSONALITY_NONE = "none"; export const CODEX_CODE_MODE_THREAD_CONFIG: JsonObject = { @@ -207,6 +211,9 @@ export async function startOrResumeThread(params: { developerInstructions?: string; config?: JsonObject; finalConfigPatch?: JsonObject; + buildFinalConfigPatch?: ( + decision: CodexThreadFinalConfigPatchDecision, + ) => CodexThreadFinalConfigPatchResult; nativeHookRelayGeneration?: string; nativeCodeModeEnabled?: boolean; nativeCodeModeOnlyEnabled?: boolean; @@ -393,10 +400,17 @@ export async function startOrResumeThread(params: { } else { try { const authProfileId = params.params.authProfileId ?? binding.authProfileId; + const finalConfigPatch = params.buildFinalConfigPatch?.({ + action: "resume", + binding, + }) ?? { + configPatch: params.finalConfigPatch, + nativeHookRelayGeneration: params.nativeHookRelayGeneration, + }; const resumeConfig = mergeCodexThreadConfigs( params.config, userMcpServersConfigPatch, - params.finalConfigPatch, + finalConfigPatch.configPatch, ); const resumeParams = lifecycleTiming.measureSync("thread_resume_params", () => buildThreadResumeParams(params.params, { @@ -440,7 +454,7 @@ export async function startOrResumeThread(params: { userMcpServersFingerprint, mcpServersFingerprint: nextMcpServersFingerprint, nativeHookRelayGeneration: - params.nativeHookRelayGeneration ?? binding.nativeHookRelayGeneration, + finalConfigPatch.nativeHookRelayGeneration ?? binding.nativeHookRelayGeneration, pluginAppsFingerprint: binding.pluginAppsFingerprint, pluginAppsInputFingerprint: binding.pluginAppsInputFingerprint, pluginAppPolicyContext: binding.pluginAppPolicyContext, @@ -484,7 +498,7 @@ export async function startOrResumeThread(params: { userMcpServersFingerprint, mcpServersFingerprint: nextMcpServersFingerprint, nativeHookRelayGeneration: - params.nativeHookRelayGeneration ?? binding.nativeHookRelayGeneration, + finalConfigPatch.nativeHookRelayGeneration ?? binding.nativeHookRelayGeneration, pluginAppsFingerprint: binding.pluginAppsFingerprint, pluginAppsInputFingerprint: binding.pluginAppsInputFingerprint, pluginAppPolicyContext: binding.pluginAppPolicyContext, @@ -510,12 +524,16 @@ export async function startOrResumeThread(params: { params.pluginThreadConfig?.build(), ))) : undefined; + const finalConfigPatch = params.buildFinalConfigPatch?.({ action: "start" }) ?? { + configPatch: params.finalConfigPatch, + nativeHookRelayGeneration: params.nativeHookRelayGeneration, + }; const config = lifecycleTiming.measureSync("merge_thread_config", () => mergeCodexThreadConfigs( params.config, userMcpServersConfigPatch, pluginThreadConfig?.configPatch, - params.finalConfigPatch, + finalConfigPatch.configPatch, ), ); const startParams = lifecycleTiming.measureSync("thread_start_params", () => @@ -558,7 +576,7 @@ export async function startOrResumeThread(params: { dynamicToolsFingerprint, userMcpServersFingerprint, mcpServersFingerprint: nextMcpServersFingerprint, - nativeHookRelayGeneration: params.nativeHookRelayGeneration, + nativeHookRelayGeneration: finalConfigPatch.nativeHookRelayGeneration, pluginAppsFingerprint: pluginThreadConfig?.fingerprint, pluginAppsInputFingerprint: pluginThreadConfig?.inputFingerprint, pluginAppPolicyContext: pluginThreadConfig?.policyContext, @@ -603,7 +621,7 @@ export async function startOrResumeThread(params: { dynamicToolsFingerprint, userMcpServersFingerprint, mcpServersFingerprint: nextMcpServersFingerprint, - nativeHookRelayGeneration: params.nativeHookRelayGeneration, + nativeHookRelayGeneration: finalConfigPatch.nativeHookRelayGeneration, pluginAppsFingerprint: pluginThreadConfig?.fingerprint, pluginAppsInputFingerprint: pluginThreadConfig?.inputFingerprint, pluginAppPolicyContext: pluginThreadConfig?.policyContext, @@ -618,94 +636,6 @@ export async function startOrResumeThread(params: { }; } -export function resolveCodexNativeHookRelayBindingReuse(params: { - binding: CodexAppServerThreadBinding | undefined; - attemptParams: EmbeddedRunAttemptParams; - agentId?: string; - dynamicTools: CodexDynamicToolSpec[]; - nativeCodeModeEnabled?: boolean; - userMcpServersEnabled?: boolean; - mcpServersFingerprint?: string; - mcpServersFingerprintEvaluated?: boolean; - environmentSelection?: CodexTurnEnvironmentParams[]; - pluginThreadConfig?: Pick< - CodexPluginThreadConfigProvider, - "enabled" | "inputFingerprint" | "enabledPluginConfigKeys" - >; - contextEngineProjection?: CodexContextEngineThreadBootstrapProjection; -}): CodexNativeHookRelayBindingReuseDecision { - const binding = params.binding; - if (!binding?.threadId || params.nativeCodeModeEnabled === false) { - return { canReuseBinding: false }; - } - - const contextEngineBinding = buildContextEngineBinding( - params.attemptParams, - params.contextEngineProjection, - ); - if (binding.contextEngine || contextEngineBinding) { - if ( - !contextEngineBinding || - !isContextEngineBindingCompatible(binding.contextEngine, contextEngineBinding) - ) { - return { canReuseBinding: false }; - } - } - - const userMcpServersConfigPatch = - params.userMcpServersEnabled === false - ? undefined - : buildCodexUserMcpServersThreadConfigPatch(params.attemptParams.config, { - agentId: params.agentId ?? params.attemptParams.agentId, - }); - if ( - binding.userMcpServersFingerprint !== - fingerprintUserMcpServersConfigPatch(userMcpServersConfigPatch) - ) { - return { canReuseBinding: false }; - } - - if ( - binding.environmentSelectionFingerprint !== - fingerprintEnvironmentSelection(params.environmentSelection) - ) { - return { canReuseBinding: false }; - } - - if ( - params.mcpServersFingerprintEvaluated === true && - binding.mcpServersFingerprint !== params.mcpServersFingerprint - ) { - return { canReuseBinding: false }; - } - - if ( - isCodexPluginThreadBindingStale({ - codexPluginsEnabled: params.pluginThreadConfig?.enabled ?? false, - bindingFingerprint: binding.pluginAppsFingerprint, - bindingInputFingerprint: binding.pluginAppsInputFingerprint, - currentInputFingerprint: params.pluginThreadConfig?.inputFingerprint, - hasBindingPolicyContext: Boolean(binding.pluginAppPolicyContext), - }) || - shouldRecheckRecoverablePluginBinding({ - binding, - pluginThreadConfig: params.pluginThreadConfig as CodexPluginThreadConfigProvider | undefined, - }) - ) { - return { canReuseBinding: false }; - } - - const dynamicToolsFingerprint = fingerprintDynamicTools(params.dynamicTools); - if ( - binding.dynamicToolsFingerprint && - !areDynamicToolFingerprintsCompatible(binding.dynamicToolsFingerprint, dynamicToolsFingerprint) - ) { - return { canReuseBinding: false }; - } - - return { canReuseBinding: true, generation: binding.nativeHookRelayGeneration }; -} - export function buildContextEngineBinding( params: EmbeddedRunAttemptParams, projection?: CodexContextEngineThreadBootstrapProjection,