fix: cancel dead pids on non-English Windows (#423)

plugins/codex/scripts/codex-companion.mjs

@@ -983,7 +983,17 @@ async function handleCancel(argv) {
     );
   }
 
-  terminateProcessTree(job.pid ?? Number.NaN);
+  // Termination failures must not leave the job stuck as "running": the user is
+  // abandoning this job, so persist the cancelled state even if the (possibly
+  // already-dead) worker cannot be terminated.
+  try {
+    terminateProcessTree(job.pid ?? Number.NaN);
+  } catch (error) {
+    appendLogLine(
+      job.logFile,
+      `Process termination failed during cancel${error?.message ? `: ${error.message}` : "."}`
+    );
+  }
   appendLogLine(job.logFile, "Cancelled by user.");
 
   const completedAt = nowIso();

plugins/codex/scripts/lib/process.mjs

@@ -1,15 +1,30 @@
 import { spawnSync } from "node:child_process";
 import process from "node:process";
 
+function resolveShellOption(shell) {
+  // Normalize null/undefined to the platform default so callers can pass
+  // through optional values without tripping spawnSync's ERR_INVALID_ARG_TYPE.
+  if (shell === undefined || shell === null) {
+    return process.platform === "win32" ? process.env.SHELL || true : false;
+  }
+  if (typeof shell !== "boolean" && typeof shell !== "string") {
+    throw new TypeError(
+      `runCommand "shell" option must be a boolean or string, received ${typeof shell}`
+    );
+  }
+  return shell;
+}
+
 export function runCommand(command, args = [], options = {}) {
+  const shell = resolveShellOption(options.shell);
   const result = spawnSync(command, args, {
     cwd: options.cwd,
     env: options.env,
     encoding: "utf8",
     input: options.input,
     maxBuffer: options.maxBuffer,
     stdio: options.stdio ?? "pipe",
-    shell: process.platform === "win32" ? (process.env.SHELL || true) : false,
+    shell,
     windowsHide: true
   });
 
@@ -64,15 +79,25 @@ export function terminateProcessTree(pid, options = {}) {
   const killImpl = options.killImpl ?? process.kill.bind(process);
 
   if (platform === "win32") {
+    // taskkill needs no shell; forcing shell:false avoids MSYS/Git-Bash argument
+    // conversion mangling "/PID" into a path, and sidesteps the DEP0190 warning.
     const result = runCommandImpl("taskkill", ["/PID", String(pid), "/T", "/F"], {
       cwd: options.cwd,
-      env: options.env
+      env: options.env,
+      shell: false
     });
 
     if (!result.error && result.status === 0) {
       return { attempted: true, delivered: true, method: "taskkill", result };
     }
 
+    // taskkill exits 128 when the target process does not exist. Its stderr is
+    // localized (and becomes mojibake when a non-UTF-8 console codepage is
+    // decoded as utf8), so the exit code is the only locale-independent signal.
+    if (!result.error && result.status === 128) {
+      return { attempted: true, delivered: false, method: "taskkill", result };
+    }
+
     const combinedOutput = `${result.stderr}\n${result.stdout}`.trim();
     if (!result.error && looksLikeMissingProcessMessage(combinedOutput)) {
       return { attempted: true, delivered: false, method: "taskkill", result };

tests/process.test.mjs

@@ -53,3 +53,47 @@ test("terminateProcessTree treats missing Windows processes as already stopped",
   assert.equal(outcome.result.status, 128);
   assert.match(outcome.result.stdout, /not found/i);
 });
+
+test("terminateProcessTree treats exit 128 as not-found even with localized/mojibake output", () => {
+  // cp950 rendering of a localized "process not found" message decoded as utf8.
+  const outcome = terminateProcessTree(36564, {
+    platform: "win32",
+    runCommandImpl(command, args) {
+      return {
+        command,
+        args,
+        status: 128,
+        signal: null,
+        stdout: "",
+        stderr: "\uFFFD\uFFFD: \uFFFD\uFFFD\uFFFD\uFFFD\uFFFD",
+        error: null
+      };
+    }
+  });
+
+  assert.equal(outcome.attempted, true);
+  assert.equal(outcome.delivered, false);
+  assert.equal(outcome.method, "taskkill");
+  assert.equal(outcome.result.status, 128);
+});
+
+test("terminateProcessTree invokes taskkill without a shell", () => {
+  let capturedOptions = null;
+  terminateProcessTree(1234, {
+    platform: "win32",
+    runCommandImpl(command, args, options) {
+      capturedOptions = options;
+      return {
+        command,
+        args,
+        status: 0,
+        signal: null,
+        stdout: "",
+        stderr: "",
+        error: null
+      };
+    }
+  });
+
+  assert.equal(capturedOptions.shell, false);
+});