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.