stack/agents/filbert/work/chat-02-console-review-2026-09-26.md

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.