diff --git a/src/agents/cli-runner.e2e.test.ts b/src/agents/cli-runner.e2e.test.ts index 8c5acd97d6..1383be1edb 100644 --- a/src/agents/cli-runner.e2e.test.ts +++ b/src/agents/cli-runner.e2e.test.ts @@ -36,7 +36,8 @@ describe("runCliAgent resume cleanup", () => { ].join("\n"), stderr: "", }) // cleanupResumeProcesses (ps) - .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (kill) + .mockResolvedValueOnce({ stdout: "", stderr: "" }) // cleanupResumeProcesses (kill -TERM) + .mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (kill -9) runCommandWithTimeoutMock.mockResolvedValueOnce({ stdout: "ok", stderr: "", @@ -62,19 +63,23 @@ describe("runCliAgent resume cleanup", () => { return; } - expect(runExecMock).toHaveBeenCalledTimes(3); + expect(runExecMock).toHaveBeenCalledTimes(4); // Second call: cleanupResumeProcesses ps const psCall = runExecMock.mock.calls[1] ?? []; expect(psCall[0]).toBe("ps"); - // Third call: kill with only the child PID - const killCall = runExecMock.mock.calls[2] ?? []; + // Third call: TERM, only the child PID + const termCall = runExecMock.mock.calls[2] ?? []; + expect(termCall[0]).toBe("kill"); + const termArgs = termCall[1] as string[]; + expect(termArgs).toEqual(["-TERM", String(selfPid + 1)]); + + // Fourth call: KILL, only the child PID + const killCall = runExecMock.mock.calls[3] ?? []; expect(killCall[0]).toBe("kill"); const killArgs = killCall[1] as string[]; - expect(killArgs[0]).toBe("-9"); - expect(killArgs[1]).toBe(String(selfPid + 1)); - expect(killArgs).toHaveLength(2); // only -9 + one PID + expect(killArgs).toEqual(["-9", String(selfPid + 1)]); }); it("falls back to per-agent workspace when workspaceDir is missing", async () => { @@ -318,6 +323,7 @@ describe("cleanupResumeProcesses", () => { ].join("\n"), stderr: "", }) + .mockResolvedValueOnce({ stdout: "", stderr: "" }) .mockResolvedValueOnce({ stdout: "", stderr: "" }); await cleanupResumeProcesses( @@ -333,8 +339,13 @@ describe("cleanupResumeProcesses", () => { return; } - expect(runExecMock).toHaveBeenCalledTimes(2); - const killCall = runExecMock.mock.calls[1] ?? []; + expect(runExecMock).toHaveBeenCalledTimes(3); + + const termCall = runExecMock.mock.calls[1] ?? []; + expect(termCall[0]).toBe("kill"); + expect(termCall[1]).toEqual(["-TERM", String(selfPid + 1)]); + + const killCall = runExecMock.mock.calls[2] ?? []; expect(killCall[0]).toBe("kill"); expect(killCall[1]).toEqual(["-9", String(selfPid + 1)]); }); diff --git a/src/agents/cli-runner/helpers.ts b/src/agents/cli-runner/helpers.ts index 5b7acd3560..95c703a9d8 100644 --- a/src/agents/cli-runner/helpers.ts +++ b/src/agents/cli-runner/helpers.ts @@ -18,6 +18,31 @@ import { buildAgentSystemPrompt } from "../system-prompt.js"; const CLI_RUN_QUEUE = new Map>(); +function buildLooseArgOrderRegex(tokens: string[]): RegExp { + // Scan `ps` output lines. Keep matching flexible, but require whitespace arg boundaries + // to avoid substring matches like `codexx` or `/path/to/codexx`. + const [head, ...rest] = tokens.map((t) => String(t ?? "").trim()).filter(Boolean); + if (!head) { + return /$^/; + } + + const headEscaped = escapeRegExp(head); + const headFragment = `(?:^|\\s)(?:${headEscaped}|\\S+\\/${headEscaped})(?=\\s|$)`; + const restFragments = rest.map((t) => `(?:^|\\s)${escapeRegExp(t)}(?=\\s|$)`); + return new RegExp([headFragment, ...restFragments].join(".*")); +} + +async function psWithFallback(argsA: string[], argsB: string[]): Promise { + try { + const { stdout } = await runExec("ps", argsA); + return stdout; + } catch { + // fallthrough + } + const { stdout } = await runExec("ps", argsB); + return stdout; +} + export async function cleanupResumeProcesses( backend: CliBackendConfig, sessionId: string, @@ -47,9 +72,11 @@ export async function cleanupResumeProcesses( } try { - // Use wide output to reduce false negatives from argv truncation. - const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,command="]); - const patternRegex = new RegExp(pattern); + const stdout = await psWithFallback( + ["-axww", "-o", "pid=,ppid=,command="], + ["-ax", "-o", "pid=,ppid=,command="], + ); + const patternRegex = buildLooseArgOrderRegex([commandToken, ...resumeTokens]); const toKill: number[] = []; for (const line of stdout.split("\n")) { @@ -77,7 +104,18 @@ export async function cleanupResumeProcesses( } if (toKill.length > 0) { - await runExec("kill", ["-9", ...toKill.map((pid) => String(pid))]); + const pidArgs = toKill.map((pid) => String(pid)); + try { + await runExec("kill", ["-TERM", ...pidArgs]); + } catch { + // ignore + } + await new Promise((resolve) => setTimeout(resolve, 250)); + try { + await runExec("kill", ["-9", ...pidArgs]); + } catch { + // ignore + } } } catch { // ignore errors - best effort cleanup @@ -146,8 +184,10 @@ export async function cleanupSuspendedCliProcesses( } try { - // Use wide output to reduce false negatives from argv truncation. - const { stdout } = await runExec("ps", ["-axww", "-o", "pid=,ppid=,stat=,command="]); + const stdout = await psWithFallback( + ["-axww", "-o", "pid=,ppid=,stat=,command="], + ["-ax", "-o", "pid=,ppid=,stat=,command="], + ); const suspended: number[] = []; for (const line of stdout.split("\n")) { const trimmed = line.trim();