From 71a09dcfe368e60c6e67f3aabed95bf3d67ba8d4 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 1 Aug 2026 17:01:42 +0800 Subject: [PATCH] fix(codex): preserve app-server close diagnostics (#117296) --- extensions/codex/src/app-server/client.ts | 5 +++++ .../src/app-server/run-attempt-finalize.ts | 21 +++++++++++-------- .../codex/src/app-server/run-attempt-route.ts | 7 +++++++ .../app-server/run-attempt-test-harness.ts | 13 +++++++----- .../src/app-server/run-attempt-turn-state.ts | 1 + .../run-attempt.turn-watches.test.ts | 8 ++++++- .../codex/src/app-server/turn-router.test.ts | 11 ++++++++-- .../codex/src/app-server/turn-router.ts | 11 +++++++--- src/agents/embedded-agent-runner/run/types.ts | 1 + 9 files changed, 58 insertions(+), 20 deletions(-) diff --git a/extensions/codex/src/app-server/client.ts b/extensions/codex/src/app-server/client.ts index 2c83958f0f1d..033d2cc87a20 100644 --- a/extensions/codex/src/app-server/client.ts +++ b/extensions/codex/src/app-server/client.ts @@ -358,6 +358,11 @@ export class CodexAppServerClient { return redactCodexAppServerLinePreview(this.stderrTail) || undefined; } + /** Returns the terminal transport error that closed this physical client. */ + getCloseError(): Error | undefined { + return this.closeError; + } + /** Stable generation id for this exact physical client instance. */ getInstanceId(): string { return this.instanceId; diff --git a/extensions/codex/src/app-server/run-attempt-finalize.ts b/extensions/codex/src/app-server/run-attempt-finalize.ts index cac1ee52c8c4..e4ab60414ea5 100644 --- a/extensions/codex/src/app-server/run-attempt-finalize.ts +++ b/extensions/codex/src/app-server/run-attempt-finalize.ts @@ -244,15 +244,18 @@ export async function finalizeCodexAttempt( turnWatchTimeoutKind: state.turnWatchTimeoutKind, }); const failureDiagnostics = - codexAppServerFailureKind === "turn_completion_idle_timeout" && - state.turnWatchTimeoutKind === "completion" - ? buildCodexAppServerTimeoutDiagnostics({ - idleMs: state.turnWatchTimeoutIdleMs, - timeoutMs: state.turnWatchTimeoutMs, - lastActivityReason: state.turnWatchTimeoutLastActivityReason, - details: state.turnWatchTimeoutDetails, - }) - : undefined; + codexAppServerFailureKind === "client_closed_before_turn_completed" && + state.clientClosedDiagnostic + ? { transportError: state.clientClosedDiagnostic } + : codexAppServerFailureKind === "turn_completion_idle_timeout" && + state.turnWatchTimeoutKind === "completion" + ? buildCodexAppServerTimeoutDiagnostics({ + idleMs: state.turnWatchTimeoutIdleMs, + timeoutMs: state.turnWatchTimeoutMs, + lastActivityReason: state.turnWatchTimeoutLastActivityReason, + details: state.turnWatchTimeoutDetails, + }) + : undefined; const codexAppServerFailure = codexAppServerFailureKind ? ({ kind: codexAppServerFailureKind, diff --git a/extensions/codex/src/app-server/run-attempt-route.ts b/extensions/codex/src/app-server/run-attempt-route.ts index 866b836b744c..09b97c7340f4 100644 --- a/extensions/codex/src/app-server/run-attempt-route.ts +++ b/extensions/codex/src/app-server/run-attempt-route.ts @@ -38,9 +38,15 @@ export async function prepareCodexAttemptRoute( } const reasonText = formatErrorMessage(route.signal.reason); const closedClient = reasonText.includes("turn router closed"); + const closeCause = + route.signal.reason instanceof Error && route.signal.reason.cause instanceof Error + ? route.signal.reason.cause + : undefined; state.clientClosedPromptError = closedClient ? "codex app-server client closed before turn completed" : `codex app-server turn route closed before turn completed: ${reasonText}`; + state.clientClosedDiagnostic = + closedClient && closeCause ? formatErrorMessage(closeCause) : undefined; state.clientClosedAbort = closedClient; const activeTurnId = turnIdRef.current; if (activeTurnId) { @@ -52,6 +58,7 @@ export async function prepareCodexAttemptRoute( embeddedAgentLog.warn(state.clientClosedPromptError, { threadId: resourceState.thread.threadId, turnId: activeTurnId, + ...(state.clientClosedDiagnostic ? { transportError: state.clientClosedDiagnostic } : {}), }); runAbortController.abort(closedClient ? "client_closed" : "turn_route_closed"); state.completed = true; diff --git a/extensions/codex/src/app-server/run-attempt-test-harness.ts b/extensions/codex/src/app-server/run-attempt-test-harness.ts index 15ae8d534e04..05ef6a1a6581 100644 --- a/extensions/codex/src/app-server/run-attempt-test-harness.ts +++ b/extensions/codex/src/app-server/run-attempt-test-harness.ts @@ -85,7 +85,7 @@ export function setCodexAppServerClientFactoryForTest( codexAppServerClientFactoryForTest = adaptCodexTestClientFactory(async (...args) => { const client = await factory(...args); const testClient = client as unknown as { - addCloseHandler?: (handler: () => void) => () => void; + addCloseHandler?: (handler: (client: CodexAppServerClient) => void) => () => void; }; // Narrow test doubles still need the client lifecycle hook installed by // the keyed router, even when the test never simulates transport closure. @@ -413,7 +413,8 @@ export function createAppServerHarness( (notification: CodexServerNotification) => Promise | void >(); const serverRequestHandlers = new Set(); - const closeHandlers = new Set<() => void>(); + const closeHandlers = new Set<(client: CodexAppServerClient) => void>(); + let closeError: Error | undefined; const request = vi.fn(async (method: string, params?: unknown, requestOptions?: unknown) => { requests.push({ method, params }); const result = await requestImpl( @@ -455,10 +456,11 @@ export function createAppServerHarness( serverRequestHandlers.add(handler); return () => serverRequestHandlers.delete(handler); }, - addCloseHandler: (handler: () => void) => { + addCloseHandler: (handler: (client: CodexAppServerClient) => void) => { closeHandlers.add(handler); return () => closeHandlers.delete(handler); }, + getCloseError: () => closeError, } as unknown as CodexAppServerClient; setCodexAppServerClientFactoryForTest( async (_startOptions, authProfileId, agentDir, _config, clientOptions) => { @@ -543,9 +545,10 @@ export function createAppServerHarness( }, }); }, - close: () => { + close: (error?: Error) => { + closeError = error; for (const handler of closeHandlers) { - handler(); + handler(client); } }, }; diff --git a/extensions/codex/src/app-server/run-attempt-turn-state.ts b/extensions/codex/src/app-server/run-attempt-turn-state.ts index 780c67b43179..ff900e58b9e4 100644 --- a/extensions/codex/src/app-server/run-attempt-turn-state.ts +++ b/extensions/codex/src/app-server/run-attempt-turn-state.ts @@ -59,6 +59,7 @@ export function createCodexAttemptTurnState(resources: CodexAttemptResources) { turnWatchTimeoutDetails: undefined as Record | undefined, turnCompletionIdleTimeoutMessage: undefined as string | undefined, clientClosedPromptError: undefined as string | undefined, + clientClosedDiagnostic: undefined as string | undefined, clientClosedAbort: false, shouldDelayNativeHookRelayUnregister: false, lifecycleStarted: false, diff --git a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts index 2c3d52c68f75..64306a44e511 100644 --- a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts +++ b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts @@ -3745,7 +3745,9 @@ describe("runCodexAppServerAttempt turn watches", () => { await new Promise((resolve) => { setImmediate(resolve); }); - harness.close(); + harness.close( + new Error('codex app-server exited: code=137 signal=SIGKILL stderr="worker exhausted"'), + ); const result = await run; expect(readAttemptTerminal(result).promptError).toBe( @@ -3759,6 +3761,10 @@ describe("runCodexAppServerAttempt turn watches", () => { threadId: "thread-1", turnId: "turn-1", replaySafe: true, + diagnostics: { + transportError: + 'codex app-server exited: code=137 signal=SIGKILL stderr="worker exhausted"', + }, }); }); diff --git a/extensions/codex/src/app-server/turn-router.test.ts b/extensions/codex/src/app-server/turn-router.test.ts index 4673d83d43eb..10ae5b7df4d6 100644 --- a/extensions/codex/src/app-server/turn-router.test.ts +++ b/extensions/codex/src/app-server/turn-router.test.ts @@ -925,11 +925,18 @@ describe("CodexAppServerTurnRouter", () => { threadId: "thread-close", onRequest: requestHandler, }); - harness.client.close(); + harness.process.stderr.write("fatal transport detail\n"); + harness.process.emit("exit", 17, "SIGTERM"); await expect(closingRoute.bindTurn("turn-close")).rejects.toThrow("turn router closed"); expect(closingRoute.signal.aborted).toBe(true); - expect(closingRoute.signal.reason).toEqual(new Error("codex app-server turn router closed")); + expect(closingRoute.signal.reason).toEqual( + new Error("codex app-server turn router closed", { + cause: new Error( + 'codex app-server exited: code=17 signal=SIGTERM stderr="fatal transport detail"', + ), + }), + ); expect(() => router.reserveThread({ threadId: "thread-late", onRequest: requestHandler }), ).toThrow("turn router is closed"); diff --git a/extensions/codex/src/app-server/turn-router.ts b/extensions/codex/src/app-server/turn-router.ts index 7eca3c477450..da1321272c21 100644 --- a/extensions/codex/src/app-server/turn-router.ts +++ b/extensions/codex/src/app-server/turn-router.ts @@ -128,7 +128,9 @@ class ClientTurnRouter implements CodexAppServerTurnRouter { constructor(client: CodexAppServerClient) { client.addNotificationHandler((notification) => this.routeNotification(notification)); client.addRequestHandler((request, signal) => this.routeRequest(request, signal)); - client.addCloseHandler(() => this.dispose()); + client.addCloseHandler((closedClient) => { + this.dispose(closedClient.getCloseError()); + }); } reserveThread(options: RouteOptions): CodexThreadRouteReservation { @@ -209,13 +211,16 @@ class ClientTurnRouter implements CodexAppServerTurnRouter { return { completion, cancel: () => finish(false) }; } - private dispose(): void { + private dispose(cause?: Error): void { if (this.disposed) { return; } this.disposed = true; + const closeError = cause + ? new Error("codex app-server turn router closed", { cause }) + : new Error("codex app-server turn router closed"); for (const route of this.routes.values()) { - this.release(route, new Error("codex app-server turn router closed")); + this.release(route, closeError); } for (const watchers of this.nativeTurnCompletionWatchers.values()) { for (const watcher of watchers) { diff --git a/src/agents/embedded-agent-runner/run/types.ts b/src/agents/embedded-agent-runner/run/types.ts index 80d0938535b3..9213acee1ca8 100644 --- a/src/agents/embedded-agent-runner/run/types.ts +++ b/src/agents/embedded-agent-runner/run/types.ts @@ -222,6 +222,7 @@ export type EmbeddedRunAttemptResult = { | "potential_side_effect" | "active_item"; diagnostics?: { + transportError?: string; idleMs?: number; timeoutMs?: number; lastActivityReason?: string;