test(clients): bound transaction fixture child completion

tests/clients/client-connect.test.ts

@@ -14,7 +14,7 @@ import {
 import { handleConnectCommand } from "../../src/cli/connect";
 import { removeTreeWithRetry } from "../helpers/remove-tree";
 import { repoRoot as findRepoRoot } from "../helpers/repo-root";
-import { INTERNAL_DEADLINE_MS } from "../helpers/test-budget";
+import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "../helpers/test-budget";
 
 const repoRoot = findRepoRoot();
 
@@ -316,7 +316,10 @@ describe("remote hub client boundary", () => {
 /** A catalog the user already had before ever connecting. */
 const PRIOR_CATALOG_BYTES = '{"models":[{"slug":"local/only-model"}]}';
 
-function runTransactionScenario(stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator") {
+function runTransactionScenario(
+  stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator",
+  options: { script?: string; timeoutMs?: number } = {},
+) {
   const opencodexHome = mkdtempSync(join(tmpdir(), "ocx-client-connect-home-"));
   const codexHome = mkdtempSync(join(tmpdir(), "ocx-client-connect-codex-"));
   const configPath = join(opencodexHome, "config.json");
@@ -335,7 +338,7 @@ function runTransactionScenario(stage: "success" | "catalog" | "preflight" | "co
     const { mkdirSync } = require("node:fs") as typeof import("node:fs");
     mkdirSync(join(opencodexHome, "config-mutation.sqlite"));
   }
-  const script = `
+  const script = options.script ?? `
     const { existsSync, readFileSync } = require("node:fs");
     const { createHash } = require("node:crypto");
     const { connectClient, disconnectClient } = require("./src/client/connect");
@@ -396,26 +399,110 @@ function runTransactionScenario(stage: "success" | "catalog" | "preflight" | "co
       console.log(JSON.stringify({ connected, error, beforeDisconnect, artifacts, disconnected, catalogAfter, after: readClientConnectionState(), calls, commitFaultTriggered }));
     })();
   `;
-  const result = spawnSync(process.execPath, ["--eval", script], {
-    cwd: repoRoot,
-    env: { ...process.env, OPENCODEX_HOME: opencodexHome, CODEX_HOME: codexHome, OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR: join(opencodexHome, "desktop") },
-    encoding: "utf8",
-  });
-  const output = result.stdout.trim().split("\n").at(-1) ?? "{}";
-  const parsed = JSON.parse(output) as Record<string, any>;
-  return {
-    status: result.status,
-    stderr: result.stderr,
-    parsed,
-    configBytes: readFileSync(configPath, "utf8"),
-    cleanup: () => {
-      removeTreeWithRetry(opencodexHome);
-      removeTreeWithRetry(codexHome);
-    },
+  const cleanup = () => {
+    const failures: unknown[] = [];
+    for (const home of [opencodexHome, codexHome]) {
+      try { removeTreeWithRetry(home); }
+      catch (error) { failures.push(error); }
+    }
+    if (failures.length) throw new AggregateError(failures, "Could not clean client transaction homes");
   };
+  try {
+    const result = spawnSync(process.execPath, ["--eval", script], {
+      cwd: repoRoot,
+      env: { ...process.env, OPENCODEX_HOME: opencodexHome, CODEX_HOME: codexHome, OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR: join(opencodexHome, "desktop") },
+      encoding: "utf8",
+      timeout: options.timeoutMs ?? INTERNAL_DEADLINE_MS,
+      killSignal: "SIGKILL",
+    });
+    if (result.error || result.status !== 0 || result.signal !== null) {
+      throw new ClientStateProbeError(result.pid, result.status, result.signal, (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT");
+    }
+    let parsed: Record<string, any>;
+    try { parsed = JSON.parse(result.stdout.trim().split("\n").at(-1) ?? "{}"); }
+    catch { throw new ClientStateProbeError(result.pid, result.status, result.signal, false); }
+    return { status: result.status, stderr: result.stderr, parsed, configBytes: readFileSync(configPath, "utf8"), cleanup };
+  } catch (error) {
+    try { cleanup(); }
+    catch (cleanupError) { throw new AggregateError([error, cleanupError], "Client transaction failed and fixture cleanup failed"); }
+    throw error;
+  }
 }
 
 describe("connect transaction and offline disconnect", () => {
+  test("transaction fixture stops a child retained after valid output", async () => {
+    const proofHome = mkdtempSync(join(tmpdir(), "ocx-transaction-child-proof-"));
+    const markerPath = join(proofHome, "child-started.json");
+    const naturalExitPath = join(proofHome, "natural-exit");
+    const script = `
+      const fs = require("node:fs");
+      fs.writeFileSync(${JSON.stringify(markerPath)}, JSON.stringify({
+        pid: process.pid, home: process.env.OPENCODEX_HOME, codexHome: process.env.CODEX_HOME,
+      }));
+      fs.writeSync(1, '{"ok":true}\\n');
+      setTimeout(() => { fs.writeFileSync(${JSON.stringify(naturalExitPath)}, "exited"); }, 5_000);
+    `;
+    let run: Awaited<ReturnType<typeof runTransactionScenario>> | undefined;
+    try {
+      let failure: unknown;
+      const startedAt = performance.now();
+      try { run = await runTransactionScenario("coordinator", { script, timeoutMs: 2_000 }); }
+      catch (error) { failure = error; }
+      expect(performance.now() - startedAt).toBeLessThan(10_000);
+      expect(failure).toBeInstanceOf(ClientStateProbeError);
+      if (!(failure instanceof ClientStateProbeError)) throw new Error("Expected bounded transaction child failure");
+      expect(failure.timedOut).toBe(true);
+      expect(existsSync(naturalExitPath)).toBe(false);
+      const proof = JSON.parse(readFileSync(markerPath, "utf8")) as { pid: number; home: string; codexHome: string };
+      expect(proof.pid).toBe(failure.pid);
+      expect(existsSync(proof.home)).toBe(false);
+      expect(existsSync(proof.codexHome)).toBe(false);
+      let exitCode: string | undefined;
+      try { process.kill(proof.pid, 0); }
+      catch (error) { exitCode = (error as NodeJS.ErrnoException).code; }
+      expect(exitCode).toBe("ESRCH");
+    } finally {
+      run?.cleanup();
+      removeTreeWithRetry(proofHome);
+    }
+  }, SPAWN_BUDGET_MS);
+
+  for (const [mode, output, status] of [
+    ["nonzero exit", '{"ok":true}', 7],
+    ["invalid JSON", "private-child-output", 0],
+  ] as const) {
+    test(`transaction fixture cleans homes after ${mode}`, () => {
+      const proofHome = mkdtempSync(join(tmpdir(), "ocx-transaction-child-proof-"));
+      const markerPath = join(proofHome, "child-started.json");
+      const script = `
+        const fs = require("node:fs");
+        fs.writeFileSync(${JSON.stringify(markerPath)}, JSON.stringify({
+          pid: process.pid, home: process.env.OPENCODEX_HOME, codexHome: process.env.CODEX_HOME,
+        }));
+        fs.writeSync(1, ${JSON.stringify(output)});
+        process.exit(${status});
+      `;
+      let run: ReturnType<typeof runTransactionScenario> | undefined;
+      try {
+        let failure: unknown;
+        try { run = runTransactionScenario("coordinator", { script }); }
+        catch (error) { failure = error; }
+        expect(failure).toBeInstanceOf(ClientStateProbeError);
+        if (!(failure instanceof ClientStateProbeError)) throw new Error("Expected transaction child failure");
+        expect(failure.status).toBe(status);
+        expect(failure.timedOut).toBe(false);
+        expect(failure.message).not.toContain(output);
+        const proof = JSON.parse(readFileSync(markerPath, "utf8")) as { pid: number; home: string; codexHome: string };
+        expect(failure.pid).toBe(proof.pid);
+        expect(existsSync(proof.home)).toBe(false);
+        expect(existsSync(proof.codexHome)).toBe(false);
+      } finally {
+        try { run?.cleanup(); }
+        finally { removeTreeWithRetry(proofHome); }
+      }
+    }, SPAWN_BUDGET_MS);
+  }
+
   test("an unavailable config coordinator refuses before issuing any hub key", () => {
     const run = runTransactionScenario("coordinator");
     try {