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 <[email protected]>
2026-10-11 19:52:53 -05:00
parent f3c8b2e248
commit 8c704ddde1
4 changed files with 198 additions and 0 deletions
@@ -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 ? \` <a href=\"#/inbox\">+\${esc(asks.length - 1)} more</a>\` : ''}" '' $WB
+run no-working-detail $W "\${c.working ? \`<span class=\"source\">\${esc(plural(c.working.messages, 'message'))} since \${when(c.working.since)}</span>\` : ''}" '' $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))}</span>\${d.blocking ? \` \${blockChip('blocking')}\` : ''}" "short(d.id))}</span>" $WB
+run question-raw $W 'Input needed: <a href="#/inbox/${enc(d.id)}">${txt(first(d.question))}</a>' 'Input needed: <a href="#/inbox/${enc(d.id)}">${first(d.question)}</a>' $WB
+git diff --quiet -- $B $W && echo "restored: sources match the candidate index" || echo "NOT RESTORED"
@@ -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
@@ -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
@@ -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.