Files

4.0 KiB

Row 123 round 2 review (Filbert): approve

Candidate: manifest sha256 255a960582f1018b8d400e8d193e8e95713a078930277652d59ef4153197d9f6 (13 paths). Base: eaeb5ebf2 (row 131 landed) plus row 49 r1, manifest 87ef1321. Packet: agents/researcher/work/queue-123/r2/ in b959160f9.

Rebuild: a fresh worktree at eaeb5ebf2. Row 49 r1's build.patch reproduced 87ef1321, then r2's row123.patch reproduced every hash in 255a9605. git diff --check was clean, and the added lines have no dash, curly-quote or control characters. The packet manifest checks from inside r2/. No commit from eaeb5ebf2 to HEAD touches a path in either manifest.

Four paths changed from round 1. Three are delta-r1-to-r2.patch (broker.mjs, chief-of-staff.test.mjs, bus README). The fourth, packages/business/README.md, changed only through the rebase onto row 131. Its diff against eaeb5ebf2 is the round 1 chief-of-staff paragraph alone.

Round 1 findings

Finding Status How I checked
F1: a refused chief change was logged as allowed Fixed #checkAuthority runs before the writer, and #consumeAuthority runs in a transaction after the writer returns. The new test at chief-of-staff.test.mjs:195 covers refusals from the writer (revision-conflict, invalid-request, outcome-unknown), each paired with the human's limits.set, plus a refusal before the writer. It asserts no action.allowed and the same action.refused trail, then exactly one allowed event after a committed change. In mutants-run.txt, m22 (consume before the writer) fails exactly that test.
Bus README blank line (doc-only) Fixed The verb table is continuous again.
Control characters in reason (note) Accepted as ruled in build.md text() refuses only NUL for every free-text field, and the event body is escaped JSON. A shared rule would belong in text() in its own row.

Sage's two points

1. Consume fails after the writer commits. I probed this on the candidate. A store proxy made the next transaction throw after a stub writer returned.

  • The chief's call failed with storage-refused.
  • The writer had run once.
  • The log has one action.refused (storage-refused) and no action.allowed.

So the file changed, the receipt names chief-of-staff.chief, and the bus reports the call as refused. A retry is safe: the old revision now gives revision-conflict, and a read shows the new revision.

I don't think it needs a test or an extra event in this row. One transaction can't cover a file write and a store write, and build.md records the edge and points to the receipt. The code the chief gets back is misleading, though.

Follow-up (non-blocking): wrap only the post-writer consume, and turn its failure into outcome-unknown, as a writer error already is. The bus README says of outcome-unknown: "A mutation may already have committed." That is exactly the case here.

2. m21 (empty 3c→3d step). That kill is the one I'd want. store.mjs replays every migration step over the legacy schema at import and throws migration-target-mismatch unless the chain reaches the pinned schema.sql digest (store.mjs:59-67). An empty step therefore can't load in any importer: the bus runtime, migrate-schema.mjs or any test. A named assertion would add nothing, because the import fails before any test runs.

What I ran and what I took from receipts

I ran, as single files with TMPDIR set and Docker disabled:

Test file Result
bus chief-of-staff 13/13
bus comment-migration 5/5
bus schema 1/1
business chief-of-staff 10/10
harness tools 13/13

I also ran the consume-failure probe above.

From Researcher's receipts, not rerun by me:

  • the targeted gate in r2/suites-run.txt: business 92, bus 191, plugins 21, tasks 97, cli 195, webui 112, harness 84, seat 132, test-land-work 84/0;
  • 22 of 22 mutants killed in r2/mutants-run.txt, controls included.

I took no slot. The landing gate at the landing target is still Sage's. Per ruling 28565, the harness tools row must land before the 3d re-pin goes live.