fix(core): always force-kill on Windows; fix stale .cjs JSDoc
killProcessTree: drop the SIGTERM-without-/F branch on Windows. taskkill without /F sends WM_CLOSE which is unreliable for headless Node.js console processes — they may silently survive, leaving orphaned processes. Always pass /F so termination is guaranteed. Callers that do SIGTERM→wait→SIGKILL escalation are unaffected: SIGKILL simply finds the process already dead. agent-workspace-hooks: correct JSDoc that still said <name>.js after the extension was changed to .cjs (forced CJS mode). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
1b43f5eea1
commit
5ce89025af
|
|
@ -129,16 +129,18 @@ describe("resolveWindowsShell", () => {
|
|||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("killProcessTree", () => {
|
||||
it("calls taskkill with /T /PID (no /F) for SIGTERM on Windows", async () => {
|
||||
it("calls taskkill with /T /F /PID for SIGTERM on Windows (always force-kill)", async () => {
|
||||
setPlatform("win32");
|
||||
resolveExecFile(""); // taskkill succeeds
|
||||
|
||||
const mod = await import("../platform.js");
|
||||
await mod.killProcessTree(1234, "SIGTERM");
|
||||
|
||||
// On Windows we always pass /F regardless of signal — taskkill without /F sends
|
||||
// WM_CLOSE which is unreliable for headless Node.js console processes.
|
||||
expect(mockExecFile).toHaveBeenCalledWith(
|
||||
"taskkill",
|
||||
["/T", "/PID", "1234"],
|
||||
["/T", "/F", "/PID", "1234"],
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -271,8 +271,8 @@ exit \$exit_code
|
|||
* Build a Node.js wrapper script for a given binary (gh or git).
|
||||
*
|
||||
* On Windows, bash scripts cannot be executed directly, so we generate:
|
||||
* - <name>.js — the actual interception logic (Node.js)
|
||||
* - <name>.cmd — a tiny CMD shim: @node "%~dp0<name>.js" %*
|
||||
* - <name>.cjs — the actual interception logic (Node.js, forced CJS mode)
|
||||
* - <name>.cmd — a tiny CMD shim: @node "%~dp0<name>.cjs" %*
|
||||
*
|
||||
* The .js script replicates what the bash wrapper does:
|
||||
* - gh: intercepts `gh pr create` and `gh pr merge`
|
||||
|
|
|
|||
|
|
@ -82,10 +82,11 @@ export async function killProcessTree(
|
|||
// kill AO itself. pid<0 is never valid. Guard both.
|
||||
if (pid <= 0) return;
|
||||
if (isWindows()) {
|
||||
// taskkill /T kills the process tree; /F forces termination (SIGKILL equivalent).
|
||||
// Without /F, taskkill sends WM_CLOSE allowing the process to shut down gracefully
|
||||
// (SIGTERM equivalent). This preserves the SIGTERM→wait→SIGKILL escalation pattern.
|
||||
const args = signal === "SIGKILL" ? ["/T", "/F", "/PID", String(pid)] : ["/T", "/PID", String(pid)];
|
||||
// Always use /F (force) on Windows. taskkill without /F sends WM_CLOSE, which
|
||||
// only works for GUI windows; headless Node.js console processes may ignore it,
|
||||
// leaving orphaned processes. Callers that do SIGTERM→wait→SIGKILL escalation
|
||||
// are unaffected: the SIGKILL step simply finds the process already dead.
|
||||
const args = ["/T", "/F", "/PID", String(pid)];
|
||||
try {
|
||||
await execFileAsync("taskkill", args);
|
||||
} catch {
|
||||
|
|
|
|||
Loading…
Reference in New Issue