diff --git a/CHANGELOG.md b/CHANGELOG.md index 9badeaf3..4ef4cb22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Tab completion (and other line editing) now stays out of the way over `ssh`. + A remote shell that emits its own prompt marks — fish 4.x on a Linux server, + most visibly, which ships OSC 133 on by default — used to engage tty7's + *local* line editor, so Tab ran completion against the local machine's + filesystem instead of reaching the remote shell. tty7 now only drives the + inline editor while the shell it launched is itself idle at its prompt; + whenever a foreground command (ssh, a TUI, a nested shell) owns the terminal, + keystrokes pass straight through to it. (#26, follow-up to #18) + ## [0.6.0] - 2026-07-08 ### Added diff --git a/src/daemon/pane.rs b/src/daemon/pane.rs index 48b173db..d9942881 100644 --- a/src/daemon/pane.rs +++ b/src/daemon/pane.rs @@ -262,11 +262,13 @@ struct PaneState { pub struct DaemonPane { pub id: u64, /// The PTY master. Kept for the pane's lifetime to `resize` it and to query the - /// foreground process group (macOS title / cwd fallback). Behind a `Mutex` - /// because the trait object is `Send` but not `Sync`, and the pane is shared - /// across connection threads via `Arc`; the lock is uncontended in practice - /// (resize is rare, the proc query rarer). - master: Mutex>, + /// foreground process group (macOS title / cwd fallback, and the reader + /// thread's remote-prompt gate — see [`foreground_command_running`]). Behind a + /// `Mutex` because the trait object is `Send` but not `Sync`, and the pane is + /// shared across connection threads via `Arc`; the lock is uncontended in + /// practice (resize is rare, the proc query rarer). Wrapped in an `Arc` so the + /// reader thread can hold its own handle for that gate. + master: Arc>>, /// The PTY's input side (keyboard input / pasted text). Behind a `Mutex` /// because writes can arrive from different connection threads. writer: Mutex>, @@ -406,9 +408,11 @@ impl DaemonPane { let shutting_down = Arc::new(AtomicBool::new(false)); let gate = Arc::new(OutputGate::new()); + let master = Arc::new(Mutex::new(pair.master)); + let pane = Arc::new(Self { id, - master: Mutex::new(pair.master), + master: master.clone(), writer: Mutex::new(writer), child: Mutex::new(child), shell_pid, @@ -419,7 +423,19 @@ impl DaemonPane { reader: Mutex::new(None), }); - let reader = Self::spawn_reader(state, shutting_down, gate, reader_handle, on_dead); + // The reader gates a foreground program's OSC 133 prompt marks (a remote + // shell over ssh emitting its own) out of `at_prompt`, so tty7's local + // line editor stays disengaged for whatever is really reading the + // keyboard — see `foreground_command_running` / issue #26. + let fg_master = master.clone(); + let reader = Self::spawn_reader( + state, + shutting_down, + gate, + reader_handle, + move || foreground_command_running(&fg_master, shell_pid), + on_dead, + ); *pane.reader.lock().unwrap() = Some(reader); Ok(pane) @@ -436,6 +452,10 @@ impl DaemonPane { shutting_down: Arc, gate: Arc, mut reader: Box, + // "Is a foreground command (not the shell) currently on the PTY?" Consulted + // when a prompt mark arrives, to reject marks a foreground program emits — + // see the call site and [`foreground_command_running`]. + foreground_running: impl Fn() -> bool + Send + 'static, on_dead: impl FnOnce() + Send + 'static, ) -> JoinHandle<()> { std::thread::Builder::new() @@ -489,7 +509,29 @@ impl DaemonPane { let bytes = &buf[..n]; // Sniff first (cheap, over the same bytes); collect any // cwd/prompt change to emit while we hold the lock. - let signals = sniffer.feed(bytes); + let mut signals = sniffer.feed(bytes); + + // Reject a prompt mark emitted by a *foreground program* + // rather than the shell tty7 spawned. The shell only + // emits its OSC 133 marks while it is the PTY's own + // foreground group (idle at its prompt); a mark arriving + // while a command owns the PTY therefore comes from that + // command — most visibly a remote shell over ssh drawing + // its own prompt. Trusting it would flip `at_prompt` true + // and engage tty7's *local* line editor, whose completion + // and history are local-only and wrong for the remote + // session (Tab completed local paths instead of the + // remote's). Drop the flag so keys pass raw to whatever is + // really reading them. The proc query runs only when a + // mark actually claims the prompt — about once per prompt. + // See issue #26. + if signals.shell.as_ref().is_some_and(|s| s.at_prompt) + && foreground_running() + { + if let Some(s) = signals.shell.as_mut() { + s.at_prompt = false; + } + } let tr1 = trace.then(std::time::Instant::now); let mut st = state.lock().unwrap(); @@ -971,6 +1013,49 @@ fn apply_signals(st: &mut PaneState, signals: SniffSignals) { } } +/// Whether a foreground command — not the shell itself — currently owns the +/// PTY. True while e.g. `ssh`, `vim`, or a nested shell runs; false when the +/// shell sits idle at its own prompt (it is then the terminal's foreground +/// process group). Unknown/missing data answers false, so a bad reading never +/// suppresses a real local prompt. +/// +/// This is the signal that keeps a foreground program's OSC 133 marks — a fish +/// session over ssh emitting its own prompt marks, most visibly — from engaging +/// tty7's local line editor, whose completion and history are local-only and +/// wrong for whatever is really reading the keyboard. See issue #26. +fn foreground_command_running( + master: &Mutex>, + shell_pid: Option, +) -> bool { + is_foreground_command(pty_foreground_pgid(master), shell_pid) +} + +/// The PTY's foreground process-group id (`pid_t`, i.e. `i32`), read from the +/// terminal via `tcgetpgrp`, or `None` when it can't be read. +#[cfg(unix)] +fn pty_foreground_pgid(master: &Mutex>) -> Option { + master.lock().ok().and_then(|m| m.process_group_leader()) +} + +/// Windows conpty has no foreground-process-group concept — portable-pty doesn't +/// implement `process_group_leader` there — so there is nothing to gate on: we +/// answer `None`, leaving prompt marks handled exactly as before. (ssh from a +/// Windows tty7 is rare and uses a different model anyway.) +#[cfg(not(unix))] +fn pty_foreground_pgid(_master: &Mutex>) -> Option { + None +} + +/// Pure core of [`foreground_command_running`]: given the PTY's foreground +/// process group and the shell's pid, is the foreground group some *other* +/// process (a running command) rather than the shell idling at its prompt? +fn is_foreground_command(fg_pgid: Option, shell_pid: Option) -> bool { + match (fg_pgid, shell_pid) { + (Some(pg), Some(shell)) if pg > 0 => pg as u32 != shell, + _ => false, + } +} + // --------------------------------------------------------------------------- // OSC sniffer (cwd + prompt). The byte-level OSC framing lives in // `core::osc::OscTokenizer` (shared with the client's notification scanner); @@ -1425,6 +1510,57 @@ mod tests { assert_eq!(d.shell.as_ref().unwrap().last_exit_code, Some(130)); } + /// The foreground-command predicate: only a process group *other* than the + /// shell counts as a running command; matching pids, or missing data, mean + /// the shell is idle at its own prompt (so we never suppress a real prompt). + #[test] + fn foreground_command_distinguishes_the_shell_from_a_command() { + // Shell idle at its prompt: the shell is the PTY's foreground group. + assert!(!is_foreground_command(Some(1000), Some(1000))); + // A command (ssh, vim, a nested shell) owns the PTY: a different group. + assert!(is_foreground_command(Some(2000), Some(1000))); + // Unknown foreground group, unknown shell pid, or a non-positive pgid all + // answer "shell is foreground" — a bad reading must not disengage editing. + assert!(!is_foreground_command(None, Some(1000))); + assert!(!is_foreground_command(Some(2000), None)); + assert!(!is_foreground_command(Some(0), Some(1000))); + } + + /// The reader's gate for issue #26: a remote shell over ssh emits its own + /// OSC 133 marks, which the sniffer reads as "at prompt" — but because a + /// foreground command (ssh) owns the PTY, the reader drops that flag so + /// tty7's local line editor stays disengaged and Tab reaches the remote shell. + #[test] + fn foreground_program_prompt_marks_do_not_claim_the_prompt() { + let mut s = OscSniffer::new(); + // The remote fish draws its prompt: A (start) then B (input begins). + let mut signals = s.feed(b"\x1b]133;A\x1b]133;B\x07"); + assert!( + signals.shell.as_ref().unwrap().at_prompt, + "the raw marks read as at-prompt" + ); + + // The reader consults the foreground gate before reporting. With ssh (a + // different process group) on the PTY, the prompt flag is cleared. + let ssh_running = is_foreground_command(Some(2000), Some(1000)); + if signals.shell.as_ref().is_some_and(|st| st.at_prompt) && ssh_running { + signals.shell.as_mut().unwrap().at_prompt = false; + } + assert!( + !signals.shell.as_ref().unwrap().at_prompt, + "a foreground program's prompt marks must not engage the local editor" + ); + + // Sanity: the very same marks with the shell itself foreground (idle at a + // local prompt) keep at_prompt true — the local editor still engages. + let mut local = s.feed(b"\x1b]133;A\x1b]133;B\x07"); + let shell_idle = is_foreground_command(Some(1000), Some(1000)); + if local.shell.as_ref().is_some_and(|st| st.at_prompt) && shell_idle { + local.shell.as_mut().unwrap().at_prompt = false; + } + assert!(local.shell.as_ref().unwrap().at_prompt); + } + /// Regression: a well-formed OSC marker directly following an *unterminated* /// one must not be dropped. A bare ESC inside an OSC aborts the current /// sequence (VT semantics) and — when the next byte is `]` — introduces a new @@ -1710,6 +1846,7 @@ mod tests { Arc::new(AtomicBool::new(false)), Arc::new(OutputGate::new()), Box::new(std::io::Cursor::new(b"tail".to_vec())), + || false, // no PTY here → treat the shell as foreground move || dead_flag.store(true, Ordering::SeqCst), ); handle.join().unwrap(); // the Cursor EOFs immediately after "tail" @@ -1740,6 +1877,7 @@ mod tests { Arc::new(AtomicBool::new(false)), Arc::new(OutputGate::new()), Box::new(std::io::Cursor::new(Vec::new())), + || false, move || dead_tx.send(()).unwrap(), ); handle.join().unwrap(); @@ -1761,6 +1899,7 @@ mod tests { Arc::new(AtomicBool::new(true)), // teardown already initiated Arc::new(OutputGate::new()), Box::new(std::io::Cursor::new(Vec::new())), + || false, move || dead_flag.store(true, Ordering::SeqCst), ); handle.join().unwrap();