diff --git a/docs/plans/2026-10-13_landing-follow-ups.md b/docs/plans/2026-10-13_landing-follow-ups.md new file mode 100644 index 000000000..001cba498 --- /dev/null +++ b/docs/plans/2026-10-13_landing-follow-ups.md @@ -0,0 +1,235 @@ +# Follow-ups from the 2026-10-13 landings + +Three rows filed by Sage on 2026-10-13, after rows 85, 101, 102, 103 and +105 landed and refactor was pushed to 847897636. Each one collects the +non-blocking notes from a review. None of them blocks anything live +today. The live bus, Console and CLI run from the pinned copy +465bbdfbb, so code here reaches them only through a new copy and a +re-pin under Mos's go. + +## Bus host lock: release on a late throw, a vanishing lock, the third racer + +### Problem + +Row 103 (#1597) moved the bus host's claim to `host.lock`. The +reviews left four notes, Sage's N1 and Dewey's 28260 a, c and d, plus +one test weakness, Dewey's b. + +- N1 (Sage 28252). `startHost` in `packages/cli/src/host.mjs` calls + `claimHostLock` and defines `release()`. The paths that `await` a + child reply call `release()` on failure, but a synchronous throw + between the claim and the first `try` (for example `fork` throwing, + or `broker.send` throwing on a closed channel) leaves the lock held + by a host that never started. The next start then reads a stale lock + and moves it aside, so nothing breaks for good, but the lock outlives + its failed start for no reason. +- a (Dewey). `readState` checks `existsSync` and then calls + `readFileSync`. If the lock vanishes between the two (another start + moved a stale lock aside, or a live host's close released it), the + ENOENT surfaces as `bus host state is unreadable: .../host.lock`, exit + 3. The start still refuses, so there are never two hosts, but the + message points at a corrupt file that doesn't exist when a retry was + right. +- b (Dewey). The `have` order test in + `packages/harness/tests/git-tools.test.mjs` tells per-clone order from + a cross-clone sort only when `alpha` lists first. On a filesystem that + lists newest first (tmpfs), both orders give the same result and the + test passes either way. +- c (Dewey). Neither README mentions the third racer. The `claimHostLock` + comment does. With three starts on one stale lock, two can claim it, + and the broker's `writer.lock` refuses the second broker. The seat + README says two launchers "can't both run", which is true only + through that refusal. +- d (Dewey). The line row 103 added to `roles/reviewer.md` is one long + line, and the bullets around it wrap at about 70 columns. + +### Owner and reviewer + +Owner: Darkwing, after row 104 (both rows touch `packages/seat/README.md`). +Reviewer: Dewey, who reviewed row 103. + +### Files owned + +- `packages/cli/src/host.mjs`, `startHost` and `claimHostLock`/`readState` only +- `packages/cli/tests/host.test.mjs`, the host lock cases only (the + bus unit row below owns its `bus-service.sh` cases and lands after this + row) +- `packages/harness/tests/git-tools.test.mjs`, the `have` order test only +- `packages/cli/README.md` and `packages/seat/README.md`, one sentence each at most +- `roles/reviewer.md`, the wrap of the one line only +- `agents/darkwing/work/queue-/` + +### What ships + +- Everything in `startHost` after `claimHostLock` that can throw before + the broker's reply is awaited runs inside a `try` whose failure path + calls `release()`. A test makes the fork or the first send throw and + finds no `host.lock` afterward. +- `claimHostLock` treats an ENOENT on the lock read as "try again", + within `CLAIM_TRIES`, the same way it treats a lock that is gone + before the read. A test removes the lock between the existence check + and the read through a test hook, and the claim succeeds or retries, + never refusing with "unreadable". A lock that exists and can't be + parsed still refuses as unreadable. +- The `have` order test reads the directory order first and gives the + first-listed clone the older refs, so it discriminates whatever order + the filesystem lists them in. +- One sentence in the seat or CLI README says that three starts racing + one stale lock can both claim it and that the broker's `writer.lock` + then refuses the second broker. +- The `roles/reviewer.md` line wraps like its neighbours. No wording change. + +Each new check is shown to fail on a mutant that removes it. + +Suites: the cli and harness node suites and every `scripts/test-*.sh`. + +### Out of scope + +Closing the third-racer window itself. Any change to `host.json`, +`stopHost`, `hostStatus` or the broker. + +### Gate + +Dewey approves on the row's issue, naming the candidate manifest, and +the suites above pass on Sage's gate rerun. Decision 109 notices go to +Mos before and after landing. The host side is live only with a new +copy, a re-pin and a bus restart, all under Mos's go. + +## Queue add: pin the retry order and drop the row history from the refusal + +### Problem + +Row 101 (#1595) made a privileged `queue add` without `--gate-owner` +refuse. Filbert approved it in 28245 with two notes. + +- N1. The new check runs after the step that answers a retried op with + `(already recorded at rev N)`. That order is right, but no test pins + it. Every retry test passes `--gate-owner`, so hoisting the check + above the retry answer would still pass the suite. A retry of an add + recorded before row 101 landed (with `gateOwner: null`) would then be + refused instead of answered with its receipt. +- N2. The refusal message ends with "(row 100 got jason by default)". A + permanent CLI message that cites one row's history will read oddly + later. The `store.mjs` comment already carries that history. + +### Owner and reviewer + +Owner: Researcher, who wrote row 101. Reviewer: Filbert (decision 130, +not a sandbox row). + +### Files owned + +- `packages/queue/src/store.mjs`, the refusal message text only +- `packages/queue/tests/`, the row 101 test file or a new one +- `agents/researcher/work/queue-/` + +### What ships + +- A test appends a `sage` add with a null gate owner to the log, the way + `addUnder` does, retries the same op and arguments through the CLI + with no `--gate-owner`, and expects `(already recorded at rev N)` with + queue.json, QUEUE.md and the revision unchanged. +- A mutant that hoists the gate-owner check above the retry answer fails + that test. The packet shows the kill. +- The refusal message still names `--gate-owner` and why the flag is + required, without citing row 100. The existing message test changes + with it. +- `SEMANTICS` stays 2 and the log format doesn't change. The live + Console holds the old `store.mjs` (see the queue semantics note in + `packages/queue/README.md`), so nothing here may change what it reads. + +Suites: the queue node suite and `scripts/test-queue.sh`, the latter run +from a canonical-root gate, since a worktree skips its `queue verify` +case (Filbert's N3). + +### Out of scope + +Any change to who may pass `--gate-owner`, or to the default for +non-privileged seats. + +### Gate + +Filbert approves on the row's issue, naming the candidate manifest, and +the suites above pass on Sage's gate rerun. + +## Bus unit drop-in guard, script usage, and the copy wrapper + +### Problem + +Row 85 (#1579) put the bus, Console and CLI on one pinned copy. Filbert +(28256) and Darkwing left these notes. + +- Filbert N1, also Darkwing's N1. `scripts/bus-service.sh install` + refuses when a `mosaic-bus@*.service.d/*.conf` drop-in overrides + `ExecStart` or `WorkingDirectory`. systemd also applies prefix + drop-ins (`mosaic-.service.d/`, `mosaic-bus@.service.d/` is already + covered) and the all-services `service.d/`, and either could set + `WorkingDirectory`. Neither directory exists on this host today. +- Darkwing N2. The header comment of `scripts/bus-service.sh` lists + `render` and `install` without `--copy`, `--copies` or `--pm`. The + `USAGE` string has them. +- Darkwing N3. The fake `systemctl` in the install tests still has + `show` arms that no test reaches since the guard stopped calling + `show` on the template. +- Sage, at the live window. `scripts/stack-copy.sh wrapper` rewrites + `~/.npm-global/bin/mosaic` with only the Q81 marker, the lease line + and the exec line. The three comment lines that said where the CLI + runs and why the lease probe still answers at `mosaic-old` were lost. +- Sage, at the live window. `npm ci` in `stack-copy.sh copy` warned + that three install scripts were blocked by `allowScripts`: + `@google/genai` (preinstall, a no-op echo), `esbuild` (postinstall) + and `protobufjs` (postinstall). The copy's bus, Console and CLI + checks passed, but nobody has recorded why the copy doesn't need + them. + +### Owner and reviewer + +Owner: Rocko, who wrote row 85, after the host lock row above (both +touch `packages/cli/tests/host.test.mjs`). Reviewer: Filbert (decision +130). + +### Files owned + +- `scripts/bus-service.sh`, the drop-in guard and the header comment +- `scripts/stack-copy.sh`, the `wrapper` output only +- `packages/cli/tests/host.test.mjs`, the `bus-service.sh` cases only, + and `packages/cli/tests/stack-copy.test.mjs` +- `docs/TOOLS.md` and `packages/cli/README.md`, the bus unit and copy + paragraphs only +- `agents/rocko/work/queue-/` + +### What ships + +- The guard also globs `mosaic-.service.d/*.conf` and + `service.d/*.conf` in every directory from + `systemd-analyze --user unit-paths`, with the same refusals on an + unreadable directory or file. A test puts a `WorkingDirectory` line + in each and gets a refusal with the unit file unchanged. +- The header comment lists every flag the `USAGE` string has. +- The dead `show` arms are gone from the install fakes, or a test + reaches them. The packet says which. +- `wrapper` writes a comment after the marker that names the copy root + and says the lease probe answers at `mosaic-old` (Q81). `check` still + reads the marker on line 2 and the exec on the last line, and a test + shows `check` passes on the new wrapper and refuses a foreign one. +- The packet runs `npm install-scripts ls` (or reads the lockfile) and + says for each of the three packages whether any file in the copy's + paths loads it at runtime. If none does, `packages/cli/README.md` + says the warning is expected and why. If one does, the row stops and + Sage decides on `allowScripts`. + +Each new check is shown to fail on a mutant that removes it. + +Suites: the cli node suite and every `scripts/test-*.sh`. + +### Out of scope + +Running `install`, `wrapper` or `copy` against the live units or +`~/.npm-global/bin`. Those are Mos-gated live steps. Changing which +paths the copy holds. + +### Gate + +Filbert approves on the row's issue, naming the candidate manifest, and +the suites above pass on Sage's gate rerun. The scripts take effect on +the live setup only at the next copy and re-pin under Mos's go.