fix(input): stop split mouse reports leaking when the ESC timer fires (#2793) - #2836
Open
sinelaw wants to merge 2 commits into
Open
fix(input): stop split mouse reports leaking when the ESC timer fires (#2793)#2836sinelaw wants to merge 2 commits into
sinelaw wants to merge 2 commits into
Conversation
A lone `0x1b` is undecidable in the legacy encoding: it is both the Escape key and the prefix of every sequence containing it. Callers therefore guess on a timer, and when the guess fires mid-sequence the remainder used to parse from ground as literal keystrokes — which fresh forwards straight into a focused embedded terminal's child pty, printing `^[[MCH4` at the user's shell prompt (#2793). Raising the timer only narrows the window; it cannot close it. Two changes, neither of which is a bigger constant: * After a flush, hold a following `[` for one byte. A mouse introducer (`M` or `<`) proves the flush split a mouse report, so the rest is handed to the existing X10/SGR machinery and decodes into the `Mouse` event the terminal meant to send. Anything else was a literal `[` keystroke and is emitted in order; an idle flush releases it, so it can never be swallowed. The spurious Escape is unavoidable once the timer has fired, but the report's own bytes no longer reach the child. * Let callers confirm the ambiguity is gone instead of assuming it. The kitty keyboard-flags reply (`CSI ? <flags> u`) was parsed and discarded; its flags are now readable, and `set_escape_unambiguous` makes a lone `ESC` wait indefinitely rather than resolve on a timer. On terminals that confirm "disambiguate escape codes", Escape arrives as `CSI 27 u`, so the leak becomes structurally impossible rather than merely rarer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VNJuB113xDhzn7mHdbjaL
…race Three loose ends from #2793, all about the lone-`ESC` guess: * fresh pushed `CSI > 1 u` optimistically and never learned whether the terminal honoured it, so it kept guessing on a timer even where Escape arrives unambiguously as `CSI 27 u`. It now also sends `CSI ? u` and adopts the reply: terminals confirming "disambiguate escape codes" retire the timer entirely. The query costs nothing on terminals that ignore it — unlike `crossterm::supports_keyboard_enhancement`, we never wait for an answer; the reply is picked up by the normal read path. * The session (daemon) path flushed a lone `ESC` after a single 15ms window while the tty path effectively allowed two, making `fresh -a` twice as easy to tear a split sequence apart. Both now take the same grace, exposed as `editor.keyboard_escape_time_ms` and defaulted to 50ms — matching Neovim's `ttimeoutlen` and libtermkey's waittime, where 15ms was the most aggressive of any comparable tool (tmux's `escape-time` defaults to 500ms). Users on high-latency links can raise it. (`server::runner`, whose `ServerConfig` carries no editor config, is referenced only by its own tests and keeps the default.) * `flush_pending_escape` claimed deferring the flush made the leak "structurally impossible". It does not — it widens the window from one grace period to two, which is why the report still reproduced past ~28ms. The comment now says what the code actually guarantees, and the regression test that only exercised the buffered window is joined by one that drives the flush path where the leak lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VNJuB113xDhzn7mHdbjaL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2793. The issue is closed as an inherent limitation of the legacy encoding, which is right —
0x1bis both the Escape key and a sequence prefix, so a loneESCis undecidable without a timer. These changes are the parts that aren't just raising the constant.What leaked
A mouse report split immediately after its introducing
ESC, with the halves far enough apart that the grace window expires in between, was flushed as an Escape key and the remainder parsed from ground as literal keystrokes — forwarded verbatim into the focused embedded terminal's child pty, printing^[[MCH4at the user's shell prompt.Changes
Resync instead of spraying keystrokes. After a flush, a following
[is held for one byte. A mouse introducer (M/<) proves the flush split a mouse report, so the rest goes to the existing X10/SGR machinery and decodes into theMouseevent the terminal meant to send. Anything else was a literal[keystroke, emitted in order; an idle flush releases it, so it can never be swallowed indefinitely (worst case it is delayed by one grace window). The spurious Escape is unavoidable once the timer has fired, but the report's own bytes no longer reach the child.Confirm the kitty protocol rather than assuming it.
terminal_modespushedCSI > 1 uoptimistically and never learned whether it took effect, so the timer stayed in charge even on terminals where Escape arrives unambiguously asCSI 27 u. It now also sendsCSI ? uand adopts the reply — the parser already decoded and discarded it. Where a terminal confirms "disambiguate escape codes", the guess is retired entirely and the leak becomes structurally impossible. The query costs nothing on terminals that ignore it: unlikecrossterm::supports_keyboard_enhancement(whose 2s probe timeout the existing comment rightly rejects), we never wait for an answer.One grace, configurable, defaulted sanely. The session path flushed after a single 15ms window while the tty path effectively allowed two, making
fresh -atwice as easy to tear a split sequence apart. Both now take the same value, exposed aseditor.keyboard_escape_time_ms, default 50ms — matching Neovim'sttimeoutlenand libtermkey's waittime. 15ms was the most aggressive of any comparable tool (tmux'sescape-timedefaults to 500ms), which put ordinary ssh jitter inside the leak window. Users on high-latency links can raise it.Corrected an over-claiming comment, and a test that proved less than it looked.
flush_pending_escapeclaimed deferring the flush made the leak "structurally impossible"; it widens the window from one grace period to two, which is why the report still reproduced past ~28ms. The existing regression test calleddrain_stdindirectly and never went through thepolltimeout where the flush actually happens, so it passed regardless of the constant. It is joined by one that drives the real flush path.Verification
Measured in tmux against the release binary, with two embedded terminals (one opting into mouse reporting, the focused one never doing so) logging raw stdin. Gap sweep of a well-formed X10 report split after the
ESC:LEAK b'\x1b[MCH4'LEAK b'\x1b[MCH4'b'\x1b'— the Escape key only; report resyncedOffset sweep at 100ms: previously
\x1b|[MCH4and\x1b|[<35;41;20Mleaked the whole report; now both leak only the single\x1bof the legitimately-delivered Escape keypress, and all 14 other offsets stay clean as before. The issue's ownrepro-045-residual.pywith its inter-chunk sleep at 50ms goes fromFAIL C1/FAIL C2toPASS/PASS; its other cases (fixed-width X10 coordinate consumption, trailing 1005-style bytes) are protocol-correct and unchanged.Also confirmed live end-to-end: default config, terminal opened from the command palette running
cat -v,ESC+ 100ms +[MCH4injected — previously printed^[[MCH4on screen, now prints nothing.Tests
fresh-input-parser: 6 new cases (X10 and SGR resync, literal[preserved and released on idle, unambiguous-Escape terminals never flushing, flags reply surfaced once). Verified failing without the fix — reverting just the resync transition fails 3 of them.tty_input:flushed_escape_does_not_leak_a_split_mouse_report_as_keysandconfirmed_disambiguate_mode_retires_the_escape_guess, both driving the reader's real call sequence with the idle flush invoked directly, so no timing is involved.#2810'sPAST_ESC_GRACEnow derives from the default grace instead of hard-coding 50ms, where it had come to sit exactly on the>=boundary.cargo test -p fresh-input-parser(106),-p fresh-editor --libinput/config subsets, and thee2e_teststerminal +issue_2810suites pass;cargo fmtandclippyclean on the touched files. Config schema regenerated viascripts/gen_schema.sh.🤖 Generated with Claude Code
https://claude.ai/code/session_018VNJuB113xDhzn7mHdbjaL
Generated by Claude Code