darkwing: row 76 round 2 review packet, approve, comment 27688 (#1570)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,43 @@
|
||||
#!/usr/bin/env bash
|
||||
# Row 76 round 2, Darkwing's mutants. Run from the candidate worktree root:
|
||||
# bash mutants.sh <mut.mjs>. Round 1's 16 with anchors updated to round 2's lines
|
||||
# and every page or Reply mutant pointed at send-browser, then 6 aimed at round 2's
|
||||
# changes. Each mutant edits one site, runs one test file, and restores the file
|
||||
# from the worktree index.
|
||||
set -u
|
||||
M=$1
|
||||
W=packages/webui
|
||||
S=$W/src/serve.mjs B=$W/src/bus.mjs C=$W/src/cli.mjs P=$W/src/public/bus.js A=$W/src/public/app.js
|
||||
run() { # name file from to testfile
|
||||
local name=$1 f=$2 from=$3 to=$4 t=$5
|
||||
echo "== $name"
|
||||
if ! node "$M" "$f" "$from" "$to"; then echo "$name: DID NOT APPLY"; return; fi
|
||||
node --test "$W/tests/$t.test.mjs" 2>&1 | grep -E '^ℹ (pass|fail)|^✖ ' | grep -v '^✖ failing tests' | sort -u | sed "s/^/$name: /"
|
||||
git checkout -q -- "$f"; echo "restored"
|
||||
}
|
||||
REFUSED='const refused = err.status >= 400 && err.status < 500 && err.status !== 408;'
|
||||
PAGE4XX='((res.status >= 400 && res.status < 500 && res.status !== 408) || body?.sent === false)'
|
||||
# Round 1's 16.
|
||||
run cli-no-bus-ok "$C" ' if (sendOn) throw new Error(`--send needs the bus configured (${why})`);' '' send
|
||||
run start-send-nonfunction "$S" "(preview === null || typeof send !== 'function')" '(preview === null)' send
|
||||
run open-no-invalid-response "$B" "const OPEN = ['outcome-unknown', 'invalid-response', 'response-too-large'];" "const OPEN = ['outcome-unknown', 'response-too-large'];" send
|
||||
run open-no-too-large "$B" "const OPEN = ['outcome-unknown', 'invalid-response', 'response-too-large'];" "const OPEN = ['outcome-unknown', 'invalid-response'];" send
|
||||
run nonbus-definite "$B" "err instanceof BusReadError ? err.code : 'outcome-unknown'" "err?.code ?? 'invalid-request'" send
|
||||
run missing-to-passes "$S" " || !('to' in value)" '' send
|
||||
run route-any-method "$S" " if (req.method !== 'POST') return json(res, 405, { error: 'method not allowed' });" '' send
|
||||
run status-mnf-502 "$B" "'message-not-found': 400, " '' send
|
||||
run success-unlogged "$S" ' line(`id=${result.id}`);' '' send
|
||||
run page-never-clears "$P" "if (draft.body === text) draft.body = '';" '' send-browser
|
||||
run page-caret-lost "$P" " if (k.caret && el.tagName === 'TEXTAREA') { el.setSelectionRange(k.caret[0], k.caret[1]); el.scrollTop = k.caret[2]; }" '' send-browser
|
||||
run page-sentfalse-ignored "$P" "$PAGE4XX" '(res.status >= 400 && res.status < 500 && res.status !== 408)' send-browser
|
||||
run page-no-config-gate "$P" "if (c?.send === true) {" "if (c) {" send-browser
|
||||
run page-composer-always "$P" " if (!canSend) return '';" '' send-browser
|
||||
run app-reply-4xx-unknown "$A" "$REFUSED" 'const refused = false;' send-browser
|
||||
run app-reply-5xx-refused "$A" "$REFUSED" 'const refused = err.status >= 400 && err.status !== 408;' send-browser
|
||||
# Round 2's changes.
|
||||
run page-code-never "$P" '${r.message.includes(`(${r.code})`) ? '"''"' : ` (<code>${txt(r.code)}</code>)`}' '' send-browser
|
||||
run page-code-any-paren "$P" 'r.message.includes(`(${r.code})`)' "r.message.includes('(')" send-browser
|
||||
run page-prefix-anywhere "$P" "body.message.replace(/^not sent: /, '')" "body.message.replace(/not sent: /g, '')" send-browser
|
||||
run page-408-only-with-body "$P" "$PAGE4XX" '((res.status >= 400 && res.status < 500 && (res.status !== 408 || body?.error)) || body?.sent === false)' send-browser
|
||||
run app-reply-4xx-all "$A" "$REFUSED" 'const refused = err.status >= 400 && err.status < 500;' send-browser
|
||||
run send-log-uncut "$S" 'JSON.stringify(to.slice(0, 64))' 'JSON.stringify(to)' send
|
||||
@@ -0,0 +1,125 @@
|
||||
start 2026-10-12T03:21:14Z
|
||||
== cli-no-bus-ok
|
||||
cli-no-bus-ok: ℹ fail 1
|
||||
cli-no-bus-ok: ℹ pass 12
|
||||
cli-no-bus-ok: ✖ the CLI refuses --send without the preview flags, and without a configured bus (10180.683387ms)
|
||||
restored
|
||||
== start-send-nonfunction
|
||||
start-send-nonfunction: ℹ fail 1
|
||||
start-send-nonfunction: ℹ pass 12
|
||||
start-send-nonfunction: ✖ startServer takes send only as a function and only with the preview mode (8.75863ms)
|
||||
restored
|
||||
== open-no-invalid-response
|
||||
open-no-invalid-response: ✖ a bus refusal is not sent, with its status; an outcome the bus could not confirm is unknown and never retried (7.076077ms)
|
||||
open-no-invalid-response: ℹ fail 2
|
||||
open-no-invalid-response: ℹ pass 11
|
||||
open-no-invalid-response: ✖ through the human transport the request is exactly the business, message.send and the keys (99.730383ms)
|
||||
restored
|
||||
== open-no-too-large
|
||||
open-no-too-large: ✖ a bus refusal is not sent, with its status; an outcome the bus could not confirm is unknown and never retried (6.896152ms)
|
||||
open-no-too-large: ℹ fail 1
|
||||
open-no-too-large: ℹ pass 12
|
||||
restored
|
||||
== nonbus-definite
|
||||
nonbus-definite: ✖ a bus refusal is not sent, with its status; an outcome the bus could not confirm is unknown and never retried (6.755831ms)
|
||||
nonbus-definite: ℹ fail 1
|
||||
nonbus-definite: ℹ pass 12
|
||||
restored
|
||||
== missing-to-passes
|
||||
missing-to-passes: ✖ each request check refuses before anything is sent, and logs the refusal without the body (9.521583ms)
|
||||
missing-to-passes: ℹ fail 1
|
||||
missing-to-passes: ℹ pass 12
|
||||
restored
|
||||
== route-any-method
|
||||
route-any-method: ✖ each request check refuses before anything is sent, and logs the refusal without the body (12.40535ms)
|
||||
route-any-method: ℹ fail 1
|
||||
route-any-method: ℹ pass 12
|
||||
restored
|
||||
== status-mnf-502
|
||||
status-mnf-502: ✖ a bus refusal is not sent, with its status; an outcome the bus could not confirm is unknown and never retried (3.622498ms)
|
||||
status-mnf-502: ✖ against a real broker: the message is written once as a human send, and its refusals map (100.667453ms)
|
||||
status-mnf-502: ℹ fail 2
|
||||
status-mnf-502: ℹ pass 11
|
||||
restored
|
||||
== success-unlogged
|
||||
success-unlogged: ✖ a send passes exactly to, body and in_reply_to, answers the id, and logs it (4.07318ms)
|
||||
success-unlogged: ℹ fail 2
|
||||
success-unlogged: ℹ pass 11
|
||||
success-unlogged: ✖ through the human transport the request is exactly the business, message.send and the keys (92.814415ms)
|
||||
restored
|
||||
== page-never-clears
|
||||
page-never-clears: ℹ fail 1
|
||||
page-never-clears: ℹ pass 0
|
||||
page-never-clears: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (2471.636336ms)
|
||||
restored
|
||||
== page-caret-lost
|
||||
page-caret-lost: ℹ fail 1
|
||||
page-caret-lost: ℹ pass 0
|
||||
page-caret-lost: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (1693.011574ms)
|
||||
restored
|
||||
== page-sentfalse-ignored
|
||||
page-sentfalse-ignored: ℹ fail 1
|
||||
page-sentfalse-ignored: ℹ pass 0
|
||||
page-sentfalse-ignored: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (1477.391499ms)
|
||||
restored
|
||||
== page-no-config-gate
|
||||
page-no-config-gate: ℹ fail 1
|
||||
page-no-config-gate: ℹ pass 0
|
||||
page-no-config-gate: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (1048.204932ms)
|
||||
restored
|
||||
== page-composer-always
|
||||
page-composer-always: ℹ fail 1
|
||||
page-composer-always: ℹ pass 0
|
||||
page-composer-always: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (965.674476ms)
|
||||
restored
|
||||
== app-reply-4xx-unknown
|
||||
app-reply-4xx-unknown: ℹ fail 1
|
||||
app-reply-4xx-unknown: ℹ pass 0
|
||||
app-reply-4xx-unknown: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (5945.940028ms)
|
||||
restored
|
||||
== app-reply-5xx-refused
|
||||
app-reply-5xx-refused: ℹ fail 1
|
||||
app-reply-5xx-refused: ℹ pass 0
|
||||
app-reply-5xx-refused: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (6091.845304ms)
|
||||
restored
|
||||
== page-code-never
|
||||
page-code-never: ℹ fail 1
|
||||
page-code-never: ℹ pass 0
|
||||
page-code-never: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (1516.082302ms)
|
||||
restored
|
||||
== page-code-any-paren
|
||||
page-code-any-paren: ℹ fail 0
|
||||
page-code-any-paren: ℹ pass 1
|
||||
restored
|
||||
== page-prefix-anywhere
|
||||
page-prefix-anywhere: ℹ fail 0
|
||||
page-prefix-anywhere: ℹ pass 1
|
||||
restored
|
||||
== page-408-only-with-body
|
||||
page-408-only-with-body: ℹ fail 1
|
||||
page-408-only-with-body: ℹ pass 0
|
||||
page-408-only-with-body: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (2209.917646ms)
|
||||
restored
|
||||
== app-reply-4xx-all
|
||||
app-reply-4xx-all: ℹ fail 1
|
||||
app-reply-4xx-all: ℹ pass 0
|
||||
app-reply-4xx-all: ✖ the send box: held roles, the draft and caret across a refresh, each receipt, one POST per Send, and the trail link; the board Reply on a 4xx and a 5xx (6127.183563ms)
|
||||
restored
|
||||
== send-log-uncut
|
||||
send-log-uncut: ✖ a crafted role name cannot forge a log line (5.067744ms)
|
||||
send-log-uncut: ℹ fail 1
|
||||
send-log-uncut: ℹ pass 12
|
||||
restored
|
||||
end 2026-10-12T03:22:09Z
|
||||
packages/webui/README.md: OK
|
||||
packages/webui/src/bus.mjs: OK
|
||||
packages/webui/src/cli.mjs: OK
|
||||
packages/webui/src/serve.mjs: OK
|
||||
packages/webui/src/public/app.js: OK
|
||||
packages/webui/src/public/bus.js: OK
|
||||
packages/webui/tests/send.test.mjs: OK
|
||||
packages/webui/tests/send-browser.test.mjs: OK
|
||||
packages/webui/tests/shell.test.mjs: OK
|
||||
A packages/webui/tests/send-browser.test.mjs
|
||||
A packages/webui/tests/send.test.mjs
|
||||
?? node_modules
|
||||
@@ -0,0 +1,26 @@
|
||||
start 2026-10-12T02:43:32Z node v26.8.1 tree 729da72b+build.patch (manifest 04ecf1b5)
|
||||
== send
|
||||
ℹ fail 0
|
||||
ℹ pass 13
|
||||
== send-browser
|
||||
ℹ fail 0
|
||||
ℹ pass 1
|
||||
== shell
|
||||
ℹ fail 0
|
||||
ℹ pass 5
|
||||
== bus-browser
|
||||
ℹ fail 0
|
||||
ℹ pass 1
|
||||
== bus
|
||||
ℹ fail 0
|
||||
ℹ pass 9
|
||||
== browser
|
||||
ℹ fail 0
|
||||
ℹ pass 1
|
||||
== serve
|
||||
ℹ fail 0
|
||||
ℹ pass 4
|
||||
== preview
|
||||
ℹ fail 0
|
||||
ℹ pass 11
|
||||
end 2026-10-12T02:43:53Z
|
||||
@@ -0,0 +1,173 @@
|
||||
# Row 76, Console sends a message to a role, round 2 review (Darkwing)
|
||||
|
||||
Issue #1570, request comment 27672, packet `agents/dewey/work/queue-76/`
|
||||
(commit `86821889`). Base `729da72b`. Candidate manifest sha256
|
||||
`04ecf1b57c594a35d34d53277b2df915a571f4fa9f7cc29146dbedfa612f77d4`, 9
|
||||
files, matching the request. Scope, per lead decisions 99 and 100: the
|
||||
rebase on row 59, my round 1 notes N1 to N5 and the Q1 proxy line, and
|
||||
Filbert's round 1 notes, each read as content.
|
||||
|
||||
Verdict: **approve**. The rebase changes no line I approved in round 1.
|
||||
Each of my notes is fixed as I asked, and N1 and N2 are now held by tests:
|
||||
the three mutants that survived my round 1 run all fail here. Filbert's 408
|
||||
change is right, and conservative in the direction that matters. Nothing
|
||||
blocks. One cosmetic note.
|
||||
|
||||
## Method
|
||||
|
||||
- A detached worktree at `729da72b` with `git apply --index build.patch`,
|
||||
then `sha256sum -c` on the manifest: 9 OK, and again after the mutants.
|
||||
`git diff --cached 729da72b --binary -- packages/webui` there hashes the
|
||||
same as `build.patch` (`6481006b…`). No commit from `729da72b` to
|
||||
`2bbc3097` touches the 9 paths.
|
||||
- To split the rebase from the content: a second worktree at `729da72b`
|
||||
with round 1's `build.patch` (`1deb6143`) applied by `git apply --3way
|
||||
--index`, then `diff -ru` of `packages/webui` between the two trees.
|
||||
- I read the content diff against my round 1 review (comment 27612), the
|
||||
bus code behind N3 and N4, and the row 59 lines near the merged hunks.
|
||||
- Eight WebUI test files, then 22 mutants (`r2/mutants.sh`): round 1's 16
|
||||
with their anchors moved to round 2's lines, and 6 aimed at round 2's
|
||||
changes. Each edits one site, runs one test file and restores from the
|
||||
worktree index.
|
||||
|
||||
Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv76r2/tmp`.
|
||||
|
||||
## The rebase
|
||||
|
||||
Round 1's patch merges onto `729da72b` by `--3way` without a conflict.
|
||||
On that tree, round 1's manifest (`ca870314`) checks 7 OK, and fails only
|
||||
`README.md` and `public/bus.js`, the two files row 59 also changed. That
|
||||
matches `build.md`.
|
||||
|
||||
None of the patch's removed lines in those two files are row 59's. The
|
||||
README's "Agent state" section and `bus.js`'s `stateMark`, `attention`
|
||||
and `agentsView` lines are untouched, and `bus-browser.test.mjs`, which
|
||||
holds row 59's agents view, passes on the candidate.
|
||||
|
||||
## The content changes
|
||||
|
||||
Between the rebased round 1 tree and the candidate, five files differ
|
||||
(291 lines of `diff -ru`). `serve.mjs`, `bus.mjs`, `cli.mjs` and
|
||||
`shell.test.mjs` are byte for byte round 1's. Every change is one
|
||||
`build.md` lists under decisions 99 or 100.
|
||||
|
||||
**My N1, the board Reply.** `send-browser` now opens an inspection, types a
|
||||
reply, and answers `/api/reply` with 403, 502 and 408 from the stub. It
|
||||
asserts the exact receipt each time, that the reply text is kept, and that
|
||||
the fake board captured nothing. `app-reply-4xx-unknown` and
|
||||
`app-reply-5xx-refused` survived six files in round 1 and fail here.
|
||||
|
||||
**My N2, the caret.** The refresh case focuses the text box, selects 3 to
|
||||
7, clicks Refresh from script (which doesn't move focus) and asserts focus
|
||||
and `{start: 3, end: 7}` on the redrawn box. `page-caret-lost` fails here.
|
||||
|
||||
**My N3.** The README and the box's note now say the message goes the same
|
||||
way as one from the CLI, with class INFO. `mosaic talk` sends `class:
|
||||
"REQUEST"` (`packages/cli/src/cli.mjs:199`), so the README's "where `mosaic
|
||||
talk` uses REQUEST" is accurate.
|
||||
|
||||
**My N4.** `message.receive` selects messages with no `delivered` or
|
||||
`read` row and writes `delivered` for each (`broker.mjs` 777 to 784); talk
|
||||
calls `message.read` for each one it prints (`cli.mjs:208`). The new
|
||||
sentence says exactly that.
|
||||
|
||||
**My N5.** The receipt drops the server's leading `not sent: ` and adds
|
||||
the code only when the message doesn't already contain `(code)`. Both
|
||||
browser receipts are now asserted as exact strings. A refusal before the
|
||||
bus (`Content-Type must be application/json`) still gets its code, and the
|
||||
sign-in receipt keeps `(sign-in)`, which the test asserts. Dropping the
|
||||
prefix goes past what I asked; I agree with it, since without it the
|
||||
receipt read "Not sent to coder: not sent: ...".
|
||||
|
||||
**Q1.** The new bullet in "What the proxy must do" is the line I proposed,
|
||||
with "a 4xx other than 408" after Filbert's N1.
|
||||
|
||||
**Filbert's N1, 408.** Both checks now leave 408 out of "not sent". I
|
||||
agree. Reading a 408 as unknown can only cost a "check before resending"
|
||||
where "not sent" would have been true; the other direction risks a
|
||||
duplicate message. Neither the Console's handlers (`serve.mjs` answers
|
||||
200, 400, 401, 403, 404, 405, 500, 502 and 503, plus the board's own
|
||||
status on the Reply proxy) nor the board send a 408. Node's request
|
||||
timeout can, but only while the request is still arriving, so before
|
||||
anything is sent; unknown is cautious there, not wrong. A 408 carrying the
|
||||
Console's `sent: false` would still read as not sent, and `build.md` says
|
||||
so. The send box's
|
||||
unknown loop and the Reply case each gain a 408.
|
||||
|
||||
**Filbert's N3.** The six tests are where `build.md` says: the caret and
|
||||
focus case, `pm` dropped by `endLaunch` and still selected as `pm (not held
|
||||
now)`, Inbox current on a message trail, `in_reply_to: ""` passed through,
|
||||
the `ENOENT` error now carrying `code`, and the log's `to` cut at exactly 64.
|
||||
|
||||
## Tests and mutants
|
||||
|
||||
On the candidate, 02:43:32Z to 02:43:53Z on 2026-10-12
|
||||
(`r2/tests-light.txt`):
|
||||
|
||||
| File | Result |
|
||||
|---|---|
|
||||
| `send.test.mjs` | 13 pass, 0 fail |
|
||||
| `send-browser.test.mjs` | 1 pass, 0 fail |
|
||||
| `shell.test.mjs` | 5 pass, 0 fail |
|
||||
| `bus-browser.test.mjs` | 1 pass, 0 fail |
|
||||
| `bus.test.mjs` | 9 pass, 0 fail |
|
||||
| `browser.test.mjs` | 1 pass, 0 fail |
|
||||
| `serve.test.mjs` | 4 pass, 0 fail |
|
||||
| `preview.test.mjs` | 11 pass, 0 fail |
|
||||
|
||||
I didn't run the full gate. Dewey's is green with Docker, Filbert was
|
||||
running theirs while I reviewed, and Sage reruns it.
|
||||
|
||||
I started my mutants after Filbert's own mutant run on this row had ended
|
||||
(03:21:03Z), so the two didn't share the machine. 03:21:14Z to 03:22:09Z
|
||||
(`r2/mutants.txt`), the manifest 9 OK afterwards:
|
||||
|
||||
| Mutant | Round 1 | Round 2 fails |
|
||||
|---|---|---|
|
||||
| `cli-no-bus-ok` | killed | the CLI refusal test |
|
||||
| `start-send-nonfunction` | killed | the `startServer` test |
|
||||
| `open-no-invalid-response` | killed | the outcome test, the transport test |
|
||||
| `open-no-too-large` | killed | the outcome test |
|
||||
| `nonbus-definite` | killed | the outcome test |
|
||||
| `missing-to-passes` | killed | the request checks test |
|
||||
| `route-any-method` | killed | the request checks test |
|
||||
| `status-mnf-502` | killed | the outcome test, the real broker test |
|
||||
| `success-unlogged` | killed | the send test, the transport test |
|
||||
| `page-never-clears` | killed | `send-browser` |
|
||||
| `page-sentfalse-ignored` | killed | `send-browser` |
|
||||
| `page-no-config-gate` | killed | `send-browser` |
|
||||
| `page-composer-always` | killed | `send-browser` |
|
||||
| `page-caret-lost` | survived | `send-browser` |
|
||||
| `app-reply-4xx-unknown` | survived | `send-browser` |
|
||||
| `app-reply-5xx-refused` | survived | `send-browser` |
|
||||
| `page-code-never`: no code appended, ever | new | `send-browser` |
|
||||
| `page-408-only-with-body`: a 408 with an `error` reads as not sent | new | `send-browser` |
|
||||
| `app-reply-4xx-all`: the Reply reads a 408 as not sent | new | `send-browser` |
|
||||
| `send-log-uncut`: the log's `to` not cut | new | the forged-line test |
|
||||
| `page-code-any-paren`: any `(` in the message drops the code | new | survives |
|
||||
| `page-prefix-anywhere`: `not sent: ` removed anywhere, not only at the start | new | survives |
|
||||
|
||||
All three of round 1's survivors fail now. Round 1's seven page and Reply
|
||||
mutants ran against `send-browser` alone this time, and that one file
|
||||
catches all seven.
|
||||
|
||||
The two survivors are equivalent for every message the Console sends. The
|
||||
only one with a parenthesis or `not sent: ` in it is the bus refusal,
|
||||
`not sent: the bus refused it (${code})` (`serve.mjs:98`), where both
|
||||
edits give the same receipt. They could differ only for a proxy's JSON
|
||||
body with a `message` field, which is outside the contract. No test
|
||||
needed.
|
||||
|
||||
## Note
|
||||
|
||||
**1. The new sentence in `send-browser.test.mjs`'s header** ("A role's run
|
||||
is ended in the broker to show a role no longer held") went into the middle
|
||||
of line 4 without a reflow. The line is 157 characters, and the sentence
|
||||
sits between "recording what the page sent" and "Server-side checks are
|
||||
send.test.mjs's", where it reads as part of the stub's description. It
|
||||
belongs with the broker sentence after it. Cosmetic; a follow-up or the
|
||||
landing commit can move it.
|
||||
|
||||
## Round 1 questions
|
||||
|
||||
Q1 is in the README as above. Q2 stays as built.
|
||||
Reference in New Issue
Block a user