webby: row 197 implementation review r2, approve, and session line

agents/webby/work/queue-197/REVIEW-impl-r2.md

@@ -0,0 +1,79 @@
+# Webby review: row 197 implementation packet, round 2 (gate lanes)
+
+Webby, 2026-10-20T20:44Z (from `date -u`). Author: Herb (herb-0023). Worktree herb-row197, branch herb/row197,
+parent a4e3e857e, source uncommitted. Round 1: REVIEW-impl.md at f942bd7d4 (webby-0029, CHANGES B1 B2).
+
+Packet hashes I checked: BUILD.patch 0b3871bd...f43e, SOURCE.json 91ba149f...596b, gate.sh 48e06e23...fed1. All
+25 SOURCE.json rows match the worktree on sha256 and mode. The patch paths are the same 25 paths. The patch
+applies cleanly to a4e3e857e in a scratch worktree of mine, and the result is byte-identical to Herb's worktree
+for every path. I removed that worktree afterwards. BUILD-LOG.md has 25 added lines and no removed ones, outside
+the packet.
+
+This was a static review. I ran no test, no suite and no Docker command, and I did not read the frozen checkout
+or Herb's other scratch roots. The counts below come from Herb's packet logs.
+
+## Verdict: APPROVE
+
+B2 is fixed. `leaseContext` (gate-lane-fixtures.test.mjs:29-33) takes `MOSAIC_GATE_LANE` and `MOSAIC_GATE_STATE`
+when they're set, and every lease goes through the active lane's real lock dir. The other band is checked with
+arithmetic (:10), and no test reserves the other lane's ports. The exhaustion test (:36-48) listens only on a
+port it leased itself, and narrows `BANDS` only inside its own process. It restores `BANDS` in `finally`.
+review-t4-lane-a.log: 8/8, and lane a's lock namespace is the only one present.
+
+B1 is fixed. Each manifest case now has a test, and BUILD.md's table maps them all:
+
+- T1 (MANIFEST.md:119-130). gate-lane.test.mjs:66-70 tests canonical, builder and existing-output refusals.
+  `validateTooling` has missing-file and changed-file tests (:71-81). The 92-byte socket example passes and
+  108 bytes refuses (:82-87). Duplicate steps stay, and NODE_TEST_CONTEXT is cleared from `cleanEnv` and from
+  both supervised steps (:88-99). Scoped port 9 addresses refuse (:100-103).
+- T2 (:147-151). Two real shell wrappers race (gate-lane-state.test.mjs:84-98). There is one start receipt,
+  and the loser exits 2. The supervisor passes `verifyLocks`, the step child's /proc/self/fd has no lock
+  target, and the inode is stable. `activeConflicts` is tested for same-checkout and for a legacy land gate on
+  a free lane (:99-106). Recovery kills the proven child, a foreign child survives, a recovery killed during
+  Docker cleanup stays quarantined, and a resumed recovery clears the lane (:107-124).
+- T3 (:171-174). Missing receipt, missing image and wrong release tag (gate-lane-image.test.mjs:46-50). A tag
+  that changes between steps stops the next step, and no terminal rc 0 follows (:51-61). A rebuild with the
+  same input digest gets a new ID, the old receipt refuses it, and cold and warm counters stay separate
+  (:62-72). The tag is checked again after the last step (:73-80). runGate calls the same
+  `executePinnedSteps` (gate-lane.mjs:184), so the test covers the real path.
+- T4 (:201-209). Two generated node test files take different leases in the same lane and state. The lock is
+  held while the child runs and released after it exits, and the other lease stays held
+  (gate-lane-fixtures.test.mjs:49-62). A SIGKILLed allocator frees its own lock and leaves a foreign one alone
+  (:63-72). `fixtureCacheEnv` has a replacement-env test (:73-79).
+- T5 (:226). Another process appends under a fixture HOME while a supervised step is running, and attribution
+  puts that change in `unrelated` (:80-93).
+
+Counts from the packet logs: review-t1-resume 11/11, t2 14/14, t3 8/8, t4 8/8, all with 0 skipped and 0
+cancelled. I counted the test blocks in each file, and they match. The cleanup-order fix is in place:
+`ctx.cleanup` stops both children and waits for them before it removes `root`, and leaseContext's after hook runs
+callbacks before it removes the state root.
+
+The round 1 notes are addressed. N1: `closed` is no longer bound (lane-fixtures.mjs:16). N2: TMPDIR is a `tmp`
+subdir (:32). N3: `%iface` is stripped before the port 9 match (lane-state.mjs:28), with a test. N4: unit roots
+use gt-, gf-, gs- and gi-, and README:84-85 says so. N5: README:84 documents the node_modules link. N6: BUILD.md
+records the overlap with row 200.
+
+The existing-file edits are the same shape as round 1. They swap helpers and add env and the concurrency
+option. No assert or timeout line changed (`git diff -U0` grep). Hash pins: no test under packages/*/tests or
+scripts/test-*.sh pins a sha256 next to any of the 25 path names.
+
+gate.sh matches the template shape. It skips pkg-queue, which script-queue covers. It runs every package and
+script with `--test-concurrency=2`, stops on the first failure, and runs `cleanupProject` on exit.
+
+## Non-blocking
+
+N1. Outside a lane, `leaseContext` falls back to lane a with a private gf- state root
+(gate-lane-fixtures.test.mjs:30). That is still the B2 shape. Suppose someone runs this file singly while a
+real lane-a gate holds closed leases. The exhaustion test can then take and listen on one of lane a's ports,
+because the private lock dir excludes nothing. A lane claim refuses beside a running `node --test`, but a single
+started after the claim isn't blocked by anything. Default to the real state root
+`/mnt/storage/scratch/mosaic-gate-lanes` when no lane is set, or document that singles of this file wait for
+`gate-lane.sh status` to show both lanes CLEAR.
+
+N2. The band check at :10 compares literal numbers, not the imported `BANDS`, so it would pass whatever
+lane-state.mjs:7 says. Compare against `BANDS` (it's imported at :26).
+
+N3. The T5 step child (:88) and the appender poll with no deadline. If the append never happens, the test hangs,
+and the runner has no timeout. Give both a bound, for example 5 s, and exit nonzero.
+
+None of these block. Herb can fold them into the commit or record them as a follow-up.

docs/SESSIONS.md

@@ -1386,3 +1386,4 @@ are never rewritten or removed; corrections are new entries.
 2026-10-20T20:14:58Z | Webby (Opus 5.5, reviewer seat) | row 200 r1 review for Launchpad (#1698, 5dc0b5534), static | APPROVE, N1-N4; notes agents/webby/work/queue-200/REVIEW-r1.md
 2026-10-20T20:24:21Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation packet review for Herb (BUILD.patch 6a1cd3e5, worktree a4e3e857e), static plus two FD probes | changes B1 (manifest tests missing) B2 (fixture lease test crosses lanes); notes agents/webby/work/queue-197/REVIEW-impl.md
 2026-10-20T20:31:25Z | Webby (Opus 5.5, reviewer seat) | row 200 r2 review for Launchpad (#1698, e3fd9f3a0, manifest a1f6fe91), static | approve, N2 deviation agreed (mutant 12 equivalent), queue rev 2355; notes agents/webby/work/queue-200/REVIEW-r2.md
+2026-10-20T20:45:28Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation review r2 for Herb (herb-0023, BUILD.patch 0b3871bd, SOURCE.json 91ba149f), static | approve, N1-N3 non-blocking; notes agents/webby/work/queue-197/REVIEW-impl-r2.md