row 250: K12 holds the stop's release while it reads the shim, the release answers the events read's reason, and R13 closes its controller (#1755)

Candidate gizmoduck/row250-r2 c1a858975 on 3b0aef995, manifest eca0b8e2.
Filbert APPROVE rev 2957 (#1755 c30130). Gate on Sage's slot (grant sage-2109,
gizmoduck-0150): P1/P2/M1 probes as predicted, conversation whole 3x 183/183
and a concurrent pair 183/183 each, control-board whole 124/124.

agents/gizmoduck/work/queue-250/BUILD.md

@@ -0,0 +1,100 @@
+# Row 250 build notes (Gizmoduck)
+
+Vikunja #450, Mos ruling mos-1182 item 3. Brief: docs/plans/2026-10-21_k12-cohort-events.md. Reviewer
+Filbert, gate owner Sage. Branch gizmoduck/row250 from origin/refactor 3b0aef995. Line numbers below are on
+this branch unless a commit is named.
+
+## Step 1
+
+cbba6ff07 changes only the K12 assertion (packages/conversation/tests/cohort.test.mjs:292 on 3b0aef995).
+On failure it prints the shim's answer, with `ok` and `unavailable`, and `h.ctrl.evidence.releases`.
+
+## Step 2: the cause
+
+K12 reads the shim after the controller has started to release it.
+
+- The force stop sets the binding `stopped` (packages/conversation/src/controller.mjs:1615). Then it pauses at
+  `scope-release` (:1623) and calls `#releaseScope` (:1624).
+- `#releaseScope` calls `releaseCohort` (:1639). That sends `hello`, then `release`
+  (packages/conversation/src/cohort.mjs:261-265).
+- The shim answers `release` and exits 10 ms later. It unlinks its socket first (shim.mjs:165-176). systemd
+  then collects the empty scope, `engine` included.
+- K12's `forceStop` helper returns once it sees `stopped` (cohort.test.mjs:148, polling every 10 ms). The
+  test then reads `engine/cgroup.freeze` (:281 on 3b0aef995) and asks the shim for `events` (:291).
+
+Nothing in K12 orders those reads before the release. Unloaded, the test usually sends its `events` request
+within the 10 ms before the shim exits, so it passes. Under load the release wins. `shimRequest` then
+returns `{ ok: false, unavailable: "the shim socket is gone" }` (cohort.mjs:46), or "unreachable" or
+"closed the connection" (:62-63), and `ev.populated` is undefined. Both recorded failures fit this
+pattern:
+
+- Row 197 proof 4, pair1-b: K12 failed in 339 ms on `undefined !== 0` at :292. 339 ms is far below the
+  3 s `shimRequest` timeout (cohort.mjs:56), so the request didn't time out. The `cgroup.freeze` read at
+  :281 passed, so the scope still existed then.
+- BUILD-LOG.md:4092, row 56's gate: "`populated` undefined with load around 12".
+
+Row 50 (a9cc522a0, #1536) added the release after a proven stop. K12 is older. Its comment still says
+"the shim holds the scope", and that stopped being true in row 50. E1 (cohort.test.mjs:923-925) already
+holds `scope-release` so it can read the shim while the shim still answers. K12 never got the hold. No
+other test reads the shim after a proven stop and expects an answer. R1 expects the shim gone, R3 is an
+uncertain stop, and K15 reads before its stop.
+
+The cgroup read itself has no race. The kill's `waitFor` (shim.mjs:117-126) had already read `populated 0`
+for the stop to be proven. The read is the shim's call to `events()` (shim.mjs:83-96), and it reads one
+kernel file. So the brief's allowance for a bounded retry doesn't apply, and there is none.
+
+The fix goes in the test's setup, which the brief allows ("Use the shim, the cohort controller or the
+test's setup and teardown, whichever the evidence points to"). The controller's release is correct and
+specified (README "Scope release", R1). Delaying it for a reader would change row 50's behavior.
+
+## Step 2: the change
+
+packages/conversation/tests/cohort.test.mjs, K12:
+
+- It runs with `gate()`, holds `scope-release` before the stop, and waits until the hold is reached. Then it
+  does every read that needs the scope: `cgroup.freeze`, `events` and `members`.
+- After the reads it lets the release go and asserts one entry, `["force-stop", "released"]`. This is new
+  coverage: K12 now also shows the forking cohort's scope released.
+- The assertions are unchanged. `populated` must be 0 and the member list must be empty. The step 1 message
+  stays.
+
+packages/conversation/src/shim.mjs:167-168, `release` (:167 on 3b0aef995). It answered a failed `events()` with "the engine
+cgroup is not empty", so it reported unavailable as not empty. The refusal was already safe, but the reason
+was false. Now an `events()` answer that isn't `ok` is returned as it is, with the read's own reason. Only
+`populated` 1 gives "not empty". Nothing else matched that string (grep of packages/).
+
+## Tests
+
+- K12 as above.
+- R13, the brief's new case, placed after R3. After a proven stop, with `scope-release` held, the test makes
+  `engine/cgroup.events` mode 000. Then:
+  - `events` returns `ok: false`, has no `populated`, and the reason is "engine/cgroup.events unreadable
+    (EACCES)".
+  - On release, the controller records one entry `["force-stop", "unavailable", <that reason>]`.
+  - The unit stays active, and the shim still answers `hello`.
+
+  Then `close()` reuses the stop's release, so there is still one entry. The `finally` closes the
+  controller again (a second close is a no-op), restores mode 444, the kernel's mode for cgroup.events,
+  and releases the scope. On
+  3b0aef995's shim.mjs, R13 fails on the reason, because it reads "the engine cgroup is not empty".
+- K15 already covers a stop, as opposed to a release, with `events` unavailable: the shim gone, the file
+  mode 000, or `engine` missing, each ending `uncertain` with its reason.
+
+## File added beyond the brief
+
+packages/conversation/README.md:599 listed "R1–R3, R5–R12" for cohort.test.mjs. With R13 it now reads
+"R1–R3, R5–R13". The brief's files don't include the README. Sage added it to this row in sage-2082.
+
+## Runs
+
+None yet. They need Sage's slot. Sage's sage-2082 says the run is on the pushed candidate in a clean
+worktree, and that it adds the control-board suite whole, so it counts as the gate run.
+
+## Round 2
+
+Filbert's C1 (#1755 comment 30125): R13 never closed its in-process controller, so the controller's
+socket server (controller.mjs:548) stayed open, and `reap` only kills units and engines. Only `close()`
+closes it (:1789). R13 now calls `h.ctrl.close()` after its checks and asserts one release entry, as R2
+does. Its `finally` also closes the controller, for a failure partway through. The second close is safe:
+`server` is null by then (:1791), and `#releaseScope` returns the cached release (:1634-1648). Branch
+gizmoduck/row250-r2, on e9cfd92f8.

packages/conversation/README.md

@@ -596,7 +596,7 @@ no prompt:
 | `claim.test.mjs` | W1–W17, W20, G1–G3: the writer claim, crash barriers, the guard |
 | `races.test.mjs` | H1–H4, H9–H23: takeover, Interrupt and force stop, retries, incarnations |
 | `turns.test.mjs` | N1–N25, N24b: the turn tracker against the fake engine's Pi behaviors; the engine seal and environment |
-| `cohort.test.mjs` | K1–K19, R1–R3, R5–R12, E1–E6: scopes, force stop, proofs, recovery, eligibility, the scope's environment, scope release and its retry after a crash, the engine-exit stop (needs a systemd user manager) |
+| `cohort.test.mjs` | K1–K19, R1–R3, R5–R13, E1–E6: scopes, force stop, proofs, recovery, eligibility, the scope's environment, scope release and its retry after a crash, the engine-exit stop (needs a systemd user manager) |
 | `flows.test.mjs` | S1–S7, P3, E1–E7, the terminal, and a CHAT-01 schema check of every record produced |
 | `smoke.test.mjs` | the pinned Pi binary, as above |
 

packages/conversation/src/shim.mjs

@@ -164,7 +164,8 @@ async function handle(req) {
       return waitFor((e) => e.populated === 0, Number(req.timeoutMs) || 5000);
     case "release": {
       const e = events();
-      if (!e.ok || e.populated !== 0) return { ok: false, unavailable: "the engine cgroup is not empty" };
+      if (!e.ok) return e;
+      if (e.populated !== 0) return { ok: false, unavailable: "the engine cgroup is not empty" };
       setTimeout(() => {
         try {
           unlinkSync(socketPath);

packages/conversation/tests/cohort.test.mjs

@@ -265,14 +265,19 @@ test("K5: a stop during a tool call leaves the effect uncertain, and it is shown
 // every child the loop forked after its SIGTERM, which nothing else ends
 // before the kill, is in the proof's list. Mutants r2-B5 (no freeze write,
 // `frozen 1` answered) and r2-B5b (freeze written, not waited on) fail here.
+// The stop's own release ends the shim and the scope (#1536), so the test
+// holds it until the reads are done (row 250).
 test("K12: a member forking in a loop: the freeze stops it, enumeration is complete, populated 0 after cgroup.kill", NEEDS_SCOPE, async () => {
-  const h = await live();
+  const g = gate();
+  const h = await live({ barrier: g.barrier });
   try {
     const pidLog = join(h.fx.base, "fork-pids.log");
     const forker = await childOf(h, { forkLoop: true, ignoreTerm: true, pidLog });
     await until(async () => (await memberPids(h.rec().shim)).length >= 6, 4000, "the fork loop");
     const engineDir = join("/sys/fs/cgroup", (await shimRequest(h.rec().shim, "hello")).scope, "engine");
+    g.hold("scope-release");
     await forceStop(h);
+    await g.waitHeld("scope-release");
     assert.equal(h.ctrl.binding.state, "stopped", JSON.stringify(h.ctrl.evidence.stops));
     const { members } = h.ctrl.stoppedProof.proof;
     assert.ok(members.some((m) => m.pid === forker));
@@ -289,9 +294,14 @@ test("K12: a member forking in a loop: the freeze stops it, enumeration is compl
       assert.ok(listed.has(pid), `child ${pid} forked after TERM is in the proof's list`);
     }
     const ev = await shimRequest(h.rec().shim, "events");
-    assert.equal(ev.populated, 0);
+    // On failure, name the cause: the answer with `ok` and `unavailable`, and any release already run (row 250).
+    assert.equal(ev.populated, 0, JSON.stringify({ ev, releases: h.ctrl.evidence.releases }));
     assert.deepEqual(await memberPids(h.rec().shim), []);
+    g.release("scope-release");
+    await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the release");
+    assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome]), [["force-stop", "released"]]);
   } finally {
+    g.release("scope-release");
     await h.close();
   }
 });
@@ -852,6 +862,45 @@ test("R3: no release after an unavailable stop or an unverified proof: the scope
   }
 });
 
+// `release` reads engine/cgroup.events again. An unreadable file answers with
+// the read's own reason, never "not empty", and the controller records the
+// release unavailable with that reason (row 250).
+test("R13: a release with engine/cgroup.events unreadable answers the read's reason; the controller records it unavailable and the scope stays", NEEDS_SCOPE, async () => {
+  const g = gate();
+  const h = await live({ barrier: g.barrier });
+  const unit = h.rec().unitName, shim = h.rec().shim;
+  let events = null;
+  try {
+    g.hold("scope-release");
+    await forceStop(h);
+    await g.waitHeld("scope-release");
+    assert.equal(h.ctrl.binding.state, "stopped", JSON.stringify(h.ctrl.evidence.stops));
+    events = join("/sys/fs/cgroup", (await shimRequest(shim, "hello")).scope, "engine", "cgroup.events");
+    chmodSync(events, 0o000);
+    const ev = await shimRequest(shim, "events");
+    assert.equal(ev.ok, false, JSON.stringify(ev));
+    assert.equal("populated" in ev, false, JSON.stringify(ev));
+    assert.equal(ev.unavailable, "engine/cgroup.events unreadable (EACCES)");
+    g.release("scope-release");
+    await until(() => h.ctrl.evidence.releases.length > 0, 8000, "the release");
+    assert.deepEqual(h.ctrl.evidence.releases.map((r) => [r.why, r.outcome, r.reason]), [["force-stop", "unavailable", ev.unavailable]]);
+    assert.ok(unitActive(unit), "the scope stays up");
+    assert.equal((await shimRequest(shim, "hello")).ok, true, "the shim got no release");
+    await h.ctrl.close();
+    assert.equal(h.ctrl.evidence.releases.length, 1, "close reuses the stop's release");
+  } finally {
+    g.release("scope-release");
+    await h.ctrl.close();
+    if (events) {
+      chmodSync(events, 0o444);
+      await releaseCohort({ kind: "scope", unitName: unit, invocationId: h.rec().invocationId, shimSocket: shim });
+    }
+    h.c.close();
+    h.fake.close();
+    reap(h.fx);
+  }
+});
+
 // A normal engine exit leaves the shim and the scope: the shim exits only on
 // `release`. The controller doesn't release then: the binding is uncertain
 // and the scope is what a force stop reads. The proven stop releases it.