From 9a60624a82d43749a03536ef9f83d0324bbc6eb3 Mon Sep 17 00:00:00 2001 From: pj991207 Date: Tue, 15 Sep 2026 00:55:44 +0900 Subject: [PATCH] fix(windows): support background task cancellation --- package.json | 2 +- scripts/lib/claude-cli.mjs | 25 ++++++++++++++--- scripts/lib/process.mjs | 40 ++++++++++++++++++++------ tests/claude-cli.test.mjs | 25 +++++++++++++++++ tests/process.test.mjs | 57 ++++++++++++++++++++++++++++++++++---- 5 files changed, 130 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index bf8873c..6a8fd1e 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "prepack": "npm run check:version-sync && npm run check:changelog", "setup:git-hooks": "node scripts/setup-git-hooks.mjs", "test": "node --test tests/*.test.mjs", - "test:cross-platform": "node --test tests/args.test.mjs tests/changelog.test.mjs tests/claude-cli.test.mjs tests/fs.test.mjs tests/prompts.test.mjs tests/render.test.mjs tests/sandbox-modes.test.mjs tests/skills-contracts.test.mjs tests/structured-output.test.mjs tests/version-sync.test.mjs", + "test:cross-platform": "node --test tests/args.test.mjs tests/changelog.test.mjs tests/claude-cli.test.mjs tests/fs.test.mjs tests/process.test.mjs tests/prompts.test.mjs tests/render.test.mjs tests/sandbox-modes.test.mjs tests/skills-contracts.test.mjs tests/structured-output.test.mjs tests/version-sync.test.mjs", "test:integration": "node --test tests/integration/*.test.mjs", "test:e2e": "node --test tests/e2e/*.test.mjs", "uninstall:codex": "node scripts/installer-cli.mjs uninstall", diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 68ffdbb..66b56b7 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -14,7 +14,11 @@ import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { normalizePathSlashes, resolvePluginRuntimeRoot } from "./codex-paths.mjs"; -import { getProcessIdentity, validateProcessIdentity } from "./process.mjs"; +import { + getProcessIdentity, + terminateProcessTree, + validateProcessIdentity, +} from "./process.mjs"; const CLAUDE_PACKAGE_EXE_PARTS = [ "node_modules", @@ -927,15 +931,28 @@ export async function runClaudeAdversarialReview( * Cancel a running Claude Code process. * Uses process group kill with PID identity verification. */ -export async function cancelClaudeProcess(pid, pidIdentity) { +export async function cancelClaudeProcess(pid, pidIdentity, options = {}) { + const platform = options.platform ?? process.platform; + const validateProcessIdentityImpl = + options.validateProcessIdentityImpl ?? validateProcessIdentity; + // Verify PID identity to prevent killing recycled PIDs - if (pidIdentity && !validateProcessIdentity(pid, pidIdentity)) { + if (pidIdentity && !validateProcessIdentityImpl(pid, pidIdentity)) { return { cancelled: true, note: "Process already exited (PID recycled)", }; } + if (platform === "win32") { + const terminateProcessTreeImpl = + options.terminateProcessTreeImpl ?? terminateProcessTree; + const termination = terminateProcessTreeImpl(pid, { platform }); + return termination.delivered + ? { cancelled: true } + : { cancelled: true, note: "Process not found" }; + } + // SIGTERM to entire process group try { process.kill(-pid, "SIGTERM"); @@ -950,7 +967,7 @@ export async function cancelClaudeProcess(pid, pidIdentity) { } // Escalate to SIGKILL - if (pidIdentity && !validateProcessIdentity(pid, pidIdentity)) { + if (pidIdentity && !validateProcessIdentityImpl(pid, pidIdentity)) { return { cancelled: true, note: "Process exited during SIGTERM wait", diff --git a/scripts/lib/process.mjs b/scripts/lib/process.mjs index 448dbbf..6352545 100644 --- a/scripts/lib/process.mjs +++ b/scripts/lib/process.mjs @@ -143,16 +143,40 @@ export function formatCommandFailure(result) { * Get stable process identity for PID reuse detection. * Returns a string that is immutable for the process lifetime. */ -export function getProcessIdentity(pid) { - if (process.platform === 'darwin') { - const row = runCommandChecked('ps', ['-o', 'lstart=,comm=', '-p', String(pid)]); +export function getProcessIdentity(pid, options = {}) { + const platform = options.platform ?? process.platform; + + if (platform === "darwin") { + const row = runCommandChecked("ps", [ + "-o", + "lstart=,comm=", + "-p", + String(pid), + ]); return row.stdout.trim(); - } else { - const stat = readFileSync(`/proc/${pid}/stat`, 'utf8'); - const closeParen = stat.lastIndexOf(')'); - const fields = stat.slice(closeParen + 2).split(' '); - return fields[19]; // starttime field } + + if (platform === "win32") { + if (!Number.isSafeInteger(pid) || pid <= 0) { + throw new TypeError("PID must be a positive safe integer."); + } + const runCommandCheckedImpl = + options.runCommandCheckedImpl ?? runCommandChecked; + // Process start time is stable for the lifetime of a Windows PID. + const row = runCommandCheckedImpl("powershell.exe", [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + `[System.Diagnostics.Process]::GetProcessById(${pid}).StartTime.ToUniversalTime().Ticks`, + ]); + return row.stdout.trim(); + } + + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + const closeParen = stat.lastIndexOf(")"); + const fields = stat.slice(closeParen + 2).split(" "); + return fields[19]; // starttime field } export function validateProcessIdentity(pid, expectedIdentity, options = {}) { diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index 5c37212..d006f09 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -14,6 +14,7 @@ import { resolveDefaultModel, resolveClaudeBin, buildArgs, + cancelClaudeProcess, EFFORT_ALIASES, VALID_EFFORTS, DEFAULT_MODEL, @@ -421,6 +422,30 @@ describe("validateTurnCompletion", () => { }); }); +describe("cancelClaudeProcess", () => { + it("uses Windows process-tree termination after identity validation", async () => { + let termination = null; + const result = await cancelClaudeProcess(12345, "recorded-identity", { + platform: "win32", + validateProcessIdentityImpl: () => true, + terminateProcessTreeImpl: (pid, options) => { + termination = { pid, options }; + return { + attempted: true, + delivered: true, + method: "taskkill", + }; + }, + }); + + assert.deepEqual(result, { cancelled: true }); + assert.deepEqual(termination, { + pid: 12345, + options: { platform: "win32" }, + }); + }); +}); + // =========================================================================== // resolveModel // =========================================================================== diff --git a/tests/process.test.mjs b/tests/process.test.mjs index bf94afc..004e092 100644 --- a/tests/process.test.mjs +++ b/tests/process.test.mjs @@ -25,7 +25,10 @@ const NODE_BIN = process.execPath; describe("runCommand", () => { it("runs a simple command and captures stdout", () => { - const result = runCommand("echo", ["hello"]); + const result = runCommand(NODE_BIN, [ + "-e", + "process.stdout.write('hello')", + ]); assert.equal(result.status, 0); assert.equal(result.stdout.trim(), "hello"); assert.equal(result.signal, null); @@ -49,13 +52,18 @@ describe("runCommand", () => { }); it("preserves command and args in result", () => { - const result = runCommand("echo", ["a", "b"]); - assert.equal(result.command, "echo"); - assert.deepEqual(result.args, ["a", "b"]); + const args = ["-e", "", "a", "b"]; + const result = runCommand(NODE_BIN, args); + assert.equal(result.command, NODE_BIN); + assert.deepEqual(result.args, args); }); it("accepts input via options.input", () => { - const result = runCommand("cat", [], { input: "stdin data" }); + const result = runCommand( + NODE_BIN, + ["-e", "process.stdin.pipe(process.stdout)"], + { input: "stdin data" }, + ); assert.equal(result.stdout, "stdin data"); }); @@ -105,7 +113,10 @@ describe("runCommand", () => { describe("runCommandChecked", () => { it("returns result for successful command", () => { - const result = runCommandChecked("echo", ["ok"]); + const result = runCommandChecked(NODE_BIN, [ + "-e", + "process.stdout.write('ok')", + ]); assert.equal(result.status, 0); assert.equal(result.stdout.trim(), "ok"); }); @@ -361,6 +372,40 @@ describe("isProcessAlive", () => { // --------------------------------------------------------------------------- describe("getProcessIdentity", () => { + it("uses the process start time as the Windows identity", () => { + let invocation = null; + const identity = getProcessIdentity(12345, { + platform: "win32", + runCommandCheckedImpl: (command, args) => { + invocation = { command, args }; + return { stdout: "638935200000000000\r\n" }; + }, + }); + + assert.equal(identity, "638935200000000000"); + assert.equal(invocation?.command, "powershell.exe"); + assert.deepEqual(invocation?.args, [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + "[System.Diagnostics.Process]::GetProcessById(12345).StartTime.ToUniversalTime().Ticks", + ]); + }); + + it("rejects an invalid Windows pid before invoking PowerShell", () => { + assert.throws( + () => + getProcessIdentity("12345; Write-Output unsafe", { + platform: "win32", + runCommandCheckedImpl: () => { + throw new Error("PowerShell should not run"); + }, + }), + TypeError, + ); + }); + it("returns a non-empty string for the current process", () => { const identity = getProcessIdentity(process.pid); assert.ok(typeof identity === "string");