feat(conversation): CHAT-03 I1, mediated control of a sealed headless Pi (#1507)
Controller, claim store, live-session guard, engine link and seal, turn tracker, cohort force stop and recovery, client library, transcript and mediated terminal, with the fake engine and tests. Fixtures only; no live cutover. Dewey built it. Darkwing (comment 26690) and Filbert (comment 26694) approved round 2. Manifest I1-r2-manifest.sha256 (2b48e333, 27 files). Suites on an export: conversation 152/152, control-board 124, webui 14, seat 19, chat-00/01/01c checks, and all nine scripts/test-*.sh green. Follow-ups for I3 are in DEFERRED. Gate E stays with Jason. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,140 @@
|
||||
# CHAT-03 I1, row 5, round 2 review (Filbert)
|
||||
|
||||
Issue #1507. Candidate: manifest `agents/dewey/work/chat-03/I1-r2-manifest.sha256`,
|
||||
sha256 `2b48e333a0f09185364359ae6f8277cc88c0b9ff39058de45cc2c1f0ec9d5c4a`,
|
||||
27 files on base `1c724958`. Packet: `agents/dewey/work/chat-03/BUILD-I1-r2.md`.
|
||||
Round 1 review: `agents/filbert/work/chat-03-i1-review-r1-2026-10-04.md` (comment 26683).
|
||||
|
||||
Verdict: **approve.** All six of my round 1 blockers are fixed. Three
|
||||
follow-ups below; none of them blocks the commit.
|
||||
|
||||
## Method
|
||||
|
||||
- Scratch clone at `~/filbert-scratch/f5r2/repo` (push URL `DISABLED`),
|
||||
checked out at `1c724958`, plus the 27 candidate files copied from the
|
||||
canonical checkout. `sha256sum -c` on the manifest: 27 OK.
|
||||
`node_modules` is a symlink to the canonical checkout's, as in round 1.
|
||||
- Round 1 to round 2 diff (`r1-r2.diff`, against my round 1 export, which
|
||||
matches the round 1 manifest): 11 files changed. `cohort.mjs` and
|
||||
`shim.mjs` are unchanged since round 1.
|
||||
- Mutants run in a separate copy (`~/filbert-scratch/f5r2/mut`) with the
|
||||
same 27 files. Baseline there: 152/152.
|
||||
|
||||
## Suites (one run each, in the clone)
|
||||
|
||||
| Suite | Result |
|
||||
|---|---|
|
||||
| `node --test packages/conversation/tests/` | 152 pass, 0 fail (49.7 s) |
|
||||
| control-board | 124 pass, 0 fail |
|
||||
| webui | 14 pass, 0 fail |
|
||||
| seat | 19 pass, 0 fail |
|
||||
| chat-00 / chat-01 / chat-01c checks | 48 checks / R3 PASS / PASS |
|
||||
| test-auth, conductor, config | 15, 17, 24 passed, 0 failed |
|
||||
| test-discord, extension-package, foundation | 64, 18, 44 passed, 0 failed |
|
||||
| test-queue | 27 passed, 0 failed (node 148/148) |
|
||||
| test-release, test-task | 14, 90 passed, 0 failed |
|
||||
|
||||
test-queue's 27 against Dewey's 29: two checks (`queue verify` and
|
||||
`render --check`) skip outside the canonical root. The node part matches.
|
||||
My first conversation run had no `node_modules` and refused
|
||||
`engine-pin-mismatch` everywhere; that was my setup. The log is kept as
|
||||
`conv-0-no-node_modules.txt`.
|
||||
|
||||
## Round 1 blockers
|
||||
|
||||
| # | Finding | Check | Result |
|
||||
|---|---|---|---|
|
||||
| B1 | Text after Enter joins the message | Round 1 probe: `key("first\rsecond\r")` | `["first","second"]` (round 1: `["firstsecond"]`). Mutant TB1 (send reads the composer when it runs): killed by the new flows test |
|
||||
| B2 | Paste-start split after its ESC | Round 1 probe: `key("\x1b")`, then `key("[200~a\rb\x1b[201~")` | Nothing sent; stays a paste (round 1: `["[200~ab"]`). Mutant TB2 (round 1's `>= 2` carry rule): killed |
|
||||
| B3 | A second force stop runs a parallel escalation | Read `controller.mjs` 680–690 and 1451–1460; new H10 test | Fixed. The new H10 test asserts one stop record, the confirmation not consumed, `launcher.stops` 1, every claim revision on the first stop and no engine bytes. Mutant (drop `\|\| this.escalating`): H10 and H17 fail |
|
||||
| B4 | Decision 34 untested | Round 1 mutant: `turns.mjs:163` disabled | Killed: both new N9 tests fail (150/152) |
|
||||
| B5 | K12 doesn't prove the freeze | Round 1 mutant C3: no freeze write, answers `frozen 1` | Killed by K12's `cgroup.freeze` read. The code waits for `frozen 1` before enumerating (below). The remaining test gap is follow-up F1 |
|
||||
| B6 | K15 has no missing-path case | Round 1 mutant D: ENOENT in `events()` answers `populated 0` | Killed by the new third K15 block (17/18) |
|
||||
|
||||
### The freeze path (Sage's ruling)
|
||||
|
||||
I read the code myself:
|
||||
|
||||
- `shim.mjs` `freeze` op: it writes `engine/cgroup.freeze`, then
|
||||
`waitFor(e => e.frozen === 1 || e.populated === 0, timeoutMs)`. That
|
||||
polls `engine/cgroup.events` every 10 ms. An unreadable file returns
|
||||
`ok: false`, and the timeout returns `timedOut: true`.
|
||||
- `cohort.mjs` 181–185: `freeze` is sent with `timeoutMs: 3000` and a
|
||||
6000 ms client limit. `!frozen.ok` or `frozen.timedOut` returns
|
||||
`unavailable` at phase `kill` before `members` is called. `members` runs
|
||||
only after a `frozen 1` (or `populated 0`) was read.
|
||||
|
||||
So the controller does wait for the frozen state before it lists the pids.
|
||||
Under the ruling, the missing test is a follow-up and doesn't block.
|
||||
|
||||
## Darkwing's blockers (touched files only; the verdicts are Darkwing's)
|
||||
|
||||
- B1 seal. `checkSeal` is an allow-list. A direct probe of
|
||||
`buildPiArgs` + `checkSeal` with 24 extra-argument lists accepted only
|
||||
the three that use `--model`, `--provider` and `--thinking` with plain
|
||||
values. It refused `--session /outside`, `--mode json`, `--no-session`, a
|
||||
repeat, a `-x` or `@f` value, a missing or empty value, a bare word,
|
||||
`@file`, `--model=m`, `--approve`, `--fork`, `--export`, `--print`, `-p`,
|
||||
`-e`, `--extension`, `--continue`, `--api-key`, `--system-prompt` and
|
||||
`--tools`. A relative session path refuses too. Note for Darkwing: a
|
||||
caller can keep the default `engine.command` and pass its own `preArgs`,
|
||||
for example the Pi `cli.js` path plus `--no-session`. Only `--extension`
|
||||
in `preArgs` is checked, so that argv reaches real Pi unsealed. The README
|
||||
documents a non-default `preArgs` as a test hook, and I1 has no
|
||||
production caller. I list it as a follow-up (F3), not a blocker.
|
||||
- B2 session key. The key is the header ID, read at construction. Every
|
||||
later read refuses `target` if it changed, and a missing file now refuses
|
||||
`configuration` instead of throwing. `claim.mjs` `sessionKey` is
|
||||
unchanged and takes any value. The three new W4 tests pass.
|
||||
|
||||
## Round 1 notes
|
||||
|
||||
n1, n2 and n3 are code changes. I read them: `stopLink` walks the
|
||||
supersede chain for an abort written, the re-check after `before-abort`
|
||||
covers `overlapped`, `gap` and `poisoned`, and `#stopUncertain` sets
|
||||
`nativeQueue` to `unknown`. Dewey's r2-n1, r2-n2 and r2-n3 mutants are in
|
||||
the table; I didn't rerun them. Dewey's dispositions of n4–n20 are
|
||||
acceptable as stated.
|
||||
|
||||
## Follow-ups (none blocks the commit)
|
||||
|
||||
- **F1. No test proves enumeration happens under the freeze.** The packet
|
||||
says both K12 checks "don't depend on timing". That's true of the
|
||||
`cgroup.freeze` read, but the read only proves the file was written at
|
||||
some point. I added mutant C4: no write at the `freeze` op, an answer of
|
||||
`frozen 1`, and `cgroup.freeze` written just before `cgroup.kill`. It
|
||||
survives the cohort suite 3 out of 3 runs (18/18 each). The pid-log check
|
||||
catches a missing freeze only if the 5 ms fork loop forks inside the gap
|
||||
between `members` and `kill`, which is about one socket round trip.
|
||||
Dewey's r2-B5b (written, not waited on) also survives here (18/18).
|
||||
Proposal: a test-only hold after `members` and before `kill` would let
|
||||
the loop fork into the gap, which makes C4 fail every time. r2-B5b still
|
||||
needs the FUSE or privileged fixture already proposed.
|
||||
- **F2. Takeover and Enter in one input chunk.** `key("\x14hi\r")` from an
|
||||
observer: round 1 took over and sent `hi`. Round 2 takes the composer
|
||||
when it parses the Enter, which happens before the queued takeover runs.
|
||||
The Enter is refused as observer, so `hi` stays in the composer and is
|
||||
sent on the next Enter. Nothing is sent that shouldn't be; it costs an
|
||||
extra keypress. Fix: either document it or check control when the send
|
||||
runs (TB1 already holds the text).
|
||||
- **F3. Overriding `preArgs` with the default command** (see Darkwing B1
|
||||
above). Refuse a non-default `engine.command` or `preArgs` unless an
|
||||
explicit test option is set, so a production caller can't use the test
|
||||
hook by accident.
|
||||
|
||||
## Mutants run in this round
|
||||
|
||||
| Mutant | Target | Result |
|
||||
|---|---|---|
|
||||
| B3-no-escalating-fence | `controller.mjs` force-stop guard | killed (H10, H17) |
|
||||
| D34-line163 | `turns.mjs:163` | killed (two N9 tests) |
|
||||
| TB1-send-at-run | `terminal.mjs` Enter | killed (flows) |
|
||||
| TB2-lone-esc | `terminal.mjs` ESC carry | killed (flows) |
|
||||
| C3-no-freeze-answer-frozen | `shim.mjs` freeze | killed (K12) |
|
||||
| C4-freeze-at-kill ×3 | `shim.mjs` freeze moved to the kill | survived 3/3 (F1) |
|
||||
| C2-no-wait-frozen | `shim.mjs` freeze not awaited (= r2-B5b) | survived (F1) |
|
||||
| D-enoent-populated0 | `shim.mjs` `events()` | killed (K15) |
|
||||
|
||||
Logs: `~/filbert-scratch/f5r2/logs/` (`mut-*.txt`, `mut-summary.txt`,
|
||||
`conv-1.txt`, `summary.txt` and one log per suite). Probes:
|
||||
`b1probe.mjs`, `sealprobe.mjs` and `tb-edge.mjs` in `~/filbert-scratch/f5r2/`.
|
||||
Reference in New Issue
Block a user