From a2542cc2dae84b78ee61ad0c8eea5f26e2afe4c0 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Wed, 30 Sep 2026 08:25:55 -0400 Subject: [PATCH 1/2] test: reproduce leaked Codex initialization processes --- agent-chat/test/codex-initialize.test.ts | 77 ++++++++++++++++++++++++ agent-chat/test/fake-codex-initialize.ts | 32 ++++++++++ 2 files changed, 109 insertions(+) create mode 100644 agent-chat/test/codex-initialize.test.ts create mode 100644 agent-chat/test/fake-codex-initialize.ts diff --git a/agent-chat/test/codex-initialize.test.ts b/agent-chat/test/codex-initialize.test.ts new file mode 100644 index 000000000000..59e73f93e1e4 --- /dev/null +++ b/agent-chat/test/codex-initialize.test.ts @@ -0,0 +1,77 @@ +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { codexAdapter } from "../adapters/codex"; + +const directory = mkdtempSync(join(import.meta.dir, ".codex-initialize-")); +const originalSpawn = Bun.spawn; +// Select a real disposable child by absolute path. Do not depend on PATH +// lookup, which could resolve the installed Codex binary on a contributor Mac. +Bun.spawn = ((command: string[], options: Bun.SpawnOptions<"pipe", "pipe", "pipe">) => { + assert.deepEqual(command, ["codex", "app-server"]); + return originalSpawn([process.execPath, join(import.meta.dir, "fake-codex-initialize.ts")], { + ...options, + env: { ...options.env, CMUX_CODEX_INIT_TEST_DIRECTORY: directory }, + }); +}) as typeof Bun.spawn; + +function processes(): { pid: number; mode: string }[] { + const journal = join(directory, "processes.jsonl"); + return existsSync(journal) ? readFileSync(journal, "utf8").trim().split("\n").filter(Boolean).map((line) => JSON.parse(line)) : []; +} + +function alive(pid: number): boolean { + try { process.kill(pid, 0); return true; } + catch (error) { + if ((error as NodeJS.ErrnoException).code === "ESRCH") return false; + throw error; + } +} + +try { + assert.ok(codexAdapter.listOptions); + writeFileSync(join(directory, "mode"), "reject"); + for (let attempt = 1; attempt <= 2; attempt++) { + // Concurrent callers must share the same failed startup, and a later retry + // must get a new startup rather than a cached rejected promise. + const results = await Promise.allSettled([codexAdapter.listOptions(), codexAdapter.listOptions()]); + for (const result of results) { + assert.equal(result.status, "rejected"); + if (result.status === "rejected") assert.match(String(result.reason), /fixture initialization rejected/); + } + const children = processes(); + assert.equal(children.length, attempt, "concurrent initialization must remain single-flight"); + for (const child of children) { + assert.equal(alive(child.pid), false, "failed initialization must reap its child before returning the error"); + } + } + + writeFileSync(join(directory, "mode"), "hang"); + await assert.rejects(codexAdapter.listOptions(), /codex app-server did not initialize within 30s/); + assert.equal(processes().length, 3); + assert.ok(processes().every((child) => !alive(child.pid)), "timed-out initialization must reap its uncooperative child"); + + writeFileSync(join(directory, "mode"), "accept"); + const options = await codexAdapter.listOptions(); + assert.ok(options.find((option) => option.id === "model")?.choices?.some((choice) => choice.value === "fixture-model")); + const children = processes(); + assert.equal(children.length, 4, "a later successful retry must create exactly one replacement server"); + assert.equal(children[3]!.mode, "accept"); + assert.equal(alive(children[3]!.pid), true, "successful initialization must keep its server running"); + await codexAdapter.listOptions(); + assert.equal(processes().length, 4, "a successfully initialized server should be reused"); + console.log("Codex initialization rejection/timeout cleanup, single-flight and successful retry: OK"); +} finally { + const children = processes(); + // Only PIDs recorded by this disposable fixture are eligible for cleanup. + for (const child of children) { + if (alive(child.pid)) process.kill(child.pid, "SIGKILL"); + } + const deadline = Date.now() + 2_000; + while (children.some((child) => alive(child.pid)) && Date.now() < deadline) { + await Bun.sleep(10); + } + Bun.spawn = originalSpawn; + rmSync(directory, { recursive: true, force: true }); + assert.ok(children.every((child) => !alive(child.pid)), "the fixture must leave no child processes behind"); +} diff --git a/agent-chat/test/fake-codex-initialize.ts b/agent-chat/test/fake-codex-initialize.ts new file mode 100644 index 000000000000..9a597aabeb9a --- /dev/null +++ b/agent-chat/test/fake-codex-initialize.ts @@ -0,0 +1,32 @@ +import { appendFileSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { readLines } from "../adapters/lines"; + +const directory = process.env.CMUX_CODEX_INIT_TEST_DIRECTORY; +if (!directory) throw new Error("missing initialization fixture directory"); +const mode = readFileSync(join(directory, "mode"), "utf8").trim(); +appendFileSync(join(directory, "processes.jsonl"), JSON.stringify({ pid: process.pid, mode }) + "\n"); + +// A rejected initialization need not make an app-server exit voluntarily. +// Ignore a graceful signal too, so cleanup cannot hang waiting for cooperation. +process.on("SIGTERM", () => {}); +const keepAlive = setInterval(() => {}, 1_000); +await readLines(Bun.stdin.stream(), (line) => { + const request = JSON.parse(line); + if (request.id === undefined) return; + if (request.method === "initialize" && mode === "hang") return; + let result: unknown = {}; + if (request.method === "initialize" && mode === "reject") { + console.log(JSON.stringify({ jsonrpc: "2.0", id: request.id, + error: { code: -32603, message: "fixture initialization rejected" } })); + return; + } + if (request.method === "model/list") { + result = { data: [{ id: "fixture-model", model: "fixture-model", displayName: "Fixture model" }], nextCursor: null }; + } + if (request.method === "collaborationMode/list") { + result = { data: [{ mode: "default", name: "Default" }] }; + } + console.log(JSON.stringify({ jsonrpc: "2.0", id: request.id, result })); +}); +clearInterval(keepAlive); From 2851bb55df50b33e4bbe1015c1da92ca029a2fd0 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Wed, 30 Sep 2026 08:27:58 -0400 Subject: [PATCH 2/2] fix: reap Codex app-server after failed initialization --- agent-chat/adapters/codex.ts | 7 ++++++- agent-chat/test/codex-initialize.test.ts | 10 +++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/agent-chat/adapters/codex.ts b/agent-chat/adapters/codex.ts index 6cc494701b5d..c3552b5094cb 100644 --- a/agent-chat/adapters/codex.ts +++ b/agent-chat/adapters/codex.ts @@ -304,7 +304,7 @@ async function startServer(): Promise { let initTimedOut = false; const initTimer = setTimeout(() => { initTimedOut = true; - proc.kill(); + proc.kill("SIGKILL"); }, 30_000); try { await request("initialize", { @@ -312,6 +312,11 @@ async function startServer(): Promise { capabilities: { experimentalApi: true, requestAttestation: false }, }); } catch (err) { + // This server has not been published to shared. Reap it before allowing + // another startup; even a rejected initialize can leave the child alive. + clearTimeout(initTimer); + if (proc.exitCode === null && !proc.killed) proc.kill("SIGKILL"); + await proc.exited; throw initTimedOut ? new Error("codex app-server did not initialize within 30s") : err; } finally { clearTimeout(initTimer); diff --git a/agent-chat/test/codex-initialize.test.ts b/agent-chat/test/codex-initialize.test.ts index 59e73f93e1e4..4caf148340e8 100644 --- a/agent-chat/test/codex-initialize.test.ts +++ b/agent-chat/test/codex-initialize.test.ts @@ -7,7 +7,7 @@ const directory = mkdtempSync(join(import.meta.dir, ".codex-initialize-")); const originalSpawn = Bun.spawn; // Select a real disposable child by absolute path. Do not depend on PATH // lookup, which could resolve the installed Codex binary on a contributor Mac. -Bun.spawn = ((command: string[], options: Bun.SpawnOptions<"pipe", "pipe", "pipe">) => { +Bun.spawn = ((command: string[], options: Bun.SpawnOptions.SpawnOptions<"pipe", "pipe", "pipe">) => { assert.deepEqual(command, ["codex", "app-server"]); return originalSpawn([process.execPath, join(import.meta.dir, "fake-codex-initialize.ts")], { ...options, @@ -34,7 +34,7 @@ try { for (let attempt = 1; attempt <= 2; attempt++) { // Concurrent callers must share the same failed startup, and a later retry // must get a new startup rather than a cached rejected promise. - const results = await Promise.allSettled([codexAdapter.listOptions(), codexAdapter.listOptions()]); + const results = await Promise.allSettled([codexAdapter.listOptions(directory), codexAdapter.listOptions(directory)]); for (const result of results) { assert.equal(result.status, "rejected"); if (result.status === "rejected") assert.match(String(result.reason), /fixture initialization rejected/); @@ -47,18 +47,18 @@ try { } writeFileSync(join(directory, "mode"), "hang"); - await assert.rejects(codexAdapter.listOptions(), /codex app-server did not initialize within 30s/); + await assert.rejects(codexAdapter.listOptions(directory), /codex app-server did not initialize within 30s/); assert.equal(processes().length, 3); assert.ok(processes().every((child) => !alive(child.pid)), "timed-out initialization must reap its uncooperative child"); writeFileSync(join(directory, "mode"), "accept"); - const options = await codexAdapter.listOptions(); + const options = await codexAdapter.listOptions(directory); assert.ok(options.find((option) => option.id === "model")?.choices?.some((choice) => choice.value === "fixture-model")); const children = processes(); assert.equal(children.length, 4, "a later successful retry must create exactly one replacement server"); assert.equal(children[3]!.mode, "accept"); assert.equal(alive(children[3]!.pid), true, "successful initialization must keep its server running"); - await codexAdapter.listOptions(); + await codexAdapter.listOptions(directory); assert.equal(processes().length, 4, "a successfully initialized server should be reused"); console.log("Codex initialization rejection/timeout cleanup, single-flight and successful retry: OK"); } finally {