Stop brokers started by the test suite

tests/helpers.mjs

@@ -4,8 +4,89 @@ import path from "node:path";
 import process from "node:process";
 import { spawnSync } from "node:child_process";
 
+import {
+  clearBrokerSession,
+  loadBrokerSession,
+  teardownBrokerSession
+} from "../plugins/codex/scripts/lib/broker-lifecycle.mjs";
+import { terminateProcessTree } from "../plugins/codex/scripts/lib/process.mjs";
+
+/**
+ * Workspaces handed out by makeTempDir, so cleanup can find the brokers that
+ * were started for them.
+ */
+const tempDirs = new Set();
+
 export function makeTempDir(prefix = "codex-plugin-test-") {
-  return fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  tempDirs.add(dir);
+  return dir;
+}
+
+/**
+ * Stop the broker registered for a workspace, if one is still running, and
+ * clear its session so nothing later reads a dead PID.
+ *
+ * The broker is deliberately long-lived so it can be reused across companion
+ * invocations, which means nothing in a test run ever shuts it down. Each test
+ * that runs the companion therefore leaves a broker and its app-server child
+ * behind for the lifetime of the machine.
+ *
+ * The session file has to go too. It records a bare PID, so leaving it behind
+ * for a stable workspace such as the repository root means a later run can read
+ * it and signal whatever process has since inherited that PID.
+ */
+export function stopBrokerFor(cwd) {
+  let session = null;
+  try {
+    session = loadBrokerSession(cwd);
+  } catch {
+    return;
+  }
+
+  if (!session) {
+    return;
+  }
+
+  try {
+    teardownBrokerSession({
+      endpoint: session.endpoint ?? null,
+      pidFile: session.pidFile ?? null,
+      logFile: session.logFile ?? null,
+      sessionDir: session.sessionDir ?? null,
+      pid: Number.isFinite(session.pid) ? session.pid : null,
+      killProcess: (pid) => terminateProcessTree(pid)
+    });
+  } catch {
+    // Best effort: teardown must never fail the suite.
+  }
+
+  try {
+    clearBrokerSession(cwd);
+  } catch {
+    // Best effort.
+  }
+}
+
+/**
+ * Tear down every workspace created by makeTempDir: stop its broker, then
+ * remove the directory. Extra workspaces (for example the repository root,
+ * which some tests use as the companion cwd) can be passed in.
+ */
+export function cleanupTempWorkspaces(extraWorkspaces = []) {
+  for (const cwd of [...tempDirs, ...extraWorkspaces]) {
+    stopBrokerFor(cwd);
+  }
+
+  for (const dir of tempDirs) {
+    try {
+      fs.rmSync(dir, { recursive: true, force: true });
+    } catch {
+      // Best effort: a leftover temp dir is not worth failing the suite over.
+    }
+  }
+
+  tempDirs.clear();
 }
 
 export function writeExecutable(filePath, source) {

tests/runtime.test.mjs

@@ -1,12 +1,12 @@
 import fs from "node:fs";
 import path from "node:path";
-import test from "node:test";
+import test, { after } from "node:test";
 import assert from "node:assert/strict";
 import { spawn } from "node:child_process";
 import { fileURLToPath } from "node:url";
 
 import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs";
-import { initGitRepo, makeTempDir, run } from "./helpers.mjs";
+import { cleanupTempWorkspaces, initGitRepo, makeTempDir, run } from "./helpers.mjs";
 import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs";
 import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs";
 
@@ -16,6 +16,31 @@ const SCRIPT = path.join(PLUGIN_ROOT, "scripts", "codex-companion.mjs");
 const STOP_HOOK = path.join(PLUGIN_ROOT, "scripts", "stop-review-gate-hook.mjs");
 const SESSION_HOOK = path.join(PLUGIN_ROOT, "scripts", "session-lifecycle-hook.mjs");
 
+// Companion invocations start a broker that is deliberately long-lived so it
+// can be reused. Nothing else shuts it down, so without this the suite leaves a
+// broker plus its app-server child behind for every workspace it touches.
+//
+// ROOT is a stable workspace a developer may already have a live companion
+// session on, so it is only torn down when this run is the thing that started
+// it: the session is sampled before any test and stopped only if it changed.
+const rootBrokerPidBeforeRun = readRootBrokerPid();
+
+function readRootBrokerPid() {
+  try {
+    return loadBrokerSession(ROOT)?.pid ?? null;
+  } catch {
+    return null;
+  }
+}
+
+after(() => {
+  const rootBrokerPidAfterRun = readRootBrokerPid();
+  const startedRootBroker =
+    rootBrokerPidAfterRun !== null && rootBrokerPidAfterRun !== rootBrokerPidBeforeRun;
+
+  cleanupTempWorkspaces(startedRootBroker ? [ROOT] : []);
+});
+
 async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) {
   const start = Date.now();
   while (Date.now() - start < timeoutMs) {