From 707ed1c9e87aa329f6bef03cc3a0e7875b0e86a6 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 14 Sep 2026 11:43:23 +0200 Subject: [PATCH 1/2] fix(tui): keep every terminal read on a mouse report boundary 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. --- AGENTS.md | 20 +- README.md | 9 +- internal/tui/input_reassembler.go | 139 +++++++- internal/tui/input_release_boundary_test.go | 353 ++++++++++++++++++++ 4 files changed, 497 insertions(+), 24 deletions(-) create mode 100644 internal/tui/input_release_boundary_test.go diff --git a/AGENTS.md b/AGENTS.md index 6b7681f..6c49d40 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -180,9 +180,23 @@ feat(tui): compact tool steps with Ctrl+E details toggle Ctrl+C as CSI once those modes are on. `RestoreEnhancedKeys` clears leftovers on startup and shutdown. A terminal read can end mid-sequence: `AssembleInput` (wired in `buildProgramOptions`, non-Windows) reassembles - it before Bubble Tea parses each read. Partial `ESC [ < …` heads wait for - their tail; a mouse-shaped head whose tail never arrives is dropped — never - echoed into the composer. Only the SGR form is disposable: the legacy X10 + it before Bubble Tea parses each read, under two invariants: a read from + the terminal never fetches more than the room its caller offers, and a + release never ends inside an escape sequence. Bubble Tea parses every read + on its own, so a chunk that stops inside a mouse report turns the head into + a finished CSI and the coordinate bytes behind it into typed runes — exactly + what a wheel burst from a terminal without mode 1006 produces, since + Terminal.app's `xterm-256color` advertises `kmous=\E[M` and answers with + legacy 6-byte `ESC [ M` reports that any burst past Bubble Tea's 256-byte + read cuts mid-report. Bounding the read (not the release) is what keeps + input flowing: bytes read ahead but not released are invisible to the + kqueue/epoll readiness wait that gates the next `Read`, so holding one back + stalls the rest of the burst until the next keystroke. The release clamp + covers what the read bound cannot — a non-file source (tests) reading its + full window, and a caller whose buffer shrinks mid-stream. + Partial `ESC [ < …` heads wait for their tail; a mouse-shaped head whose + tail never arrives is dropped — never echoed into the composer. Only the SGR + form is disposable: the legacy X10 form is joined but never dropped, because its coordinate bytes are indistinguishable from typed text. Bracket-paste bodies are user data and are never dropped, whatever they contain. A mouse head a fresh read cannot diff --git a/README.md b/README.md index aea10b3..79cdbb7 100644 --- a/README.md +++ b/README.md @@ -667,10 +667,11 @@ never become the weakest link: reports the mouse so the wheel can scroll. Click a reply card to copy it, or use `^Y` / `alt+y`. `--plain` keeps the terminal's own scrollback. - **Garbage like `65;75;25M[<65;75;25M` appears in the composer while - scrolling** — the terminal split a mouse report across reads. bodek - reassembles the input stream before Bubble Tea parses it, so every split - point is covered: press `^U` to clear any text from an older build, and - `bodek upgrade` if you are not on the latest release. + scrolling** — the terminal split a mouse report across reads. bodek now keeps + every read on a report boundary, so no split point can splice a report into + the draft; the old builds showed this most on terminals that never negotiate + SGR mouse mode (Terminal.app). Press `^U` to clear text left by an older + build, and `bodek upgrade`. - **Colors look wrong** — try `/theme classic`, check `TERM`; `NO_COLOR=1` forces a colorless render everywhere. - **Connection dropped mid-turn** — bodek retries with backoff (5 attempts) diff --git a/internal/tui/input_reassembler.go b/internal/tui/input_reassembler.go index 11894ad..f93181c 100644 --- a/internal/tui/input_reassembler.go +++ b/internal/tui/input_reassembler.go @@ -22,8 +22,9 @@ const inputSettle = 10 * time.Millisecond // text would type garbage into the composer. const mouseAbandon = 250 * time.Millisecond -// inputReadLen sizes each source read. Large enough that a whole wheel -// burst lands in one read, small enough to bound memory. +// inputReadLen caps one source read: a whole wheel burst lands in one read, +// while memory stays bounded. A read from the watched terminal is further +// bounded by the caller's room (see readNext). const inputReadLen = 4096 // stringSeqCap bounds how much of a string sequence (OSC / DCS / APC) is held @@ -195,8 +196,20 @@ func (r *inputReassembler) isClosed() bool { // kqueue/epoll wakeups coherent: a pre-draining pump goroutine would strand // already-read bytes where a kevent can never see them, freezing input until // an unrelated later keystroke lands. +// +// For the same reason a read never fetches more than the room in p: whatever +// is read but not released in this call sits here where the reader's own +// readiness wait — it waits on the descriptor before every Read — cannot see +// it. Holding a byte back would stall every remaining byte of the burst until +// the next keystroke. With the read bounded by p, a release is either the +// whole buffer or everything up to a held head, and the only bytes left behind +// are an incomplete sequence whose tail is still arriving. func (r *inputReassembler) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } for { + room := len(p) - len(r.buf) if r.headAt > 0 { return r.emit(p, r.headAt), nil } @@ -207,7 +220,20 @@ func (r *inputReassembler) Read(p []byte) (int, error) { if r.srcDone { return r.finish(p) } - if n, ok := r.held(p); ok { + if room <= 0 { + // The head fills the caller's whole buffer. A head that can only + // be a mouse report is noise even here: streaming a truncated + // report as text is the bug being fixed, so drop it. + if r.droppable() { + r.drop() + continue + } + // A real sequence longer than the caller's buffer (a long string + // sequence) is streamed rather than held for a tail that could + // never fit. + return r.emit(p, len(r.buf)), nil + } + if n, ok := r.held(p, room); ok { return n, nil } continue @@ -215,15 +241,15 @@ func (r *inputReassembler) Read(p []byte) (int, error) { if r.srcDone || r.isClosed() { return r.finish(p) } - c, _ := r.readNext(-1) + c, _ := r.readNext(-1, room) r.take(c) } } // held waits for the tail of the head that occupies the whole buffer. It // reports false when the head was resolved (dropped or flushed), in which case -// the caller re-plans. -func (r *inputReassembler) held(p []byte) (int, bool) { +// the caller re-plans. room is how many bytes the caller can still take. +func (r *inputReassembler) held(p []byte, room int) (int, bool) { if !r.holding { r.holding, r.since, r.heldLen = true, time.Now(), len(r.buf) } @@ -241,7 +267,7 @@ func (r *inputReassembler) held(p []byte) (int, bool) { budget = r.abandon - time.Since(r.since) } if budget > 0 { - if c, ok := r.readNext(budget); ok { + if c, ok := r.readNext(budget, room); ok { r.take(c) if r.srcDone { n, err := r.finish(p) @@ -269,21 +295,36 @@ func (r *inputReassembler) take(c inputChunk) { } // readNext performs one read from the source, waiting no longer than budget -// for input to arrive. A negative budget blocks until input or the source -// ends. It reports false only when the budget lapsed without any data — -// nothing was consumed, so the caller may retry or give up on the head. +// for input to arrive, and never fetching more than room bytes. A negative +// budget blocks until input or the source ends. It reports false only when the +// budget lapsed without any data — nothing was consumed, so the caller may +// retry or give up on the head. // // For a file source the wait is poll(2) on the descriptor followed by the read // itself, so unread bytes stay in the kernel buffer and kqueue/epoll wakeups // remain coherent. Non-file sources only occur in unit tests (the wrapper // itself is attached to terminals only); for those, a single buffered read // goroutine provides the bounded wait without ever losing a read. -func (r *inputReassembler) readNext(budget time.Duration) (inputChunk, bool) { +func (r *inputReassembler) readNext(budget time.Duration, room int) (inputChunk, bool) { + if room <= 0 && r.file != nil { + return inputChunk{}, false + } + // A read from the watched descriptor never exceeds the caller's room: + // bytes read ahead of an unfilled buffer sit where the readiness wait that + // gates every Read (kqueue/epoll, see waitReadable) cannot see them, which + // would stall the rest of the burst until the next keystroke. A non-file + // source has no such wait — the wrapper is only ever attached to terminals, + // so that path exists for tests — and reads its full window instead; the + // release clamp in emit is what keeps its sequences whole. + size := inputReadLen + if r.file != nil { + size = min(inputReadLen, room) + } if r.file != nil { if budget >= 0 && !r.waitReadable(budget) { return inputChunk{}, false } - b := make([]byte, inputReadLen) + b := make([]byte, size) n, err := r.file.Read(b) return inputChunk{data: b[:n], err: err}, true } @@ -291,7 +332,7 @@ func (r *inputReassembler) readNext(budget time.Duration) (inputChunk, bool) { ch := make(chan inputChunk, 1) r.pending = ch go func() { - b := make([]byte, inputReadLen) + b := make([]byte, size) n, err := r.src.Read(b) ch <- inputChunk{data: b[:n], err: err} }() @@ -346,22 +387,50 @@ func (r *inputReassembler) absorb(c inputChunk) { } // emit copies n buffered bytes into p, tracks paste state across them, and -// never ends a release inside a bracketed-paste marker: a marker torn by the -// caller's buffer size would be released as text and desync paste tracking. +// never ends a release inside an escape sequence or a bracketed-paste marker. +// +// The caller's buffer is what forces the cut: Bubble Tea reads input 256 bytes +// at a time and parses every read on its own, so a chunk that stops inside a +// mouse report is not "incomplete" from its side. The head in front of the cut +// is a finished CSI (ESC [ M is the legacy form's final byte) and the bytes +// behind it are decoded as typed runes — one stray character per boundary, +// repeated through a wheel burst. A torn marker would likewise desync paste +// tracking. func (r *inputReassembler) emit(p []byte, n int) int { if n > len(r.buf) { n = len(r.buf) } + // The caller's buffer is the hard cut, and it is applied before the + // sequence rules below: a clamp inside copy would silently tear whatever + // sequence happens to sit at the boundary. + if n > len(p) { + n = len(p) + } if len(p) >= len(pasteStart) { - if cut := cutBeforeMarker(r.buf, n); cut > 0 { + if cut := cutBeforeMarker(r.buf, n); cut > 0 && cut < n { n = cut } } + if cut := cutBeforeSequence(r.buf, n); cut > 0 && cut < n { + n = cut + } written := copy(p, r.buf[:n]) r.trackPaste(r.buf[:written]) r.buf = r.buf[written:] if r.headAt >= 0 { - r.headAt -= written + // A release can stop short of the head's start — the rules above cut + // it back — and the remainder is then the head's own tail, so the + // head begins at offset 0. It must never go negative: a negative + // offset reads as "no head", and the tail would be released unheld + // on the next call. + if r.headAt >= written { + r.headAt -= written + } else { + r.headAt = 0 + } + } + if len(r.buf) == 0 { + r.headAt = -1 } r.holding = false return written @@ -572,3 +641,39 @@ func cutBeforeMarker(b []byte, n int) int { } return n } + +// cutBeforeSequence shortens n so a release never ends inside an escape +// sequence. It returns the offset the release must stop at, or 0 when no +// sequence straddles n. +// +// The room-bounded read (see Read) already lands releases from a watched +// descriptor on sequence boundaries; this is the release-side guarantee, and +// it is load-bearing wherever the buffer can hold more than the caller asked +// for — a non-file source reading its full window, and a caller that hands a +// smaller buffer than the read that filled the buffer. Bubble Tea cannot +// recover from the split itself: ESC [ M is a valid final byte on its own, so +// it consumes a torn legacy X10 report's head as an unknown CSI and decodes +// the coordinate bytes behind it as text, while a torn SGR report leaves +// ESC [ < … with no final byte and is reported as an Alt+[ keypress with the +// digits behind it as text. Either way the bytes reach the composer. The tail +// is held here instead and released with the next read. +// +// Only the last escape before n can straddle it, so that is the one examined. +// An escape at offset 0 is left alone: stopping there would return an empty +// read and stall the caller — a sequence longer than the caller's buffer (a +// long OSC string) is streamed instead, which Bubble Tea already handles by +// waiting for its terminator. +func cutBeforeSequence(b []byte, n int) int { + if n <= 1 || n >= len(b) { + return 0 + } + i := bytes.LastIndexByte(b[:n], 0x1b) + if i <= 0 { + return 0 + } + length, complete := escapeLen(b[i:]) + if !complete || i+length > n { + return i + } + return 0 +} diff --git a/internal/tui/input_release_boundary_test.go b/internal/tui/input_release_boundary_test.go new file mode 100644 index 0000000..9927e8a --- /dev/null +++ b/internal/tui/input_release_boundary_test.go @@ -0,0 +1,353 @@ +package tui + +import ( + "fmt" + "io" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +// Bubble Tea reads input 256 bytes at a time and parses each read on its own +// (key.go: `var buf [256]byte`). It has no way to tell that a read ended in +// the middle of a mouse report, so a release that stops there is not "held" — +// the head in front of the cut is a well-formed CSI and the bytes behind it +// are decoded as typed runes. +// +// Both mouse encodings tear this way: +// - legacy X10 (ESC [ M plus three bytes): ESC [ M matches Bubble Tea's +// unknownCSIRe on its own — M is a final byte — so the coordinate bytes +// that follow are typed into the composer as single characters. Terminal.app +// never negotiates mode 1006 (its xterm-256color terminfo advertises +// kmous=\E[M and no XM), so a wheel burst is a stream of these 6-byte +// reports and the 256-byte boundary lands inside one most of the time. +// - SGR (ESC [ < … M/m): the consumed head makes the parser report an +// Alt+[ keypress and the digits behind it become text. +// +// The tests below pin the invariant that keeps both out of the composer: a +// release never ends inside a mouse report. + +// x10Report is a legacy wheel-up report: ESC [ M plus the button and the +// two 1-based coordinate bytes (0x60 = wheel up, 0x41 = column/row 1). +const x10Report = "\x1b[M\x60\x41\x41" + +// sgrReport is the same event in the SGR (mode 1006) encoding. +const sgrReport = "\x1b[<64;1;1M" + +// reportSpans locates the mouse reports in a stream with a scanner of its own +// — deliberately not the production escapeLen — so a release boundary can be +// judged against the byte forms terminals actually put on the wire. +func reportSpans(s string) [][2]int { + var out [][2]int + for i := 0; i < len(s); { + switch { + case strings.HasPrefix(s[i:], "\x1b[M") && i+6 <= len(s): + out = append(out, [2]int{i, i + 6}) + i += 6 + case strings.HasPrefix(s[i:], "\x1b[<"): + j := i + 3 + for j < len(s) && (s[j] == ';' || (s[j] >= '0' && s[j] <= '9')) { + j++ + } + if j < len(s) && (s[j] == 'M' || s[j] == 'm') { + out = append(out, [2]int{i, j + 1}) + i = j + 1 + continue + } + i++ + default: + i++ + } + } + return out +} + +// assertBoundariesClear fails when any release boundary lands inside a mouse +// report: the report's head and tail are then parsed as two separate reads, +// and the tail is typed into the composer. +func assertBoundariesClear(t *testing.T, stream string, cuts []int) { + t.Helper() + spans := reportSpans(stream) + for _, k := range cuts { + for _, sp := range spans { + if sp[0] < k && k < sp[1] { + t.Fatalf("release boundary %d lands inside the mouse report at %d..%d of %d bytes: "+ + "Bubble Tea parses that read alone and types the bytes behind the head into the composer", + k, sp[0], sp[1], len(stream)) + } + } + } +} + +// TestAssembleInputReleaseNeverSplitsAReport drives bursts longer than the +// 256-byte read Bubble Tea uses and asserts that every release ends between +// reports — the invariant the composer depends on. +func TestAssembleInputReleaseNeverSplitsAReport(t *testing.T) { + for _, tc := range []struct { + name string + burst string + }{ + {"legacy x10 wheel burst", strings.Repeat(x10Report, 60)}, // 360 bytes + {"sgr wheel burst", strings.Repeat(sgrReport, 40)}, // 440 bytes + {"x10 burst with typing behind it", strings.Repeat(x10Report, 50) + "hi"}, // 302 bytes + } { + t.Run(tc.name, func(t *testing.T) { + src := &replayReader{chunks: [][]byte{[]byte(tc.burst)}, block: make(chan struct{})} + re := newInputReassembler(src, 5*time.Second, 5*time.Second) + var got []byte + var cuts []int + for len(got) < len(tc.burst) { + chunk, ok := readWithin(t, re, time.Second) + if !ok || len(chunk) == 0 { + t.Fatalf("released %d of %d bytes and then stalled", len(got), len(tc.burst)) + } + got = append(got, chunk...) + if len(got) < len(tc.burst) { + cuts = append(cuts, len(got)) + } + } + close(src.block) + if string(got) != tc.burst { + t.Fatalf("burst released as %d bytes, want the original %d byte-for-byte", len(got), len(tc.burst)) + } + assertBoundariesClear(t, tc.burst, cuts) + }) + } +} + +// msgSummary renders the message mix of a run, so a failure says what the +// burst turned into instead of only how much of it survived. +func msgSummary(msgs []tea.Msg) string { + counts := map[string]int{} + var order []string + for _, m := range msgs { + kind := fmt.Sprintf("%T", m) + if n, ok := counts[kind]; !ok { + order = append(order, kind) + counts[kind] = 1 + } else { + counts[kind] = n + 1 + } + } + parts := make([]string, 0, len(order)) + for _, kind := range order { + parts = append(parts, fmt.Sprintf("%s×%d", kind, counts[kind])) + } + return strings.Join(parts, " ") +} + +// assertBurstIsAllMouse checks that a burst arrived as exactly reps mouse +// events and nothing else: no typed runes, and no unknown-sequence garbage +// either — both mean the boundary tore a report. +func assertBurstIsAllMouse(t *testing.T, msgs []tea.Msg, reps int, what string) { + t.Helper() + if got := keyMsgs(msgs); len(got) > 0 { + t.Fatalf("%s leaked %d key message(s) into the composer: %v", what, len(got), got) + } + if got := mouseMsgs(msgs); len(got) != reps { + t.Fatalf("%s → %d mouse message(s), want %d (%s)", what, len(got), reps, msgSummary(msgs)) + } + if len(msgs) != reps { + t.Fatalf("%s → %d message(s) in total, want exactly %d mouse events (%s)", what, len(msgs), reps, msgSummary(msgs)) + } +} + +// TestProgramKeepsLegacyX10BurstIntact is the same invariant through Bubble +// Tea's real parser: the burst must arrive as one mouse event per report with +// nothing typed in between. This is the Terminal.app shape. +func TestProgramKeepsLegacyX10BurstIntact(t *testing.T) { + const reps = 60 + burst := strings.Repeat(x10Report, reps) // 360 bytes: past the 256-byte read + + assertBurstIsAllMouse(t, runProgram(t, [][]byte{[]byte(burst)}, reps, 2*time.Second), reps, "x10 burst") +} + +// TestProgramKeepsSGRBurstIntact covers the mode-1006 terminals (VSCode, +// iTerm2, kitty): the same boundary must not tear their longer reports either. +func TestProgramKeepsSGRBurstIntact(t *testing.T) { + const reps = 40 + burst := strings.Repeat(sgrReport, reps) // 440 bytes: past the 256-byte read + + assertBurstIsAllMouse(t, runProgram(t, [][]byte{[]byte(burst)}, reps, 2*time.Second), reps, "sgr burst") +} + +// TestProgramKeepsLongX10WheelBurstIntact scales the same shape up: a fast +// scroll is kilobytes of reports, and every byte must be delivered without +// waiting for further input. A byte read off the descriptor but not released +// in the same read is invisible to the reader's own readiness wait (the +// kqueue/epoll cancel reader waits on the descriptor first), so the burst +// would stall mid-flight — which is why the reassembler reads no further than +// the room its caller offers. +func TestProgramKeepsLongX10WheelBurstIntact(t *testing.T) { + const reps = 400 + burst := strings.Repeat(x10Report, reps) // 2.4K: many reads past the room + + assertBurstIsAllMouse(t, runProgram(t, [][]byte{[]byte(burst)}, reps, 5*time.Second), reps, "long x10 burst") +} + +// TestProgramDeliversLargePasteWhole guards the other side of the read cap: a +// paste larger than the caller's buffer is delivered in many reads, and +// Bubble Tea only recognises the body as pasted once it has seen both markers +// — so bounding the read must not tear the paste apart or delay its tail. +func TestProgramDeliversLargePasteWhole(t *testing.T) { + body := strings.Repeat("paste body 42 ", 120) // 1.6K + stream := pasteStart + body + pasteEnd + + msgs := runProgram(t, [][]byte{[]byte(stream)}, 1, 5*time.Second) + + if len(msgs) != 1 { + t.Fatalf("large paste → %d message(s), want a single paste (%s)", len(msgs), msgSummary(msgs)) + } + km, ok := msgs[0].(tea.KeyMsg) + if !ok { + t.Fatalf("large paste → %T, want a key message", msgs[0]) + } + if !km.Paste { + t.Fatalf("large paste → KeyMsg{Paste: false}: Bubble Tea did not see it as pasted") + } + if got := string(km.Runes); got != body { + t.Fatalf("large paste → %d runes, want %d byte-for-byte", len(km.Runes), len(body)) + } +} + +// drainWithBuffer reads everything r produces in chunks of at most size bytes, +// like a caller whose buffer is smaller than Bubble Tea's 256. It returns the +// bytes and every release boundary, so the same boundary invariant can be +// judged for any caller size. +func drainWithBuffer(t *testing.T, r io.Reader, size int, budget time.Duration) (string, []int) { + t.Helper() + var got []byte + var cuts []int + var spent time.Duration + step := 5 * time.Millisecond + for { + buf := make([]byte, size) + n, err := r.Read(buf) + if n > 0 { + got = append(got, buf[:n]...) + cuts = append(cuts, len(got)) + } + if err != nil { + break + } + if n == 0 { + spent += step + if spent > budget { + t.Fatalf("reader stalled after %d bytes", len(got)) + } + time.Sleep(step) + } + } + if len(cuts) > 0 { + cuts = cuts[:len(cuts)-1] // the last release ends the stream, not a cut + } + return string(got), cuts +} + +// TestAssembleInputSmallCallerBufferNeverTearsAReport pins the release clamp. +// A caller with a smaller buffer than the read that filled the wrapper's own +// buffer forces a cut mid-stream; every such cut must still land between +// reports. Without the clamp in emit the boundary lands inside a report and +// Bubble Tea decodes the bytes behind the head as text — this test fails. +// +// The guarantee needs a buffer that can hold one whole report: a buffer +// shorter than the longest sequence in the stream cannot be served without +// tearing it, because cutting at offset 0 would release nothing and stall the +// caller (that is why the clamp stops there). Bubble Tea reads 256 bytes, an +// order of magnitude above the longest report a terminal emits. +func TestAssembleInputSmallCallerBufferNeverTearsAReport(t *testing.T) { + for _, tc := range []struct { + name string + burst string + sizes []int + }{ + {"legacy x10 reports", strings.Repeat(x10Report, 30), []int{6, 7, 11, 64, 256}}, + {"sgr reports", strings.Repeat(sgrReport, 30), []int{10, 11, 64, 256}}, + } { + for _, size := range tc.sizes { + t.Run(fmt.Sprintf("%s buffer %d", tc.name, size), func(t *testing.T) { + src := &replayReader{chunks: [][]byte{[]byte(tc.burst)}} + re := newInputReassembler(src, time.Second, time.Second) + got, cuts := drainWithBuffer(t, re, size, 2*time.Second) + if got != tc.burst { + t.Fatalf("released %d bytes, want the original %d byte-for-byte", len(got), len(tc.burst)) + } + assertBoundariesClear(t, tc.burst, cuts) + }) + } + } +} + +// TestReadWithNoRoomIsANoOp pins the empty-buffer guard: Read must not reach +// for the source to fill a caller that has no room. +func TestReadWithNoRoomIsANoOp(t *testing.T) { + src := &replayReader{chunks: [][]byte{[]byte(x10Report)}, block: make(chan struct{})} + re := newInputReassembler(src, time.Second, time.Second) + n, err := re.Read(nil) + close(src.block) + if n != 0 || err != nil { + t.Fatalf("Read(nil) → (%d, %v), want (0, nil)", n, err) + } +} + +// TestReadDropsAReportFillingTheCallersBuffer pins the room<=0 branch: when a +// mouse head fills the caller's whole buffer it is noise, not input, so it is +// dropped rather than streamed out as text. +func TestReadDropsAReportFillingTheCallersBuffer(t *testing.T) { + head := strings.Repeat("\x1b[<64;75", 3) // a mouse-shaped head, no terminator + src := &replayReader{chunks: [][]byte{[]byte(head), []byte("ok")}, block: make(chan struct{})} + re := newInputReassembler(src, 5*time.Millisecond, 30*time.Millisecond) + + buf := make([]byte, len(head)) // exactly the head: room == 0 + n, _ := re.Read(buf) + close(src.block) + if string(buf[:n]) == head { + t.Fatalf("a report-shaped head filling the buffer was streamed out as text: %q", string(buf[:n])) + } +} + +// TestCutBeforeSequenceClampsAtTheStraddlingReport pins the helper directly: +// it must name the byte offset a release has to stop at. +func TestCutBeforeSequenceClampsAtTheStraddlingReport(t *testing.T) { + for _, tc := range []struct { + name string + buf string + n int + want int + }{ + {"x10 report straddles the cut", x10Report + x10Report, 8, 6}, + {"sgr report straddles the cut", sgrReport + sgrReport, 12, 10}, + {"cut inside the head", x10Report + x10Report, 2, 0}, + {"cut exactly between reports", x10Report + x10Report, 6, 0}, + {"complete sequence before the cut", x10Report + "typed", 11, 0}, + {"no sequence at all", "plain typing here", 12, 0}, + {"whole buffer is the cut", x10Report, len(x10Report), 0}, + {"escape at offset 0 is not a stall", x10Report + x10Report, 3, 0}, + {"typing then a straddling report", "hi" + x10Report, 5, 2}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := cutBeforeSequence([]byte(tc.buf), tc.n); got != tc.want { + t.Fatalf("cutBeforeSequence(%q, %d) = %d, want %d", tc.buf, tc.n, got, tc.want) + } + }) + } +} + +// clamping releases to report boundaries must not delay or drop the typing +// that follows a burst. +func TestProgramKeepsTypingThroughABurst(t *testing.T) { + const reps = 50 + burst := strings.Repeat(x10Report, reps) + "ok" + // One mouse message per report, then the two runes: 52 messages total. + msgs := runProgram(t, [][]byte{[]byte(burst)}, reps+1, 2*time.Second) + + if got := mouseMsgs(msgs); len(got) != reps { + t.Fatalf("burst → %d mouse message(s), want %d", len(got), reps) + } + keys := keyMsgs(msgs) + if len(keys) != 1 || string(keys[0].Runes) != "ok" { + t.Fatalf("typing behind a burst → %v, want a single %q key", keys, "ok") + } +} From 41c0e7067567f70989fd556f0b54281de892c79f Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 14 Sep 2026 11:47:31 +0200 Subject: [PATCH 2/2] docs(tui): scope the read-bound claim in readNext to the terminal source 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. --- internal/tui/input_reassembler.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/internal/tui/input_reassembler.go b/internal/tui/input_reassembler.go index f93181c..50c41d6 100644 --- a/internal/tui/input_reassembler.go +++ b/internal/tui/input_reassembler.go @@ -197,13 +197,15 @@ func (r *inputReassembler) isClosed() bool { // already-read bytes where a kevent can never see them, freezing input until // an unrelated later keystroke lands. // -// For the same reason a read never fetches more than the room in p: whatever -// is read but not released in this call sits here where the reader's own -// readiness wait — it waits on the descriptor before every Read — cannot see -// it. Holding a byte back would stall every remaining byte of the burst until -// the next keystroke. With the read bounded by p, a release is either the -// whole buffer or everything up to a held head, and the only bytes left behind -// are an incomplete sequence whose tail is still arriving. +// For the same reason a read from the terminal never fetches more than the +// room in p: whatever is read but not released in this call sits here where the +// reader's own readiness wait — it waits on the descriptor before every Read — +// cannot see it. Holding a byte back would stall every remaining byte of the +// burst until the next keystroke. With the read bounded by p, a release is +// either the whole buffer or everything up to a held head, and the only bytes +// left behind are an incomplete sequence whose tail is still arriving. A +// non-file source has no readiness wait to strand bytes behind (the wrapper is +// only ever attached to terminals) and reads its full window instead. func (r *inputReassembler) Read(p []byte) (int, error) { if len(p) == 0 { return 0, nil @@ -295,10 +297,11 @@ func (r *inputReassembler) take(c inputChunk) { } // readNext performs one read from the source, waiting no longer than budget -// for input to arrive, and never fetching more than room bytes. A negative -// budget blocks until input or the source ends. It reports false only when the -// budget lapsed without any data — nothing was consumed, so the caller may -// retry or give up on the head. +// for input to arrive. A read from the watched descriptor never fetches more +// than room bytes; a non-file source (tests only) reads its full window. A +// negative budget blocks until input or the source ends. It reports false only +// when the budget lapsed without any data — nothing was consumed, so the +// caller may retry or give up on the head. // // For a file source the wait is poll(2) on the descriptor followed by the read // itself, so unread bytes stay in the kernel buffer and kqueue/epoll wakeups