diff --git a/codex-cli/tests/raw-exec-process-group.test.ts b/codex-cli/tests/raw-exec-process-group.test.ts index a7515946c..8dfc28212 100644 --- a/codex-cli/tests/raw-exec-process-group.test.ts +++ b/codex-cli/tests/raw-exec-process-group.test.ts @@ -6,7 +6,6 @@ import { exec as rawExec } from "../src/utils/agent/sandbox/raw-exec.js"; // the direct child. The original logic only sent `SIGTERM` to the immediate // child which meant that grandchildren (for instance when running through a // `bash -c` wrapper) were left running and turned into "zombie" processes. - // Strategy: // 1. Start a Bash shell that spawns a long‑running `sleep`, prints the PID // of that `sleep`, and then waits forever. This guarantees we can later @@ -15,7 +14,6 @@ import { exec as rawExec } from "../src/utils/agent/sandbox/raw-exec.js"; // 3. After `rawExec()` resolves we probe the previously printed PID with // `process.kill(pid, 0)`. If the call throws `ESRCH` the process no // longer exists – the desired outcome. Otherwise the test fails. - // The negative‑PID process‑group trick employed by the fixed implementation is // POSIX‑only. On Windows we skip the test. @@ -26,49 +24,59 @@ describe("rawExec – abort kills entire process group", () => { } const abortController = new AbortController(); - // Bash script: spawn `sleep 30` in background, print its PID, then wait. const script = "sleep 30 & pid=$!; echo $pid; wait $pid"; const cmd = ["bash", "-c", script]; - // Kick off the command. - const execPromise = rawExec(cmd, {}, [], abortController.signal); + // Start a bash shell that: + // - spawns a background `sleep 30` + // - prints the PID of the `sleep` + // - waits for `sleep` to exit + const { stdout, exitCode } = await (async () => { + const p = rawExec(cmd, {}, [], abortController.signal); - // Give Bash a tiny bit of time to start and print the PID. - await new Promise((r) => setTimeout(r, 100)); + // Give Bash a tiny bit of time to start and print the PID. + await new Promise((r) => setTimeout(r, 100)); - // Cancel the task – this should kill *both* bash and the inner sleep. - abortController.abort(); + // Cancel the task – this should kill *both* bash and the inner sleep. + abortController.abort(); - const { exitCode, stdout } = await execPromise; + // Wait for rawExec to resolve after aborting + return p; + })(); // We expect a non‑zero exit code because the process was killed. expect(exitCode).not.toBe(0); - // Attempt to extract the grand‑child PID from stdout. - const pidMatch = /^(\d+)/.exec(stdout.trim()); - - if (pidMatch) { - const sleepPid = Number(pidMatch[1]); - - // Verify that the sleep process is no longer alive. - let alive = true; - try { - process.kill(sleepPid, 0); - } catch (error: any) { - // Check if error is ESRCH (No such process) - if (error.code === "ESRCH") { - alive = false; // Process is dead, as expected. - } else { - throw error; - } - } - expect(alive).toBe(false); - } else { - // If PID was not printed, it implies bash was killed very early. - // The test passes implicitly in this scenario as the abort mechanism - // successfully stopped the command execution quickly. - expect(true).toBe(true); + // Extract the PID of the sleep process that bash printed + const pid = Number(stdout.trim().match(/^\d+/)?.[0]); + if (pid) { + // Confirm that the sleep process is no longer alive + await ensureProcessGone(pid); } }); }); + +/** + * Waits until a process no longer exists, or throws after timeout. + * @param pid - The process ID to check + * @throws {Error} If the process is still alive after 500ms + */ +async function ensureProcessGone(pid: number) { + const timeout = 500; + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + try { + process.kill(pid, 0); // check if process still exists + await new Promise((r) => setTimeout(r, 50)); // wait and retry + } catch (e: any) { + if (e.code === "ESRCH") { + return; // process is gone — success + } + throw e; // unexpected error — rethrow + } + } + throw new Error( + `Process with PID ${pid} failed to terminate within ${timeout}ms`, + ); +}