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

Filbert review: row 182 round 1 (#1680), approve

Request 29387. Candidate manifest 9b5cc63c37f57a7a9954d6d3ecd1c5ee23a014b003c653d5ff7c0e055d57e866 on base 841c56d05. The brief is blob 3b4313cb9f. The design was approved in sage-1204, and the messaging.test.mjs ruling is sage-1212.

Verdict: approve. No findings block. N1 and N2 are notes.

What I ran

• I built ~/filbert-scratch/r182-r1/wt from 841c56d05, then ran git apply --index build.patch.
  • sha256sum -c on the manifest passed 4/4, and the manifest file hashes to 9b5cc63c...d57e866.
  • The patch-id is fa9a8988ebfc: 4 files, +88/-6.
• Base drift: git log 841c56d05..refactor (02482094a) on the four paths is empty.
• Hash pins and text readers:
  • No 64-hex pin in packages//tests or scripts/test-.sh names a changed path.
  • Two bus tests read broker.mjs as text: limit-checks.test.mjs:323 and project-runtime.test.mjs:13. Both are in the bus suite.
  • No test reads packages/bus/README.md.
• I took the slot on sage-1219, with the /proc busy check CLEAR before and after, and TMPDIR ~/fs182.
  • The bus suite (node --test tests/*.test.mjs in packages/bus): 451/451 pass, 0 fail, rc 0, from 05:36:39Z to 05:36:55Z. The counts are an awk sum over the spec log.
  • My mutant M1 applies the owner check to every sender (s.tester && removed in #testerOwns). Killed: rc 1, 2 of the 4 row 182 tests failed. Those were the chief's reply up a chain into the tester's thread, and the spec notice test, where the chief's relay into the human's thread now refuses. So the reverse direction the brief keeps is tested. broker.mjs was restored by git checkout.
• Darkwing's run 2 (bus 451/451, m1-m3 killed) is their receipt, not mine.

Checked against the brief

• The hole. #threadFor (broker.mjs:322) is the only place a tester's message.send gets a thread. It's called at :1893, after the in_reply_to existence check, and before the tester-target-not-running check and the message INSERT (:1900). So a refusal stores nothing, and test 1 checks that in the messages and message_threads counts. The other caller, #reviewMessage (:2523), is the review verbs', which a tester can't call (TESTER_VERBS).
• The check. #testerOwns (:352) compares the thread's owner with #owner(s) only when s.tester. It's applied on both branches: the in-thread one at :337 and the spec branch at :342.
• Spec branch. #specThread binds t.owner to the business's human, so for a tester it can return only a thread the tester doesn't own. BUILD.md's answer is right, and test 4 covers it.
  • Test 4's fixture body matches what #specNotice (:2595) writes up to the project name, which is all #specThread parses.
  • The chief's relay on the same chain shows the branch is reached.
• The code. Reusing thread-not-owned is reasonable: it's the REQ-USER-2 code the tester's thread verbs already return, and the webui and cli already map it. The README paragraph says the chain picks the thread.
• Tests 1-3 are the brief's three. Test 2 checks the tester's own thread through message_threads, thread.messages and the run's message.receive. Test 3 is the round trip with the chief as a plain run.
• messaging.test.mjs:203. It sits outside the brief's file list, as BUILD.md names. The old tester leg replied into a message in the human's thread, which is exactly the send this row refuses. The new leg keeps a passing tester send in a tester-owned thread, and refusals() stays [], as sage-1212 ruled.

Notes

• N1 (pre-existing order, doesn't block): in the in-thread branch the thread-instance-mismatch check (:336) runs before the owner check. A tester whose chain reaches a human thread of a different instance gets thread-instance-mismatch, not thread-not-owned. Nothing is stored either way, so it's only which code the tester sees.
• N2 (BUILD.md, doc-only): BUILD.md names limit-checks.test.mjs:323 as the one test that reads broker.mjs as text. project-runtime.test.mjs:13 reads it too. That file is in the bus suite and passed in both Darkwing's run and mine.