diff --git a/packages/coding-agent/src/utils/shell.ts b/packages/coding-agent/src/utils/shell.ts index fbe81f965..9289d27e4 100644 --- a/packages/coding-agent/src/utils/shell.ts +++ b/packages/coding-agent/src/utils/shell.ts @@ -236,6 +236,19 @@ export function killProcessTree(pid: number): void { // Ignore errors if taskkill fails. } } else { + if (process.platform === "darwin") { + // macOS signals group members individually after releasing the group lock. A killed + // child can wake its waiting shell before that shell is signalled, letting `a; b` + // start b during cancellation (#11252). Stop the group first to reduce this race; + // this is best-effort, not an atomic freeze. SIGKILL also kills stopped processes. + // Linux holds tasklist_lock across group signalling; child exit notification needs + // its write lock, so the shell is signalled before observing the killed child's exit. + try { + process.kill(-pid, "SIGSTOP"); + } catch { + // A failed stop must not prevent the kill attempt or its single-process fallback. + } + } // Use SIGKILL on Unix/Linux/Mac try { process.kill(-pid, "SIGKILL"); diff --git a/packages/coding-agent/test/shell-kill-process-tree.test.ts b/packages/coding-agent/test/shell-kill-process-tree.test.ts new file mode 100644 index 000000000..8cc300053 --- /dev/null +++ b/packages/coding-agent/test/shell-kill-process-tree.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { killProcessTree } from "../src/utils/shell.ts"; + +const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + +// Regression coverage for #11252: stop-before-kill is macOS-only and best-effort. +describe("killProcessTree", () => { + beforeEach(() => { + Object.defineProperty(process, "platform", { configurable: true, value: "darwin" }); + vi.spyOn(process, "kill").mockReturnValue(true); + }); + + afterEach(() => { + vi.restoreAllMocks(); + if (platformDescriptor) Object.defineProperty(process, "platform", platformDescriptor); + }); + + it("stops the macOS process group before killing it without resuming it", () => { + killProcessTree(1234); + + expect(vi.mocked(process.kill).mock.calls).toEqual([ + [-1234, "SIGSTOP"], + [-1234, "SIGKILL"], + ]); + }); + + it("still kills the group when stopping fails", () => { + vi.mocked(process.kill).mockImplementationOnce(() => { + throw new Error("stop failed"); + }); + + killProcessTree(1234); + + expect(vi.mocked(process.kill).mock.calls).toEqual([ + [-1234, "SIGSTOP"], + [-1234, "SIGKILL"], + ]); + }); + + it("falls back to killing the shell when the group kill fails", () => { + vi.mocked(process.kill).mockImplementation((pid, signal) => { + if (pid < 0 && signal === "SIGKILL") throw new Error("group kill failed"); + return true; + }); + + killProcessTree(1234); + + expect(vi.mocked(process.kill).mock.calls).toEqual([ + [-1234, "SIGSTOP"], + [-1234, "SIGKILL"], + [1234, "SIGKILL"], + ]); + }); + + it("tolerates failure of both group signals and the fallback", () => { + vi.mocked(process.kill).mockImplementation(() => { + throw new Error("process no longer exists"); + }); + + expect(() => killProcessTree(1234)).not.toThrow(); + expect(vi.mocked(process.kill).mock.calls).toEqual([ + [-1234, "SIGSTOP"], + [-1234, "SIGKILL"], + [1234, "SIGKILL"], + ]); + }); + + it("keeps Linux group termination unchanged", () => { + Object.defineProperty(process, "platform", { configurable: true, value: "linux" }); + + killProcessTree(1234); + + expect(vi.mocked(process.kill).mock.calls).toEqual([[-1234, "SIGKILL"]]); + }); +});