test(conversation): Mr flows test and no shim left after the suite (#1533)

Row 47, the S5 follow-up. flows.test gains a test for a hold set inside
held input, which kills mutant Mr. claim W5 and W13 reap their own
force-stopped scopes, K19 kills its scope if its kill or release fails,
and the harness gains liveShims, killShims and shimsGone with a final
sweep in reap. claim, cohort and races each end by asserting no shim
outlives the file.

Author: Dewey. Approved round 1 by Filbert (comment 27041) and Darkwing
(comment 27043) on candidate 0587e393. The src side, where a force stop
never sends release, is row 50 (#1536).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

packages/conversation/tests/claim.test.mjs

@@ -21,14 +21,17 @@ import { bootId } from "../../discord/src/journal.mjs";
 import { FakeLauncher } from "./fake-pi.mjs";
 import {
   REPO, FAST, fixture, controllerFor, started, spawnController, killChildren, cleanupAll, reap, claimRecords,
-  userEntry, assistantEntry, thinkingEntry, receiptState, noUnits, tick,
+  userEntry, assistantEntry, thinkingEntry, receiptState, noUnits, tick, killShims, shimsGone,
 } from "./harness.mjs";
 
 const reaped = [];
-after(() => {
+after(async () => {
   for (const fx of reaped) reap(fx);
+  killShims();
+  const left = await shimsGone();
   killChildren();
   cleanupAll();
+  assert.deepEqual(left, [], "no shim outlives this file (#1533)");
 });
 const track = (fx) => (reaped.push(fx), fx);
 
@@ -317,6 +320,8 @@ test("W5: SIGKILL between every publication barrier of release; restart finishes
   const { ch, mark } = await runRelease(probe);
   ch.send("close");
   await ch.exited;
+  // The force stop leaves the probe's shim running: nothing sends `release`.
+  reap(probe);
   const points = ch.msgs.slice(mark).filter((m) => m.barrier).map((m) => [m.barrier, m.n]);
   assert.ok(points.some(([b]) => b === "phase-kill"));
   for (const [name, n] of points) {
@@ -434,6 +439,8 @@ test("W13: crash after the engine spawns, before active; restart finds the live
   c.close();
   b.send("close");
   await b.exited;
+  // The force stop leaves the shim running: nothing sends `release`.
+  reap(fx);
 });
 
 test("W14: crash after reservation, before the spawn marker: stopped with a no-unit observation; the pair is free", async () => {
@@ -705,3 +712,9 @@ test("G3: a fixture path swapped for a live path after construction is refused a
     rmSync(live, { recursive: true, force: true });
   }
 });
+
+// Last in the file. A shim ignores SIGTERM and outlives its controller, so a
+// test that leaves one running relies on the after hook's kill (#1533).
+test("no shim from this file's tests is left running for the after hook", async () => {
+  assert.deepEqual(await shimsGone(), [], "a test left its shim running");
+});

packages/conversation/tests/cohort.test.mjs

@@ -19,11 +19,11 @@ import { Controller, ELIGIBILITY, TEST_ENGINE } from "../src/controller.mjs";
 import { ENGINE_ENV, ENGINE_PIN_MISMATCH, engineEnv } from "../src/pi-pin.mjs";
 import { FixtureVerifier, newId } from "../src/records.mjs";
 import { ControlClient, FakeLauncher } from "./fake-pi.mjs";
-import { FAST, REPO, assistantEntry, claimRecords, cleanupAll, controllerFor, fixture, killChildren, noUnits, reap, receiptState, spawnController, started, tick } from "./harness.mjs";
+import { FAST, REPO, assistantEntry, claimRecords, cleanupAll, controllerFor, fixture, killChildren, killShims, noUnits, reap, receiptState, shimsGone, spawnController, started, tick } from "./harness.mjs";
 
 const reaped = [];
 const strays = new Set();
-after(() => {
+after(async () => {
   for (const pid of strays) {
     try {
       process.kill(pid, "SIGKILL");
@@ -32,8 +32,11 @@ after(() => {
     }
   }
   for (const fx of reaped) reap(fx);
+  killShims();
+  const left = await shimsGone();
   killChildren();
   cleanupAll();
+  assert.deepEqual(left, [], "no shim outlives this file (#1533)");
 });
 const track = (fx) => (reaped.push(fx), fx);
 const SCOPE = scopeAvailable();
@@ -735,8 +738,18 @@ test("K19: a scope launched with only the engine environment still reaches the u
     for (const k of names) assert.ok(ENGINE_ENV.includes(k) || ScopeLauncher.MANAGER_ENV.includes(k) || set.includes(k), k);
     for (const k of ScopeLauncher.MANAGER_ENV) if (typeof process.env[k] === "string") assert.ok(names.includes(k), k);
   } finally {
-    assert.equal((await shimRequest(socketPath, "kill", { timeoutMs: 5000 }, 8000)).ok, true);
-    assert.equal((await shimRequest(socketPath, "release")).ok, true);
+    // A failed kill or release still ends the scope here, not in the after hook.
+    const killed = await shimRequest(socketPath, "kill", { timeoutMs: 5000 }, 8000);
+    const released = killed.ok ? await shimRequest(socketPath, "release") : killed;
+    if (!released.ok) spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${unitName}.scope`], { stdio: "ignore", timeout: 5000 });
     await proc.exited;
+    assert.equal(killed.ok, true, JSON.stringify(killed));
+    assert.equal(released.ok, true, JSON.stringify(released));
   }
 });
+
+// Last in the file. A shim ignores SIGTERM and outlives its controller, so a
+// test that leaves one running relies on the after hook's kill (#1533).
+test("no shim from this file's tests is left running for the after hook", async () => {
+  assert.deepEqual(await shimsGone(), [], "a test left its shim running");
+});

packages/conversation/tests/flows.test.mjs

@@ -703,6 +703,27 @@ test("terminal: input held behind Ctrl-T waits for that takeover while an earlie
   }
 });
 
+test("terminal: a hold inside held input holds again, and once it is drained later input and Ctrl-C still reach the terminal (Darkwing note 1 on #1522)", async () => {
+  // Ctrl-T, then Ctrl-O inside the text held behind it, then "hi" Enter. The
+  // drain re-parses the held text, Ctrl-O holds again, and that hold must be
+  // drained too; otherwise `held` stays set and every later chunk is held
+  // for good.
+  for (const chunks of [["\x14", "\x0f", "hi\r"], ["\x14\x0fhi\r"]]) {
+    const { stub, sent, open } = takeoverStub();
+    let quit = false;
+    const term = new Terminal({ client: stub, onQuit: () => (quit = true) });
+    const done = Promise.all(chunks.map((c) => term.key(c)));
+    open();
+    await done;
+    assert.deepEqual(sent, ["hi"], JSON.stringify(chunks));
+    assert.equal(term.held, null, `${JSON.stringify(chunks)}: nothing stays held`);
+    await term.key("yo\r");
+    assert.deepEqual(sent, ["hi", "yo"], `${JSON.stringify(chunks)}: later input runs`);
+    await term.key("\x03");
+    assert.equal(quit, true, `${JSON.stringify(chunks)}: Ctrl-C quits`);
+  }
+});
+
 test("terminal: an action that throws still releases the input held behind it, in order, then rethrows", async () => {
   const { stub, sent, open } = takeoverStub();
   stub.takeover = async () => {

packages/conversation/tests/harness.mjs

@@ -216,4 +216,50 @@ export function reap(fx) {
       // gone
     }
   }
+  // A shim the records don't name (a direct launch), or one the systemctl
+  // calls above didn't reach: neither result is checked.
+  killShims(fx.base);
+}
+
+// The shims under `root` still running, read from /proc. A shim ignores
+// SIGTERM and only exits on `release` (src/shim.mjs), and a force stop
+// never sends `release`, so a stopped scope stays up until it is killed.
+export function liveShims(root = tmp) {
+  const out = [];
+  for (const pid of lsdir("/proc").filter((n) => /^\d+$/.test(n))) {
+    let argv, cgroup;
+    try {
+      argv = readFileSync(`/proc/${pid}/cmdline`, "utf8").split("\0");
+      if (!argv[1]?.endsWith("/shim.mjs") || argv[2] !== "--socket" || !argv[3]?.startsWith(root + "/")) continue;
+      cgroup = readFileSync(`/proc/${pid}/cgroup`, "utf8");
+    } catch {
+      continue; // gone
+    }
+    const scope = cgroup.trim().split("/").find((s) => s.endsWith(".scope"));
+    out.push({ pid: Number(pid), socket: argv[3], unit: scope ? scope.slice(0, -".scope".length) : null });
+  }
+  return out;
+}
+
+// SIGKILLs each shim's scope, which takes the engine with it, and the shim
+// itself in case the scope kill doesn't land.
+export function killShims(root = tmp) {
+  for (const s of liveShims(root)) {
+    if (s.unit) spawnSync("systemctl", ["--user", "kill", "--signal=SIGKILL", `${s.unit}.scope`], { stdio: "ignore", timeout: 5000 });
+    try {
+      process.kill(s.pid, "SIGKILL");
+    } catch {
+      // gone
+    }
+  }
+}
+
+// Waits up to `ms` for every shim under `root` to go; returns those left.
+export async function shimsGone(root = tmp, ms = 5000) {
+  const end = Date.now() + ms;
+  for (;;) {
+    const left = liveShims(root);
+    if (left.length === 0 || Date.now() >= end) return left;
+    await tick(50);
+  }
 }

packages/conversation/tests/races.test.mjs

@@ -15,13 +15,16 @@ import { EngineLink } from "../src/engine.mjs";
 import { newId } from "../src/records.mjs";
 import { ControlClient, FakeLauncher } from "./fake-pi.mjs";
 import { scopeAvailable } from "../src/cohort.mjs";
-import { controllerFor, started, receiptState, spawnController, killChildren, cleanupAll, reap, fixture, tick } from "./harness.mjs";
+import { controllerFor, started, receiptState, spawnController, killChildren, cleanupAll, reap, fixture, tick, killShims, shimsGone } from "./harness.mjs";
 
 const reaped = [];
-after(() => {
+after(async () => {
   for (const fx of reaped) reap(fx);
+  killShims();
+  const left = await shimsGone();
   killChildren();
   cleanupAll();
+  assert.deepEqual(left, [], "no shim outlives this file (#1533)");
 });
 const track = (fx) => (reaped.push(fx), fx);
 const SCOPE = scopeAvailable();
@@ -894,3 +897,9 @@ test("H23: requests pending at a restart are not resent; each shows outcome unkn
   await b.exited;
   reap(fx);
 });
+
+// Last in the file. A shim ignores SIGTERM and outlives its controller, so a
+// test that leaves one running relies on the after hook's kill (#1533).
+test("no shim from this file's tests is left running for the after hook", async () => {
+  assert.deepEqual(await shimsGone(), [], "a test left its shim running");
+});