fix(tooling): let check implementations own cancellation (#157272)

* fix(tooling): let check implementations own cancellation

* fix(tooling): integrate cancellation ownership with current main

Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

scripts/check-changed.mjs

@@ -2,4 +2,6 @@ import { runTsxCliShim } from "./lib/tsx-cli-shim.mjs";
 
 await runTsxCliShim(import.meta.url, {
   implementation: "./check-changed.mts",
+  // The implementation must join active checks before the outer wrapper exits.
+  terminationOwner: "implementation",
 });

scripts/run-oxlint.mjs

@@ -3,4 +3,6 @@ import { runTsxCliShim } from "./lib/tsx-cli-shim.mjs";
 await runTsxCliShim(import.meta.url, {
   implementation: "./run-oxlint.mts",
   failureTool: "oxlint",
+  // The implementation must join active lint/preparation commands before cleanup.
+  terminationOwner: "implementation",
 });

scripts/run-tsgo-core-test-shards.mjs

@@ -5,4 +5,6 @@ import { runTsxCliShim } from "./lib/tsx-cli-shim.mjs";
 await runTsxCliShim(import.meta.url, {
   implementation: "./run-tsgo-core-test-shards.mts",
   failureTool: "tsgo:core:test",
+  // The implementation must settle started shards before releasing artifact ownership.
+  terminationOwner: "implementation",
 });

scripts/run-tsgo.mjs

@@ -3,7 +3,6 @@ import { runNodeCliShim } from "./lib/tsx-cli-shim.mjs";
 await runNodeCliShim(import.meta.url, {
   implementation: "./run-tsgo.mts",
   failureTool: "tsgo",
-  // The implementation owns a detached compiler group and escalates after 5s.
-  // Its outer shim must stay alive long enough to finish that cleanup.
-  forceKillDelayMs: 10_000,
+  // The implementation must join its compiler group before releasing artifact ownership.
+  terminationOwner: "implementation",
 });

test/scripts/check-changed-cancellation.test.ts

@@ -0,0 +1,182 @@
+import type { ChildProcess } from "node:child_process";
+import fs from "node:fs";
+import path from "node:path";
+import { pathToFileURL } from "node:url";
+import { afterEach, describe, expect, it } from "vitest";
+import { createFixtureLifetime } from "../helpers/fixture-lifetime.js";
+import { isProcessAlive, waitForDead, waitForFixtureFile } from "../helpers/process-wait.js";
+import { withTestTimeout } from "../helpers/promise.js";
+import { runNodeScript } from "../helpers/run-node-script.js";
+
+const fixture = createFixtureLifetime();
+afterEach(() => fixture.cleanup());
+
+describe.skipIf(process.platform === "win32")("check-changed public wrapper cancellation", () => {
+  it.each(["resistant", "cooperative", "failure"] as const)(
+    "joins a %s check and stops admitting commands",
+    async (mode) => {
+      await fixture.run(async () => {
+        const cwd = fixture.createTempDir("check-changed-cancellation-");
+        const wrapperPath = path.resolve("scripts/check-changed.mjs");
+        const implementationPath = path.resolve("scripts/check-changed.mts");
+        const commandsPath = path.join(cwd, "commands.jsonl");
+        const pidPaths = ["implementation", "command", "descendant"].map((name) =>
+          path.join(cwd, `${name}.pid`),
+        );
+        const readyPath = path.join(cwd, "ready.pid");
+        const clockPath = path.join(cwd, "supervisor-clock.mjs");
+        const binDir = path.join(cwd, "bin");
+        fs.mkdirSync(binDir);
+        // Exercise the real wrapper and managed owner. Scale their deadlines together;
+        // fixture readiness, leaf processes, and harness cleanup keep real time.
+        fs.writeFileSync(
+          clockPath,
+          `if (process.argv[1] === ${JSON.stringify(implementationPath)}) {
+  const realNow = Date.now.bind(Date);
+  const startedAt = realNow();
+  Date.now = () => startedAt + (realNow() - startedAt) * 10;
+} else if (process.argv[1] === ${JSON.stringify(wrapperPath)}) {
+  const realSetTimeout = globalThis.setTimeout;
+  globalThis.setTimeout = (callback, delay, ...args) =>
+    realSetTimeout(callback, delay / 10, ...args);
+}
+`,
+        );
+        const descendantSource = `
+const fs = require("node:fs");
+for (const signal of ["SIGTERM", "SIGINT", "SIGHUP"]) {
+  process.on(signal, () => fs.writeSync(2, "descendant received " + signal + "\\n"));
+}
+process.on("message", () => {
+  fs.writeSync(1, "descendant output tail\\n");
+  process.exit(0);
+});
+setTimeout(() => process.exit(98), 15000);
+process.send("ready");
+`;
+        fs.writeFileSync(
+          path.join(binDir, "pnpm"),
+          `#!/usr/bin/env node
+const fs = require("node:fs");
+const { spawn } = require("node:child_process");
+const args = process.argv.slice(2);
+fs.appendFileSync(${JSON.stringify(commandsPath)}, JSON.stringify(args) + "\\n");
+if (args.length !== 1 || args[0] !== "check:no-conflict-markers") {
+  console.error("unexpected fixture command: " + JSON.stringify(args));
+  process.exit(99);
+}
+if (${JSON.stringify(mode)} === "failure") {
+  console.error("fixture check failed");
+  process.exit(7);
+}
+fs.writeFileSync(${JSON.stringify(pidPaths[0])}, String(process.ppid));
+fs.writeFileSync(${JSON.stringify(pidPaths[1])}, String(process.pid));
+const child = spawn(process.execPath, ["-e", ${JSON.stringify(descendantSource)}], {
+  stdio: ["ignore", "inherit", "inherit", "ipc"],
+});
+if (child.pid) fs.writeFileSync(${JSON.stringify(pidPaths[2])}, String(child.pid));
+for (const signal of ["SIGTERM", "SIGINT", "SIGHUP"]) {
+  process.on(signal, () => {
+    fs.writeSync(2, "command received " + signal + "\\n");
+    if (${JSON.stringify(mode)} === "cooperative" && child.connected) child.send("finish");
+  });
+}
+child.once("close", () => {
+  fs.writeSync(1, "command output tail\\n");
+  process.exit(0);
+});
+setTimeout(() => process.exit(98), 15000);
+child.once("message", () => {
+  fs.writeFileSync(${JSON.stringify(readyPath)}, String(process.pid));
+});
+`,
+          { mode: 0o755 },
+        );
+        const env: NodeJS.ProcessEnv = {
+          ...process.env,
+          // This fixture deliberately exercises the local pnpm leaf on CI too.
+          CI: "",
+          GITHUB_ACTIONS: "",
+          OPENCLAW_TESTBOX: "",
+          PATH: [binDir, path.dirname(process.execPath), process.env.PATH].join(path.delimiter),
+          NODE_OPTIONS: [process.env.NODE_OPTIONS, `--import=${pathToFileURL(clockPath).href}`]
+            .filter(Boolean)
+            .join(" "),
+        };
+        let wrapper: ChildProcess | undefined;
+        const completion = fixture.track(
+          runNodeScript([wrapperPath, "--staged", "--", "README.md"], env, 10_000, {
+            cwd,
+            maxBuffer: 64 * 1024,
+            onReady(child) {
+              wrapper = child;
+            },
+          }),
+        );
+        const readOwnedPids = () =>
+          pidPaths.flatMap((pidPath) => {
+            if (!fs.existsSync(pidPath)) {
+              return [];
+            }
+            const pid = Number(fs.readFileSync(pidPath, "utf8"));
+            if (!Number.isSafeInteger(pid) || pid <= 1) {
+              throw new Error(`Invalid fixture pid in ${pidPath}`);
+            }
+            return [pid];
+          });
+        try {
+          if (mode !== "failure") {
+            // The receipt is written only after both leaf signal handlers are installed.
+            await withTestTimeout(
+              waitForFixtureFile(readyPath, completion),
+              5_000,
+              "changed-check command did not become ready",
+            );
+            expect(readOwnedPids()).toHaveLength(3);
+            expect(wrapper?.kill("SIGTERM")).toBe(true);
+          }
+          const result = await completion;
+          expect(result.error, result.stderr).toBeUndefined();
+          expect(result.status, result.stderr).toBe(mode === "failure" ? 7 : 143);
+          expect(result.stderr.trim().split("\n").at(-1)).toBe(
+            `[check:changed] FAILED (exit ${mode === "failure" ? 7 : 143})`,
+          );
+          expect(fs.readFileSync(commandsPath, "utf8").trim()).toBe(
+            JSON.stringify(["check:no-conflict-markers"]),
+          );
+          // Check before fixture teardown: a closed outer pipe alone does not prove
+          // the implementation's detached command group has stopped.
+          expect(readOwnedPids().filter(isProcessAlive)).toEqual([]);
+          if (mode === "cooperative") {
+            expect(result.stdout).toContain("descendant output tail\ncommand output tail\n");
+          } else if (mode === "resistant") {
+            expect(result.stderr).toContain("command received SIGTERM");
+            expect(result.stderr).toContain("descendant received SIGTERM");
+          }
+        } finally {
+          await fixture.verifyCleanup(async () => {
+            // A pre-fix wrapper can orphan these exact fixture PIDs. Reap only those
+            // recorded by the fixture leaf; never broaden cleanup to process-name scans.
+            const pids = [...readOwnedPids(), ...(wrapper?.pid ? [wrapper.pid] : [])];
+            for (const pid of pids) {
+              if (isProcessAlive(pid)) {
+                try {
+                  process.kill(pid, "SIGKILL");
+                } catch (error) {
+                  if ((error as NodeJS.ErrnoException).code !== "ESRCH") {
+                    throw error;
+                  }
+                }
+              }
+            }
+            await Promise.all([
+              ...pids.map((pid) => waitForDead(pid, 2_000)),
+              withTestTimeout(completion, 2_000, "wrapper output did not close during cleanup"),
+            ]);
+          });
+        }
+      });
+    },
+    20_000,
+  );
+});

test/scripts/direct-run-entrypoints.test-support.ts

@@ -75,6 +75,10 @@ console.log(JSON.stringify({ value: first.value, evaluations: globalThis.pluginE
 export async function withShimFixture<T>(
   wrapper:
     | (typeof TSX_SHIM_WRAPPERS)[number]
+    | "scripts/check-changed.mjs"
+    | "scripts/run-tsgo.mjs"
+    | "scripts/run-oxlint.mjs"
+    | "scripts/run-tsgo-core-test-shards.mjs"
     | "scripts/run-node.mjs"
     | "scripts/crabbox-wrapper.mjs",
   run: (paths: {

test/scripts/direct-run-entrypoints.test.ts

@@ -177,13 +177,30 @@ function expectShimLoader(
 }
 
 describe("script direct-run entrypoints", () => {
-  it.skipIf(process.platform === "win32")(
-    "lets the Vitest implementation finish cleanup beyond the shim force-kill window",
-    async () => {
-      await withShimFixture("scripts/run-vitest.mjs", async (fixture) => {
+  it
+    .skipIf(process.platform === "win32")
+    .each([
+      "scripts/run-vitest.mjs",
+      "scripts/check-changed.mjs",
+      "scripts/run-tsgo.mjs",
+      "scripts/run-oxlint.mjs",
+      "scripts/run-tsgo-core-test-shards.mjs",
+    ] as const)(
+    "lets %s finish implementation cleanup beyond the shim force-kill window",
+    async (wrapper) => {
+      await withShimFixture(wrapper, async (fixture) => {
         const { checkoutRoot, fixtureRoot, implementationPath, wrapperPath, runNode } = fixture;
         const ownerPath = path.join(fixtureRoot, "owner.pid");
         const settledPath = path.join(fixtureRoot, "cleanup-settled");
+        const clockPath = path.join(fixtureRoot, "supervisor-clock.mjs");
+        // Scale both owners equally: a competing 5s or 10s cutoff must still fail.
+        // Readiness and the test harness retain real time.
+        writeFileSync(
+          clockPath,
+          `const realSetTimeout = globalThis.setTimeout;
+globalThis.setTimeout = (callback, delay, ...args) =>
+  realSetTimeout(callback, delay / 20, ...args);\n`,
+        );
         writeTsxFixture(path.join(checkoutRoot, "node_modules"), "checkout");
         writeFileSync(
           implementationPath,
@@ -194,12 +211,21 @@ process.once("SIGTERM", () => {
     fs.writeFileSync(${JSON.stringify(settledPath)}, "settled");
     clearInterval(keepAlive);
     process.exitCode = 143;
-  }, 5500);
+  }, 11000);
 });
 fs.writeFileSync(${JSON.stringify(ownerPath)}, String(process.ppid));
 `,
         );
-        const completion = runNode([wrapperPath], process.env, fixtureRoot);
+        const completion = runNode(
+          [wrapperPath],
+          {
+            ...process.env,
+            NODE_OPTIONS: [process.env.NODE_OPTIONS, `--import=${pathToFileURL(clockPath).href}`]
+              .filter(Boolean)
+              .join(" "),
+          },
+          fixtureRoot,
+        );
         const owner = await waitForPidFile(ownerPath, 10_000);
         process.kill(owner, "SIGTERM");
         const result = await completion;