review(darkwing): row 76 round 1 review packet, approve (#1570)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,20 @@
|
||||
#!/usr/bin/env bash
|
||||
# The four survivors of mutants.sh, rerun against the other test files that could catch them.
|
||||
set -u
|
||||
M=$1
|
||||
W=packages/webui
|
||||
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; shift 4
|
||||
for t in "$@"; do
|
||||
echo "== $name on $t"
|
||||
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"
|
||||
done
|
||||
}
|
||||
run page-caret-lost "$P" " if (k.caret && el.tagName === 'TEXTAREA') { el.setSelectionRange(k.caret[0], k.caret[1]); el.scrollTop = k.caret[2]; }" '' bus-browser
|
||||
run page-no-config-gate "$P" "if (c?.send === true) {" "if (c) {" send-browser shell
|
||||
run page-composer-always "$P" " if (!canSend) return '';" '' send-browser shell
|
||||
run app-reply-4xx-unknown "$A" 'const refused = err.status >= 400 && err.status < 500;' 'const refused = false;' return-flow browser-edge history-return-flow shell
|
||||
run app-reply-5xx-refused "$A" 'const refused = err.status >= 400 && err.status < 500;' 'const refused = err.status >= 400;' return-flow browser-edge
|
||||
@@ -0,0 +1,48 @@
|
||||
start 2026-10-12T01:19:14Z
|
||||
== page-caret-lost on bus-browser
|
||||
page-caret-lost: ℹ fail 0
|
||||
page-caret-lost: ℹ pass 1
|
||||
restored
|
||||
== page-no-config-gate on send-browser
|
||||
page-no-config-gate: ℹ fail 1
|
||||
page-no-config-gate: ℹ pass 0
|
||||
page-no-config-gate: ✖ the send box: held roles, the draft across a refresh, each receipt, one POST per Send, and the trail link (3517.666605ms)
|
||||
restored
|
||||
== page-no-config-gate on shell
|
||||
page-no-config-gate: ℹ fail 0
|
||||
page-no-config-gate: ℹ pass 5
|
||||
restored
|
||||
== page-composer-always on send-browser
|
||||
page-composer-always: ℹ fail 1
|
||||
page-composer-always: ℹ pass 0
|
||||
page-composer-always: ✖ the send box: held roles, the draft across a refresh, each receipt, one POST per Send, and the trail link (1171.730778ms)
|
||||
restored
|
||||
== page-composer-always on shell
|
||||
page-composer-always: ℹ fail 0
|
||||
page-composer-always: ℹ pass 5
|
||||
restored
|
||||
== app-reply-4xx-unknown on return-flow
|
||||
app-reply-4xx-unknown: ℹ fail 0
|
||||
app-reply-4xx-unknown: ℹ pass 1
|
||||
restored
|
||||
== app-reply-4xx-unknown on browser-edge
|
||||
app-reply-4xx-unknown: ℹ fail 0
|
||||
app-reply-4xx-unknown: ℹ pass 1
|
||||
restored
|
||||
== app-reply-4xx-unknown on history-return-flow
|
||||
app-reply-4xx-unknown: ℹ fail 0
|
||||
app-reply-4xx-unknown: ℹ pass 1
|
||||
restored
|
||||
== app-reply-4xx-unknown on shell
|
||||
app-reply-4xx-unknown: ℹ fail 0
|
||||
app-reply-4xx-unknown: ℹ pass 5
|
||||
restored
|
||||
== app-reply-5xx-refused on return-flow
|
||||
app-reply-5xx-refused: ℹ fail 0
|
||||
app-reply-5xx-refused: ℹ pass 1
|
||||
restored
|
||||
== app-reply-5xx-refused on browser-edge
|
||||
app-reply-5xx-refused: ℹ fail 0
|
||||
app-reply-5xx-refused: ℹ pass 1
|
||||
restored
|
||||
end 2026-10-12T01:22:00Z
|
||||
@@ -0,0 +1,31 @@
|
||||
#!/usr/bin/env bash
|
||||
# Row 76 round 1, Darkwing's mutants. Run from the candidate worktree root:
|
||||
# bash mutants.sh <mut.mjs>. 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"
|
||||
}
|
||||
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" '((res.status >= 400 && res.status < 500) || body?.sent === false)' '(res.status >= 400 && res.status < 500)' send-browser
|
||||
run page-no-config-gate "$P" "if (c?.send === true) {" "if (c) {" bus-browser
|
||||
run page-composer-always "$P" " if (!canSend) return '';" '' bus-browser
|
||||
run app-reply-4xx-unknown "$A" 'const refused = err.status >= 400 && err.status < 500;' 'const refused = false;' browser
|
||||
run app-reply-5xx-refused "$A" 'const refused = err.status >= 400 && err.status < 500;' 'const refused = err.status >= 400;' preview
|
||||
@@ -0,0 +1,80 @@
|
||||
start 2026-10-12T01:17:37Z
|
||||
== 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 (10200.769622ms)
|
||||
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 (11.492433ms)
|
||||
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.441283ms)
|
||||
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 (117.993747ms)
|
||||
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 (8.767124ms)
|
||||
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 (8.725677ms)
|
||||
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 (16.621682ms)
|
||||
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 (28.066332ms)
|
||||
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 (6.018709ms)
|
||||
status-mnf-502: ✖ against a real broker: the message is written once as a human send, and its refusals map (97.093713ms)
|
||||
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.473721ms)
|
||||
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 (135.764088ms)
|
||||
restored
|
||||
== page-never-clears
|
||||
page-never-clears: ℹ fail 1
|
||||
page-never-clears: ℹ pass 0
|
||||
page-never-clears: ✖ the send box: held roles, the draft across a refresh, each receipt, one POST per Send, and the trail link (7056.220257ms)
|
||||
restored
|
||||
== page-caret-lost
|
||||
page-caret-lost: ℹ fail 0
|
||||
page-caret-lost: ℹ pass 1
|
||||
restored
|
||||
== page-sentfalse-ignored
|
||||
page-sentfalse-ignored: ℹ fail 1
|
||||
page-sentfalse-ignored: ℹ pass 0
|
||||
page-sentfalse-ignored: ✖ the send box: held roles, the draft across a refresh, each receipt, one POST per Send, and the trail link (6137.191886ms)
|
||||
restored
|
||||
== page-no-config-gate
|
||||
page-no-config-gate: ℹ fail 0
|
||||
page-no-config-gate: ℹ pass 1
|
||||
restored
|
||||
== page-composer-always
|
||||
page-composer-always: ℹ fail 0
|
||||
page-composer-always: ℹ pass 1
|
||||
restored
|
||||
== app-reply-4xx-unknown
|
||||
app-reply-4xx-unknown: ℹ fail 0
|
||||
app-reply-4xx-unknown: ℹ pass 1
|
||||
restored
|
||||
== app-reply-5xx-refused
|
||||
app-reply-5xx-refused: ℹ fail 0
|
||||
app-reply-5xx-refused: ℹ pass 11
|
||||
restored
|
||||
end 2026-10-12T01:18:51Z
|
||||
@@ -0,0 +1,20 @@
|
||||
start 2026-10-12T01:16:27Z
|
||||
== send
|
||||
ℹ pass 13
|
||||
ℹ fail 0
|
||||
== send-browser
|
||||
ℹ pass 1
|
||||
ℹ fail 0
|
||||
== shell
|
||||
ℹ pass 5
|
||||
ℹ fail 0
|
||||
== preview
|
||||
ℹ pass 11
|
||||
ℹ fail 0
|
||||
== bus
|
||||
ℹ pass 9
|
||||
ℹ fail 0
|
||||
== serve
|
||||
ℹ pass 4
|
||||
ℹ fail 0
|
||||
end 2026-10-12T01:16:44Z
|
||||
@@ -0,0 +1,180 @@
|
||||
# Row 76, Console sends a message to a role, round 1 review (Darkwing)
|
||||
|
||||
Issue #1570, request comment 27594, packet `agents/dewey/work/queue-76/`
|
||||
(commit `1deb6143`). Brief: `docs/plans/2026-10-11_dogfood-cutover.md`,
|
||||
"Console sends a message to a role". Base `3d298920`. Candidate manifest
|
||||
sha256 `ca8703145aed98142c1fdf0553ab846a9b9a808cd58c35f7cbb4fa35d7f279a2`,
|
||||
9 files, matching the request. Sage asked for this round on content now;
|
||||
the row re-rounds after row 59's landing, which changes the README and
|
||||
`bus.js` digests.
|
||||
|
||||
Verdict: **approve**. The route refuses everything the brief lists before
|
||||
it calls the bus, and logs one line per send without the body. Dewey's
|
||||
decision 5, that a coded bus answer means nothing was written, holds on
|
||||
every path I traced from the socket to the page. The page sends once per
|
||||
press and never resends. Nothing blocks. Five notes and my answers to the
|
||||
two questions are below.
|
||||
|
||||
## Method
|
||||
|
||||
- A detached worktree at `3d298920` with `git apply --index build.patch`,
|
||||
then `sha256sum -c` on the manifest: 9 OK, and again after the mutants.
|
||||
None of the 9 paths changed from `3d298920` to `1ab6e181` at the start.
|
||||
- I read the whole patch, and the bus code the send passes through:
|
||||
`Broker.request` and the `message.send` case, `Store.transaction`, the
|
||||
socket server, `Client.call`, `human-cli.mjs` and `humanCall`.
|
||||
- Six WebUI test files, then 16 mutants of my own (`r1/mutants.sh`) and a
|
||||
rerun of the four survivors against other files (`r1/m2.sh`). Each mutant
|
||||
edits one site, runs one test file and restores from the worktree index,
|
||||
using `agents/darkwing/work/queue-59-review/r1/mut.mjs`.
|
||||
|
||||
Node v26.8.1, `TMPDIR=~/darkwing-scratch/rv76/tmp`.
|
||||
|
||||
While I was reviewing, Dewey's working copy of the manifest and
|
||||
`build.patch` changed (uncommitted, the row 59 re-round). Everything here
|
||||
is against the committed `ca870314`, which still checks 9 OK on my tree.
|
||||
|
||||
## Is a coded answer really "not written"?
|
||||
|
||||
This is what the page's "Not sent" rests on, so I traced each way an error
|
||||
code can reach `sendOutcome` (`packages/webui/src/bus.mjs:78`).
|
||||
|
||||
- The broker runs `#dispatch` and the result's secret check inside one
|
||||
`BEGIN IMMEDIATE` transaction (`broker.mjs` `request`, `store.mjs`
|
||||
`transaction`). Anything thrown there, a failed `COMMIT` included, rolls
|
||||
back. `#refused` then writes its `action.refused` event in a separate
|
||||
transaction, or answers `storage-unavailable` if it can't. Both come
|
||||
after the rollback.
|
||||
- The socket server (`server.mjs`) calls `broker.request` synchronously for
|
||||
`message.send`, so its `request-timeout` (line 27) can only fire before
|
||||
the request is handled. `invalid-envelope`, `incomplete-request` and
|
||||
`request-too-large` also come before the broker.
|
||||
- `Client.call` turns every socket error, end or timeout into
|
||||
`outcome-unknown`, and an answer it can't parse into `invalid-response`.
|
||||
It refuses an oversized request before connecting.
|
||||
- `human-cli.mjs` writes `invalid-request` for an uncoded error (line 37).
|
||||
Inside its `try`, the only steps that can throw without a code run
|
||||
before `client.call`. A failure writing the result to stdout surfaces as
|
||||
an `error` event, not in that `catch`, and the parent reads that as
|
||||
`invalid-response`.
|
||||
- `humanCall` maps a kill, a timeout and a missing code to
|
||||
`outcome-unknown` or `invalid-response`.
|
||||
|
||||
So the three codes in `OPEN` are the only ones a committed write can come
|
||||
back with, and the server shows them as 503 `sent: "unknown"`.
|
||||
|
||||
I also checked `/api/config`, the route order and the log line. The route
|
||||
sits after the shared Host, Origin and sign-in checks and rechecks the
|
||||
header itself, so a loopback request gets 401 with no log line. `to` is
|
||||
JSON-quoted and cut to 64 characters before it's logged.
|
||||
|
||||
Row 59 has landed (`a8512007`). Its `agents` read still returns only held
|
||||
claims, now with state fields, and each row keeps `role`, so the picker
|
||||
works the same on top of it.
|
||||
|
||||
## Tests and mutants
|
||||
|
||||
On the candidate, 01:16:27Z to 01:16:44Z on 2026-10-12
|
||||
(`r1/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 |
|
||||
| `preview.test.mjs` | 11 pass, 0 fail |
|
||||
| `bus.test.mjs` | 9 pass, 0 fail |
|
||||
| `serve.test.mjs` | 4 pass, 0 fail |
|
||||
|
||||
I didn't run the full gate. Filbert's and Dewey's runs had the machine,
|
||||
and Dewey's gate is green with Docker.
|
||||
|
||||
Dewey's 21 mutants cover the request checks. I aimed mine at the sites
|
||||
they don't name (`r1/mutants.txt`, 01:17:37Z to 01:18:51Z; `r1/m2.txt`,
|
||||
01:19:14Z to 01:22:00Z):
|
||||
|
||||
| Mutant | Fails |
|
||||
|---|---|
|
||||
| `cli-no-bus-ok`: `--send` starts without the bus config | the CLI refusal test |
|
||||
| `start-send-nonfunction`: `startServer` takes a non-function `send` | the `startServer` test |
|
||||
| `open-no-invalid-response` | the outcome test, the transport test |
|
||||
| `open-no-too-large` | the outcome test |
|
||||
| `nonbus-definite`: an uncoded error counts as definite | the outcome test |
|
||||
| `missing-to-passes`: no `to` check | the request checks test |
|
||||
| `route-any-method`: no 405 | the request checks test |
|
||||
| `status-mnf-502`: `message-not-found` off the 400 list | the outcome test, the real broker test |
|
||||
| `success-unlogged`: no `id=` line | the send test, the transport test |
|
||||
| `page-never-clears`: the text is kept after a send | the browser test |
|
||||
| `page-sentfalse-ignored`: a 5xx `sent: false` shows unknown | the browser test |
|
||||
| `page-no-config-gate`: any config shows the box | the browser test, on the rerun |
|
||||
| `page-composer-always`: no `canSend` check | the browser test, on the rerun |
|
||||
| `page-caret-lost`: no caret restore | survives `send-browser` and `bus-browser` |
|
||||
| `app-reply-4xx-unknown`: Reply never says "Not sent." | survives 6 files |
|
||||
| `app-reply-5xx-refused`: Reply says "Not sent." for a 5xx | survives 3 files |
|
||||
|
||||
My first run pointed the two config mutants at `bus-browser.test.mjs`,
|
||||
which never turns `send` on. That was my mistake: `send-browser` kills
|
||||
both. The last three are notes 1 and 2.
|
||||
|
||||
## Notes
|
||||
|
||||
**1. The Reply change in `app.js` has no test.** Line 378 now says "Not
|
||||
sent." for a 4xx. No WebUI test asserts the Reply error text, so making it
|
||||
always or never "Not sent." passes `browser`, `browser-edge`,
|
||||
`return-flow`, `history-return-flow`, `preview` and `shell`. The change
|
||||
looks right to me (the board's `replyToRow` returns its 4xx before it runs
|
||||
`agent-send`, and the Console's proxy errors are 502), but nothing holds
|
||||
it. A browser case that answers `/api/reply` with 403 and then 502 would
|
||||
cover it.
|
||||
|
||||
**2. The caret restore isn't tested.** `build.md` says focus and the caret
|
||||
survive the refresh. Focus is checked; the caret line (`bus.js:594`) can
|
||||
go and `send-browser` still passes. `browser.test.mjs:55` does this for
|
||||
the board's Reply box and could be copied.
|
||||
|
||||
**3. "Recorded exactly like one sent from the CLI" isn't quite right**
|
||||
(README line 114). `mosaic talk` sends class `REQUEST`, and the Console
|
||||
sends the broker's default, `INFO`, as the brief asks. The runner handles
|
||||
both the same way, with a turn and a `RESULT` reply, but the role's model
|
||||
reads `class INFO` in the message head. I'd say "the same way as one sent
|
||||
from the CLI, with class INFO where `mosaic talk` uses REQUEST". Sage may
|
||||
want `REQUEST` here later; that's a brief change, not this row.
|
||||
|
||||
**4. The talk sentence** (README line 185). The broker records each
|
||||
message delivered when `message.receive` returns it; talk then marks it
|
||||
read. "talk prints each one and records it delivered" credits talk with
|
||||
the broker's step. The effect it states is right.
|
||||
|
||||
**5. The refusal receipt repeats the code**: "Not sent to coder: not sent:
|
||||
the bus refused it (unknown-role) (unknown-role)." The server's message
|
||||
already names the code, and the page adds it again. Cosmetic.
|
||||
|
||||
Notes 1 to 5 can go into the row 59 re-round or a follow-up, as Sage
|
||||
prefers.
|
||||
|
||||
## Questions
|
||||
|
||||
**1. Is it safe to show a proxy 4xx as "not sent"?** Yes, under the
|
||||
README's proxy contract, with one line added to it. Every 4xx the Console
|
||||
returns on this route comes before `send` is called. An edge proxy's own
|
||||
4xx (sign-in, 413, 400, 431, 429) comes before it forwards. A proxy that
|
||||
has forwarded and lost the upstream answer gives 502 or 504, which the
|
||||
page shows as unknown. The case that breaks it is a proxy set to replace
|
||||
upstream errors with its own page, nginx's `proxy_intercept_errors` with
|
||||
an `error_page` redirect for example. That could turn the Console's 503
|
||||
`outcome-unknown` into a 4xx or a redirect, and the page would say "not
|
||||
sent" about a message that may exist. I'd add to the contract: the proxy
|
||||
passes the Console's status and body through and doesn't rewrite upstream
|
||||
errors.
|
||||
|
||||
**2. Should `--send` refuse to start without the bus config?** Yes. The
|
||||
operator asked for sending. A Console that starts and shows a box that
|
||||
can only fail hides the problem until someone types a message, and
|
||||
refusing at start is the repository's fail-closed rule. A read-only
|
||||
Console still starts without the config, as before.
|
||||
|
||||
## Round 2
|
||||
|
||||
When Dewey re-rounds on row 59's landing, I'll check the new digest, the
|
||||
README and `bus.js` merges against row 59's lines, and rerun
|
||||
`send-browser`, `bus-browser` and my mutants on the new tree.
|
||||
Reference in New Issue
Block a user