fix(server): reject pending Codex app-server requests on dispose (#4283)

The provider refresh deadline aborts a Codex catalog probe by disposing
the app-server client, but dispose killed the child without settling
in-flight requests. With the request fallback timeout at 14 days, an
unanswered `initialize` left `fetchCatalog` pending forever, so the
workspace snapshot entry stayed "loading" and the model picker showed
"Loading" for every Codex profile with no timeout ever surfacing.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Ivan Malison
2026-09-04 00:50:26 +02:00
committed by GitHub
co-authored by Claude Fable 5.1
parent 75eb9d44a3
commit 92442e7435
3 changed files with 48 additions and 6 deletions
@@ -1,7 +1,9 @@
import { describe, expect, test } from "vitest";
import { describe, expect, test, vi } from "vitest";
import { CodexAppServerAgentClient } from "./codex-app-server-agent.js";
import { runProviderRefreshWithDeadline } from "../provider-refresh-deadline.js";
import { createTestLogger } from "../../../test-utils/test-logger.js";
import { createCodexAppServerChildProcess } from "./codex/test-utils/fake-app-server.js";
describe("CodexAppServerAgentClient spawn error handling", () => {
const logger = createTestLogger();
@@ -32,6 +34,31 @@ describe("CodexAppServerAgentClient spawn error handling", () => {
}
});
test("fetchCatalog settles when the refresh deadline aborts an unanswered initialize", async () => {
const client = new CodexAppServerAgentClient(logger, {
command: {
mode: "replace",
argv: ["/nonexistent/codex-binary-that-does-not-exist"],
},
});
const child = createCodexAppServerChildProcess();
const kill = vi.spyOn(child, "kill");
Object.assign(client, { spawnAppServer: async () => child });
await expect(
runProviderRefreshWithDeadline({
label: "Codex",
timeoutMs: 50,
operation: (context) =>
client.fetchCatalog(
{ scope: "workspace", cwd: "/tmp/codex-models", force: false },
context,
),
}),
).rejects.toThrow("Timed out refreshing Codex after 50ms");
expect(kill).toHaveBeenCalled();
});
test("listImportableSessions rejects gracefully when the codex binary does not exist", async () => {
const client = new CodexAppServerAgentClient(logger, {
command: {
@@ -22,6 +22,16 @@ describe("Codex app-server transport", () => {
child.stdin.end();
});
test("dispose rejects pending requests instead of leaving them hanging", async () => {
const child = createCodexAppServerChildProcess();
const client = new CodexAppServerClient(child, createTestLogger());
const request = client.request("initialize", {});
await client.dispose();
await expect(request).rejects.toThrow("Codex app-server client is closed");
});
test.each([
"item/commandExecution/requestApproval",
"item/fileChange/requestApproval",
@@ -261,6 +261,7 @@ export class CodexAppServerClient {
this.disposed = true;
this.unexpectedTerminationHandler = null;
this.rl.close();
this.rejectPending(new Error("Codex app-server client is closed"));
try {
this.child.stdin.end();
} catch {
@@ -290,11 +291,7 @@ export class CodexAppServerClient {
}
this.disposed = true;
this.rl.close();
for (const pending of this.pending.values()) {
clearTimeout(pending.timer);
pending.reject(error);
}
this.pending.clear();
this.rejectPending(error);
const handler = this.unexpectedTerminationHandler;
this.unexpectedTerminationHandler = null;
if (!handler) {
@@ -307,6 +304,14 @@ export class CodexAppServerClient {
}
}
private rejectPending(error: Error): void {
for (const pending of this.pending.values()) {
clearTimeout(pending.timer);
pending.reject(error);
}
this.pending.clear();
}
private writeJsonRpcResponse(response: JsonRpcResponse): void {
if (this.disposed || this.child.stdin.destroyed || !this.child.stdin.writable) {
return;