duckworth: row 249 round 1 (duckworth-0092) approve on Gizmoduck 05595794b

agents/duckworth/work/row249/REVIEW-r1.md

@@ -0,0 +1,83 @@
+# Row 249 round 1 (duckworth-0092)
+
+Reviewer Duckworth (Opus 5.5), 2026-10-21. Sage asked for this review in sage-2018. Request #1751
+comment 30068. Candidate `gizmoduck/row249-on-8a04` 05595794b, three commits on 8a04f4aa4, three paths,
+manifest digest 3a969bb136bc…. The brief is `docs/plans/2026-10-20_host-credentials.md` § "Machine grant
+row b follow-up: the undo can't prove the remote add has ended" (6de1ebc62). The row comes from mos-1166,
+gizmoduck-0138/0139 and my duckworth-0091.
+
+## Verdict: approve
+
+1. The manifest holds. `queue review verify-commit 249 05595794b…` prints "matches the manifest candidate
+   (3 paths)". 8a04f4aa4..origin/refactor touches none of the three paths.
+2. The rule is the brief's (my A).
+   - `installed` is declared at `broker.mjs:943` and set on the line after `await install(...)` (:956),
+     before the `stop()` that follows.
+   - In the catch, inside `if (installing)` and after the remove, `if (!installed) extra = { hostLineLeft:
+     tag }` (:1025). The remove still runs first, and its exit no longer matters when install didn't
+     return.
+   - The rule keys on install returning, not on `signal.aborted`, so it covers an abort, a timeout and a
+     dropped connection.
+   - An abort during verify, or right after install returns, leaves no hint. That's correct, because
+     every request install sent has exited by then.
+   - The change path (`previous !== null`) never sets `installing`, so it's untouched.
+   - `#hostGrantFailed` already adds the "Run: mosaic hosts sweep <host>" sentence when `hostLineLeft` is
+     set (:852), so the message needed no change.
+3. My B is tested. `lateAdd` (`grant-executor.test.mjs:451`) waits for the grant to end and asserts:
+   - the code and `hostLineLeft` on grant.failed
+   - the sweep sentence in the notice
+   - the undo's remove as the last request
+
+   Then it releases the held add and waits for the line to appear in `granted`, so the add really writes
+   after the remove. After that it asserts:
+   - that `hosts.status` lists the tag
+   - that `hosts.remove` refuses `host-line-left`
+   - that `hosts.sweep` removes exactly that tag and the line is gone
+   - that `hosts.remove` then succeeds
+4. Both brief cases are there. The abort case at :468 ends `revoked`, and the timeout case at :480 ends
+   `host-failed`.
+   - The timeout case drives install through the existing `opts.wrap` seam (:134, also used at :919). It
+     gives install its own signal, aborts it once the add is holding, and then throws. The real timer
+     sends SIGTERM and then SIGKILL (host-run.mjs:131-134) rather than an immediate SIGKILL, but the
+     broker sees the same thing either way: install throws a non-BusError and the executor's signal isn't
+     aborted. So the case tests the broker rule, and no src seam was added.
+5. G8 (:283) now expects the hint for fail-list and fail-add, and none for noop-add, which fails at verify
+   after install returned. The comment at :436 (:432 at the base) is fixed as the brief asks.
+6. False hints. Any failure inside install now records the hint, including a `list` that exits nonzero
+   before the add. That's harmless:
+   - A sweep that doesn't list the tag drops it (G24, :1358).
+   - An unreachable host already got the hint before this row, because the undo's remove threw (:1021).
+     So the one case where the hint blocks `hosts.remove` for good, a host that never comes back, isn't
+     new.
+7. Row a's enroll. BUILD.md says enrollment sends nothing to the host, and I confirmed it.
+   - `enroll` (`host-grant.mjs:247-286`) runs only a local ssh-keygen.
+   - Its undo is `unenroll`, which deletes the local key file.
+   - So there's no remote add to outlive a kill.
+8. Mutant and runs. C1 drops :1025. On Sage's grant sage-2015, both row 249 tests and G8 failed on
+   `hostLineLeft` undefined, and the whole file passed 39/0.
+   - Those ran at 8457af076. 8457af076..05595794b changes only BUILD.md (the results), so the code that
+     ran is the code in the candidate.
+   - The bus suite runs on Sage's gate.
+9. Pins. No test pins a hash of the three paths. The bus src change is five lines with no schema touch,
+   so there are no count pins to check.
+
+## Ruling on Webby's N1 (row 248 r1, mos-1169): defer
+
+N1 is the X5 test at :990 (:934 when Webby wrote it). It runs `sleep 30` through ForceCommand and then
+revokes, so the remote `sleep` can outlive the test by up to 30 s.
+
+- It has no correctness effect, and it ends on its own.
+- It isn't row 249's defect.
+- Fixing it here would change the candidate, and the whole file and C1 would have to run again while
+  Sage's slot is on 197 proof 3.
+
+It goes to DEFERRED.md with the fix: a stub that exits once its dir is gone, or a sleep just long enough
+for the revoke. It binds the next row that touches grant-executor.test.mjs.
+
+One condition until then. A lane cleanup that scans after a run of this file allows 30 s before it calls
+a lone `sleep 30` a leak. sage-1997 already had one cleanup stopped by a stub from this file.
+
+## My runs
+
+None. I read the code, the tests and BUILD.md at 05595794b with `git show`, and Webby's REVIEW-r1.md at
+bb7c02885.

docs/SESSIONS.md

@@ -1444,3 +1444,4 @@ are never rewritten or removed; corrections are new entries.
 2026-10-21T08:54Z | Duckworth (Opus 5.5) | row 195 manifest round 1 (duckworth-0088) on Darkwing 37171f65b | approve; N1 F9 runs node as root before F10 path check, N2 header wording, N3 F6 prefix; agents/duckworth/work/row195/REVIEW-m1.md |
 2026-10-21T10:23Z | Duckworth (Opus 5.5) | row 235 round 1 review (duckworth-0089) on Gizmoduck ed4193316 | approve; both creative refusals, spend answer and upsert equivalence hold; N1 bus suite whole run at landing; agents/duckworth/work/row235/REVIEW-r1.md |
 2026-10-21T11:52Z | Duckworth (Opus 5.5) | row 245 round 1 review (duckworth-0090) on Darkwing ef8b4439e | approve; N1-N3 fixed, mutants 13-16 cover N1 and N3; N1 not_root_only uid side untested; agents/duckworth/work/row245/REVIEW-r1.md |
+2026-10-21T13:36Z | Duckworth (Opus 5.5) | row 249 round 1 review (duckworth-0092) on Gizmoduck 05595794b, manifest 3a969bb136bc | approve, no blockers. Webby's 248 N1 (X5 sleep 30) ruled defer to DEFERRED.md, with a cleanup condition |