From 1a6798022052a91b90104335a6b1d39b120ca39e Mon Sep 17 00:00:00 2001 From: Yogthos Date: Wed, 2 Sep 2026 13:35:30 -0400 Subject: [PATCH] Stop terminal reports from typing themselves into the input box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user watched text pour into the compose field on its own — "nonstop input, as if I had been holding something down". crossterm 0.29 has no parser for OSC / DCS / APC sequences. `parse_event` handles `ESC O`, `ESC [` and `ESC ESC`; every other byte after ESC falls through to a recursive call that reports the introducer as an Alt-modified character, and `Parser::advance` then clears its buffer — so the payload is parsed byte-by-byte as ordinary text. A background-colour report, `ESC ] 11 ; rgb:2e2e/3434/3636 BEL`, therefore arrived as Alt+`]` plus twenty-one characters, every one of which the compose editor inserted. Terminals emit these unprompted (colour reports on a theme change or an alt-screen transition, an OSC 52 clipboard reply, DCS XTGETTCAP replies, kitty's OSC 99 notification reports), and in a multiplexer one pane can receive the reply to another pane's query. dirge already drained that chatter at startup, at teardown and around a suspended subprocess; nothing stood between it and the editor mid-session. Four changes, all in the input path: - The input reader recognises an OSC / DCS / APC / SOS / PM introducer and swallows the run through its terminator, including an ST split across two parses. A run it cannot close — no terminator, a human-scale gap, or more than 4096 events — is released as ordinary keystrokes rather than dropped, so a deliberate Alt+`]` is delayed by 25ms and never lost, and an AltGr-composed `]` (Ctrl+Alt on the Windows layouts) is still typing. (dirge-v4xf) - Readers carry a generation. The suspend path proceeds after 150ms even when the reader has not exited and the resume path then clears the shutdown flag, so a stale reader woke to a `false` flag and kept reading fd 0 next to its replacement — and next to the stdin drains, which splits escape sequences and lands the same junk in the compose box by a different route. Claiming a generation retires every older reader at its next tick, and only the live generation may report the reader gone. (dirge-xxo9) - Poll/read errors are logged and retried a few times instead of ending the thread for the session, which left a healthy-looking UI that accepted no keys. The dead-tty probe still owns the unrecoverable case. (dirge-sp1x) - `?1003h` (any-event mouse tracking) is no longer enabled. Nothing consumed it — the reader maps the wheel and left button down/drag/up and drops the rest, `MouseEventKind::Moved` appears nowhere in the tree, and `?1002h` already covers the wheel and the drag-selection. All it bought was the only continuous input byte stream in the program, which is what turns a one-off desync on fd 0 into a sustained flood rather than a single burst. `?1015h` (urxvt encoding, which crossterm cannot parse) goes with it; the teardown strings still clear both. (dirge-hn6e) Tests: unit tests for the filter (report swallowed whole, both terminators, split ST, cap released not dropped, AltGr `]` untouched, ordinary typing untouched) and PTY-level tests that push the real bytes through fd 0 and the production reader — which is what pins the crossterm behaviour the filter assumes — plus one that a retired reader stops consuming input. --- CHANGELOG.md | 55 +++ src/ui/input_reader.rs | 451 ++++++++++++++++++++++++- src/ui/relay_tests/mod.rs | 4 + src/ui/relay_tests/terminal_reports.rs | 241 +++++++++++++ src/ui/renderer.rs | 9 +- src/ui/renderer_tests.rs | 19 +- src/ui/terminal.rs | 44 ++- 7 files changed, 800 insertions(+), 23 deletions(-) create mode 100644 src/ui/relay_tests/terminal_reports.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 143362128..4363f2286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,47 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] ### Fixed +- Terminal reports no longer type themselves into the input box. A user + watched text pour into the compose field on its own — "nonstop input, as if + I had been holding something down". A terminal emits reports unprompted: + OSC 10/11/12 colour reports on a theme change or an alt-screen transition + (kitty, ghostty, foot, iTerm2), an OSC 52 clipboard reply, DCS XTGETTCAP / + DECRQSS replies, and in a multiplexer a reply to a query some other pane's + program made. crossterm has no parser for any of them: it reports the + introducer as an Alt-modified character, clears its buffer, and then parses + the whole payload as ordinary text — so `ESC ] 11 ; rgb:… BEL` arrived as + Alt+`]` followed by twenty-one characters, every one of which the editor + inserted. dirge already drained that chatter at startup, at teardown and + around a suspended subprocess, but nothing stood between it and the editor + mid-session. The input reader now recognises an OSC / DCS / APC / SOS / PM + introducer and swallows the run through its terminator (BEL or ST, including + an ST split across two parses). It is conservative in the other direction + too: a run it cannot close — no terminator, a human-scale gap, or more than + 4096 events — is released as ordinary keystrokes rather than dropped, so a + deliberate Alt+`]` is delayed by 25ms and never lost. An AltGr-composed `]` + (Ctrl+Alt on Windows layouts) is still typing. (dirge-v4xf) +- A subprocess could leave two input readers running. `suspend_tui_for_ + subprocess` gives the reader 150ms to exit and then proceeds anyway (it says + so on stderr); `resume_tui_after_subprocess` clears the shutdown flag and + starts a replacement. The loop never latched the flag, so a reader that had + not exited woke to a `false` flag and kept reading fd 0 next to its + replacement — and next to the stdin drains, and `EVENT_READER_EXITED` was + then set by whichever of them exited first, so the next suspend's barrier + could pass while a reader was still consuming keystrokes. Two consumers on + one descriptor split escape sequences, and the tail of a split sequence + parses as plain text: the same junk-in-the-compose-box symptom as above, by + a different route. Readers now carry a generation; claiming a new one + retires every older reader at its next tick, and only the live generation + may report that the reader is gone. (dirge-xxo9) +- One error from the terminal no longer kills input for the session. Both + `event::poll` and `event::read` errors ended the reader thread, and nothing + outside the suspend/resume path restarts it — so a single transient failure + left dirge painting a healthy UI that accepted no keys, indistinguishable + from a hang, with the dead-tty watchdog quiet because the terminal was + alive. Errors are now logged and retried a few times before the thread + gives up; the dead-tty probe still owns the case that cannot recover. + (dirge-sp1x) + - Compaction can now fold an autonomous stretch. Both cuts of the compress window snapped to a *user* message, which guarantees no tool_use↔tool_result pair is split — but one prompt followed by a hundred tool iterations, the @@ -168,6 +209,20 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). with a substitution, redirect, pipe, subshell or background), so ordinary shell work is untouched. (#808) +### Changed +- dirge no longer enables `?1003h` (any-event mouse tracking). It made the + terminal report every cell of pointer motion with no button held, and + nothing consumed it: the reader maps the wheel and left button + down/drag/up and drops the rest, and `MouseEventKind::Moved` appears + nowhere in the tree. `?1002h` (button-event tracking) already covers the + wheel and the drag-selection. All the mode bought was the only continuous + input byte stream in the program — a few KB/s parsed and thrown away + whenever the pointer crossed the window, and the fuel that turns a one-off + desync on fd 0 into a sustained flood rather than a single burst. `?1015h` + (the urxvt encoding, which crossterm cannot parse) is dropped with it. The + teardown and panic-reset strings still clear both, since another program — + or an older dirge — may have set them. (dirge-hn6e) + ## [0.25.3] - 2026-08-31 ### Fixed diff --git a/src/ui/input_reader.rs b/src/ui/input_reader.rs index e6c7549b0..e3c11bd7b 100644 --- a/src/ui/input_reader.rs +++ b/src/ui/input_reader.rs @@ -3,18 +3,220 @@ //! key/mouse/paste/resize events and forwards them to the UI loop as //! [`UserEvent`]s over an mpsc channel. Kept off the async runtime because //! `event::read()` is blocking; cooperative shutdown via the terminal -//! module's `EVENT_READER_SHUTDOWN` / `EVENT_READER_EXITED` flags. +//! module's `EVENT_READER_SHUTDOWN` / `EVENT_READER_EXITED` flags and the +//! `READER_GENERATION` counter. +//! +//! The thread is also where unsolicited terminal reports are stopped +//! ([`ReportFilter`]): crossterm cannot parse them, so left alone they +//! arrive as ordinary text and get typed into the compose box. + +use std::time::{Duration, Instant}; use crossterm::event; -use crossterm::event::{MouseButton, MouseEventKind}; +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEventKind}; +use smallvec::SmallVec; use crate::event::UserEvent; +/// How long a gap between two key events ends a suspected terminal report +/// (dirge-v4xf). A real report arrives inside a single `read()` of the tty, +/// so its events land microseconds apart; the fastest human typing leaves +/// ~100ms between keys. 25ms sits clear of both, with slack for a report +/// split across two reads on a laggy link. +const REPORT_BURST_GAP: Duration = Duration::from_millis(25); + +/// How many events one suspected report may swallow before the filter gives +/// up and releases them as ordinary keys. An OSC 52 clipboard reply is the +/// longest report seen in practice; this covers it while keeping a +/// mis-detected introducer from eating an unbounded amount of typing. +const REPORT_MAX_EVENTS: usize = 4096; + +/// How many consecutive `event::poll` / `event::read` errors the reader +/// tolerates before giving up (dirge-sp1x). A single transient error used to +/// end the thread for the rest of the session, which reads to the user as a +/// dead keyboard on a UI that still paints. +const MAX_CONSECUTIVE_ERRORS: usize = 8; + +/// Pause between retries after a poll/read error, so a persistently failing +/// source is abandoned in tens of milliseconds rather than spun on. +const ERROR_RETRY_PAUSE: Duration = Duration::from_millis(5); + +/// Does `key` open an OSC / DCS / APC / SOS / PM sequence? +/// +/// crossterm 0.29 has no parser for any of them. `parse_event` handles only +/// `ESC O`, `ESC [` and `ESC ESC`; every other byte after ESC falls through +/// to a recursive call that reports the introducer as an Alt-modified +/// character, and `Parser::advance` then clears its buffer — so the whole +/// payload is parsed byte-by-byte as ordinary text +/// (`src/event/sys/unix/parse.rs`, `src/event/source/unix/tty.rs`). +/// +/// SHIFT is ignored because crossterm sets it for the uppercase introducers +/// (`P`, `X`). CONTROL must NOT be set: Windows reports AltGr as +/// Ctrl+Alt, so an AltGr-composed `]` on the Italian/German/Spanish layouts +/// arrives with both modifiers and is real typing, not an introducer +/// (`ui::input::normalize_altgr`, GH #659). +fn is_report_introducer(key: &KeyEvent) -> bool { + key.modifiers.contains(KeyModifiers::ALT) + && !key.modifiers.contains(KeyModifiers::CONTROL) + && matches!(key.code, KeyCode::Char(']' | 'P' | '_' | 'X' | '^')) +} + +/// BEL (`\x07`), which crossterm reports as Ctrl+G — one of the two +/// terminators a terminal may use for an OSC report. +fn is_report_bel(key: &KeyEvent) -> bool { + key.code == KeyCode::Char('g') && key.modifiers.contains(KeyModifiers::CONTROL) +} + +/// ST (`ESC \`), the other terminator, which crossterm reports as Alt+`\` +/// when both bytes land in the same parse. +fn is_report_st(key: &KeyEvent) -> bool { + key.code == KeyCode::Char('\\') && key.modifiers.contains(KeyModifiers::ALT) +} + +/// Swallows unsolicited terminal reports before they reach the compose +/// editor as text (dirge-v4xf). +/// +/// A terminal emits these on its own: OSC 10/11/12 colour reports on a theme +/// change or an alt-screen transition (kitty, ghostty, foot, iTerm2), an +/// OSC 52 clipboard reply, DCS XTGETTCAP / DECRQSS replies, kitty's OSC 99 +/// notification reports — and in a multiplexer, a reply to a query some other +/// pane's program made. dirge drains that chatter at startup, at teardown and +/// around a suspended subprocess (`ui::terminal::sync_and_drain_via_sentinel`), +/// but nothing stood between it and the editor mid-session, so a report typed +/// its payload into the input box one character at a time. +/// +/// The filter is deliberately conservative: a suspected report it cannot +/// close — no terminator, a human-scale gap, more events than +/// [`REPORT_MAX_EVENTS`] — is released as ordinary keys rather than dropped, +/// so the worst case for a real Alt+`]` keystroke is a delay of +/// [`REPORT_BURST_GAP`]. +#[derive(Default)] +pub(crate) struct ReportFilter { + /// The introducer plus every payload event captured so far. Empty when + /// not inside a suspected report. + pending: Vec, + /// When the last event joined `pending` (or closed a report). + last: Option, + /// A bare `Esc` closed a report: the `\` of an ST split across two + /// parses may still be in flight and must not leak into the editor. + expect_st_backslash: bool, +} + +/// What one call to [`ReportFilter::feed`] hands back: nothing while a report +/// is being swallowed, one key for ordinary typing, and occasionally a run +/// being released. Inline capacity keeps the common path allocation-free — +/// this is the program's hottest input path. +type Forward = SmallVec<[KeyEvent; 4]>; + +impl ReportFilter { + /// Feed one key event; returns the events to forward, in order. + pub(crate) fn feed(&mut self, key: KeyEvent, now: Instant) -> Forward { + // The `\` right after the `Esc` that closed a report is the tail of a + // split ST, not typing. + if self.expect_st_backslash { + self.expect_st_backslash = false; + if self.within_burst(now) && key.code == KeyCode::Char('\\') { + self.last = Some(now); + return Forward::new(); + } + } + + if self.pending.is_empty() { + if is_report_introducer(&key) { + self.pending.push(key); + self.last = Some(now); + return Forward::new(); + } + let mut out = Forward::new(); + out.push(key); + return out; + } + + // Inside a suspected report. + if !self.within_burst(now) { + // Too slow to be one: release what we held, then treat this key as + // fresh (it may open a report of its own). `release` empties + // `pending`, so the recursion is one level deep. + let mut out = self.release(); + out.extend(self.feed(key, now)); + return out; + } + if is_report_bel(&key) || is_report_st(&key) { + tracing::debug!( + events = self.pending.len() + 1, + "swallowed an unsolicited terminal report" + ); + self.pending.clear(); + self.last = Some(now); + return Forward::new(); + } + if key.code == KeyCode::Esc { + // Possibly the first half of an ST that straddles two parses. + tracing::debug!( + events = self.pending.len() + 1, + "swallowed an unsolicited terminal report (bare ESC terminator)" + ); + self.pending.clear(); + self.last = Some(now); + self.expect_st_backslash = true; + return Forward::new(); + } + if self.pending.len() >= REPORT_MAX_EVENTS { + let mut out = self.release(); + out.push(key); + return out; + } + self.pending.push(key); + self.last = Some(now); + Forward::new() + } + + /// Release a suspected report that has gone quiet. Called from the + /// reader's idle tick, so a real Alt+`]` reaches the editor after + /// [`REPORT_BURST_GAP`] instead of waiting for the next keystroke. + pub(crate) fn flush_stale(&mut self, now: Instant) -> Forward { + if self.within_burst(now) { + return Forward::new(); + } + self.expect_st_backslash = false; + self.release() + } + + fn within_burst(&self, now: Instant) -> bool { + self.last + .is_some_and(|t| now.saturating_duration_since(t) < REPORT_BURST_GAP) + } + + fn release(&mut self) -> Forward { + self.last = None; + if self.pending.is_empty() { + return Forward::new(); + } + tracing::debug!( + events = self.pending.len(), + "releasing a suspected terminal report as keystrokes" + ); + Forward::from_vec(std::mem::take(&mut self.pending)) + } +} + /// Spawn the blocking crossterm reader thread. `user_tx` is consumed (pass /// a clone — the caller keeps its own sender for other event sources). The /// `JoinHandle` is stored in `READER_HANDLE` so the sandbox attach path /// can fully join the thread before draining stdin. +/// +/// Bumping `READER_GENERATION` here is what retires a previous reader +/// (dirge-xxo9): the shutdown flag alone could not, because it is cleared +/// again on resume and the loop never latched it, so a reader the suspend +/// path failed to join woke up to a `false` flag and kept reading fd 0 +/// alongside its replacement — and alongside the stdin drains, which is how +/// a split escape sequence ends up typed into the compose box. pub(crate) fn spawn_input_reader(user_tx: tokio::sync::mpsc::UnboundedSender) { + // Claim a generation before the thread starts, so any older reader sees + // the new value on its very next tick. + let generation = crate::ui::terminal::READER_GENERATION + .fetch_add(1, std::sync::atomic::Ordering::SeqCst) + + 1; let handle = std::thread::spawn(move || { // ── CFS priority boost for the input reader ────────────── // nice -20 gives ~5900x scheduling weight over KVM (nice 19) @@ -110,7 +312,9 @@ pub(crate) fn spawn_input_reader(user_tx: tokio::sync::mpsc::UnboundedSender {} + Ok(true) => consecutive_errors = 0, Ok(false) => { - std::thread::sleep(std::time::Duration::from_millis(1)); + consecutive_errors = 0; + // Idle: a suspected report that has gone quiet is released + // here, so a real Alt+`]` reaches the editor after + // REPORT_BURST_GAP rather than on the next keystroke. + for key in filter.flush_stale(Instant::now()) { + if user_tx.send(UserEvent::Key(key)).is_err() { + break 'reader; + } + } + std::thread::sleep(Duration::from_millis(1)); + continue; + } + Err(e) => { + // dirge-sp1x: a transient error must not end input for the + // session. The dead-tty probe above owns the case that + // genuinely cannot recover. + consecutive_errors += 1; + tracing::warn!( + error = %e, + consecutive = consecutive_errors, + "input reader: event::poll failed" + ); + if consecutive_errors >= MAX_CONSECUTIVE_ERRORS { + tracing::error!("input reader: giving up after repeated poll failures"); + break; + } + std::thread::sleep(ERROR_RETRY_PAUSE); continue; } - Err(_) => break, } // Re-check the shutdown flag between poll and read. // poll() returning true means there are bytes on fd 0; // if shutdown was signalled during poll, we must not // consume those bytes — they belong to the drain pass. if crate::ui::terminal::EVENT_READER_SHUTDOWN.load(std::sync::atomic::Ordering::Relaxed) + || crate::ui::terminal::READER_GENERATION.load(std::sync::atomic::Ordering::Relaxed) + != generation { break; } @@ -161,12 +394,19 @@ pub(crate) fn spawn_input_reader(user_tx: tokio::sync::mpsc::UnboundedSender { @@ -231,9 +471,24 @@ pub(crate) fn spawn_input_reader(user_tx: tokio::sync::mpsc::UnboundedSender break, + Err(e) => { + // dirge-sp1x: same retry policy as the poll error above. + consecutive_errors += 1; + tracing::warn!( + error = %e, + consecutive = consecutive_errors, + "input reader: event::read failed" + ); + if consecutive_errors >= MAX_CONSECUTIVE_ERRORS { + tracing::error!("input reader: giving up after repeated read failures"); + break; + } + std::thread::sleep(ERROR_RETRY_PAUSE); + continue; + } _ => {} } + consecutive_errors = 0; } // Tell `TerminalGuard::drop` we've actually exited so it can // proceed past the wait barrier without sleeping on a @@ -242,12 +497,24 @@ pub(crate) fn spawn_input_reader(user_tx: tokio::sync::mpsc::UnboundedSender KeyEvent { + KeyEvent::new(code, modifiers) + } + + fn ch(c: char) -> KeyEvent { + key(KeyCode::Char(c), KeyModifiers::NONE) + } + + fn alt(c: char) -> KeyEvent { + key(KeyCode::Char(c), KeyModifiers::ALT) + } + + fn ctrl(c: char) -> KeyEvent { + key(KeyCode::Char(c), KeyModifiers::CONTROL) + } + + /// Feed a whole burst at one instant, returning everything forwarded. + fn burst(f: &mut ReportFilter, at: Instant, keys: &[KeyEvent]) -> Vec { + let mut out: Vec = Vec::new(); + for k in keys { + out.extend(f.feed(*k, at)); + } + out + } + + /// `\x1b]11;rgb:2e2e/3434/3636\x07` — a background-colour report, the + /// chatter kitty / ghostty / foot / iTerm2 emit unprompted. + fn osc_11_report() -> Vec { + let mut keys = vec![alt(']')]; + keys.extend("11;rgb:2e2e/3434/3636".chars().map(ch)); + keys.push(ctrl('g')); // BEL + keys + } + + #[test] + fn osc_report_is_swallowed_whole() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + assert!( + burst(&mut f, t0, &osc_11_report()).is_empty(), + "an OSC report must not reach the editor" + ); + // Typing right behind it is unaffected. + assert_eq!( + f.feed(ch('x'), t0 + Duration::from_millis(1)).as_slice(), + [ch('x')] + ); + } + + #[test] + fn dcs_report_closed_by_st_is_swallowed() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + // `\x1bP1+r5463=787465726d\x1b\` — an XTGETTCAP reply. + let mut keys = vec![key( + KeyCode::Char('P'), + KeyModifiers::ALT | KeyModifiers::SHIFT, + )]; + keys.extend("1+r5463=787465726d".chars().map(ch)); + keys.push(alt('\\')); // ST + assert!(burst(&mut f, t0, &keys).is_empty()); + } + + #[test] + fn st_split_across_two_parses_leaks_no_backslash() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + // The ESC of the ST landed at the end of a read, so crossterm + // reported it as a bare Esc and the `\` came in the next parse. + let mut keys = vec![alt(']')]; + keys.extend("52;c;SGVsbG8=".chars().map(ch)); + keys.push(key(KeyCode::Esc, KeyModifiers::NONE)); + keys.push(ch('\\')); + assert!(burst(&mut f, t0, &keys).is_empty()); + assert_eq!( + f.feed(ch('q'), t0 + Duration::from_millis(1)).as_slice(), + [ch('q')] + ); + } + + #[test] + fn ordinary_typing_passes_through_untouched() { + let mut f = ReportFilter::default(); + let mut t = Instant::now(); + for c in "hello".chars() { + assert_eq!(f.feed(ch(c), t).as_slice(), [ch(c)]); + t += Duration::from_millis(80); + } + } + + #[test] + fn altgr_composed_bracket_is_not_an_introducer() { + // Windows reports AltGr as Ctrl+Alt, and `]` needs AltGr on the + // Italian / German / Spanish layouts (GH #659). It is typing. + let mut f = ReportFilter::default(); + let k = key( + KeyCode::Char(']'), + KeyModifiers::CONTROL | KeyModifiers::ALT, + ); + assert_eq!(f.feed(k, Instant::now()).as_slice(), [k]); + } + + #[test] + fn a_human_scale_gap_releases_the_introducer_and_the_next_key() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + assert!(f.feed(alt(']'), t0).is_empty()); + assert_eq!( + f.feed(ch('a'), t0 + Duration::from_millis(100)).as_slice(), + [alt(']'), ch('a')], + "a deliberate Alt+] is delayed, never dropped" + ); + } + + #[test] + fn idle_flush_releases_a_lone_introducer() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + assert!(f.feed(alt(']'), t0).is_empty()); + assert!( + f.flush_stale(t0 + Duration::from_millis(1)).is_empty(), + "still inside the burst window" + ); + assert_eq!( + f.flush_stale(t0 + REPORT_BURST_GAP).as_slice(), + [alt(']')], + "the reader's idle tick releases it" + ); + assert!(f.flush_stale(t0 + Duration::from_secs(1)).is_empty()); + } + + #[test] + fn a_report_past_the_cap_is_released_not_dropped() { + let mut f = ReportFilter::default(); + let t0 = Instant::now(); + assert!(f.feed(alt(']'), t0).is_empty()); + let mut released = Forward::new(); + for i in 0..=REPORT_MAX_EVENTS { + released = f.feed(ch('z'), t0 + Duration::from_millis(1)); + if !released.is_empty() { + // The introducer + everything held + the event that hit the + // cap, in order. + assert_eq!(released.len(), i + 2, "released at event {i}"); + break; + } + } + assert!( + !released.is_empty(), + "the cap must release the run, not swallow forever" + ); + assert_eq!(released[0], alt(']')); + } } diff --git a/src/ui/relay_tests/mod.rs b/src/ui/relay_tests/mod.rs index a42e01717..bbb1469a1 100644 --- a/src/ui/relay_tests/mod.rs +++ b/src/ui/relay_tests/mod.rs @@ -60,3 +60,7 @@ mod poll_latency; #[cfg(test)] #[cfg(all(unix, feature = "sandbox-microvm"))] mod tty_hangup; + +#[cfg(test)] +#[cfg(all(unix, feature = "sandbox-microvm"))] +mod terminal_reports; diff --git a/src/ui/relay_tests/terminal_reports.rs b/src/ui/relay_tests/terminal_reports.rs new file mode 100644 index 000000000..d061c2632 --- /dev/null +++ b/src/ui/relay_tests/terminal_reports.rs @@ -0,0 +1,241 @@ +//! Unsolicited terminal reports must not reach the UI as text (dirge-v4xf). +//! +//! These drive real bytes through a PTY on fd 0 and the production input +//! reader, so they check what `ui::input_reader::ReportFilter`'s unit tests +//! have to assume: that crossterm 0.29 reports an OSC / DCS introducer as an +//! Alt-modified character and then parses the payload as ordinary text. +//! Before the filter, phase 1 below collected 22 events — `Alt+]` plus every +//! character of `11;rgb:2e2e/3434/3636`, all of which the compose editor +//! would have inserted. +//! +//! One test with phases, like `crossterm_suite`: crossterm's event source is +//! a process-wide singleton bound to whatever fd 0 was when it was first +//! created, so only the first fd-0 swap in a process takes effect. Split into +//! separate `#[test]`s, everything after the first sees no input at all. +//! +//! Failures are collected rather than asserted in place so fd 0 is always +//! restored — a panic mid-test would strand the process's stdin on a closed +//! PTY and take every later test with it. +//! +//! All tests in this module require `sandbox-microvm`. + +#[cfg(test)] +#[cfg(all(unix, feature = "sandbox-microvm"))] +mod tests { + use super::super::common::*; + use crate::event::UserEvent; + use crossterm::event::{KeyCode, KeyModifiers}; + use std::io::Write; + use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd}; + use std::sync::atomic::Ordering; + use std::time::{Duration, Instant}; + + type Rx = tokio::sync::mpsc::UnboundedReceiver; + + /// Collect events for `window`, stopping early once `want` have arrived — + /// then keep draining briefly, because a leak arrives right behind the + /// events we expected. + fn collect(rx: &mut Rx, window: Duration, want: usize) -> Vec { + let deadline = Instant::now() + window; + let mut out = Vec::new(); + while Instant::now() < deadline { + match rx.try_recv() { + Ok(ev) => { + out.push(ev); + if out.len() >= want { + std::thread::sleep(Duration::from_millis(20)); + while let Ok(ev) = rx.try_recv() { + out.push(ev); + } + return out; + } + } + Err(tokio::sync::mpsc::error::TryRecvError::Empty) => { + std::thread::sleep(Duration::from_millis(2)); + } + Err(tokio::sync::mpsc::error::TryRecvError::Disconnected) => break, + } + } + out + } + + fn describe(events: &[UserEvent]) -> String { + events + .iter() + .map(|e| match e { + UserEvent::Key(k) => format!("{:?}+{:?}", k.code, k.modifiers), + other => format!("{other:?}"), + }) + .collect::>() + .join(" ") + } + + /// One report followed by one keystroke: only the keystroke may arrive. + /// The trailing keystroke is what proves the filter *closed* the report + /// rather than swallowing everything after it, and the gap before it + /// exceeds the burst window so an unclosed run would be released as text + /// (a visible failure) instead of vanishing. + fn check_report( + label: &str, + report: &[u8], + follow: u8, + pty: &mut std::fs::File, + rx: &mut Rx, + failures: &mut Vec, + ) { + pty.write_all(report).expect("write report"); + pty.flush().ok(); + std::thread::sleep(Duration::from_millis(30)); + pty.write_all(&[follow]).expect("write keystroke"); + pty.flush().ok(); + + let events = collect(rx, Duration::from_millis(500), 1); + let ok = match events.as_slice() { + [UserEvent::Key(k)] => { + k.code == KeyCode::Char(follow as char) && !k.modifiers.contains(KeyModifiers::ALT) + } + _ => false, + }; + if !ok { + failures.push(format!( + "{label}: expected only the trailing keystroke {:?}, got [{}]", + follow as char, + describe(&events) + )); + } + } + + #[test] + fn terminal_report_filter_suite() { + let _guard = serial_fd_test(); + + // ── save fd 0 ── + let saved_stdin = unsafe { OwnedFd::from_raw_fd(libc::dup(0)) }; + if saved_stdin.as_raw_fd() < 0 { + eprintln!("skipping: dup(0) failed"); + return; + } + let saved_termios: Option = unsafe { + let mut t: libc::termios = std::mem::zeroed(); + if libc::tcgetattr(0, &mut t) >= 0 { + Some(t) + } else { + None + } + }; + crate::ui::terminal::EVENT_READER_SHUTDOWN.store(false, Ordering::Relaxed); + crate::ui::terminal::EVENT_READER_EXITED.store(false, Ordering::Relaxed); + + // ── PTY as fd 0, one pair for every phase ── + let Some((mut pty, secondary)) = open_pty_pair() else { + eprintln!("skipping: no PTY available"); + return; + }; + assert!( + unsafe { libc::dup2(secondary.as_raw_fd(), 0) } >= 0, + "dup2 to fd 0 failed" + ); + make_raw_fd(0).expect("make_raw on fd 0"); + + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); + crate::ui::input_reader::spawn_input_reader(tx); + // Let the reader reach its poll loop before the first write. + std::thread::sleep(Duration::from_millis(20)); + + let mut failures: Vec = Vec::new(); + + // ── Phase 1: OSC 11 background-colour report, BEL-terminated ── + // What kitty / ghostty / foot / iTerm2 emit on a theme change or an + // alt-screen transition, and what a multiplexer may hand to the + // wrong pane. + check_report( + "osc-11-bel", + b"\x1b]11;rgb:2e2e/3434/3636\x07", + b'x', + &mut pty, + &mut rx, + &mut failures, + ); + + // ── Phase 2: the same shape terminated by ST, long enough to stand + // in for an OSC 52 clipboard reply ── + check_report( + "osc-52-st", + b"\x1b]52;c;SGVsbG8sIGNsaXBib2FyZCwgdGhpcyBpcyBhIGxvbmcgcmVwbHk=\x1b\\", + b'y', + &mut pty, + &mut rx, + &mut failures, + ); + + // ── Phase 3: a DCS reply (XTGETTCAP) ── + check_report( + "dcs-xtgettcap", + b"\x1bP1+r5463=787465726d\x1b\\", + b'z', + &mut pty, + &mut rx, + &mut failures, + ); + + // ── Phase 4: a deliberate Alt+] is delayed by the burst window, + // never dropped ── + pty.write_all(b"\x1b]").expect("write alt-]"); + pty.flush().ok(); + let events = collect(&mut rx, Duration::from_millis(500), 1); + let released = match events.as_slice() { + [UserEvent::Key(k)] => { + k.code == KeyCode::Char(']') && k.modifiers.contains(KeyModifiers::ALT) + } + _ => false, + }; + if !released { + failures.push(format!( + "alt-]: expected the introducer to be released, got [{}]", + describe(&events) + )); + } + + // ── Phase 5: a retired reader stops consuming input (dirge-xxo9) ── + // Spawning a replacement — what `resume_tui_after_subprocess` does — + // used to leave the previous thread running: it re-read a shutdown + // flag the resume path had just cleared and kept pulling bytes off + // fd 0 next to its replacement. The first reader must go quiet and + // the second must see every keystroke. Last phase: it retires the + // reader the phases above share. + let (second_tx, mut second_rx) = tokio::sync::mpsc::unbounded_channel::(); + crate::ui::input_reader::spawn_input_reader(second_tx); + std::thread::sleep(Duration::from_millis(20)); + pty.write_all(b"abc").expect("write keystrokes"); + pty.flush().ok(); + let second = collect(&mut second_rx, Duration::from_millis(500), 3); + let first = collect(&mut rx, Duration::from_millis(50), 1); + if second.len() != 3 { + failures.push(format!( + "retired-reader: the live reader must see every keystroke, got [{}]", + describe(&second) + )); + } + if !first.is_empty() { + failures.push(format!( + "retired-reader: a retired reader kept consuming input: [{}]", + describe(&first) + )); + } + + // ── restore fd 0 before asserting ── + crate::ui::terminal::EVENT_READER_SHUTDOWN.store(true, Ordering::Relaxed); + crate::ui::terminal::join_reader(Duration::from_millis(100)); + unsafe { + libc::dup2(saved_stdin.as_raw_fd(), 0); + if let Some(ref t) = saved_termios { + libc::tcsetattr(0, libc::TCSANOW, t); + } + } + crate::ui::terminal::EVENT_READER_SHUTDOWN.store(false, Ordering::Relaxed); + crate::ui::terminal::EVENT_READER_EXITED.store(false, Ordering::Relaxed); + drop(secondary); + + assert!(failures.is_empty(), "\n{}", failures.join("\n")); + } +} diff --git a/src/ui/renderer.rs b/src/ui/renderer.rs index 663d5a92f..1cb562661 100644 --- a/src/ui/renderer.rs +++ b/src/ui/renderer.rs @@ -197,7 +197,7 @@ pub(crate) const PANEL_AUTO_MIN_COLS: u16 = 153; const SHELL_BOX_MAX_ROWS: u16 = 12; /// Global terminal modes dirge owns and must keep asserted for its whole -/// session: SGR mouse capture (`?1000`/`?1002`/`?1003`/`?1006`) so wheel + +/// session: SGR mouse capture (`?1000`/`?1002`/`?1006`) so wheel + /// click reach the app, bracketed paste (`?2004`), and focus reporting /// (`?1004`). These are set once at startup /// ([`crate::ui::terminal::TerminalGuard::new`]); this is the exact same @@ -220,8 +220,7 @@ const SHELL_BOX_MAX_ROWS: u16 = 12; /// and shell-session children are detached via `setsid()` with no /// controlling terminal, so a `/dev/tty` open fails with ENXIO — see /// `bash::exec::detach_session` and the `dirge-tc2q` test.) -const TERMINAL_MODE_REASSERT: &[u8] = - b"\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1006h\x1b[?2004h\x1b[?1004h"; +const TERMINAL_MODE_REASSERT: &[u8] = b"\x1b[?1000h\x1b[?1002h\x1b[?1006h\x1b[?2004h\x1b[?1004h"; /// Full terminal re-assert (dirge-ph60, dirge-173j). Unlike /// [`TERMINAL_MODE_REASSERT`] this DOES re-enter the alternate screen @@ -238,7 +237,7 @@ const TERMINAL_MODE_REASSERT: &[u8] = /// so the alt-screen re-entry only happens at discrete moments (focus / /// keypress), never per-second. Mirrors the mode set in /// [`crate::ui::terminal::resume_tui_after_subprocess`]. -const TERMINAL_FULL_REASSERT: &[u8] = b"\x1b[?2026h\x1b[?1049h\x1b[2J\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1006h\x1b[?2004h\x1b[?1004h\x1b[?2026l"; +const TERMINAL_FULL_REASSERT: &[u8] = b"\x1b[?2026h\x1b[?1049h\x1b[2J\x1b[?1000h\x1b[?1002h\x1b[?1006h\x1b[?2004h\x1b[?1004h\x1b[?2026l"; /// How often [`tui_redraw`](Renderer::tui_redraw) re-asserts the terminal /// modes. Long enough that the extra `/dev/tty` write is negligible, short @@ -2999,7 +2998,7 @@ impl Renderer { /// the mouse stays dead. pub fn reassert_terminal_modes(&mut self) { // Don't write to /dev/tty while the user is mid-drag selecting - // text: re-sending mouse-tracking enable sequences (?1003h et al.) + // text: re-sending mouse-tracking enable sequences (?1002h et al.) // resets internal tracking state on some terminals, dropping the // drag so MouseUp never fires and copy_to_clipboard is never called. if self.selection_active { diff --git a/src/ui/renderer_tests.rs b/src/ui/renderer_tests.rs index d8dc663eb..65af44141 100644 --- a/src/ui/renderer_tests.rs +++ b/src/ui/renderer_tests.rs @@ -1155,6 +1155,18 @@ fn mode_reassert_payload_re_enables_mouse_paste_and_focus() { "must re-enable SGR mouse encoding" ); assert!(s.contains("\x1b[?2004h"), "must re-enable bracketed paste"); + // dirge-hn6e: NOT any-event tracking. `?1003h` reports every cell of + // pointer motion with no button held and nothing consumes it; `?1002h` + // already covers the wheel and the drag-selection. Re-arming it would + // put the program's only continuous input byte stream back. + assert!( + !s.contains("\x1b[?1003h"), + "must not re-enable any-event mouse tracking" + ); + assert!( + s.contains("\x1b[?1002h"), + "must re-enable button-event mouse tracking" + ); // The genuine fix (dirge-tc2q follow-up): focus reporting MUST be in the // periodic payload. The primary recovery is FocusGained → // force_terminal_reassert, but that event can't fire while focus @@ -1263,6 +1275,11 @@ fn full_reassert_re_enters_alt_screen_synchronized() { "must re-enable SGR mouse encoding" ); assert!(s.contains("\x1b[?2004h"), "must re-enable bracketed paste"); + // dirge-hn6e: same exclusion as the periodic payload. + assert!( + !s.contains("\x1b[?1003h"), + "must not re-enable any-event mouse tracking" + ); // dirge-ph60: focus reporting must be re-armed too, or the next // FocusGained-driven recovery never fires — the automatic self-heal // depends on the terminal continuing to report focus changes. @@ -1354,7 +1371,7 @@ fn reassert_terminal_modes_arms_and_respects_throttle() { /// When the user is mid-drag selecting text, `reassert_terminal_modes` must /// not write to /dev/tty: re-sending mouse-tracking enable sequences -/// (?1003h et al.) resets internal tracking state on some terminals, +/// (?1002h et al.) resets internal tracking state on some terminals, /// dropping the drag so MouseUp never fires and copy_to_clipboard is never /// called. #[test] diff --git a/src/ui/terminal.rs b/src/ui/terminal.rs index 7fa432c85..9552ec996 100644 --- a/src/ui/terminal.rs +++ b/src/ui/terminal.rs @@ -1,14 +1,13 @@ use std::io::Write; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; use std::time::Duration; use crossterm::ExecutableCommand; use crossterm::cursor::Hide; use crossterm::event::{ - EnableBracketedPaste, EnableFocusChange, EnableMouseCapture, KeyboardEnhancementFlags, - PushKeyboardEnhancementFlags, + EnableBracketedPaste, EnableFocusChange, KeyboardEnhancementFlags, PushKeyboardEnhancementFlags, }; use crossterm::terminal::{ self, Clear, ClearType, EnterAlternateScreen, supports_keyboard_enhancement, @@ -155,10 +154,33 @@ pub(crate) static EVENT_READER_SHUTDOWN: AtomicBool = AtomicBool::new(false); /// the common case (reader exits within a few ms). pub(crate) static EVENT_READER_EXITED: AtomicBool = AtomicBool::new(false); +/// Which generation of input reader is the live one (dirge-xxo9). +/// +/// `spawn_input_reader` bumps this and captures the new value; a reader +/// whose captured generation no longer matches exits at its next tick. +/// +/// The shutdown flag cannot carry that meaning on its own. The suspend path +/// proceeds after 150ms even when the reader has not exited, and the resume +/// path then clears the flag — so a stale reader woke to a `false` flag and +/// kept reading fd 0 next to its replacement, and next to the stdin drains. +/// Two consumers on one descriptor split escape sequences, and the tail of a +/// split sequence parses as plain text: that is how terminal bytes ended up +/// typed into the compose box. +pub(crate) static READER_GENERATION: AtomicU64 = AtomicU64::new(0); + /// Stored `JoinHandle` of the crossterm input-reader thread. /// Set by `spawn_input_reader`, consumed by `join_reader`. pub(crate) static READER_HANDLE: Mutex>> = Mutex::new(None); +/// Mouse modes dirge actually uses: X10 button reporting (`?1000h`), +/// button-event tracking so a held-button drag reports (`?1002h`), and the +/// SGR encoding so coordinates past column 95 survive (`?1006h`). +/// Deliberately NOT `?1003h` / `?1015h` — see the call site in +/// `TerminalGuard::new` (dirge-hn6e). The teardown strings still clear +/// `?1003l` / `?1015l`: a terminal may have them set from an older dirge or +/// another program. +const MOUSE_CAPTURE_ON: &[u8] = b"\x1b[?1000h\x1b[?1002h\x1b[?1006h"; + pub struct TerminalGuard { /// Original stdout (fd 1) saved before we redirected fd 1 to /// the log file. Restored on drop so the shell that spawned @@ -212,7 +234,19 @@ impl TerminalGuard { // so native text selection requires the standard // bypass-modifier: Option/Alt+drag on macOS terminals, Shift // +drag on most Linux terminals. - tty_writer.execute(EnableMouseCapture)?; + // + // Written out rather than `EnableMouseCapture` because that also + // sets `?1003h` and `?1015h` (dirge-hn6e). `?1003h` is any-event + // tracking: the terminal reports every cell of pointer motion with + // no button held, and nothing consumes it — the reader maps wheel + // and left button down/drag/up and drops the rest, and + // `MouseEventKind::Moved` appears nowhere in the tree. `?1002h` + // (button-event tracking) already covers the wheel and the + // drag-selection. All it bought was the only continuous input byte + // stream in the program, which is what turns a one-off desync on + // fd 0 into a sustained flood of junk characters. `?1015h` is the + // urxvt encoding, which crossterm cannot parse at all. + tty_writer.write_all(MOUSE_CAPTURE_ON)?; // Focus reporting (`?1004h`): the terminal sends `\x1b[I` on // focus-in / `\x1b[O` on focus-out, which crossterm delivers as // FocusGained / FocusLost. dirge-ph60 uses FocusGained to @@ -747,7 +781,7 @@ pub(crate) fn resume_tui_after_subprocess( // suspend path emitted `?1004l`, so re-arm it or FocusGained // recovery goes dark after any sandbox attach). let _ = tty.write_all( - b"\x1b[?1049h\x1b[2J\x1b[?25l\x1b[?2004h\x1b[?1000h\x1b[?1002h\x1b[?1003h\x1b[?1006h\x1b[?1004h", + b"\x1b[?1049h\x1b[2J\x1b[?25l\x1b[?2004h\x1b[?1000h\x1b[?1002h\x1b[?1006h\x1b[?1004h", ); let _ = tty.flush(); }