Skip to content

fix(tui): keep every terminal read on a mouse report boundary - #122

Merged
jkyberneees merged 2 commits into
mainfrom
fix/composer-report-boundary
Sep 14, 2026
Merged

jkyberneees merged 2 commits into
mainfrom
fix/composer-report-boundary

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Problem

The composer kept picking up odd characters in Terminal.app on macOS — repeating
fragments like ;1;1M / 64;5;13M, sometimes [, spliced into the draft while
scrolling. It reproduced in Terminal.app and never in VSCode's integrated
terminal.

Root cause

Bubble Tea reads input 256 bytes at a time and parses each read on its own
(key.go: var buf [256]byte; leftOverFromPrevIteration is only consulted
when detectOneMsg reports w == 0). A read that ends in the middle of a mouse
report is therefore not "incomplete" from Bubble Tea's side: the head in front of
the cut is a finished CSI, and the bytes behind it are decoded as typed runes.

That is fatal for the legacy X10 encoding, where ESC [ M (the head) is matched
by Bubble Tea's unknownCSIRe on its own — M is a final byte — so the three
coordinate bytes that follow become single typed characters. And Terminal.app
never negotiates SGR mouse mode 1006: the system's xterm-256color terminfo
advertises kmous=\E[M and no XM, so a wheel burst is a stream of 6-byte
ESC [ M reports. Any burst longer than 256 bytes gets cut mid-report, and every
such cut types one stray character into the composer.

AssembleInput already reassembled reports between reads, but nothing kept a
read from ending inside one: the reader filled its own 4096-byte buffer and let
the caller's 256-byte buffer clamp the release inside copy.

Measured before the fix (60 wheel reports = 360 bytes, one write):

x10 burst -> 42 mouse message(s), want 60 (tea.MouseMsg x42 tea.unknownCSISequenceMsg x1)
sgr burst leaked 1 key message(s) into the composer: [alt+[]   <- the alt+[ is the torn head

Fix

Two invariants in internal/tui/input_reassembler.go:

  1. A read from the terminal never fetches more than the caller's room.
    Bytes read ahead but not released in the same call sit where the
    kqueue/epoll readiness wait that gates every Read cannot see them, so
    holding one back would stall the rest of the burst until the next keystroke.
    (The first attempt clamped only the release; input then stalled mid-burst —
    that is why the bound belongs on the read.)
  2. A release never ends inside an escape sequence (cutBeforeSequence),
    which covers what the read bound cannot: a non-file source reading its full
    window, and a caller whose buffer shrinks mid-stream. Reverting this clamp
    alone fails TestAssembleInputSmallCallerBufferNeverTearsAReport at 64 and
    256 bytes — it is load-bearing, not decoration.

Plus three guards found in review: a mouse-shaped head that fills the caller's
whole buffer is dropped rather than streamed as text; a release that stops
short of a held head keeps the head at offset 0 (a negative offset would read as
"no head" and let the tail out unheld); and Read with no room is a no-op.

Tests (RED-first)

internal/tui/input_release_boundary_test.go — the boundary invariant from both
ends: the byte stream through the real os.Pipe harness, and the message mix
through Bubble Tea's real parser.

Test Pins
TestAssembleInputReleaseNeverSplitsAReport every release boundary falls between reports (X10, SGR, burst + trailing typing)
TestAssembleInputSmallCallerBufferNeverTearsAReport the release clamp, at buffer sizes 6…256
TestProgramKeepsLegacyX10BurstIntact 60 X10 reports -> exactly 60 mouse events, nothing typed (Terminal.app shape)
TestProgramKeepsSGRBurstIntact the mode-1006 shape (VSCode, iTerm2, kitty)
TestProgramKeepsLongX10WheelBurstIntact 400 reports / 2.4 KB delivered without stalling
TestProgramDeliversLargePasteWhole a 1.6 KB bracketed paste still arrives as one Paste message
TestProgramKeepsTypingThroughABurst typing behind a burst is not delayed or dropped
TestReadDropsAReportFillingTheCallersBuffer the room <= 0 drop path
TestReadWithNoRoomIsANoOp Read(nil)
TestCutBeforeSequenceClampsAtTheStraddlingReport the helper, offset by offset

RED proof: all six original tests failed on main (boundary 256 landing inside
the report at 252..258; 42/60 mouse events; the alt+[ leak). Green after the
fix. Boundary tests are timing-free — they assert byte positions and message
counts, never spinner frames or sleeps.

Verification

make fmt, make vet, make lint (0 issues) and make test (go test -race,
all packages) all green. internal/tui coverage is unchanged by this diff.

Reviewed by a three-agent adversarial panel (one read-only reviewer, two
falsification runs against the Bubble Tea and cancelreader sources). Their
findings — the unpinned clamp, the room-<= 0 drop, the negative head offset,
and the untested guards — are what the final commit incorporates. The
falsification run's verdict was NOT FALSIFIED, with the one counterexample to the
stated invariant (a report split by more than the 10 ms settle window)
reproducing identically on main.

Docs: the AGENTS.md input-path contract now states both invariants and why the
bound is on the read.

Bubble Tea reads input 256 bytes at a time and parses each read on its own,
so a read that ends inside a mouse report is not incomplete from its side:
the head is a finished CSI and the bytes behind the cut are decoded as typed
runes. Reported as odd characters splicing into the composer while
scrolling, reproducible in Terminal.app and not in VSCode.

Terminal.app never negotiates SGR mouse mode 1006 -- the system's
xterm-256color terminfo advertises kmous=\E[M and no XM -- so a wheel burst
is a stream of legacy 6-byte ESC [ M reports, and ESC [ M is a complete
sequence on its own. A burst past the 256-byte read was therefore cut
mid-report and typed garbage into the draft: 60 reports arrived as 42 mouse
events, with the torn head surfacing as an alt+[ keypress.

The reassembler now holds two invariants: a read from the terminal never
fetches more than the caller's room (bytes read ahead are invisible to the
kqueue/epoll wait that gates the next Read, so holding one back stalls the
rest of the burst), and a release never ends inside an escape sequence, which
covers a non-file source reading its full window and a caller whose buffer
shrinks mid-stream. A mouse-shaped head that fills the buffer is dropped
rather than streamed as text, a release that stops short of a held head keeps
it at offset 0 instead of letting the tail out unheld, and Read with no room
is a no-op.

Six boundary tests failed before the change and pass after; reverting the
release clamp alone fails the small-buffer test at 64 and 256 bytes.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bodek 41c0e70 Commit Preview URL

Branch Preview URL
Sep 14 2026, 09:47 AM

A read from the watched descriptor is bounded by the caller's room; the
non-file path a test constructs reads its full window and relies on the
release clamp in emit instead.
@jkyberneees
jkyberneees merged commit 4a372e3 into main Sep 14, 2026
9 checks passed
@jkyberneees
jkyberneees deleted the fix/composer-report-boundary branch September 14, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant