fix: stop teardown drains at the first non-mouse event - #57
Merged
Conversation
All three mouse-report drains (driver shutdown, init-exit shutdown, and the RawModeGuard drop fallback) read and discarded every event type for up to 50ms, so keystrokes typed immediately after an app exited could be eaten. The spray is contiguous: stop at the first non-mouse event, leaving everything not yet read in the tty for the shell. This shrinks the loss window but cannot cap it at one event: crossterm reads the tty in 1KiB chunks and queues every event it parses, so whatever shared a chunk with that first non-mouse event is consumed along with it — ttys have no peek and TIOCSTI is disabled on modern kernels, so consumed input cannot be handed back. A true byte-granular drain requires owning the input path instead of reading through crossterm; tracked as ATU-580.
BinaryMuse
force-pushed
the
mkt/drain-keeps-keystrokes
branch
from
August 10, 2026 16:30
2b5c986 to
be0c465
Compare
BinaryMuse
marked this pull request as ready for review
August 10, 2026 16:31
Greptile SummaryThe PR improves terminal teardown by stopping mouse-report drains at the first non-mouse event, reducing the amount of user input consumed after application exit.
Confidence Score: 5/5The PR appears safe to merge with its remaining crossterm buffering limitation clearly documented. The changed drains consistently stop after observing a non-mouse event, and no unacknowledged concrete failure was established. Important Files Changed
Reviews (1): Last reviewed commit: "fix: stop teardown drains at the first n..." | Re-trigger Greptile |
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.
Dogfooding atuin's eye-declare search surfaced this: all three teardown drains (async driver shutdown, init-exit shutdown, and the
RawModeGuarddrop fallback) read and discard every event type for up to 50ms, so keystrokes typed immediately after an app exits can be silently eaten.The spray of stale mouse reports is contiguous, so the drain now stops at the first non-mouse event, leaving everything not yet read in the tty for the shell.
Correction from review: the cap is one crossterm buffer chunk, not one event. Crossterm reads the tty up to 1 KiB per syscall and queues every event it parses, so anything sharing a chunk with the first non-mouse event is consumed with it and cannot be handed back — ttys have no peek, and
TIOCSTIis disabled on modern kernels. This PR is still a strict improvement (it stops all subsequent chunk reads, and the drain only continues through a dense spray — a 5ms gap ends it, so only keys typed into the spray are at risk), but a true byte-granular drain requires eye owning the input path instead of reading through crossterm. That design — which also addresses the CPR startup-latency and reader-thread-lock issues, with a PTY regression harness — is tracked as ATU-580.