docs: briefs for the re-test faults, seat git identity and review request held

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-13 15:12:49 -05:00
parent 0c3826910e
commit 18c29c28d3
1 changed files with 139 additions and 0 deletions
@@ -0,0 +1,139 @@
 # Faults from Jason's PM re-test on 664199f7
 Jason's PM re-test finished on 2026-10-13 (Mos, PM run rcc0f72be8b17,
 Vikunja #228). The #229 loop worked end to end: the coder pushed
 010d30cb, the reviewer approved it after running its tests, and the PM
 closed the task. Mos listed four faults. Sage checked two of them and
 neither holds, so neither gets a row:
 - The reviewer did run the tests. Its own transcript shows
  `node --test` in a clean worktree at 010d30cb (14 pass) eight seconds
  before its verdict. Its first prompt was the coder's REVIEW-REQUEST,
  so it didn't need the PM's later message.
 - The PM's clone wasn't stale. Row 95's landing 17ad6f35 is an ancestor
  of both commits the clone has held (4da68152 at clone, 664199f7 after
  the launch fast-forward), and `app.js` and `board-held.test.mjs` are
  tracked in it. The PM's search missed the code.
 The other two get rows here.
 ## Seats commit under their own name
 ### Problem
 A role session runs with HOME set to the run's empty `home/` and no git
 identity. The coder's first commit failed with "Author identity
 unknown", and the coder got past it with `git config user.name` and
 `user.email` in its clone.
 That workaround breaks the next launch. `allowed()` in
 `packages/seat/src/checkout.mjs` keeps a clone whose config holds any
 key outside its list, so the coder's clone will be kept, not
 fast-forwarded, from now on. Its config holds both keys today.
 A related confusion: the PM said its clone of `stack` on branch
 `refactor` had no Board code when it did. Nothing tells a seat which
 branch and commit its clone holds or when it refreshes.
 ### Owner and reviewer
 Owner: Darkwing, after row 103 (both rows touch `packages/seat`).
 Reviewer: Dewey (a sandbox row, decision 130).
 ### Files owned
 - `packages/seat/src/sandbox.mjs` and whichever launch file passes it
  the role and business, with their tests
 - `packages/harness/src/bundle.mjs`, one prompt line, and its test
 - `packages/seat/README.md`
 - `agents/darkwing/work/queue-<row>/`
 ### What ships
 - Every role session's sandbox sets `GIT_AUTHOR_NAME`,
  `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME` and `GIT_COMMITTER_EMAIL`:
  the name is the role, the email `<role>@<business>.mosaic.invalid`. A
  test commits in a sandboxed clone with no config and reads the author
  back.
 - The clone's config needs no new key, so `allowed()` doesn't change.
 - The bundle prompt gets one line: the clone's branch and the commit it
  was at when the session started, that it refreshes only at launch, and
  that setting git config in it stops the next launch from updating it.
 - The README says where the identity comes from.
 Each new check is shown to fail on a mutant that removes it.
 Suites: the seat and harness node suites and every `scripts/test-*.sh`.
 ### Out of scope
 Changing `allowed()`. Signing. Fixing the coder's existing clone: after
 landing, with Mos's go, Sage unsets the two keys so the next launch
 updates it again.
 ### 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. The launch side is live only at a bus restart,
 and that needs Mos's go.
 ## A review request says whether a reviewer is running
 ### Problem
 `review.request` in `packages/bus/src/broker.mjs` routes the
 REVIEW-REQUEST to whichever run holds the reviewer role, or to nobody
 when no run holds it. The message then waits until a run claims the
 role. The reply is the same either way.
 In the re-test the coder asked before any reviewer held the role. The
 PM happened to launch one, which got the request as its first prompt,
 so the loop worked. But the coder couldn't tell that its request was
 waiting on nobody, and nothing told anyone to launch a reviewer. Had
 nobody launched one, the request would have sat there unseen.
 Refusing the request isn't the fix. Delivery on claim is what lets the
 PM launch a reviewer after the coder asks, and that path worked. The
 fix is to say so.
 ### Owner and reviewer
 Owner: Researcher (a Haiku subagent may build it, decision 129).
 Reviewer: Filbert (decision 130).
 ### Files owned
 - `packages/bus/src/broker.mjs`, the `review.request` case only, and
  its tests
 - `packages/harness/src/tools.mjs`, the `request_review` description
  only, and its test
 - `packages/bus/README.md`, the review paragraph only
 - `agents/researcher/work/queue-<row>/`
 ### What ships
 - The reply adds `held`: the run id holding the reviewer role when the
  request was routed, or `null`. Tests cover both, and the existing
  reply fields don't change.
 - The `review.requested` event records the same value.
 - The `request_review` description says what `null` means: no reviewer
  is running, the request waits for one, and the requester should tell
  whoever launches roles.
 - The README says the same.
 Each new check is shown to fail on a mutant that removes it.
 Suites: the bus and harness node suites and every `scripts/test-*.sh`.
 ### Out of scope
 Refusing a request to an unheld role. Launching a reviewer
 automatically. The Console. Plain `message.send` to an unheld role.
 ### Gate
 Filbert 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. The bus side is live only at a bus restart, and
 that needs Mos's go.