darkwing: row 81 (#1575) review, round 1, approve (comment 27783)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
+54
@@ -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 '<a href="#/trail/message/${enc(m.id)}">trail</a>' '<a href="#/trail/message/${enc(receipt.id)}">trail</a>'
|
||||
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;"
|
||||
@@ -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;
|
||||
@@ -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
|
||||
@@ -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.
|
||||
Reference in New Issue
Block a user