stack/agents/darkwing/work/queue-60-review/review-r1.md

Row 60, bus read: credential status on the broker, round 1 review (Darkwing)

Issue #1549, request comment 27383, packet agents/filbert/work/queue-60/. Brief: docs/plans/2026-10-10_design-implementation.md, "Bus read: credential status on the broker". Base 4100fa68. Candidate manifest sha256 99d67e44e226fdf15aeb850bfa2f36a61f3b7f940fad36249ff4bd101f24eedc, 9 files, matching the request.

Verdict: approve. The read gives human and reader sessions service, account, role, date kind, date and state for their own business, and nothing that names a token, file or variable. A role session gets human-required, and any argument gets invalid-request. The broker gets a status function, never the Credentials object, so it has no path to use(). The read asks the status on every request, so a date that crosses a boundary while the host runs shows up without a restart. Nothing blocks. Two notes and the answers to Filbert's two questions are below.

Method

• I read the whole patch: the credentials case and the constructor option in broker.mjs, the runtime wiring, views.mjs, the WebUI route, both READMEs and the tests.
• A detached worktree at 4100fa68 with build.patch applied, then sha256sum -c on the manifest: 9 OK.
• I checked the account rule against the business file loader (packages/business/src/business.mjs lines 91 to 101): a role whose definition uses Vikunja must have tracker.bot, and a role without Vikunja can't have tracker.
• 16 mutants of my own (r1/mutants.sh, using agents/darkwing/work/queue-59-review/r1/mut.mjs), each run against one test file and restored from the worktree index. I aimed some at sites Filbert's 16 name, to check the tests kill them for the right reason, and the rest at sites they don't.

Node v26.8.1, TMPDIR=~/darkwing-scratch/rv60/tmp.

Suites

On the candidate, one at a time (r1/tests.sh, r1/gate.sh), from 23:48Z to 23:53:59Z on 2026-10-11:

Suite	Result
packages/bus	84 pass, 0 fail
packages/webui	41 pass, 0 fail
test-auth	15 passed
test-conductor	17 passed
test-config	24 passed
test-discord	66 passed
test-extension-package	18 passed
test-foundation	44 passed
test-queue	27 passed
test-release	14 passed
test-task	26 passed

test-task ran with DOCKER_HOST set to a missing socket and skipped its Docker cases. I didn't run packages/control-board or packages/cli; Filbert's run has them green, and Sage's gate rerun covers landing. test-discord passed first time for me, so I didn't see Filbert's timing failure.

Mutants

13 of 16 fail on the first run (r1/mutants.txt). Each kill failed the test meant for it.

Mutant	Fails
reader-refused: credentials left off the reader list	all 4 credential tests
role-allowed: no human-or-reader check	the role and argument test, the process test
args-accepted: keys(a, [], []) removed	the role and argument test
prefix-no-slash	the shape test (demo sees demox), the process test
no-filter	the shape test, the process test
sync-label-raw: role @sync	the shape test, the process test
sync-no-bot: the sync account null	the shape test, the process test
gitea-gets-bot: Gitea takes tracker.bot	the shape test, the process test
datekind-fixed: always expires	the shape test, the process test
leak-instance: instance kept in the row	the shape test, the process test
status-cached: status read once at construction	the per-request test
runtime-no-status: the runtime passes no status	the process test
serve-no-verb: credentials off BUS_VERBS	2 WebUI bus tests
views-no-verb: credentials() removed from views	2 WebUI bus tests, on a rerun
account-undefined: bot for bot ?? null	survives, equivalent
no-result-check: no #secretCheck(result)	survives, base code

views-no-verb survived the first run because I pointed it at the bus suite. The WebUI is the only caller of views().credentials, and the rerun against packages/webui/tests/bus.test.mjs kills it. That was my mistake in the script, not a gap in the tests.

account-undefined is equivalent under a loaded business file. The only row where bot could be undefined is a Vikunja credential on a role with no tracker.bot, and the loader refuses that file. The broker tests build their businesses by hand, so they'd accept one, but the runtime gets its businesses from the loader.

no-result-check removes a line the base already has for every read, so it isn't this row's code. No credential row in the fixtures holds a known token, so nothing reaches it. The process test covers the same leak from the outside: it seeds a token in a file and in the environment and greps the output, the log and every bus.sqlite* file.

Notes

1. The re-round meets two rows on four of these paths. Row 74 (351464ce) changed packages/webui/README.md and packages/webui/src/serve.mjs after 4100fa68. Row 59 edits broker.mjs and the bus README, and Filbert's overlap note already shows git apply --3way merging it cleanly. Row 58 touches the webui README too. When this row re-rounds after 58, the serve.mjs merge is the one to read by hand: row 74 put its preview checks ahead of routing in the same request handler that reads BUS_VERBS. Nothing in this review depends on those lines, so my findings carry over, and on the re-round I'll check the digest, the merged handler and the WebUI bus and preview tests.

2. test-release is 14, not 4. The gate table in build.md says "release 4". My run has 14 passed, the same as the other runs today. A typo in the packet; it doesn't touch the candidate.

Questions

1. The Business view. Leave it alone in this row. The brief asks for the endpoint, and the Business view working without a bus host is worth keeping. The reason this read exists is the case the view can't show: a token file rotated after startup, which the host doesn't reread until restart. That only helps Jason if a page puts the two side by side. A later row, after the cutover, could add a "host loaded" column to the Business view when the bus answers and flag a row whose date or state differs from the file's, with the file metadata alone when it doesn't.
2. The missing decisions row. Yes, add it. Since this row re-rounds anyway and already edits that table, I'd put the one line in the re-round rather than open a docs row for it. If Sage would rather keep the re-round to the digest, a docs follow-up is fine.