From 8c704ddde1ffb40213ca34ddde5f7dd151a47976 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sun, 11 Oct 2026 19:52:53 -0500 Subject: [PATCH] review(darkwing): row 59 round 2, approve (#1548) Rebase-only round on 3d298920. Merged hunks against rows 58 and 60 keep both sides; 21 round 1 mutants give round 1's results. Comment 27591. Co-Authored-By: Claude Opus 5.5 --- .../work/queue-59-review/r2/mutants.sh | 36 ++++++++ .../work/queue-59-review/r2/mutants.txt | 68 ++++++++++++++ .../work/queue-59-review/r2/tests-light.txt | 5 ++ .../work/queue-59-review/review-r2.md | 89 +++++++++++++++++++ 4 files changed, 198 insertions(+) create mode 100755 agents/darkwing/work/queue-59-review/r2/mutants.sh create mode 100644 agents/darkwing/work/queue-59-review/r2/mutants.txt create mode 100644 agents/darkwing/work/queue-59-review/r2/tests-light.txt create mode 100644 agents/darkwing/work/queue-59-review/review-r2.md diff --git a/agents/darkwing/work/queue-59-review/r2/mutants.sh b/agents/darkwing/work/queue-59-review/r2/mutants.sh new file mode 100755 index 000000000..c464418d3 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r2/mutants.sh @@ -0,0 +1,36 @@ +#!/bin/bash +# Round 2: the same mutants as r1/mutants.sh and r1/m2.sh, on the round 2 tree (base 3d298920). +export TMPDIR=~/darkwing-scratch/rv59r2/tmp +cd ~/darkwing-scratch/rv59r2/wt +M=~/darkwing-scratch/rv59r2/mut.mjs +B=packages/bus/src/broker.mjs; W=packages/webui/src/public/bus.js +BT=packages/bus/tests/agents.test.mjs; WE=packages/webui/tests/bus.test.mjs; WB=packages/webui/tests/bus-browser.test.mjs +run() { # name file from to test-file + node $M "$2" "$3" "$4" || { echo "$1: MUTATION DID NOT APPLY"; return; } + env -u NODE_TEST_CONTEXT node --test --test-timeout=60000 "$5" > $TMPDIR/mut-$1.log 2>&1 + echo "$1: rc=$? $(grep -E '^ℹ (pass|fail) ' $TMPDIR/mut-$1.log | tr '\n' ' ')" + grep -E '^✖ ' $TMPDIR/mut-$1.log | sed 's/ ([0-9.]*ms)$//; s/^/ /' | sort -u | head -4 + git checkout -q -- "$2" +} +run any-holder $B 'AND d.holder_run=? AND' 'AND (d.holder_run=? OR 1) AND' $BT +run read-ignored $B " AND NOT EXISTS(SELECT 1 FROM deliveries r WHERE r.message=m.id AND r.op='read')\"" '"' $BT +run any-business-msg $B 'WHERE m.business=? AND m.to_role=?' 'WHERE (m.business=? OR 1) AND m.to_role=?' $BT +run any-role-msg $B 'AND m.to_role=? AND' 'AND (m.to_role=? OR 1) AND' $BT +run since-max $B 'min(d.at) AS since' 'max(d.at) AS since' $BT +run any-route $B "raised_by_role=? AND route_to='human' AND" 'raised_by_role=? AND' $BT +run closed-kept $B "AND route_to='human' AND NOT EXISTS(SELECT 1 FROM decision_events x WHERE x.decision=d.id AND x.op IN \${CLOSED}) ORDER" "AND route_to='human' ORDER" $BT +run any-raiser $B 'AND raised_by_role=? AND' 'AND (raised_by_role=? OR 1) AND' $BT +run waiting-wins $B "const state = working ? 'working' : inputNeeded.length ? 'waiting' : 'idle';" "const state = inputNeeded.length ? 'waiting' : working ? 'working' : 'idle';" $BT +run role-gets-state $B 'return s.human || s.reader ? claims.map((c) => this.#agentState(c)) : claims;' 'return claims.map((c) => this.#agentState(c));' $BT +run reader-plain $B 'return s.human || s.reader ? claims.map' 'return s.human ? claims.map' $BT +run working-zero $B 'const working = busy.messages ? { messages: busy.messages, since: busy.since } : null;' 'const working = { messages: busy.messages, since: busy.since };' $BT +run endpoint-plain $B 'return s.human || s.reader ? claims.map((c) => this.#agentState(c)) : claims;' 'return claims;' $WE +run last-ask $W 'const d = asks[0];' 'const d = asks[asks.length - 1];' $WB +run no-more-link $W "\${asks.length > 1 ? \` +\${esc(asks.length - 1)} more\` : ''}" '' $WB +run no-working-detail $W "\${c.working ? \`\${esc(plural(c.working.messages, 'message'))} since \${when(c.working.since)}\` : ''}" '' $WB +run no-state-col $W "['Role', 'State', 'Attention', 'Holder'" "['Role', 'Attention', 'Holder'" $WB +run seq-order $B "route_to='human' AND NOT EXISTS(SELECT 1 FROM decision_events x WHERE x.decision=d.id AND x.op IN \${CLOSED}) ORDER BY CASE class WHEN 'gated' THEN 0 ELSE 1 END,seq" "route_to='human' AND NOT EXISTS(SELECT 1 FROM decision_events x WHERE x.decision=d.id AND x.op IN \${CLOSED}) ORDER BY seq" $BT +run blocking-raw $B "class: d.class, blocking: !!d.blocking, task_ref" "class: d.class, blocking: d.blocking, task_ref" $BT +run no-blocking-chip $W "short(d.id))}\${d.blocking ? \` \${blockChip('blocking')}\` : ''}" "short(d.id))}" $WB +run question-raw $W 'Input needed: ${txt(first(d.question))}' 'Input needed: ${first(d.question)}' $WB +git diff --quiet -- $B $W && echo "restored: sources match the candidate index" || echo "NOT RESTORED" diff --git a/agents/darkwing/work/queue-59-review/r2/mutants.txt b/agents/darkwing/work/queue-59-review/r2/mutants.txt new file mode 100644 index 000000000..619df9436 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r2/mutants.txt @@ -0,0 +1,68 @@ +start 2026-10-12T00:51:06Z +any-holder: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: a new holder doesn't inherit what the last one left unread; decisions stay with the role + ✖ failing tests: +read-ignored: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: working counts what the holder received and has not read; routed alone is not working + ✖ failing tests: +any-business-msg: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: state reads only its own business, even for a run name both use + ✖ failing tests: +any-role-msg: rc=0 ℹ pass 6 ℹ fail 0 +since-max: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: working counts what the holder received and has not read; routed alone is not working + ✖ failing tests: +any-route: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ failing tests: +closed-kept: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ failing tests: +any-raiser: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ failing tests: +waiting-wins: rc=1 ℹ pass 3 ℹ fail 3 + ✖ agents: a new holder doesn't inherit what the last one left unread; decisions stay with the role + ✖ agents: state reads only its own business, even for a run name both use + ✖ agents: working wins over waiting, and Input needed still shows + ✖ failing tests: +role-gets-state: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: a held role with nothing on the bus is idle; a role session gets the claim rows unchanged + ✖ failing tests: +reader-plain: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: a held role with nothing on the bus is idle; a role session gets the claim rows unchanged + ✖ failing tests: +working-zero: rc=1 ℹ pass 1 ℹ fail 5 + ✖ agents: a held role with nothing on the bus is idle; a role session gets the claim rows unchanged + ✖ agents: a new holder doesn't inherit what the last one left unread; decisions stay with the role + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ agents: state reads only its own business, even for a run name both use +endpoint-plain: rc=1 ℹ pass 8 ℹ fail 1 + ✖ bus routes serve the four reads from the reader view, unescaped JSON for the page to escape + ✖ failing tests: +last-ask: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +no-more-link: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +no-working-detail: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +no-state-col: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +seq-order: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ failing tests: +blocking-raw: rc=1 ℹ pass 5 ℹ fail 1 + ✖ agents: input needed is the role's open decisions routed to the human, gated first, then oldest + ✖ failing tests: +no-blocking-chip: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +question-raw: rc=1 ℹ pass 0 ℹ fail 1 + ✖ failing tests: + ✖ S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes +restored: sources match the candidate index +end 2026-10-12T00:51:53Z diff --git a/agents/darkwing/work/queue-59-review/r2/tests-light.txt b/agents/darkwing/work/queue-59-review/r2/tests-light.txt new file mode 100644 index 000000000..9df4e428c --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r2/tests-light.txt @@ -0,0 +1,5 @@ +start 2026-10-12T00:50:46Z node v26.8.1 +packages/bus/tests/agents.test.mjs rc=0 ℹ pass 6 ℹ fail 0 +packages/webui/tests/bus.test.mjs rc=0 ℹ pass 9 ℹ fail 0 +packages/webui/tests/bus-browser.test.mjs rc=0 ℹ pass 1 ℹ fail 0 +end 2026-10-12T00:51:03Z diff --git a/agents/darkwing/work/queue-59-review/review-r2.md b/agents/darkwing/work/queue-59-review/review-r2.md new file mode 100644 index 000000000..47f5f96e4 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/review-r2.md @@ -0,0 +1,89 @@ +# Row 59, bus read: agent state per role, round 2 review (Darkwing) + +Issue #1548, request comment 27586, packet `agents/filbert/work/queue-59/` +(commit `93fc00e4`). Base `3d298920`, row 60's landing plus Sage's log +commits. Candidate manifest sha256 +`9acfac19161525d47cfaf66e11f677cf3119e10f3cd0b5b7105be5933ddbad2d`, 8 +files, matching the request. Scope: Sage's rev 571, rebase only. + +Verdict: **approve**. The rebase changes one line of round 1's code, the +`bus.css` widths line, and keeps both rows' values there. The two merges +Sage asked about, `broker.mjs` against row 58 and the bus README against +row 60, keep every line of both sides. The tests and my round 1 mutants +give the same results as round 1. Nothing blocks. + +## Method + +- A detached worktree at `3d298920` with `git apply --index build.patch`, + then `sha256sum -c` on the manifest: 8 OK, and again after the mutants. + None of the 8 paths changed from `3d298920` to `HEAD` (`f3c8b2e2`) at + the time of review. +- I compared the added and removed lines of round 1's `build.patch` + (`93fc00e4^`) with round 2's. They match except for `bus.css` line 28. +- I read each merged hunk in the round 2 tree and checked the line numbers + `build.md` gives. They're right. +- Three test files, then my 21 round 1 mutants on the new tree + (`r2/mutants.sh`, the 17 from `r1/mutants.sh` that applied plus the 4 + from `r1/m2.sh`). + +Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv59r2/tmp`. + +## The merged hunks + +**`broker.mjs`, the `agents` branch against row 58.** Lines 852 to 859 are +round 1's block unchanged: the claims query, then `#agentState` for human +and reader sessions and the plain rows for a role session. Line 860 is row +58's `if (verb === 'tasks') return this.#taskRows(s.business);` as row 58 +landed it. Row 58's `#taskRows` at line 313 is untouched. Row 60's +`credentials` case (line 886) is inside the switch above `default:` (line +906), and `#agentState` (line 914) is after the switch. The reader +allowlist already had `agents`, so row 60's edit to it doesn't matter +here. + +**The bus README against row 60.** The verb table reads `inbox, tasks` +(128), `agents` (129), `trail` (130), then row 60's `decisions` (131) and +`credentials` (132). Round 1's split of the `inbox, tasks, agents` row and +row 60's two new rows share no lines. The "Agent state" section follows +the table at line 134, as in round 1. + +**`bus.css` line 28.** Row 58 set `table.s1-tasks` to 1020px and round 1 +set `table.s1-agents` to 1000px on the same line. The line has both. The +browser test passes with it. + +**`bus.js` and the tests against row 58.** Row 58's Tasks view changes end +above the Agents hunk, and the test additions in `bus-browser.test.mjs` and +`bus.test.mjs` share no lines with rows 58 or 60. + +## Tests and mutants + +On the candidate, 00:50:46Z to 00:51:03Z on 2026-10-12 +(`r2/tests-light.txt`): + +| File | Result | +|---|---| +| `packages/bus/tests/agents.test.mjs` | 6 pass, 0 fail | +| `packages/webui/tests/bus.test.mjs` | 9 pass, 0 fail | +| `packages/webui/tests/bus-browser.test.mjs` | 1 pass, 0 fail | + +I didn't run the full suites. Sage's gate run for this row was going at +the same time, Filbert's gate is green, and the code is round 1's apart +from the CSS line. + +The mutants ran from 00:51:06Z to 00:51:53Z (`r2/mutants.txt`). 20 of 21 +fail, and each fails the same test it failed in round 1; I compared the +failing test names mutant by mutant. `any-role-msg` survives again, for +round 1's reason. The only count that moved is `endpoint-plain`, 8 pass +instead of 7, because row 60 added a test to the WebUI bus file. The +sources matched the candidate index afterwards. + +## Filbert's correction + +`build.md` takes back round 1's claim that the diff was unchanged after +the mutants. My round 1 review didn't rely on that claim: my mutants ran +on my own worktree and restored from its index. This round's full run +checks clean. + +## Round 1 notes + +Note A (a claim that outlives its runner) stays open; this round is rebase +only. Note B is row 80 now.