Files
jason.woltje 1b6158ce26 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.
2026-10-21 14:42:48 -05:00

5.9 KiB
Raw Permalink Blame History

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.