feat(webui): CHAT-02 Console, read-only conversation view (#1507)

History opens a seat's conversation from the Waiting card, table row and inspector. It pages the whole branch through the CHAT-02 board routes, renders untrusted text inert, polls with the follow cursor, and marks every switch (branch, newer, reconcile, gone). The WebUI proxy passes only the two conversation routes' queries upstream. Dewey authored it. Filbert asked for changes on r1 (24b046af) and approved r2 (d06de6a7) in review 160dd68d. A relaunch shows 'newer', not 'reconcile', a deviation from brief 2.3 item 6 that Filbert accepted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-09-26 18:05:11 -05:00
parent 3a209eeafe
commit c9e771cf59
34 changed files with 1669 additions and 26 deletions
@@ -0,0 +1,19 @@
+import { mkdirSync, writeFileSync, appendFileSync, mkdtempSync, realpathSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { createReader } from "../../../../packages/conversation/src/reader.mjs";
+const proj = realpathSync(mkdtempSync(join(tmpdir(), "fb-proj-")));
+const dir = join(proj, ".pi", "state", "x", "sessions"); mkdirSync(dir, { recursive: true });
+const f = join(dir, "s.jsonl");
+const L = (o) => JSON.stringify(o) + "\n";
+const msg = (id, parentId, role, text) => L({ type: "message", id, parentId, timestamp: "2026-09-26T00:00:00Z", message: role === "assistant" ? { role, content: [{ type: "text", text }], stopReason: "stop" } : { role, content: text } });
+writeFileSync(f, L({ type: "session", id: "sess", cwd: proj, timestamp: "2026-09-26T00:00:00Z" }) + msg("a", null, "user", "hi") + msg("b", "a", "assistant", "hello"));
+const reader = createReader({ roots: [{ seat: "x", project: "p", projectRoot: proj, dir, harness: "pi", unsupportedReason: null, engineStartedAt: null }] });
+const conv = reader.catalogue().conversations[0].conversation;
+const p1 = reader.open({ conversation: conv });
+console.log("open branch", p1.page.branch, "entries", p1.page.entries.map(e => e.id + "@" + e.branch));
+appendFileSync(f, msg("c", "b", "user", "more") + msg("d", "c", "assistant", "ok"));
+const p2 = reader.next({ cursor: p1.follow.id, conversation: conv, branch: p1.page.branch });
+console.log("follow ok", p2.ok, "page branch", p2.page?.branch, "entries", p2.page?.entries.map(e => e.id + "@" + e.branch), "view", JSON.stringify(p2.view?.branches));
+const p3 = reader.open({ conversation: conv, branch: p1.page.branch });
+console.log("reopen with first branch id:", JSON.stringify(p3.refusal ?? p3.page.branch));
@@ -0,0 +1,15 @@
+import { mkdirSync, writeFileSync, mkdtempSync, realpathSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { createReader } from "../../../../packages/conversation/src/reader.mjs";
+const proj = realpathSync(mkdtempSync(join(tmpdir(), "fb-proj-")));
+const dir = join(proj, ".pi", "state", "x", "sessions"); mkdirSync(dir, { recursive: true });
+const L = (o) => JSON.stringify(o) + "\n";
+const msg = (id, parentId, text) => L({ type: "message", id, parentId, timestamp: "2026-09-26T00:00:00Z", message: { role: "user", content: text } });
+// a -> b is one branch. X (lost, malformed) was a child of a: a fork. y -> X.
+writeFileSync(join(dir, "s.jsonl"), L({ type: "session", id: "sess", cwd: proj, timestamp: "2026-09-26T00:00:00Z" }) + msg("a", null, "root") + msg("b", "a", "ON THE OTHER BRANCH") + '{"type":"message","id":"X","parentId":"a","timest\n' + msg("y", "X", "leaf"));
+const reader = createReader({ roots: [{ seat: "x", project: "p", projectRoot: proj, dir, harness: "pi", unsupportedReason: null, engineStartedAt: null }] });
+const conv = reader.catalogue().conversations[0].conversation;
+const p = reader.open({ conversation: conv });
+console.log("branches", JSON.stringify(p.view.branches));
+for (const e of p.page.entries) console.log(e.branch, e.role, JSON.stringify(e.content[0].text).slice(0, 90));
@@ -0,0 +1,24 @@
+import { mkdirSync, writeFileSync, appendFileSync, mkdtempSync, realpathSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { createReader } from "../../../../packages/conversation/src/reader.mjs";
+const proj = realpathSync(mkdtempSync(join(tmpdir(), "fb-proj-")));
+const dir = join(proj, ".pi", "state", "x", "sessions"); mkdirSync(dir, { recursive: true });
+const f = join(dir, "s.jsonl");
+const L = (o) => JSON.stringify(o) + "\n";
+const m = (id, parentId, text) => L({ type: "message", id, parentId, timestamp: "2026-09-26T00:00:00Z", message: { role: "user", content: text } });
+writeFileSync(f, L({ type: "session", id: "s", cwd: proj, timestamp: "2026-09-26T00:00:00Z" }) + m("a", null, "A") + m("b", "a", "B") + m("c", "b", "C"));
+const reader = createReader({ roots: [{ seat: "x", project: "p", projectRoot: proj, dir, harness: "pi", unsupportedReason: null, engineStartedAt: null }] });
+const conv = reader.catalogue().conversations[0].conversation;
+const t = (p) => p.ok ? `${p.page.branch} [${p.page.entries.map((e) => e.content[0].text).join(",")}] default=${p.view.defaultBranch} branches=${p.view.branches.map((b) => b.branch).join("|")}` : JSON.stringify(p.refusal);
+let main = reader.open({ conversation: conv }); console.log("1 open", t(main));
+// Pi navigates back to b and continues: fork from the middle of main.
+appendFileSync(f, m("x", "b", "X") + m("y", "x", "Y"));
+let fm = reader.next({ cursor: main.follow.id, conversation: conv, branch: "main" }); console.log("2 main follow", t(fm));
+let dx = reader.open({ conversation: conv }); console.log("3 default open", t(dx));
+// Pi goes back to c on main and continues; then resetLeaf starts a new root.
+appendFileSync(f, m("d", "c", "D") + m("r", null, "R"));
+console.log("4 main follow", t(reader.next({ cursor: fm.follow.id, conversation: conv, branch: "main" })));
+console.log("5 b.x follow", t(reader.next({ cursor: dx.follow.id, conversation: conv, branch: dx.page.branch })));
+console.log("6 open b.x", t(reader.open({ conversation: conv, branch: "b.x" })));
+console.log("7 open default", t(reader.open({ conversation: conv })));
@@ -0,0 +1,15 @@
+import { mkdirSync, writeFileSync, mkdtempSync, realpathSync, chmodSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+import { createReader } from "../../../../packages/conversation/src/reader.mjs";
+const proj = realpathSync(mkdtempSync(join(tmpdir(), "fb-proj-")));
+const mk = (seat) => { const d = join(proj, ".pi", "state", seat, "sessions"); mkdirSync(d, { recursive: true }); writeFileSync(join(d, "s.jsonl"), JSON.stringify({ type: "session", id: "s", cwd: proj, timestamp: "2026-09-26T00:00:00Z" }) + "\n"); return d; };
+const good = mk("good"), bad = mk("bad");
+chmodSync(join(proj, ".pi", "state", "bad"), 0o000);
+const root = (seat, dir) => ({ seat, project: "p", projectRoot: proj, dir, harness: "pi", unsupportedReason: null, engineStartedAt: null });
+const reader = createReader({ roots: [root("good", good), root("bad", bad)] });
+try { const c = reader.catalogue(); console.log("catalogue ok", c.conversations.length, JSON.stringify(c.refusedRoots)); }
+catch (e) { console.log("catalogue THREW", e.code, e.message); }
+try { console.log("open", JSON.stringify(reader.open({ conversation: "pi-" + "0".repeat(32) }).refusal)); }
+catch (e) { console.log("open THREW", e.code); }
+chmodSync(join(proj, ".pi", "state", "bad"), 0o755);
@@ -0,0 +1,264 @@
+# CHAT-02 Console, revision 1: Filbert's code review
+
+Reviewer: Filbert, 2026-09-26. Requested by Dewey, assigned by Sage (#1507,
+row 5). Measured against brief R4 (`agents/dewey/work/chat-02/BRIEF.md`) §2.2,
+§2.3 and §4, and Sage's D1–D4.
+
+Candidate: packet `agents/dewey/work/chat-02/CONSOLE.md`, sha256
+`24b046af…cd9ccd`, ten files on base `3a209eea`. All ten pins verify in the
+shared tree and in my overlay.
+
+**Verdict: changes requested.** One blocking finding (B1): "Reload
+conversation" after a reconcile moves a view that sits on an older branch to
+the default branch without saying so. The rest of the candidate is sound, and
+I accept all five §3 choices. The fix is small, and I'll re-review only the
+delta.
+
+## 1. What I ran
+
+- **Overlay.** A detached worktree at `3a209eea` (`/tmp/fb-console-wt`), with
+  only the ten pinned files overlaid (`sha256sum -c` clean) and
+  `node_modules` symlinked.
+- **Suites.** Running `node --test --test-concurrency=1` over the
+  conversation, control-board, webui and seat tests gives 185/185. The
+  `chat-00`, `chat-01` and `chat-01c` `check.mjs` scripts each exit 0.
+- **Screens.** I regenerated test 1's screens with `WEBUI_EVIDENCE` at the
+  pinned hashes. The four PNGs have the same dimensions as Dewey's
+  (305×3315 at 320, 1425×1586 at 1440), and I looked at 320-dark and
+  1440-light. They show the long answer in full, the tool call and result,
+  inert hostile text (with ␛, ␇ and `[U+202E]` visible), the line-5 notice
+  and the reconcile marker. Test 1 asserts no horizontal overflow at 320 and
+  1440 in both modes.
+- **Two probes of my own,** in a scratch test file in the overlay (not the
+  shared tree). Both results are below: B1 and N1.
+
+## 2. Against the brief
+
+- **§2.2 rendering.**
+  - Every session string goes through `node()`, which sets `textContent`
+    after `inert()`. No session value reaches `innerHTML`. The existing
+    card, table and inspector templates gain only `historyButton(r)`, and
+    both values it interpolates go through `esc()`.
+  - Tool calls, tool results, thinking and compaction are closed
+    `details`. Thinking is hidden by default, and unavailable thinking
+    says so.
+  - The hostile fixture matches R1, in both assistant text and tool output.
+    The assertions cover the absence of any element, any `on*` attribute and
+    any navigation.
+  - Age is untouched. Reply keeps the board path, and the view polls.
+- **§2.3 items 1–7** are in `history-return-flow.test.mjs`, with no
+  injection and no Refresh. Item 6 is covered with `newer` in place of
+  `reconcile` (see §3.1).
+- **Proxy.** `/api/conversations` and `/api/conversation` are GET-only and
+  forward their query string unchanged. `/api/board` gets no query. The Host
+  and Origin checks run first, the JSON-only response rule still holds, and
+  so does `redirect: 'error'`. The webui serve test pins each of these.
+- **§4.**
+  - The suites and checks are green.
+  - The screens are present, and I reproduced them at the pins.
+  - The live check is still open: it needs the commit and a WebUI restart,
+    as the packet's §6 says.
+
+## 3. The five choices Dewey asked about
+
+1. **`newer` instead of `reconcile` for a relaunch: accept.**
+   - In this code, `reconcile` means "the source refused, polling stopped".
+     A relaunch refuses nothing. The open file is still accurate, and
+     polling on it should go on.
+   - A separate marker with its own action is the better reading of item 6's
+     intent, which is to keep the file and never switch silently.
+   - I wrote that line in my R2 review, and the wording was too narrow.
+     Sage should record the deviation against brief §2.3 item 6, in the
+     BUILD-LOG entry or as a brief erratum, so the brief and the test agree.
+2. **Detection only for the newest readable conversation: accept.** An
+   older session is an explicit choice from the picker, so it has nothing
+   newer to warn about. There is one latent edge in the matching; see N1.
+3. **The `conv-form` and `conv-receipt` classes: accept.**
+   - The cause is right: the hidden view comes earlier in the DOM, so a
+     bare `.reply-form` query found it.
+   - The four browser tests pass in the overlay.
+   - Dewey's "forms share the reply-form class" mutant is caught.
+   - The CSS lists both class pairs, and the submit handler matches
+     `.reply-form, #conv-form`.
+4. **The heading focus ring: accept.** The heading uses `tabindex="-1"` and
+   the shared `:focus-visible` rule, the same as the inspector title. The
+   ring shows in the screens only because a synthetic `click()` counts as
+   keyboard focus.
+5. **Darkwing's R2 notes, test-only: accept.**
+   - Only `packages/control-board/tests/serve.test.mjs` changes under
+     control-board (`git diff 3a209eea --stat`), so the board's `serve.mjs`
+     is unchanged.
+   - The refusal scan now reads every `.mjs` in `packages/conversation/src`,
+     and the pin covers the whole `REFUSAL_STATUS` object, all 16 codes.
+
+## 4. Blocking
+
+**B1. After a reconcile, Reload silently switches the branch.**
+- **Where.** The reconcile marker's button is
+  `{ id: 'reopen', label: 'Reload conversation' }`. The click handler calls
+  `openConversation(conv.row, conv.id)`, which reads `convURL({ id })` with
+  no branch, so the server returns the default branch. `openConversation`
+  resets the markers, and `apply()` raises `branch` only when
+  `view.defaultBranch !== view.branch`. On the default branch, that is
+  never.
+- **Probe A.** I opened a view, then a fork made another leaf the default,
+  and the `branch` marker showed. That part is correct. Next I did a
+  same-inode rewrite that keeps both branches, which triggered a reconcile,
+  and clicked Reload.
+  - Result: turns `[["User","QUESTION"],["Assistant","FORK_ANSWER"]]` and
+    no markers.
+  - The reader was on `MAIN_ANSWER`'s branch. They now see the fork's
+    branch, and nothing on screen says so.
+- **Why it blocks.**
+  - The branch marker's own text promises "This view stays on the branch it
+    opened".
+  - Test 2's assertion is named "no silent switch".
+  - Brief §2.1 says nothing switches silently.
+- **It is reachable in normal use, not only on a rewrite.** Cursors are held
+  in board memory with a 10-minute TTL (`reader.mjs:204`), so either of
+  these produces the reconcile:
+  - a board restart (`cursor-unknown`);
+  - Pause held for more than ten minutes (`cursor-expired`).
+- **Suggested fix.**
+  - Reload reopens with the branch it was on: `convURL({ id, branch })`.
+  - If the board answers `unknown-branch`, open the default and show a
+    marker that says the old branch is gone.
+  - Add a test in probe A's shape: fork, a refusal, Reload, then assert the
+    open branch's text or a marker. Also run a mutant that drops the branch
+    from Reload.
+  - The "Open the latest branch" button shares the `reopen` id. It should
+    keep going to the default, so the two buttons need different ids.
+
+## 5. Nonblocking
+
+**N1. The board and the catalogue disagree on "newest".**
+- **The mismatch.** The board row's `sessionId` comes from the newest file
+  by mtime (`scan.mjs:49–58`). The catalogue sorts by the last entry's
+  timestamp and puts `null` last (`reader.mjs:262`).
+  `openConversation` takes `readable[0]` as "the board's session" and, for
+  the "Open the newest session" action, again as "the newest".
+- **Probe B.** A relaunch file holding only its header:
+  - `newer` shows. Clicking "Open the newest session" reopens the **old**
+    file, clears the marker and records the new `sessionId`.
+  - After a message then lands in the new file, the view is still on the
+    old file and has no marker. It never returns.
+- **Why it doesn't block.** Real Pi doesn't write a header-only file. Its
+  `_persist` creates the file with `wx` only once an assistant message
+  exists (`session-manager.js:739–766`), so header, user and assistant land
+  together. The new file's last timestamp is then newer, and the two rules
+  agree.
+- **Cheap guard.** After the `newest` action, if `choice.conversation`
+  equals the conversation that was open, keep the marker and say the newer
+  session isn't readable yet.
+
+**N2. Raw bidi controls in the source.**
+- **Where.** `app.js:88` builds `BIDI` from raw U+202A, U+202E, U+2066 and
+  U+2069 characters inside the regex literal. `conversation.test.mjs`'s
+  `HOSTILE` string holds a raw U+202E.
+- **Why it matters.** They behave correctly, but a raw override in a diff is
+  exactly what a reviewer can't see; this is the Trojan Source pattern.
+- **Fix.** Write `/[‪-‮⁦-⁩]/g` and `'‮evil'`. The
+  behaviour is identical.
+
+**N3. `inert()` coverage.**
+- **What's missing.** It leaves out:
+  - LRM, RLM and ALM (U+200E, U+200F, U+061C);
+  - the C1 controls U+0080–U+009F, which include the single-byte CSI
+    U+009B.
+- **Why it's minor.** In a browser, the marks can only nudge neutral
+  characters next to them, and C1 has no effect. Take it or leave it. If
+  taken, the fixture gains one of each.
+
+## 6. Scratch
+
+- **Worktree.** `/tmp/fb-console-wt` stays in place for the delta re-review.
+  Its only additions are the probe file
+  `packages/webui/tests/zz-filbert-probe.test.mjs` and the symlinked
+  `node_modules`, and the ten pins still verify. Screens are in
+  `/tmp/fb-console-ev`.
+- **Nothing live was touched.** I didn't use the shared tree's served WebUI
+  or any live process.
+- No commit or push.
+
+## Revision 2: delta re-review
+
+Candidate: packet `CONSOLE.md` `d06de6a7…72d2`, and
+`evidence/console/r2-delta.patch` `c1d65628…5866`. Three files changed:
+
+- `app.js` `3c7f2f4c…0d6e`
+- `conversation.test.mjs` `52c2c663…a4a5`
+- `README.md` `5a1a4de7…acc4`
+
+The other seven pins are unchanged from revision 1.
+
+**Chain.** In my overlay, the patch applies in reverse to revision 1's ten
+pins (all ten verify). Applied forward again, it gives the three revision 2
+hashes. In both the overlay and the shared tree, all ten revision 2 pins
+verify.
+
+**Runs.** Suites 188/188: Dewey's 186, plus my probes A and B, kept for the
+run. The chat-00, chat-01 and chat-01c checks exit 0.
+
+**B1: fixed.**
+- The reconcile button is now `reload` and reopens with
+  `{ branch: c.branch }`. "Open the latest branch" is now `latest`, which
+  reopens without a branch. `newest` passes `from` and `sessionId`.
+- Probe A, rerun: after the rewrite, Reload shows
+  `[QUESTION, MAIN_ANSWER]` with the `branch` marker. Revision 1 showed the
+  fork with no marker.
+- On `unknown-branch`, the view falls back to the default and raises
+  `gone`. The fallback runs only when a branch was asked for, and the second
+  fetch checks the generation.
+- The fork test now covers:
+  - Reload keeping `main`;
+  - `latest` taking the fork;
+  - a fork rewritten away, giving `gone` with no reconcile marker.
+- Dewey's four new mutants for this (reload drops its branch, latest keeps
+  the open branch, gone not reopened, gone reopened silently) are all
+  caught.
+
+**N1: taken.**
+- `newerUnread` is set when `newest` lands on the conversation already open.
+  The view then keeps the old `sessionId`, so `newer` stays and says the new
+  history isn't readable yet.
+- Probe B, rerun: the marker survives the click and a later poll.
+- Dewey's test 3 adds the second click once the new file has an entry, and
+  that click opens it.
+
+**N2: taken.** A scan of `app.js` and both conversation test files finds no
+raw C1, LRM, RLM, ALM, bidi override or isolate, U+2028, U+2029 or ESC
+characters.
+
+**N3: taken.**
+- `UNSEEN` covers U+0080–U+009F, U+061C, U+200E, U+200F, U+202A–U+202E and
+  U+2066–U+2069, printed as `[U+XXXX]` and padded to four digits.
+- The fixture asserts `[U+009B]31mCSI [U+200E]mark [U+202E]evil`, and the
+  page check rejects all four raw characters.
+
+**Nit, no action needed.** Test 4's declaration lost a space
+(`reply',{ timeout`).
+
+**Screens.** I didn't regenerate revision 2's screens. Test 1 asserts the
+new visible text at the pins, and that overflow check passed in my run.
+
+**Verdict: approve** revision 2. These are the exact ten files:
+
+| File | sha256 |
+|---|---|
+| `packages/webui/README.md` | `5a1a4de7…acc4` |
+| `packages/webui/src/public/app.js` | `3c7f2f4c…0d6e` |
+| `packages/webui/src/public/index.html` | `b972e2f7` (unchanged) |
+| `packages/webui/src/public/live.css` | `8747b83d` (unchanged) |
+| `packages/webui/src/serve.mjs` | `1187a98f` (unchanged) |
+| `packages/webui/tests/serve.test.mjs` | `0477f66d` (unchanged) |
+| `packages/webui/tests/conversation.test.mjs` | `52c2c663…a4a5` |
+| `packages/webui/tests/history-fixture.mjs` | `b312c8a1` (unchanged) |
+| `packages/webui/tests/history-return-flow.test.mjs` | `0918aeea` (unchanged) |
+| `packages/control-board/tests/serve.test.mjs` | `105d87ec` (unchanged) |
+
+**Still open, and not part of this verdict:**
+- the brief §4 live check, after the commit and a WebUI restart;
+- Sage recording the `newer` deviation from §2.3 item 6.
+
+**Scratch.** I removed the probe file and verified the pins, then removed
+the worktree `/tmp/fb-console-wt`. No commit or push.