fix(codex): preserve app-server close diagnostics (#117296)

This commit is contained in:
Vincent Koc
2026-08-01 17:01:42 +08:00
committed by GitHub
parent 0e4fa090e2
commit 71a09dcfe3
9 changed files with 58 additions and 20 deletions

View File

@@ -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;

View File

@@ -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,

View File

@@ -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;

View File

@@ -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> | void
>();
const serverRequestHandlers = new Set<AppServerRequestHandler>();
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);
}
},
};

View File

@@ -59,6 +59,7 @@ export function createCodexAttemptTurnState(resources: CodexAttemptResources) {
turnWatchTimeoutDetails: undefined as Record<string, unknown> | undefined,
turnCompletionIdleTimeoutMessage: undefined as string | undefined,
clientClosedPromptError: undefined as string | undefined,
clientClosedDiagnostic: undefined as string | undefined,
clientClosedAbort: false,
shouldDelayNativeHookRelayUnregister: false,
lifecycleStarted: false,

View File

@@ -3745,7 +3745,9 @@ describe("runCodexAppServerAttempt turn watches", () => {
await new Promise<void>((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"',
},
});
});

View File

@@ -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");

View File

@@ -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) {

View File

@@ -222,6 +222,7 @@ export type EmbeddedRunAttemptResult = {
| "potential_side_effect"
| "active_item";
diagnostics?: {
transportError?: string;
idleMs?: number;
timeoutMs?: number;
lastActivityReason?: string;