fix(coding-agent): prevent Windows taskkill spawn crashes

closes #6596
This commit is contained in:
Vegard Stikbakke
2026-08-26 10:33:45 +02:00
parent 8fa7eebd23
commit 7af2d27dce
6 changed files with 124 additions and 13 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed Windows `NodeExecutionEnv` aborts crashing when `taskkill.exe` is unavailable on `PATH` ([#6596](https://github.com/earendil-works/pi/issues/6596)).
## [0.84.3] - 2026-08-24
### Fixed
+11 -5
View File
@@ -253,11 +253,17 @@ function getShellEnv(
function killProcessTree(pid: number): void {
if (process.platform === "win32") {
try {
spawn("taskkill", ["/F", "/T", "/PID", String(pid)], {
stdio: "ignore",
detached: true,
windowsHide: true,
});
const child = spawn(
join(process.env.SystemRoot ?? "C:\\Windows", "System32", "taskkill.exe"),
["/F", "/T", "/PID", String(pid)],
{
stdio: "ignore",
detached: true,
windowsHide: true,
},
);
// A failed spawn emits "error" asynchronously; consume it to avoid crashing Node.
child.once("error", () => {});
} catch {
// Ignore errors.
}
@@ -496,6 +496,48 @@ describe("NodeExecutionEnv", () => {
if (!result.ok) expect(result.error).toMatchObject({ code: "aborted" });
});
it.skipIf(process.platform === "win32")("ignores asynchronous taskkill spawn errors during abort", async () => {
const root = createTempDir();
const pidFile = join(root, "shell.pid");
const controller = new AbortController();
const env = new NodeExecutionEnv({ cwd: root, shellPath: "/bin/bash" });
const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform");
const previousSystemRoot = process.env.SystemRoot;
process.env.SystemRoot = "/definitely/missing/windows";
Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
let pid: number | undefined;
try {
const execution = env.exec(`echo $$ > ${toBashSingleQuotedArg(pidFile)}; exec sleep 60`, {
abortSignal: controller.signal,
});
for (let attempt = 0; attempt < 100 && !existsSync(pidFile); attempt++) {
await new Promise((resolve) => setTimeout(resolve, 10));
}
expect(existsSync(pidFile)).toBe(true);
controller.abort();
await new Promise((resolve) => setTimeout(resolve, 0));
pid = Number.parseInt(readFileSync(pidFile, "utf8"), 10);
process.kill(pid, "SIGKILL");
const result = await execution;
expect(result).toMatchObject({ ok: false, error: { code: "aborted" } });
} finally {
if (pid === undefined && existsSync(pidFile)) {
pid = Number.parseInt(readFileSync(pidFile, "utf8"), 10);
}
if (pid !== undefined && Number.isFinite(pid)) {
try {
process.kill(pid, "SIGKILL");
} catch {}
}
if (previousSystemRoot === undefined) delete process.env.SystemRoot;
else process.env.SystemRoot = previousSystemRoot;
if (platformDescriptor) Object.defineProperty(process, "platform", platformDescriptor);
}
});
it("captures large shell output to a full output file through the execution env", async () => {
const root = createTempDir();
const env = new NodeExecutionEnv({ cwd: root });
+1
View File
@@ -8,6 +8,7 @@
### Fixed
- Fixed Windows shell aborts crashing Pi when `taskkill.exe` is unavailable on `PATH` ([#6596](https://github.com/earendil-works/pi/issues/6596)).
- Fixed extension messages sent with `triggerTurn: false` while the agent is running being inserted between a tool call and its result, which made providers that validate message order reject the replayed history. They are now appended once the turn's tool results are in ([#8537](https://github.com/earendil-works/pi/issues/8537)).
## [0.84.3] - 2026-08-24
+14 -8
View File
@@ -1,5 +1,5 @@
import { existsSync } from "node:fs";
import { delimiter } from "node:path";
import { delimiter, join } from "node:path";
import { spawn, spawnSync } from "child_process";
import { getBinDir } from "../config.ts";
@@ -215,15 +215,21 @@ export function killTrackedDetachedChildren(): void {
*/
export function killProcessTree(pid: number): void {
if (process.platform === "win32") {
// Use taskkill on Windows to kill process tree
// Use the trusted System32 executable so cleanup does not depend on PATH.
try {
spawn("taskkill", ["/F", "/T", "/PID", String(pid)], {
stdio: "ignore",
detached: true,
windowsHide: true,
});
const child = spawn(
join(process.env.SystemRoot ?? "C:\\Windows", "System32", "taskkill.exe"),
["/F", "/T", "/PID", String(pid)],
{
stdio: "ignore",
detached: true,
windowsHide: true,
},
);
// A failed spawn emits "error" asynchronously; consume it to avoid crashing Node.
child.once("error", () => {});
} catch {
// Ignore errors if taskkill fails
// Ignore errors if taskkill fails.
}
} else {
// Use SIGKILL on Unix/Linux/Mac
@@ -0,0 +1,52 @@
import type { ChildProcess } from "node:child_process";
import { EventEmitter } from "node:events";
import { join } from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }));
vi.mock("child_process", async (importOriginal) => {
const actual = await importOriginal<typeof import("child_process")>();
return { ...actual, spawn: spawnMock };
});
import { killProcessTree } from "../../../src/utils/shell.ts";
function withWindowsPlatform(test: () => void): void {
const platformDescriptor = Object.getOwnPropertyDescriptor(process, "platform");
try {
Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
test();
} finally {
if (platformDescriptor) Object.defineProperty(process, "platform", platformDescriptor);
}
}
afterEach(() => {
spawnMock.mockReset();
});
describe("issue #6596 taskkill spawn failures", () => {
it("uses System32 taskkill and consumes its asynchronous spawn error", () => {
const child = new EventEmitter() as ChildProcess;
const previousSystemRoot = process.env.SystemRoot;
process.env.SystemRoot = "C:\\CustomWindows";
spawnMock.mockReturnValue(child);
try {
withWindowsPlatform(() => {
killProcessTree(1234);
});
} finally {
if (previousSystemRoot === undefined) delete process.env.SystemRoot;
else process.env.SystemRoot = previousSystemRoot;
}
expect(spawnMock).toHaveBeenCalledWith(
join("C:\\CustomWindows", "System32", "taskkill.exe"),
["/F", "/T", "/PID", "1234"],
{ detached: true, stdio: "ignore", windowsHide: true },
);
expect(() => child.emit("error", new Error("spawn taskkill ENOENT"))).not.toThrow();
});
});