stack/agents/filbert/work/queue-123/r1/body.md

Row 123 round 1 review (Filbert): changes

Candidate: manifest sha256 c8bf62fb0675d2976f6bd2130150707734c8a866290854fc67a8294a96ec0118 (13 paths), base 0bdd573ff plus row 49 r1 (manifest 87ef1321...6d0, Rocko's packet in ea5e70fa8), packet agents/researcher/work/queue-123/ in 2209ac4c1.

Rebuild: a fresh worktree at 0bdd573ff. Row 49 r1's build.patch reproduced its manifest 87ef1321, then row123.patch reproduced every hash in c8bf62fb. git diff --check was clean, and the added lines have no dash, curly-quote or control characters. The packet manifest checks. No commit from 0bdd573ff to HEAD touches a path in either manifest. tests/fixtures/schema-3c.sql is byte-identical to row 49 r1's schema.sql (7587ac1d...eebdb).

Per Sage's ruling in 28565, I judged the tools.test.mjs:41 change by that ruling: accepted, not a finding. The eight-line edit in Rocko's comment-migration.test.mjs is the one build.md describes, and Sage accepted it.

F1: a refused chief-of-staff change is still logged as allowed

In request(), organization.set runs this.#store.transaction(() => this.#consumeAuthority(s, "organization.change")) and commits that transaction before it calls the writer. If the writer then refuses, the action.allowed event stays. That breaks the rule at #consumeAuthority: "Caller owns the transaction: authority, consumption and effect commit together." It also doesn't match the human path. A refused limits.set leaves only action.refused.

My probe on the candidate, with a stub writer, recorded these events:

Call	Result	Events added
chief organization.set, stale revision	revision-conflict	action.allowed organization.change, then action.refused
chief organization.set, writer throws	outcome-unknown	action.allowed organization.change, then action.refused
human limits.set, stale revision	revision-conflict	action.refused only

The line 234 test makes seven refused chief calls: six invalid changes and one stale revision. By the same path, each of them leaves an action.allowed for organization.change. The test comment there says an invalid change "names no state change". The test checks the file bytes and the receipts but not the events, so it passes either way.

The brief asks for a chief-of-staff change "logged like a human's" and for an invalid change to be refused. Right now it's refused, but it's logged as allowed too.

A fix I tried and then reverted, to confirm it works:

• Before the writer: this.#checkAuthority(s, "organization.change"). It writes no event, and a role without the grant is still refused before the writer runs.
• After the writer returns: this.#store.transaction(() => this.#consumeAuthority(s, "organization.change")).

The writer is synchronous, so nothing runs in between. With that change, chief-of-staff.test.mjs stayed 12/12 and the probe showed only action.refused for both refusals and one action.allowed for a successful change.

One tradeoff is Researcher's to choose. After an outcome-unknown, the file may have changed but no action.allowed exists; the action.refused event and the writer's receipt naming chief-of-staff.chief remain.

The fix needs a test assertion: after the invalid changes in the line 234 test, allowed('organization.change') (or the events table) has not grown. It also needs a mutant that moves the consume back before the writer.

Doc-only, follow-up

• packages/bus/README.md: the patch adds a blank line after the new organization.set row. That ends the verb table, so decision.raise and every row after it no longer render as a table. Delete the blank line.

This is decision 137 doc-only. No test reads packages/bus/README.md: grep -rlF README.md packages/*/tests scripts/test-*.sh lists 13 files, and none of them names the bus or business README.

What I checked and found correct

• Reason path. role.revoke {role, reason} checks role.end first. A role without it gets decision-required, and reason with decision is invalid-request. The role-end-refused check for self or assistant comes after #consumeAuthority, but it runs inside the request transaction, so the allowed event rolls back. My probe confirmed an unknown target and a second revoke both give not-held.
• Decision path. It's unchanged, apart from passing reason as null.
• organization.set refusals before the writer. A human gets agent-required, pm decision-required, and the chief calling limits.set gets human-required. In all three cases the writer is not called.
• Keys. ORGANIZATION_KEYS is the same list for both verbs, so the chief can't reach launch, arbiters or roles.
• Storage. The 3c→3d step is the trigger swap and nothing else, and it's registered in Rocko's migrations array.
• Role file. roles/chief-of-staff.json grants role.end and organization.change within role, and not role.revoke.

Notes (non-blocking)

• reason accepts newlines and control characters; my probe stored "a\nb\u0007". Other free-text fields may already allow the same. If the WebUI trail shows reasons, it should escape them.
• REQ-ROLE-3 has the chief writing the organization file's role instances, arbiters and credential references. organization.set covers row 122's keys only. That matches the brief ("through step 5's write path"), and the rest is #1627.

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	12/12
bus comment-migration	5/5
bus schema	1/1
business chief-of-staff	10/10
harness tools	13/13

I also ran the event probe above, and bus chief-of-staff again (12/12) with the trial fix, which I then reverted. The tree was rechecked against c8bf62fb afterwards.

From Researcher's receipts, not rerun by me:

• the targeted gate in suites-run.txt: business 88, bus 190, tasks 97, cli 195, webui 112, harness 84 (rerun after 83/84), seat 132, test-land-work 84;
• 21 of 21 mutants failing with 2 controls in mutants-run.txt.

I took no slot.