diff --git a/agents/darkwing/work/queue-59-review/r1/gate-summary.txt b/agents/darkwing/work/queue-59-review/r1/gate-summary.txt new file mode 100644 index 000000000..f7edaba10 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/gate-summary.txt @@ -0,0 +1,10 @@ +test-auth rc=0 selftest: 15 passed, 0 failed +test-conductor rc=0 selftest: 17 passed, 0 failed +test-config rc=0 selftest: 24 passed, 0 failed +test-discord rc=0 discord suite: 66 passed, 0 failed +test-extension-package rc=0 extension package selftest: 18 passed, 0 failed +test-foundation rc=0 selftest: 44 passed, 0 failed +test-queue rc=0 queue suite: 27 passed, 0 failed +test-release rc=0 selftest: 14 passed, 0 failed +test-task rc=0 selftest: 26 passed, 0 failed +DONE diff --git a/agents/darkwing/work/queue-59-review/r1/gate.sh b/agents/darkwing/work/queue-59-review/r1/gate.sh new file mode 100755 index 000000000..d921d8aab --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/gate.sh @@ -0,0 +1,10 @@ +#!/bin/bash +# Every scripts/test-*.sh in the review worktree, one at a time; bus and webui ran in tests.sh. +export TMPDIR=~/darkwing-scratch/rv59/tmp; O=~/darkwing-scratch/rv59/out/gate; mkdir -p $O; rm -f $O/summary.txt; cd ~/darkwing-scratch/rv59/wt +[ -x node_modules/.bin/pi ] || { echo "node_modules not linked" >> $O/summary.txt; exit 1; } +for s in scripts/test-*.sh; do + n=$(basename $s .sh) + if [ $n = test-task ]; then DOCKER_HOST=unix:///nonexistent bash $s > $O/$n.log 2>&1; else bash $s > $O/$n.log 2>&1; fi + echo "$n rc=$? $(grep -iE '[0-9]+ (passed|pass)' $O/$n.log | tail -1)" >> $O/summary.txt +done +echo DONE >> $O/summary.txt diff --git a/agents/darkwing/work/queue-59-review/r1/m2.sh b/agents/darkwing/work/queue-59-review/r1/m2.sh new file mode 100644 index 000000000..9603114d2 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/m2.sh @@ -0,0 +1,15 @@ +#!/bin/bash +export TMPDIR=~/darkwing-scratch/rv59/tmp; cd ~/darkwing-scratch/rv59/wt; M=~/darkwing-scratch/rv59/mut.mjs +B=packages/bus/src/broker.mjs; W=packages/webui/src/public/bus.js; BT=packages/bus/tests/agents.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 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/r1/mut.mjs b/agents/darkwing/work/queue-59-review/r1/mut.mjs new file mode 100644 index 000000000..072bba00d --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/mut.mjs @@ -0,0 +1,6 @@ +// node mut.mjs : replace exactly one occurrence, else exit 3. +import { readFileSync, writeFileSync } from 'node:fs'; +const [f, a, b] = process.argv.slice(2); +const s = readFileSync(f, 'utf8'); const n = s.split(a).length - 1; +if (n !== 1) { console.error(`matches=${n}`); process.exit(3); } +writeFileSync(f, s.replace(a, () => b)); diff --git a/agents/darkwing/work/queue-59-review/r1/mutants-2.txt b/agents/darkwing/work/queue-59-review/r1/mutants-2.txt new file mode 100644 index 000000000..e915c31c8 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/mutants-2.txt @@ -0,0 +1,13 @@ +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 diff --git a/agents/darkwing/work/queue-59-review/r1/mutants.sh b/agents/darkwing/work/queue-59-review/r1/mutants.sh new file mode 100755 index 000000000..c69495e79 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/mutants.sh @@ -0,0 +1,37 @@ +#!/bin/bash +# Darkwing's own row 59 mutants. Each edits one source file in the review worktree, runs one +# test file, then restores the file from the worktree index, which holds the candidate. +export TMPDIR=~/darkwing-scratch/rv59/tmp +cd ~/darkwing-scratch/rv59/wt +M=~/darkwing-scratch/rv59/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 seq-order $B "ORDER BY CASE class WHEN 'gated' THEN 0 ELSE 1 END,seq" 'ORDER BY seq' $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 blocking-raw $B 'blocking: !!d.blocking' 'blocking: d.blocking' $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-blocking-chip $W "\${d.blocking ? \` \${blockChip('blocking')}\` : ''}" '' $WB +run question-raw $W '${txt(first(d.question))}' '${first(d.question)}' $WB +run no-state-col $W "['Role', 'State', 'Attention', 'Holder'" "['Role', 'Attention', 'Holder'" $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/r1/mutants.txt b/agents/darkwing/work/queue-59-review/r1/mutants.txt new file mode 100644 index 000000000..076a13b86 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/mutants.txt @@ -0,0 +1,62 @@ +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: +matches=2 +seq-order: MUTATION DID NOT APPLY +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: +matches=2 +blocking-raw: MUTATION DID NOT APPLY +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 7 ℹ 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 +matches=2 +no-blocking-chip: MUTATION DID NOT APPLY +matches=2 +question-raw: MUTATION DID NOT APPLY +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 +restored: sources match the candidate index diff --git a/agents/darkwing/work/queue-59-review/r1/tests.sh b/agents/darkwing/work/queue-59-review/r1/tests.sh new file mode 100755 index 000000000..f6054b632 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/r1/tests.sh @@ -0,0 +1,3 @@ +#!/bin/bash +export TMPDIR=~/darkwing-scratch/rv59/tmp; cd ~/darkwing-scratch/rv59/wt; O=~/darkwing-scratch/rv59/out +for s in bus webui; do env -u NODE_TEST_CONTEXT node --test "packages/$s/tests/*.test.mjs" > $O/$s-tests.log 2>&1; echo "$s rc=$? $(grep -E '^ℹ (pass|fail) ' $O/$s-tests.log | tr '\n' ' ')"; done diff --git a/agents/darkwing/work/queue-59-review/review-r1.md b/agents/darkwing/work/queue-59-review/review-r1.md new file mode 100644 index 000000000..3fa50d147 --- /dev/null +++ b/agents/darkwing/work/queue-59-review/review-r1.md @@ -0,0 +1,132 @@ +# Row 59, bus read: agent state per role, round 1 review (Darkwing) + +Issue #1548, request comment 27371, packet `agents/filbert/work/queue-59/` +(commit `e82f7c32`). Brief: `docs/plans/2026-10-10_design-implementation.md`, +section "Bus read: agent state per role". Base `f2f2861b`. Candidate +manifest sha256 +`90d5bddc545e429091bf43202ef70cb55eeb1ce964964f440a0c753decb82f71`, 8 +files, matching the request. + +Verdict: **approve**. The broker derives each held role's state from the +claim, delivery and decision rows it already keeps, writes nothing and +changes no schema. A role session still gets the plain claim rows. The +Agents view escapes everything it prints. Nothing blocks. I have two notes +on the README's limits, one bus follow-up that predates this row, and +answers to both questions. + +## Method + +- I read the whole patch: `#agentState` and the `agents` branch in + `broker.mjs`, the webui `bus.js` and `bus.css` changes, both READMEs and + the three test files. I checked the working query against `receive` + (which skips delivered messages, so a message can't count twice) and the + `inputNeeded` order against the Inbox query in the same file. I checked + the runner's claim in `packages/harness/src/runner.mjs`, lines 311 to 350: + it receives a batch, takes a turn per message, sends the result and marks + the message read, and its catch only exits on `SESSION_GONE`. +- A detached worktree at `f2f2861b` with `git apply --index build.patch`, + then `sha256sum -c` on the manifest: 8 OK. +- 21 mutants of my own (`r1/mutants.sh`, `r1/m2.sh`, and `r1/mut.mjs`, + which replaces text that must match exactly once). Each ran one test + file and restored the source from the worktree index. Four of the first + pass matched twice and didn't apply; `m2.sh` reran them with longer + matches. + +Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv59/tmp`. + +## Suites + +On the candidate tree (`f2f2861b` plus `build.patch`), one at a time. +The two package runs were at 22:36Z (`r1/tests.sh`); the scripts ran +23:23:30Z to 23:26:25Z (`r1/gate.sh`, `r1/gate-summary.txt`). `test-task` +ran with `DOCKER_HOST=unix:///nonexistent`. + +| Suite | Result | +|---|---| +| bus `node --test` | 86 pass, 0 fail | +| webui `node --test` | 40 pass, 0 fail | +| test-auth | 15 passed, 0 failed | +| test-conductor | 17 passed, 0 failed | +| test-config | 24 passed, 0 failed | +| test-discord | 66 passed, 0 failed | +| test-extension-package | 18 passed, 0 failed | +| test-foundation | 44 passed, 0 failed | +| test-queue | 27 passed, 0 failed | +| test-release | 14 passed, 0 failed | +| test-task | 26 passed, 0 failed | + +None of the 8 files changed between `f2f2861b` and `HEAD` (`d7548697`) +at the time of review. + +## Mutants + +20 of 21 fail (`r1/mutants.txt`, `r1/mutants-2.txt`). I checked each +browser kill's assertion; each failed on the check meant for it. +`question-raw` fails the injected-markup count, not a layout check. + +| Mutant | Fails | +|---|---| +| `any-holder`: deliveries to any holder count | agents 5 (new holder) | +| `read-ignored`: read deliveries still count | agents 2 | +| `any-business-msg`: messages from every business | agents 6 | +| `any-role-msg`: `m.to_role` dropped | survives, equivalent | +| `since-max`: newest delivery as `since` | agents 2 | +| `any-route`: decisions routed anywhere | agents 3 | +| `closed-kept`: closed decisions still listed | agents 3 | +| `seq-order`: no gated-first order | agents 3 | +| `any-raiser`: decisions raised by any role | agents 3 | +| `waiting-wins`: waiting over working | agents 4, 5, 6 | +| `role-gets-state`: role sessions get state too | agents 1 | +| `reader-plain`: readers get plain rows | agents 1 | +| `blocking-raw`: `blocking` not coerced to a boolean | agents 3 | +| `working-zero`: `working` set with no messages | agents 1, 3, 5, 6 | +| `endpoint-plain`: no state for anyone | webui endpoint test | +| `last-ask`: the last decision shown first | browser | +| `no-more-link`: no "+N more" | browser | +| `no-working-detail`: no message count or since | browser | +| `no-blocking-chip` | browser | +| `question-raw`: the question printed unescaped | browser | +| `no-state-col`: no State column | browser | + +`any-role-msg` is equivalent, as the packet says. `bindLaunch` refuses a +second launch record for a run name with another role +(`launch-record-mismatch`), so within a business a holder run belongs to +one role, and its deliveries are for that role's messages. + +## Notes + +**A. A runner whose claim outlives it.** The README's limits say a reply +that fails leaves its message unread. There is a second case. The +launcher calls `endLaunch` when a run's process exits, and `endLaunch` +releases the claim, so a crashed or killed runner normally clears. If the +launcher itself dies, or `endLaunch` is refused (the launcher logs +`endLaunch refused` and goes on), the claim stays and the role shows +working, with a `since` that keeps growing, or idle, until someone +revokes it. One clause in the limits paragraph would cover it. Not +blocking. + +**B. A delivered message can be stranded (predates this row).** `receive` +skips messages that already have a delivery. If a holder receives a +message and releases the role without marking it read, the next holder +never gets it. Row 59 shows this correctly: the new holder is idle and +doesn't inherit the old holder's count, as test 5 pins. But the message +sits there unanswered with nothing pointing at it. That belongs in a bus +row, not this one; I'm sending it to Sage. + +**C. Overlap with row 58.** Rows 58 and 59 both change `Broker` in +`broker.mjs` and the webui `bus.js`: row 58 changes `#taskRows` and the +Tasks view, and row 59 changes the `agents` branch, `#agentState` and the +Agents view. The hunks don't touch each other. Whichever row lands second +should rebase and rerun the browser test. + +## Questions + +1. **Working signal.** Received and not read is the right signal. The + runner marks a message read only after its turn's reply, and the bus + has no per-turn record, so it is the only signal available. The other + option, the claim's own activity, would need a new write. Note A is + where this signal gets it wrong. +2. **Attention.** The first decision and a count is right. The Inbox + already lists every decision in the same order, and one line per role + keeps the table readable. "+N more" links to the Inbox, so the rest is + one click away.