diff --git a/.adr/0004-the-statusline-run-is-bounded.md b/.adr/0004-the-statusline-run-is-bounded.md new file mode 100644 index 0000000..fd054e8 --- /dev/null +++ b/.adr/0004-the-statusline-run-is-bounded.md @@ -0,0 +1,102 @@ +# 4. The statusline run is bounded, but the bound guards less than it looks + +Status: Accepted · 2026-07-30 + +## Context + +`locateInputBox` (`web/src/lib/harness/claude/chrome.ts`) walks up from the buffer tail and accepts at +most `MAX_STATUS_LINES` non-blank, non-border rows before it requires the input box's bottom border. +That run is the statusline plus its hint rows. + +**A statusline's height is not ours to assume.** It is whatever `statusLine.command` prints, and Claude +renders that verbatim below the box. Nothing in the contract caps it: a multi-line statusline is an +ordinary configuration, generators in common use expose several line slots, a hand-rolled script can +print as many rows as it likes, and any single row can soft-wrap into two on a narrow pane — which is +the usual case on a phone. At 3, the bound was smaller than configurations that are entirely normal. + +**Statusline height also varies at runtime, which is why the failure reads as intermittent rather than +reproducible.** Common widgets are conditional — git state, PR or review state, a context-usage meter, a +session timer — so a statusline can gain or lose a row while nobody touches a setting, carrying a pane +over the threshold and back. Observed exactly that way in the field: panes that failed to send one day +were fine the next, and vice versa, with no config change in between. + +When the border is never reached, `extractInputDraft` returns `null`, so `draftCarriesSend` is false +and `sendGuardedReply` withholds Enter with *"Message didn't reach the input box"* — while the text is +already sitting in the box. + +This is the second time this walk has been broken the same way. `web/src/fixtures/panes/README.md` +records that the background-agents footer "broke `locateInputBox` (**it tolerated only the statusline +window**), so the whole box stayed visible on the mirror **and** no draft chip surfaced", fixed in +`d9521e3` by admitting more rows below the border under `MAX_FOOTER_LINES`. + +### Four justifications for keeping the bound low, all falsified + +The old comment credited the bound with protection it does not provide, which is why raising it felt +unsafe and why "just delete it" was proposed twice. Each claim was tested against the real modules: + +| Claimed role | Verdict | +| --- | --- | +| Stops the walk stripping unboundedly | **False.** Step (c) requires a border, so an over-long run degrades to *no match*. 200 000 borderless rows with the bound removed strip 0 lines. | +| Distinguishes the live box from one that scrolled into scrollback | **False for this harness.** A Claude pane runs on the terminal's alternate screen and keeps no scrollback ring, so the buffer only ever holds the visible viewport — stated in seven places in this repo (`bridge/types.ts`, `web/src/lib/types.ts`, `loaders.ts`, `nav.ts`, `api.ts`, …) and confirmed against running panes, none of which reported a non-zero `max_offset_from_bottom`. | +| Keeps a dialog below the box from being swallowed as chrome | **False.** All 20 dialog fixtures are refused by step (c), (d) or (e) — never by the row count. Step (c) fires because Claude paints a blank line above a dialog's footer hint, ending the tail run within 2 rows in **20 of 20**. The blank is the guard. | +| At least hedges against a blank-free run below the box | **Backwards.** Dialogs are 2-11 rows, so a taller ceiling admits *more* of them: a blank-free dialog is refused for 8/20 fixtures at 3, but only 2/20 at 8. | + +So on the real corpus the bound has no measurable protective effect at 3 or at 8, and at 3 it has one +measurable cost: blocked sends. + +### The two bounds compose + +Step (a) peels up to `MAX_FOOTER_LINES` rows plus their blank separator and hands its position to step +(b), which then takes up to `MAX_STATUS_LINES` more. The deepest strippable run below the box is +therefore `MAX_STATUS_LINES + 1 + MAX_FOOTER_LINES` — **12 rows at 3, 17 at 8**. Measured: + +``` +complete box + K rows + blank + 8 footer rows + K MAX_STATUS_LINES=3 MAX_STATUS_LINES=8 + 3 MATCH strips=15 MATCH strips=15 + 4 no match MATCH strips=16 + 8 no match MATCH strips=20 + 9 no match no match +``` + +## Decision + +**Keep a row bound, size it to `MAX_FOOTER_LINES` (8), and stop crediting it with protection it does +not provide.** + +- Keep it, rather than delete it, for one narrow reason: an unbounded walk has no failsafe of its own. + A 60-row blank-free run below a complete box is matched, and `stripChrome` then deletes all 63 lines. + Nothing like that shape appears in 47 real captures, but this repo has already been surprised once by + Claude adding rows below the box. +- 8 rather than some other number because `MAX_FOOTER_LINES` already bounds a run in the same region of + the same buffer. Nothing on today's corpus depends on the exact value, so "the same as its neighbour" + is the only justification available that is not a guess — and two different numbers would invite a + reader to look for a distinction that does not exist. +- The comment on the constant says what the bound is *not*, and points here. `chrome.test.ts` pins both + halves: the heights that must work, and the dialogs that must stay untouched. + +## Consequences + +- **Nothing on the current corpus depends on this number.** That is unusual for a constant and has to be + said out loud, or the next reader re-derives one of the four false justifications above — which is + exactly what happened four times while this was being written. +- **The scrollback-echo false-ENTER window widens from `r<=3` to `r<=8`.** An echo of our own sent text, + bracketed by rules with the lower rule flush beneath it, is structurally identical to the live box. + `draftCarriesSend` cannot discriminate, because the echo *is* the sent text. Zero instances in 47 real + captures, and the precondition (a `❯` line with a border flush below and only blanks up to a border + above) appears in no real render — the two real echoes in the corpus have transcript text above them + and so fail step (e). Pinned as a known limitation in `chrome.test.ts` rather than left implicit. +- **The composed ceiling rises from 12 rows to 17.** Unobserved: the deepest real run below a border in + the whole corpus is 11 rows (`claude--wizard-submit.txt`), and that is a dialog which fails + (c)/(d)/(e) anyway. +- **A statusline taller than 8 rows falls back to the raw mirror.** Safe direction — the box stays + visible instead of being stripped — but sending stalls again, and the fix is to raise the number, not + to remove it. +- **A dialog block cannot serve as an interlock in the send path.** Tried and measured: appending a box + shape below a real dialog fixture flips `buildBlocks` from `[raw, prompt-select]` to `[raw]`, and + `extractInputDraft` then returns the appended text. The grammars need the dialog at the buffer tail, + so the interlock goes silent in exactly the screen it was meant to protect. + +**What would justify revisiting this:** `web/` gaining Herdr's `agent_status` or `scroll` geometry. +Neither is plumbed into the client today. With either, box liveness could be *established* instead of +inferred from shape — and then the bound can go, along with the echo limitation it cannot fix. diff --git a/.adr/README.md b/.adr/README.md index f20a1ad..9f5d640 100644 --- a/.adr/README.md +++ b/.adr/README.md @@ -45,3 +45,4 @@ A superseded ADR is never deleted or edited into agreement with the present. Mar | [0001](./0001-one-managed-front-door.md) | Collie manages exactly one front door | Accepted | | [0002](./0002-invert-the-light-terminal-mirror.md) | The light terminal mirror is inverted, not re-themed | Accepted | | [0003](./0003-one-shared-seen.md) | "Seen" is one shared fact, and only Collie's own reads count | Accepted | +| [0004](./0004-the-statusline-run-is-bounded.md) | The statusline run is bounded, but the bound guards less than it looks | Accepted | diff --git a/CLAUDE.md b/CLAUDE.md index 2acd4a2..804c690 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,11 @@ the unit name; the Herdr action runs from anywhere. backwards in a surface that renders dark under every theme and inverts in light ([ADR 0002](./.adr/0002-invert-the-light-terminal-mirror.md)). Fails silently; `ansi-output.test.tsx` guards it. +- **The statusline-run bound in `chrome.ts` guards less than it looks** — a dialog below the box is + refused by the border/prompt checks and by the blank line Claude paints above its footer hint, never + by the row count. Size it up if a real statusline needs more rows; don't delete it, and don't credit + it with protection it doesn't provide + ([ADR 0004](./.adr/0004-the-statusline-run-is-bounded.md)). `chrome.test.ts` pins both halves. ## The journal (scrollback the mirror can't give you) diff --git a/bridge/journal/pi.test.ts b/bridge/journal/pi.test.ts index 189d656..08a92e7 100644 --- a/bridge/journal/pi.test.ts +++ b/bridge/journal/pi.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { mkdir, rm, symlink } from "node:fs/promises"; +import { mkdir, realpath, rm, symlink } from "node:fs/promises"; import { tmpdir } from "node:os"; import { isPiSessionId, parsePiTranscript, PiTranscriptSource } from "./pi.ts"; @@ -154,7 +154,9 @@ describe("PiTranscriptSource — path refs are confined to the root", () => { * base/sessions/--repo--/sneaky.jsonl → ../../outside.jsonl a symlink out of the root */ async function fixture() { - const base = `${tmpdir()}/collie-pi-${Math.floor(performance.now() * 1000)}`; + const created = `${tmpdir()}/collie-pi-${Math.floor(performance.now() * 1000)}`; + await mkdir(created, { recursive: true }); + const base = await realpath(created); const root = `${base}/sessions`; const project = `${root}/--var-home-you-repo--`; await mkdir(project, { recursive: true }); diff --git a/web/src/lib/harness/claude/chrome.test.ts b/web/src/lib/harness/claude/chrome.test.ts index c335d29..553e124 100644 --- a/web/src/lib/harness/claude/chrome.test.ts +++ b/web/src/lib/harness/claude/chrome.test.ts @@ -1,4 +1,4 @@ -import { readFileSync } from "node:fs"; +import { readdirSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -28,6 +28,36 @@ function wrappedBoxBuffer(promptLine: string, continuationLines: string[], above return splitLines(parseAnsi(rows.join("\n"))); } +function boxWithStatusRows(promptLine: string, statusRows: string[]): StyledLine[] { + const rule = "─".repeat(40); + return splitLines(parseAnsi(["earlier output", rule, promptLine, rule, ...statusRows].join("\n"))); +} + +function boxWithBlankFreeRunBelow(output: string[]): StyledLine[] { + const rule = "─".repeat(40); + return splitLines( + parseAnsi([rule, "❯ do the earlier thing", rule, "old statusline", ...output].join("\n")), + ); +} + +function echoedSendAboveDialog(sent: string, dialogRows: number): StyledLine[] { + const rule = "─".repeat(40); + const dialog = Array.from({ length: dialogRows }, (_, k) => ` ${k + 1}. dialog row`); + return splitLines( + parseAnsi(["earlier output", rule, `❯ ${sent}`, rule, ...dialog, "", "Esc to cancel"].join("\n")), + ); +} + +function trailingBlankCount(lines: StyledLine[]): number { + let count = 0; + while (count < lines.length && lineText(lines[lines.length - 1 - count]!).trim().length === 0) count++; + return count; +} + +function keptAboveTail(lines: StyledLine[]): number { + return lines.length - trailingBlankCount(lines); +} + // stripChrome peels the agent's own input-box + statusline + trailing blanks off the TAIL. It's // deliberately conservative: it strips only when the full box shape matches and never removes // content above the last real output — when unsure it returns the buffer untouched. Driven against @@ -265,3 +295,153 @@ describe("extractInputDraft — recovers a stranded prompt-line draft", () => { expect(extractInputDraft(wrappedBoxBuffer("❯ opening line", tooMany))).toBeNull(); }); }); + +describe("the statusline run — as tall as a real statusline", () => { + const DRAFT = "my draft text here"; + const statusRows = (n: number) => Array.from({ length: n }, (_, i) => `status row ${i}`); + + it.each([1, 2, 3, 4, 5, 6, 7, 8])("locates the box under %i statusline row(s)", (rows) => { + const lines = boxWithStatusRows(`❯ ${DRAFT}`, statusRows(rows)); + expect(extractInputDraft(lines)).toBe(DRAFT); + expect(extractStatusLine(lines)).toBe("status row 0"); + expect(stripChrome(lines)).not.toBe(lines); + }); + + it.each([9, 10])("falls back to the raw mirror at %i rows, the deliberate ceiling", (rows) => { + const lines = boxWithStatusRows(`❯ ${DRAFT}`, statusRows(rows)); + expect(extractInputDraft(lines)).toBeNull(); + expect(extractStatusLine(lines)).toBeNull(); + expect(stripChrome(lines)).toBe(lines); + }); +}); + +describe("dialogs are refused by the border and blank checks — not by the row bound", () => { + const DIALOG_FIXTURES = [ + "claude--permission-bash.txt", + "claude--permission-edit.txt", + "claude--plan-approval--numbered-body.txt", + "claude--plan-approval.txt", + "claude--select-menu.txt", + "claude--select-multi.txt", + "claude--select-multiselect-checked.txt", + "claude--select-multiselect-review.txt", + "claude--select-multiselect-single.txt", + "claude--select-preview-note-attached.txt", + "claude--select-preview-note-input.txt", + "claude--select-preview.txt", + "claude--trust-prompt.txt", + "claude--wizard-preview-note-attached.txt", + "claude--wizard-preview-q1.txt", + "claude--wizard-q1-revisit.txt", + "claude--wizard-q1.txt", + "claude--wizard-q2.txt", + "claude--wizard-submit-unanswered.txt", + "claude--wizard-submit.txt", + ]; + + it.each(DIALOG_FIXTURES)("%s surfaces no box, so no chrome is stripped from it", (name) => { + const lines = fixtureLines(name); + expect(extractStatusLine(lines)).toBeNull(); + expect(extractInputDraft(lines)).toBeNull(); + expect(stripChrome(lines).length).toBe(keptAboveTail(lines)); + }); + + it.each(DIALOG_FIXTURES)("%s ends its tail run with a blank within 2 rows", (name) => { + const texts = fixtureLines(name).map(lineText); + let end = texts.length; + while (end > 0 && texts[end - 1]!.trim().length === 0) end--; + let run = 0; + while (end - 1 - run >= 0 && texts[end - 1 - run]!.trim().length > 0) run++; + expect(run).toBeGreaterThan(0); + expect(run).toBeLessThanOrEqual(2); + }); +}); + +describe("the row bound only catches a run taller than any plausible statusline", () => { + const outputRows = (n: number) => Array.from({ length: n }, (_, i) => `tool output ${i}`); + + it("refuses a complete box above an 8-row blank-free run", () => { + const lines = boxWithBlankFreeRunBelow(outputRows(8)); + expect(extractStatusLine(lines)).toBeNull(); + expect(extractInputDraft(lines)).toBeNull(); + expect(stripChrome(lines)).toBe(lines); + }); + + it("known limitation: a complete box above a 7-row blank-free run reads as live", () => { + const lines = boxWithBlankFreeRunBelow(outputRows(7)); + expect(extractInputDraft(lines)).toBe("do the earlier thing"); + expect(lines.length - stripChrome(lines).length).toBe(11); + }); +}); + +describe("scrollback echo — a known limitation, pinned on purpose", () => { + const SENT = "please run the database migration now"; + + it.each([3, 8])("reads an echo of our own send back as a draft at %i dialog rows", (rows) => { + expect(extractInputDraft(echoedSendAboveDialog(SENT, rows))).toBe(SENT); + }); + + it("stops reading the echo once the run passes the bound", () => { + expect(extractInputDraft(echoedSendAboveDialog(SENT, 9))).toBeNull(); + }); + + it.each(["claude--select-menu.txt", "claude--select-multi.txt"])( + "%s: a real ❯ echo with transcript above it is not read as a draft", + (name) => { + expect(extractInputDraft(fixtureLines(name))).toBeNull(); + }, + ); +}); + +describe("real corpus — pinned so any change to the walk shows up as a diff", () => { + const PINNED: { fixture: string; statusLineFound: boolean; draft: string | null; stripped: number }[] = [ + { fixture: "done", statusLineFound: true, draft: "cat hello.txt to verify", stripped: 28 }, + { fixture: "draft-footer-empty", statusLineFound: true, draft: null, stripped: 9 }, + { fixture: "draft-footer-single", statusLineFound: true, draft: "remember to update the changelo", stripped: 9 }, + { fixture: "draft-footer-wrapped", statusLineFound: true, draft: "this stranded draft is long eno", stripped: 11 }, + { fixture: "draft-wrapped", statusLineFound: true, draft: "this stranded draft is long eno", stripped: 10 }, + { fixture: "fresh-idle", statusLineFound: true, draft: null, stripped: 47 }, + { fixture: "permission-bash", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "permission-edit", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "plan-approval", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "plan-approval--numbered-body", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "rename-resolved", statusLineFound: true, draft: null, stripped: 6 }, + { fixture: "select-menu", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-multi", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-multiselect-checked", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-multiselect-review", statusLineFound: false, draft: null, stripped: 5 }, + { fixture: "select-multiselect-single", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-preview", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-preview-note-attached", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "select-preview-note-input", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "send-inflight", statusLineFound: true, draft: "/rename", stripped: 5 }, + { fixture: "trust-prompt", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-preview-note-attached", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-preview-q1", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-q1", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-q1-revisit", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-q2", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-submit", statusLineFound: false, draft: null, stripped: 0 }, + { fixture: "wizard-submit-unanswered", statusLineFound: false, draft: null, stripped: 3 }, + { fixture: "working", statusLineFound: true, draft: null, stripped: 6 }, + ]; + + it("pins every claude fixture on disk, so a new capture can't slip past this table", () => { + const onDisk = readdirSync(PANES_DIR) + .filter((f) => f.startsWith("claude--") && f.endsWith(".txt")) + .map((f) => f.replace("claude--", "").replace(".txt", "")) + .sort(); + expect(onDisk).toEqual(PINNED.map((p) => p.fixture).sort()); + }); + + it.each(PINNED)("$fixture classifies identically", ({ fixture, statusLineFound, draft, stripped }) => { + const lines = fixtureLines(`claude--${fixture}.txt`); + expect(extractStatusLine(lines) !== null).toBe(statusLineFound); + if (draft === null) { + expect(extractInputDraft(lines)).toBeNull(); + } else { + expect(extractInputDraft(lines)).toContain(draft); + } + expect(lines.length - stripChrome(lines).length).toBe(stripped); + }); +}); diff --git a/web/src/lib/harness/claude/chrome.ts b/web/src/lib/harness/claude/chrome.ts index 1437be9..2967023 100644 --- a/web/src/lib/harness/claude/chrome.ts +++ b/web/src/lib/harness/claude/chrome.ts @@ -11,11 +11,11 @@ import type { StyledLine } from "../../blocks"; import { isBlank, isBoxBorder, lineText } from "./markers"; -// Lines allowed DIRECTLY under the input box's bottom border: the statusline plus a hint line or two -// ("← for agents", "⏵⏵ bypass permissions on …"). More than this and we don't recognise the shape, so -// we leave the buffer raw. The background-agents footer below these (see MAX_FOOTER_LINES) is peeled -// separately — this bound stays tight because it's the run that must sit flush against the border. -const MAX_STATUS_LINES = 3; +// Rows allowed DIRECTLY under the input box's bottom border: the statusline plus its hint row(s) +// ("← for agents", "⏵⏵ bypass permissions on …"). A statusline is an arbitrary user command's output, +// so this run is as tall as the user made it. The ceiling only stops a borderless buffer matching +// unboundedly — it guards less than it looks, and mirrors MAX_FOOTER_LINES: see ADR 0004. +const MAX_STATUS_LINES = 8; // A newer Claude Code UI paints a "background agents" footer BELOW the statusline/hint, separated from // them by a blank line: a bold "● main" header and one row per background agent @@ -140,7 +140,7 @@ interface InputBox { * ❯ (the prompt line) * (0..MAX_DRAFT_LINES wrapped-draft lines, no leading "❯") * - * (0..MAX_STATUS_LINES lines, matched by position not content) + * (statusline + hint rows together are 0..MAX_STATUS_LINES, by position) * * (optional — separates the background-agents footer, if present) * <● main> (0..MAX_FOOTER_LINES footer lines, matched by position not content)