From 3e797633c9bd07368cbc205344552d020dc5d271 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 12 Oct 2026 13:51:09 -0500 Subject: [PATCH] darkwing: row 81 (#1575) review, round 1, approve (comment 27783) Co-Authored-By: Claude Opus 5.5 --- .../work/queue-81-review/r1/mutants.sh | 54 +++++++++ .../work/queue-81-review/r1/mutants.txt | 9 ++ .../work/queue-81-review/r1/tests.txt | 3 + .../work/queue-81-review/review-r1.md | 110 ++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100755 agents/darkwing/work/queue-81-review/r1/mutants.sh create mode 100644 agents/darkwing/work/queue-81-review/r1/mutants.txt create mode 100644 agents/darkwing/work/queue-81-review/r1/tests.txt create mode 100644 agents/darkwing/work/queue-81-review/review-r1.md diff --git a/agents/darkwing/work/queue-81-review/r1/mutants.sh b/agents/darkwing/work/queue-81-review/r1/mutants.sh new file mode 100755 index 000000000..dadb41e52 --- /dev/null +++ b/agents/darkwing/work/queue-81-review/r1/mutants.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# Darkwing row 81 review mutants: single-site edits to the candidate in WORKTREE, +# at sites Dewey's 17 don't name. Each backs up one file, edits one site with +# queue-59-review/r1/mut.mjs (exactly one match, else BAD-SITE), runs the named +# test files and restores the backup. +# Usage: mutants.sh WORKTREE (TMPDIR must be set; outputs go there) +set -u +WT="$1" +MUT="$(cd "$(dirname "$0")/../../queue-59-review/r1" && pwd)/mut.mjs" +cd "$WT" || exit 1 +BR=packages/bus/src/broker.mjs SV=packages/webui/src/serve.mjs PG=packages/webui/src/public/bus.js +BT="tests/replies.test.mjs" WT1="tests/bus.test.mjs" WB="tests/send-browser.test.mjs" + +m() { # name file pkg tests from to + local name="$1" file="$2" pkg="$3" tests="$4" from="$5" to="$6" + cp -- "$file" "$TMPDIR/mut.bak" + if ! node "$MUT" "$file" "$from" "$to"; then echo "$name BAD-SITE"; cp -- "$TMPDIR/mut.bak" "$file"; return; fi + if (cd "packages/$pkg" && node --test $tests > "$TMPDIR/$name.out" 2>&1); then + echo "$name SURVIVED" + else + echo "$name killed: $(grep -E '^✖ ' "$TMPDIR/$name.out" | grep -v 'failing tests' | sort -u | sed 's/ ([0-9.]*ms)$//' | tr '\n' ';')" + fi + cp -- "$TMPDIR/mut.bak" "$file" +} + +# The broker. +m exists-any-business $BR bus $BT "'SELECT id FROM messages WHERE id=? AND business=?', a.id, s.business))" "'SELECT id FROM messages WHERE id=?', a.id))" +m replies-to-human $BR bus $BT "WHERE business=? AND in_reply_to=? ORDER BY seq LIMIT \${REPLIES}\`, + s.business, + a.id," "WHERE business=? AND to_role=? ORDER BY seq LIMIT \${REPLIES}\`, + s.business, + 'human'," +m no-identifier $BR bus $BT " keys(a, ['id'], ['id']); + identifier(a.id);" " keys(a, ['id'], ['id']);" +m human-only $BR bus $BT "if (!s.human && !s.reader) fail('human-required'); + keys(a, ['id'], ['id']);" "if (!s.human) fail('human-required'); + keys(a, ['id'], ['id']);" +# The server. +m id-check-after-bus $SV webui $WT1 " if (verb === 'replies') { + arg = new URLSearchParams(search).get('id'); + if (!subjectOk(arg)) return json(res, 400, { error: 'invalid-request', message: 'id must be a message id' }); + } + if (!bus) return json(res, 503, { error: 'not-configured', message: 'the Console has no bus configured' });" " if (!bus) return json(res, 503, { error: 'not-configured', message: 'the Console has no bus configured' }); + if (verb === 'replies') { + arg = new URLSearchParams(search).get('id'); + if (!subjectOk(arg)) return json(res, 400, { error: 'invalid-request', message: 'id must be a message id' }); + }" +# The page. +m shows-to-role $PG webui $WB '${txt(m.from_role)} ${when(m.at)}' '${txt(m.to_role)} ${when(m.at)}' +m link-to-send $PG webui $WB 'trail' 'trail' +m any-receipt-reads $PG webui $WB " if (receipt?.kind === 'sent') { + const r = receipt;" " if (receipt) { + const r = receipt;" +m failure-keeps-old $PG webui $WB "if (receipt === r) replies = Array.isArray(rows) ? rows : null;" "if (receipt === r && Array.isArray(rows)) replies = rows;" diff --git a/agents/darkwing/work/queue-81-review/r1/mutants.txt b/agents/darkwing/work/queue-81-review/r1/mutants.txt new file mode 100644 index 000000000..144073e07 --- /dev/null +++ b/agents/darkwing/work/queue-81-review/r1/mutants.txt @@ -0,0 +1,9 @@ +exists-any-business killed: ✖ an id that names no message in the business refuses message-not-found, and the refusal is recorded; +replies-to-human killed: ✖ an id that names no message in the business refuses message-not-found, and the refusal is recorded;✖ a reply is listed for human and reader sessions, and its delivery rows are unchanged; +no-identifier killed: ✖ strict arguments; a role session gets human-required; +human-only killed: ✖ a message with no reply gives an empty list;✖ an id that names no message in the business refuses message-not-found, and the refusal is recorded;✖ a reply is listed for human and reader sessions, and its delivery rows are unchanged;✖ replies come oldest first and stop at REPLIES;✖ strict arguments; a role session gets human-required;✖ the reply still reaches the human message.receive (mosaic talk) after the Console showed it; +id-check-after-bus killed: ✖ bus refusals map to statuses and never carry a path or a stack; +shows-to-role killed: ✖ a sent receipt lists its replies as the page refreshes, oldest first and capped, and marks none delivered or read; +link-to-send killed: ✖ a sent receipt lists its replies as the page refreshes, oldest first and capped, and marks none delivered or read; +any-receipt-reads SURVIVED +failure-keeps-old killed: ✖ a sent receipt lists its replies as the page refreshes, oldest first and capped, and marks none delivered or read; diff --git a/agents/darkwing/work/queue-81-review/r1/tests.txt b/agents/darkwing/work/queue-81-review/r1/tests.txt new file mode 100644 index 000000000..7e74f2e2b --- /dev/null +++ b/agents/darkwing/work/queue-81-review/r1/tests.txt @@ -0,0 +1,3 @@ +bus rc=0 18:45:51-18:45:55 ℹ pass 102 ℹ fail 0 +webui rc=0 18:45:55-18:46:52 ℹ pass 83 ℹ fail 0 +cli rc=0 18:46:52-18:47:21 ℹ pass 115 ℹ fail 0 diff --git a/agents/darkwing/work/queue-81-review/review-r1.md b/agents/darkwing/work/queue-81-review/review-r1.md new file mode 100644 index 000000000..81f359962 --- /dev/null +++ b/agents/darkwing/work/queue-81-review/review-r1.md @@ -0,0 +1,110 @@ +# Row 81, the Console shows the reply to a web send, round 1 review (Darkwing) + +Issue #1575, request comment 27763, packet `agents/dewey/work/queue-81/` +(commit `5afa8b0f`). Brief: `docs/plans/2026-10-11_console-reply-view.md` +§ "Show replies to a message without receiving them" (blob `dd42071d`). +Base `ff11aa4b`. Candidate manifest sha256 +`2263ff5f2866ba57d167a060668e819874c893a18b514f62c54fbe4089d6ec2f`, 12 +files, matching the request. + +Verdict: **approve**. The new `replies` read is registered everywhere +`credentials` is, refuses what the brief says it refuses, and writes no +delivery or read row. The receipt drops a late answer for an earlier send, +and a failed replies read stays inside the composer. Nothing blocks. Three +small notes follow, then my answers to Dewey's three questions. + +## Method + +- A detached worktree at `ff11aa4b` with `git apply --index build.patch`, + then `sha256sum -c` on the manifest: 12 OK, and again after my mutants. + `git diff --cached --binary ff11aa4b` there hashes the same as + `build.patch` (`0da56968…`). No commit from `ff11aa4b` to `4918d410` + touches the 12 paths. +- I read the whole patch against the brief. For the verb, I checked every + place `credentials` is registered (the reader allowlist and the dispatch + case in `broker.mjs`, `views.mjs`, `BUS_VERBS` in `serve.mjs`) and found + `replies` in each. No other verb list in `packages/` needs it. +- For the page, I read the base `reader(live)` and `render`'s fallback to + the kept reader (`public/bus.js` 49 to 70 and 640 to 650) to see what the + composer's `get('replies', id)` does on a stale render (note 2). +- The bus, webui and cli suites on the candidate, then 9 mutants of my own + (`r1/mutants.sh`) next to Dewey's 17. Most edit sites theirs don't; + `human-only` overlaps their `reader not allowed`. Each edits one site + with `queue-59-review/r1/mut.mjs`, runs one test file and restores the + file from a backup. + +Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv81/tmp`. + +## Tests and mutants + +On the candidate, 18:45:51Z to 18:47:21Z on 2026-10-12 +(`r1/tests.txt`): bus 102, webui 83 and cli 115, all pass, 0 fail. I +didn't rerun the scripts suites. Dewey's gate has them green, and Sage +reruns it. + +Mutants ran 18:47:27Z to 18:48:27Z (`r1/mutants.txt`): + +| Mutant | Edit | Result | +|---|---|---| +| `exists-any-business` | the existence check drops `business=?` | killed: the message-not-found test | +| `replies-to-human` | the query lists messages to the human, not replies | killed: two tests | +| `no-identifier` | no `identifier(a.id)` | killed: the strict arguments test | +| `human-only` | a reader session refused | killed: six tests | +| `id-check-after-bus` | the `id` check after the no-bus 503 | killed: the refusals test | +| `shows-to-role` | a reply shows `to_role`, not `from_role` | killed: the browser test | +| `link-to-send` | each reply links to the sent message's trail | killed: the browser test | +| `failure-keeps-old` | a failed read keeps the last list | killed: the browser test | +| `any-receipt-reads` | the composer reads replies for any receipt | survives, see note 3 | + +`exists-any-business` covers the defence Dewey mentions for the equivalent +mutant they didn't run: the existence check has its own business filter, +and the cross-business test catches it. + +## Notes + +**1. One comment line in `packages/webui/src/bus.mjs` isn't wrapped.** +Line 6 is 103 characters ("reachable as reads: no read writes, not +even seen or delivered (Q4). The one write is row 76's human"), where the +rest of the comment wraps near 76. Cosmetic; the landing commit or round 2 +can reflow it. + +**2. On a stale render the replies come from the kept answers.** When a +live read elsewhere on the page fails, `render` redraws with +`reader(false)`. The composer's `get('replies', id)` then returns the last +answer for that id if there is one. If there isn't, it throws `not-read`, +and the receipt says the replies couldn't be read. The cached list doesn't +say it's old, but the page's stale banner already does, with the oldest +`at` across every read, including this one. I think that's right and +needs no change. + +**3. `any-receipt-reads` survives, and I'd leave it.** With the guard +loosened to any receipt, a refused or unknown receipt has no `id`, so the +read goes to `/api/bus/replies` with no query, the server answers 400, and +the composer sets `replies` to `null`. Neither receipt kind renders +`repliesHtml`, so the page looks the same. The only effect is a wasted +request per render. A test could pin it by counting replies requests after +a refused send, but I don't think it's worth a round. + +**The overlap with row 61.** Row 61 (#1550, mine, in review) also changes +`serve.mjs`, `public/bus.js` and `packages/webui/README.md`. The hunks +differ, and both patches apply on `4918d410` with `git apply --3way` in +either order. Whichever row lands second needs a new round for its +manifest. That's a landing-order matter for Sage, not a finding here. + +## Dewey's three questions + +**1. A new verb, not a `trail` field.** Agreed. `trail` answers `[]` for +an unknown subject, and the brief wants this read to refuse an unknown id. +Folding replies into trail would either change trail's answer for every +caller or give one field its own refusal rule. A separate verb also keeps +the 10 s refresh from reading the whole trail. + +**2. The bus README verb-table row outside the reads section.** Keep it. +`decisions` and `credentials` have rows in that table, so leaving +`replies` out would make the table wrong. I read the brief's file +boundary as naming the README, not one section of it. + +**3. `packages/cli/README.md` lines 61 to 65.** Agreed, a follow-up. "The +reply still lands in the inbox" means the human's message inbox, which is +true, but next to this row's timeout wording it reads like `mosaic +inbox`. "The next `mosaic talk` prints it" would match the CLI's message.