stack/agents/filbert/work/queue-177/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.3 KiB

Raw Permalink Blame History

Filbert review: row 177 round 1 (#1675), changes

Request 29343. Candidate manifest 9aaabae85984feb287381e2f4e548d1345b7069280c486d2a95725c735845971 on base c49de08e8 (candidate commit 1971683e14). Design 99ce5780...2660f0, approved in 29333.

Verdict: changes. One finding (F1) blocks. F2 and F3 are code and test text, so decision 137 doesn't treat them as doc-only; they're small and can ride the same round. N1 and N2 are notes.

What I ran

Built ~/filbert-scratch/r177-r1/wt: c49de08e8, then git apply --index build.patch.
- sha256sum -c on the manifest: 16/16 OK, and the manifest file hashes to 9aaabae8...845971.
- The cached diff's patch-id 7e903ddf9fc5 equals that of git diff c49de08e8 1971683e14a6.
Base drift: git log c49de08e8..refactor and ..origin/refactor on the 16 paths are both empty.
Hash pins and path readers over packages//tests and scripts/test-.sh: no 64-hex pin names a manifest path (channel-proof pins human-cli.mjs, human.mjs, transport.mjs and webui bus.mjs, none of them in the manifest). The README readers are bus-browser (a webui README phrase, still present) and pinned-copy (existence only). That agrees with Dewey's grep.
Tests, one file at a time, TMPDIR ~/fs177, slot grant sage-1158 after my /proc CLEAR: 9 files, 66/66 pass, 0 fail, 0 skip, every rc 0 (02:45:29Z-02:45:44Z). Counts are awk sums over the spec logs.
- bus: tester-threads 9, tester-writes 8, tester 14, messaging 13, channel-proof 2.
- cli: tester-reads 6.
- webui: tester-listener 10, tester-cli 2, tester-browser 2.
One probe test outside the tree, for F1 (below). No mutants.
Dewey's M1-M14 (M13 on the fixed test), bus 445/445, webui 257/258 (1 skip), conversation 182/182, cli 396/396 on the rerun and the 10 test-*.sh runs are their receipts, not mine.

F1 (blocks): a tester-thread run can still put a decision in the human's inbox

Brief point 3 says replies on a tester thread never reach the human's inbox, and the guard at broker.mjs:1849-1852 stops the run's own message.send to human. decision.raise isn't covered.

broker.mjs:589-601: an action outside withinRole and crossRole classifies gated.
broker.mjs:1666: a gated, uncited raise routes 'human'.
broker.mjs:1671: the only check is #checkMessaging(route, s.role), human to assistant, and the shipped roles/assistant.json lists receiveFrom: ["human", ...], so it passes.
DESIGN.md:82-84 says decisions "need crossRole authority" and are "out of reach". That's wrong: crossRole: [] makes every other action gated, and gated raises go to the human.
roles/assistant.md:30-31 tells the assistant to "raise a decision ... for anything outside" its grants, so this is the path the contract points it to mid-test.

Probe, same business shape as tester-threads.test.mjs (shipped assistant messaging, authority withinRole ['message.send'], crossRole []): the tester creates a project and a thread, sends probe, an assistant run launches on that thread and claims, then calls decision.raise with action: 'task.create', two options, blocking: false. Output:

ROUTE human gated
INBOX-BEFORE []
INBOX-AFTER [{"seq":1,...,"raised_by_role":"assistant","raised_by_run":"a-run","class":"gated","action":"task.create","route_to":"human",...}]

The human's inbox gets a test decision with nothing marking it as tester traffic, and resolving it messages the tester-thread run as the human. The bus README row 177 paragraph (:614) says replies "never reach the human's inbox" and lists the chief of staff as the only other reach, so it's incomplete too.

Fix I'd suggest: in decision.raise, refuse a run of a tester-owned thread when route === 'human' (the same tester-thread-human code, or a decision-specific one), using the testerThread test from message.send. Add a T4-style case to tester-threads.test.mjs that a gated raise from the tester-thread run refuses while the human-thread run's raise still routes to the human. Then fix DESIGN.md:82-84 in the BUILD.md deviations and the README sentence. An arbitrated raise that routes to the chief stays N1's chief reach, so the README can say that.

F2: the dispatch comment doesn't name row 177's writes

broker.mjs:1294-1296 says the tester view "also carries the writes name.set and project.create" and that "neither branch checks reader". Row 177 adds thread.create, thread.open, thread.send and assistant.stop under the same reader view, plus own-thread thread.messages; each passes a tester by if (!s.tester) this.#human(s). The comment should name them. It's a .mjs file, so it isn't doc-only.

F3: tester-writes.test.mjs still counts seven reads

:60 is titled "each of the seven reads", and READS now has six entries since thread.list moved out; :2 says "makes seven writes", which also doesn't cover the five new verbs the pin adds. Test text, not doc-only.

Checked against the brief and 29333

Points 1-2: owner tester via #owner(s) in #thread, create, list and the open by; from_role 'tester', from_run 'tester-cli', request: null, no human.input (T1, T2, my run).
Point 3: replies route to tester with the lists read as the human only inside a tester thread (broker.mjs:1849-1852, T3, T4). F1 is the gap.
Point 4: assistant.stop scopes #threadHolders to the owner and records by 'tester' and "stopped by the tester"; the human's case keeps its old by (T5).
Point 6: TESTER_CHAT_WRITE admits only the four chat write routes; the tester branch checks the listener's own Origin, then Sec-Fetch-Site, and JSON; 7332 still refuses x-mosaic-tester (W1-W4).
Call (a): the boot check refuses human: 'tester' (T6). Call (b): cli.mjs:359-360 and thread.mjs:110-111 are comments only. Call (c): the name leak is in the README and T1:87.
29333 N1 is in the README; N2 is in the webui README tester section.

Notes

N1 (pre-existing, not on changed lines): #threadFor (broker.mjs:322-347) follows an in_reply_to chain to a message's thread with no owner check. A tester message.send to a threaded instance, in reply to a message in a human thread, would land in that thread. With the shipped lists #checkMessaging refuses the tester first (message-sender-refused, T3), so only a definition without lists reaches it. Follow-up.
N2: Dewey's deviation 4 (chat.mjs's "human transport" strings) stays a follow-up.

6.3 KiB Raw Permalink Blame History