review(55): Sage's dispatch test ruling and the transport follow-up note
BUILD.md records Sage's 2026-10-11 ruling that packages/queue/tests/ dispatch.test.mjs is in scope. FOLLOWUP-transport-code.md writes up the transport's did-not-finish error with no .code, for the 10-12 batch. The candidate (manifest 93fa48c1cdfe) is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -25,7 +25,7 @@ over them: 9 files, +1363/−16.
|
|||||||
| `packages/cli/README.md` | the intro names row 55; new section "The app" |
|
| `packages/cli/README.md` | the intro names row 55; new section "The app" |
|
||||||
| `docs/TOOLS.md` | the `mosaic` entry: the no-argument line and a four-line paragraph |
|
| `docs/TOOLS.md` | the `mosaic` entry: the no-argument line and a four-line paragraph |
|
||||||
| `scripts/mosaic` | a decided scope addition (Sage, 2026-10-11); see below |
|
| `scripts/mosaic` | a decided scope addition (Sage, 2026-10-11); see below |
|
||||||
| `packages/queue/tests/dispatch.test.mjs` | follows from `scripts/mosaic`; my addition, for Sage to confirm; see below |
|
| `packages/queue/tests/dispatch.test.mjs` | follows from `scripts/mosaic`; in scope (Sage, 2026-10-11); see below |
|
||||||
|
|
||||||
`resolveDecision` writes the same lines in the same order and asks the
|
`resolveDecision` writes the same lines in the same order and asks the
|
||||||
same question as before. `decide` passes `ask = null` for `--yes` and
|
same question as before. `decide` passes `ask = null` for `--yes` and
|
||||||
@@ -60,9 +60,10 @@ My first gate run had test-queue red, 26 passed and 1 failed
|
|||||||
pins A2's rule that every call but `queue` reaches the seat CLI exactly as
|
pins A2's rule that every call but `queue` reaches the seat CLI exactly as
|
||||||
the pre-A2 script did, and its list of calls included `[]`. Sage's approved
|
the pre-A2 script did, and its list of calls included `[]`. Sage's approved
|
||||||
change sends exactly that call somewhere else, so the pin can't hold as
|
change sends exactly that call somewhere else, so the pin can't hold as
|
||||||
written. This file is outside the brief and outside what Sage approved. I
|
written. The file is outside the brief. I changed it because the gate
|
||||||
changed it because the gate needs every suite green and the change follows
|
needs every suite green and the change follows from the approved one.
|
||||||
directly from the approved one. Sage, please confirm or rule it out.
|
Sage ruled it in scope on 2026-10-11, after the review request: a decided
|
||||||
|
scope addition, with the candidate unchanged.
|
||||||
|
|
||||||
The change, 16 lines in and 5 out:
|
The change, 16 lines in and 5 out:
|
||||||
|
|
||||||
@@ -198,6 +199,7 @@ and `node_modules` linked (`out/gate/`).
|
|||||||
`outcome-unknown: the bus transport did not finish`, exit 1, instead of
|
`outcome-unknown: the bus transport did not finish`, exit 1, instead of
|
||||||
the "may have recorded it" sentence. The app matches the message text
|
the "may have recorded it" sentence. The app matches the message text
|
||||||
and shows its outcome-unknown overlay, so only the verb has the gap. It
|
and shows its outcome-unknown overlay, so only the verb has the gap. It
|
||||||
predates this row.
|
predates this row. Written up in `FOLLOWUP-transport-code.md`; Sage
|
||||||
|
queues it with the 2026-10-12 batch.
|
||||||
- Talk and stop in the app, Decision Seen, and the web console stay out of
|
- Talk and stop in the app, Decision Seen, and the web console stay out of
|
||||||
scope, as the brief says.
|
scope, as the brief says.
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
# Follow-up: the transport's "did not finish" error has no code
|
||||||
|
|
||||||
|
Found while building row 55 (#1544). It is not part of row 55. Sage will
|
||||||
|
queue it with the 2026-10-12 batch. The code was already like this at
|
||||||
|
`fe5c68fa`.
|
||||||
|
|
||||||
|
## What happens
|
||||||
|
|
||||||
|
`humanTransport` in `packages/cli/src/transport.mjs` (line 60 at
|
||||||
|
`fe5c68fa`) throws this when the bus CLI child errors or times out (30 s)
|
||||||
|
and no exit status comes back:
|
||||||
|
|
||||||
|
```js
|
||||||
|
throw new CliError("outcome-unknown: the bus transport did not finish", 1);
|
||||||
|
```
|
||||||
|
|
||||||
|
Every other error it throws sets `error.code`. This one doesn't.
|
||||||
|
`resolveDecision` in `packages/cli/src/cli.mjs` checks
|
||||||
|
`e.code === "outcome-unknown"` to choose its sentence:
|
||||||
|
|
||||||
|
```
|
||||||
|
outcome unknown: the broker may have recorded it. Check mosaic inbox or mosaic trail <id> before trying again
|
||||||
|
```
|
||||||
|
|
||||||
|
That check misses the transport error. So if `decision.resolve` times out,
|
||||||
|
`mosaic decide` prints `mosaic: outcome-unknown: the bus transport did not
|
||||||
|
finish` and exits 1. The exit code is right. The text is wrong: it doesn't
|
||||||
|
tell the user the decision may already be recorded, or where to look. A
|
||||||
|
retry could resolve a decision that already went through. The bus should
|
||||||
|
refuse a second resolve, but the user hasn't been told to check first.
|
||||||
|
|
||||||
|
The row 55 app is not affected. It matches the message text
|
||||||
|
(`/^outcome[ -]unknown/`) and shows its outcome-unknown overlay, which has
|
||||||
|
no retry.
|
||||||
|
|
||||||
|
No other check in `packages/cli/src` compares `.code` against
|
||||||
|
`outcome-unknown`. The launcher and notifier errors have their own codes.
|
||||||
|
|
||||||
|
## Suggested fix
|
||||||
|
|
||||||
|
1. Set `error.code = "outcome-unknown"` on the transport's did-not-finish
|
||||||
|
error, the same way the other two throws do.
|
||||||
|
2. Add a test in `packages/cli/tests/cli.test.mjs`: a transport whose
|
||||||
|
`decision.resolve` throws that error makes `mosaic decide` print the
|
||||||
|
"may have recorded it" sentence, exit 1.
|
||||||
|
3. Extend the `hang` case in `packages/cli/tests/transport.test.mjs` to
|
||||||
|
check `e.code === "outcome-unknown"`.
|
||||||
|
|
||||||
|
Files: `packages/cli/src/transport.mjs`, `packages/cli/tests/cli.test.mjs`,
|
||||||
|
`packages/cli/tests/transport.test.mjs`.
|
||||||
Reference in New Issue
Block a user