fix(server): stop replayed history from re-arming mouse reporting - #4821
fix(server): stop replayed history from re-arming mouse reporting#4821Sy-D wants to merge 3 commits into
Conversation
Mouse reporting is terminal state, not text. A TUI killed before it could restore the mode leaves the enabling sequence in the session history, and reopening the terminal replays it into a fresh xterm. Every pointer move then echoes into the idle shell as a literal ESC[<..M. Clear the tracking modes at the end of a replayed snapshot, but only when no child process is running, so a live TUI keeps the mouse support the replay legitimately restores for it. Closes pingdotgg#4776. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| status: session.status, | ||
| pid: session.pid, | ||
| history: session.history, | ||
| history: replaySafeTerminalHistory(session.history, session.hasRunningSubprocess), |
There was a problem hiding this comment.
🟡 Medium terminal/Manager.ts:353
replaySafeTerminalHistory gates the mouse-reporting reset on the cached session.hasRunningSubprocess, which is only refreshed asynchronously by pollSubprocessActivity. If a TUI starts after the last poll and a client attaches before the next poll, the value is still false, so snapshot appends the reset and disables mouse reporting even though the live TUI needs it — mouse input silently breaks for the running application. Conversely, after a TUI exits but before the next poll, hasRunningSubprocess remains true, so the enbling history passes through unchanged and re-arms mouse reporting on the fresh client even though no child remains to consume the reports, leaving the original garbage-input bug timing-dependent. Consider checking live subprocess state at snapshot time instead of relying on the stale cached flag.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/terminal/Manager.ts around line 353:
`replaySafeTerminalHistory` gates the mouse-reporting reset on the cached `session.hasRunningSubprocess`, which is only refreshed asynchronously by `pollSubprocessActivity`. If a TUI starts after the last poll and a client attaches before the next poll, the value is still `false`, so `snapshot` appends the reset and disables mouse reporting even though the live TUI needs it — mouse input silently breaks for the running application. Conversely, after a TUI exits but before the next poll, `hasRunningSubprocess` remains `true`, so the enbling history passes through unchanged and re-arms mouse reporting on the fresh client even though no child remains to consume the reports, leaving the original garbage-input bug timing-dependent. Consider checking live subprocess state at snapshot time instead of relying on the stale cached flag.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 54c94dc. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This bug fix for terminal mouse reporting has good test coverage, but an unresolved review comment questions whether the race condition around cached subprocess state is fully addressed. The author added You can customize Macroscope's approvability policy. Learn more. |
…tate Review feedback on the first commit: - A DECSET may set several modes at once (`ESC[?1002;1006h`) and may use the 8-bit introducer, so parse the sequence instead of matching single-parameter spellings. - Separate tracking modes from encodings: only a tracking mode makes the terminal report, but the reset now clears both, including 1001 and 1016. - Refresh the subprocess state when a client attaches. The cached flag is only updated once a second, so a TUI that had just started could lose its mouse support and one that had just exited could still leave reporting armed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All three findings were valid and are fixed in b4ce3f5. Combined and 8-bit DECSET — correct, the Missing reset codes — correct, and chasing it surfaced a second problem: detection and reset were sharing one list, which conflated two different things. They are now separate, because only a tracking mode makes the terminal emit reports while an encoding merely shapes them:
So a history that enables just Stale Four new test cases cover the combined DECSET, the 8-bit introducer, the encoding-only history, and the completeness of the reset list.
|
Review feedback: mode 9 is a tracking mode xterm.js implements, so a history enabling it was left unreset and could keep reporting armed on an idle shell. The reset list derives from the tracking list, so it now clears ?9l too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Correct, and fixed in 7d11637 — verified rather than taken on trust. Driving a real
So mode 9 does arm reporting, exactly as you said. A tracking mode on its own emitted nothing in this harness, but that is not the realistic case — a TUI sets a tracking mode and an encoding together, and then X10 behaves like the rest.
|
7d11637 to
add5fa2
Compare

What Changed
snapshot()now clears the mouse tracking modes at the end of a replayed terminal history — but only when the session has no running child process.Why
Reported in #4776: reopening the terminal occasionally fills it with sequences like
^[[<35;65;13M.Those are SGR mouse reports (mode 1006) — something a terminal sends to a program, never the reverse. Seeing them as text means reporting is armed while nothing is left to consume it, so the tty echoes each pointer move into the shell. That also explains why Ctrl-C does not help and why only killing the shell does: it is terminal state, not a stuck process.
Mouse reporting is state, not text, but the history replays it as text. A TUI killed before it could restore the mode leaves the enabling sequence in the retained history, and every reopen — including switching threads and coming back — replays it into a fresh xterm.
The gate matters: a live TUI (vim, htop, lazygit) legitimately depends on the replay restoring its mouse support after a thread switch. Clearing unconditionally would have broken that silently. With a child process running the history is passed through untouched.
Notes For Review
sanitizeTerminalHistoryChunkalready defends against this class of replay hazard — it strips DSR, CPR, DA and OSC colour queries so a replay cannot make the terminal answer into the shell. Mode setting is the same hazard in a different shape, but it cannot be stripped on the way in, because at record time the mode may still be wanted. The decision belongs at replay time, which is why this sits insnapshot().Only the retained history is touched; live output still reaches the client verbatim.
Validation
The trigger is intermittent and I could not make it fire on demand, so here is what is measured rather than assumed:
xterm.jsinstance through the same stepswriteTerminalBufferperforms — writeESC c, then a history containingESC[?1002h ESC[?1006h— a fresh terminal emits nothing on pointer events before the replay andESC[<0;14;3M,ESC[<32;25;4Mafter it. Same shape as the reported screenshot. With the reset appended, it emits nothing again.ESC[?2004h(bracketed paste), confirming DECSET sequences reach the replayed buffer.next dev(Next.js 16.2.9, Turbopack) captured under a real PTY emits no private mode sequences at all, so the stale enable comes from something else in the long-lived session — consistent with a hard-killed TUI.Also run:
vp test run apps/server/src/terminal/Manager.test.ts— 53 passed, including three new cases (clears when idle, passes through with a running child, untouched when no mouse mode was ever set)vp run --filter t3 test— 1673 passed, 7 skippedvp run --filter t3 typecheck— no errorsvp lint/vp fmt --checkon the changed files — cleanCloses #4776.
Checklist
Note
Medium Risk
Changes terminal snapshot replay semantics for all reconnects; incorrect subprocess detection could strip mouse modes from an active TUI, though attach now refreshes that flag first.
Overview
Fixes #4776, where reconnecting to a terminal could flood the shell with literal mouse report sequences (
^[[<…M) after a TUI exited without restoring terminal modes.Replay-time sanitization:
snapshot()now runs history throughreplaySafeTerminalHistorybefore clients receive it. If the retained transcript contains a DECSET that enabled mouse tracking (modes 9, 1000–1003), the server appends matchingDECRSTsequences for tracking and related encoding modes. Encoding-only enables (e.g.1006alone) are left unchanged. WhenhasRunningSubprocessis true, history is passed through so live TUIs still get mouse support after a thread switch.Attach freshness:
attachStreamcallspollSubprocessActivity()immediately before the initial snapshot so the idle-vs-child gate does not rely on a stale poller interval.Unit tests cover idle reset, active-child passthrough, combined/8-bit CSI, X10 mode, and no-op histories.
Reviewed by Cursor Bugbot for commit add5fa2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Stop replayed terminal history from re-arming mouse reporting on reconnect
replaySafeTerminalHistoryin Manager.ts that appends DECSET reset sequences for mouse tracking modes (9, 1000–1003) and encoding modes (1006, 1015, 1016) when replaying history that enabled mouse reporting.pollSubprocessActivityimmediately before emitting the initial snapshot so the replay decision reflects current state.Macroscope summarized add5fa2.