stack/agents/filbert/work/queue-140/r1/body.md at dev-console

0 Fork 1

Files

T

jason.woltje b9ea9304fd lead: commit filbert's untracked evidence (273 files: queue-101 queue-102 queue-105 queue-107 queue-108 queue-109 queue-110 queue-112 queue-113 queue-114 queue-115 queue-116 queue-119 queue-120 queue-122 queue-123 queue-125 queue-126 queue-128 queue-129 queue-130 queue-131 queue-132 queue-133 queue-134 queue-135 queue-136 queue-137 queue-139 queue-140 queue-141 queue-142 queue-145 queue-146 queue-147 queue-148 queue-149 queue-150 queue-151 queue-152 queue-153 queue-154 queue-155 queue-156 queue-162 queue-163 queue-165 queue-166 queue-167 queue-169 queue-170 queue-175 queue-177 queue-181 queue-182 queue-184 queue-185 queue-186 queue-187 queue-188 queue-49 queue-64 queue-66 queue-70 queue-71 queue-80 queue-84 queue-85 queue-87 queue-88)

Unclaimed files left in the canonical checkout. Inventory: canonical-cleanup-inventory.md (commit-as-evidence groups). detect-secrets scan clean on 2026-11-02: hits are sha256 pins, git object ids, test sentinels already in tracked tests and unit-file lines.

2026-11-02 15:05:14 -06:00

6.2 KiB

Raw Permalink Blame History

Row 140 round 1 review (Filbert): changes

Candidate: manifest sha256 df9e3fea692c6a5edf20b9fa9be3b8435ce851bba4bcffef1921688d8f34aaaf, 11 paths, base dda7e31b0, packet agents/researcher/work/queue-140/ in d10a1a467.

The code is correct, and I found no defect in it. One finding blocks, and it is in the tests. The brief asks for tests showing the request reaches the chief of staff only from the pm. The tests can't tell the pm and chief-of-staff definitions apart from instances that happen to be named pm and chief. A check on the instance name passes all of them.

Rebuild

A fresh worktree at dda7e31b0 with row140.patch applied. Every manifest hash matched and git diff --check was clean (11 files, +354 -5).
The added lines have no dash, curly-quote or control characters. The packet manifest checks.
No commit from dda7e31b0 to refactor HEAD touches a manifest path.

What I checked and found correct

staff.request. The shape checks come first, then the caller's message.send grant is spent, then the sender's definition, the launcher's definition and the messaging lists are checked. Then one STAFF-REQUEST goes to the launcher through #reviewMessage, so it routes like any message.
- A refusal after the grant rolls the action.allowed event back, because the request runs in one transaction. My probe below confirms it.
- staff_for instead of task_ref in the event is right: an action event carrying task_ref must be filed under the task.
No other writer. message.send and thread.send keep their class lists, so STAFF-REQUEST is invalid-class there for every caller. Only this verb writes it.
It grants nothing. pm.json and chief-of-staff.json are unchanged. The chief launches with the role.launch it already holds, and the request adds no launch path.
The harness. request_staff is offered only to the pm definition, and only inside the message.send block, so a pm without message.send gets no tool. Its schema bounds match the broker's. The prompt line goes only to a session holding the tool.
The chief's reply. The shipped pm and chief-of-staff definitions have no messaging lists, so the chief's RESULT reply to the pm isn't blocked.
Role contracts. The pm.md and chief-of-staff.md text matches the code, and the chief text is limited to the staff request, as the brief asks.
Tests against the brief. The launcher end-to-end test covers "the chief's launch stays inside the organization's limits":
- the request launches nothing;
- the next coder launch works and the reviewer launch is refused capacity-full;
- nothing is attributed to the pm, and neither the pm nor the chief has the other's tool.

Finding (blocks): the "only from pm" tests don't separate definitions from instance names

packages/bus/tests/staff-request.test.mjs always names the pm-definition instance pm and the launcher chief. I ran two mutants of my own on broker.mjs against that file. Both survive, 7/7 pass:

Mutant	staff-request
N1: the sender check becomes `s.role !== STAFF_FROM` (instance name, not definition)	7/7 pass
N2: the recipient check stops reading the launcher's definition (`!b.roles[chief] \|\| chief === 'coder'`)	7/7 pass

The README and the code comment say "only an instance of the pm definition" and a launcher that is "an instance of chief-of-staff". That is the row's central property, but no test pins it.

The candidate itself is right. My probe on the candidate (a scratch script, Store and Broker in-process, the shipped pm and chief-of-staff authority) gives:

Case	Result
A: `lead` (definition pm) sends	reaches `chief`
A: an instance named `pm` with definition coder sends	`staff-request-refused`
B: the launcher is `boss` (definition chief-of-staff)	the pm's request reaches `boss`
C: the launcher is named `chief` with definition coder	`no-chief-of-staff`

action.allowed events with operation: staff.request were 1 in A, 1 in B and 0 in C. Only the accepted requests left one.

Suggested fix (test only): add these cases to staff-request.test.mjs:

a pm-definition instance with another name succeeds, and an instance named pm with another definition is refused;
a chief-of-staff launcher with another name receives, and a launcher named chief with another definition gives no-chief-of-staff.

It would also be worth asserting in the only-pm test that a refused request leaves no staff.request action.allowed event. No test asserts the rollback today. N1 and N2 should then fail.

Notes (non-blocking)

The requested role names aren't checked against the business's roles. roles[].role only has to be an identifier. That seems right, since the chief of staff decides and launch refuses what isn't an instance. I note it because a pm's typo reaches the chief as a request.
Row 146 also edits roles/chief-of-staff.md. Whichever lands second rebases, as Researcher and Sage noted.
About my launcher run. With my usual TMPDIR=/tmp/fr140/t, launcher.test.mjs failed 9 of 16 with "runner ... did not start". dda7e31b0 without the patch fails the same way (8 of 15), so it is my environment, not the row. With the default TMPDIR, the base gives 17/17 and the candidate 18/18. The spec reporter shows the new test passed, not skipped.

What I ran and what I took from receipts

I ran, as single files on the candidate with Docker disabled, all with 0 fail and 0 skipped. Every file except launcher used TMPDIR set to scratch; launcher used the default TMPDIR (note 3).

Test file	Result
bus staff-request	7/7
bus messaging	13/13
bus broker	16/16
bus csuite-messaging	16/16
bus threads	14/14
bus chief-of-staff	13/13
harness tools	18/18
harness bundle	21/21
harness runner	23/23
cli launcher	18/18 (base 17/17)

I also ran the two mutants above and the probe. The tree matched the manifest again afterwards.

From Researcher's receipts, not rerun by me:

m1 to m21, all killed by assertion, with 3 controls passing (mutants-run.txt).
The gate from 22:59:28Z to 23:03:08Z, all rc=0 (suites-run.txt): cli 208, business 106, bus 259, plugins 28, tasks 97, webui 113, harness 102, seat 135, and the ten scripts/test-*.sh.

I took no slot.

6.2 KiB Raw Permalink Blame History