From aad0a49ef7d94258791df80e6de3dd6fd704e998 Mon Sep 17 00:00:00 2001 From: thomas Date: Mon, 27 Jul 2026 13:12:41 +0800 Subject: [PATCH 1/3] fix(config): don't let a UTF-8 BOM silently reset every setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every config-dir file is read by a loader that treats any parse error as "there is no file" and falls back to defaults. serde_json rejects the U+FEFF a BOM puts before the opening brace, so a BOM never surfaced as a broken config — it surfaced as an absent one, and the app booted on defaults with nothing to explain it. Windows makes that easy to hit by accident: PowerShell's `>`, `Out-File` and `Set-Content -Encoding utf8` all write a BOM, so editing config.json from a shell was enough to lose every setting. Strip a leading BOM in the three loaders whose files people hand-edit: config.json, session.json (which dropped every workspace the same way), and themes/*.yaml. read_to_string decodes the marker to one U+FEFF char, so this strips the char, not the three raw bytes — and only the first one, since a second is content the parser should still reject. window.json and update.json are left alone: they are machine-written state a relaunch rebuilds, never hand-edited. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WCb8ZDmvdA5xbVtvs647tD --- CHANGELOG.md | 11 ++++++++ src/core/config.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++- src/core/session.rs | 21 ++++++++++++++- src/ui/presets.rs | 3 ++- 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3da0235d..3a7b3735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A config file saved with a UTF-8 BOM no longer wipes your settings** — every + config-dir file is read by a loader that treats any parse error as "there is + no file", falling back to defaults. `serde_json` rejects the U+FEFF a BOM puts + before the opening brace, so a BOM didn't report a broken config — it reported + an absent one, and tty7 came up on defaults with nothing in the log to explain + it. Windows makes this easy to hit by accident: PowerShell's `>`, `Out-File` + and `Set-Content -Encoding utf8` all write a BOM, so editing `config.json` + from a shell was enough to lose every setting. `config.json`, `session.json` + (which lost every workspace the same way) and hand-authored `themes/*.yaml` + now skip a leading BOM. + - **New tabs and splits open in the right directory even when the shell can't be instrumented** ([#187](https://github.com/l0ng-ai/tty7/issues/187)) — a pane learned its directory from `OSC 7`, which only shells tty7 injects its diff --git a/src/core/config.rs b/src/core/config.rs index 01efaed6..32e2fe79 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -679,7 +679,7 @@ impl Config { // Missing/unreadable config is the common case — start with defaults. return Config::default(); }; - match serde_json::from_str::(&text) { + match serde_json::from_str::(strip_bom(&text)) { Ok(mut cfg) => { cfg.sanitize(); cfg @@ -824,6 +824,21 @@ pub fn config_path(file: &str) -> Option { Some(config_dir()?.join(file)) } +/// Drop a leading UTF-8 BOM so a hand-edited config still parses. +/// +/// `serde_json` rejects U+FEFF before the opening brace, and every config-dir +/// file is read by a loader that treats *any* parse error as "there is no +/// config" — so a BOM doesn't surface as an error, it silently resets the +/// user's settings. Windows makes that easy to hit by accident: PowerShell's +/// `>`, `Out-File` and `Set-Content -Encoding utf8` all write one, so a quick +/// `... | Set-Content config.json` is enough to lose every setting. +/// +/// `read_to_string` decodes the BOM to the single char U+FEFF, so this strips +/// the char rather than the three raw bytes. +pub fn strip_bom(text: &str) -> &str { + text.strip_prefix('\u{FEFF}').unwrap_or(text) +} + /// Write `bytes` to `path` atomically: write to a sibling temp file, fsync, then /// rename over the target. A crash/power-loss mid-write then leaves either the /// old file or the new one intact — never a truncated/half-written file that @@ -1417,8 +1432,19 @@ mod tests { set_config_dir(dir); } + /// Serialize the tests that write the shared `config.json`: they all resolve + /// the same pinned path, so without this they clobber each other's file. + static CONFIG_FILE: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn lock_config_file() -> std::sync::MutexGuard<'static, ()> { + // A poisoned lock only means another test failed mid-sequence; every + // holder rewrites the file from scratch, so the state is still sound. + CONFIG_FILE.lock().unwrap_or_else(|e| e.into_inner()) + } + #[test] fn save_load_and_shell_command_round_trip_through_disk() { + let _guard = lock_config_file(); pin_config_dir(); // Persist a config with a non-default shell + font + an SSH profile, then // read it back. @@ -1457,6 +1483,41 @@ mod tests { assert_eq!(args, vec!["-l".to_string()]); } + #[test] + fn a_utf8_bom_does_not_silently_reset_the_config() { + let _guard = lock_config_file(); + pin_config_dir(); + let path = Config::path().expect("pinned config dir"); + // Exactly what PowerShell's `>`, `Out-File` and `Set-Content -Encoding + // utf8` leave behind. + let text = "\u{FEFF}{\"font_size\": 21.0, \"restore_session\": false}"; + write_atomic(&path, text.as_bytes()).unwrap(); + + // The failure this guards is silent by construction: `load` turns *any* + // parse error into defaults, so a BOM didn't report a bad config — it + // reported no config, and the user's settings appeared to vanish. + let loaded = Config::load(); + assert_eq!(loaded.font_size, 21.0); + assert!(!loaded.restore_session); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn strip_bom_only_removes_a_leading_marker() { + assert_eq!(strip_bom("{}"), "{}"); + assert_eq!(strip_bom("\u{FEFF}{}"), "{}"); + // Only the first U+FEFF is a marker. A second one is content, and + // content that happens to be a BOM is still invalid JSON — stripping + // it too would be guessing at a file we can't rescue. + assert_eq!(strip_bom("\u{FEFF}\u{FEFF}{}"), "\u{FEFF}{}"); + // A BOM *inside* the document is data (U+FEFF is a legal string char), + // so it must survive untouched. + let inner = "{\"tab_title\":\"\u{FEFF}\"}"; + assert_eq!(strip_bom(inner), inner); + assert_eq!(strip_bom(""), ""); + } + #[test] fn ssh_profiles_default_empty_and_parse_from_json() { // Absent key → empty (a config predating profiles still loads). diff --git a/src/core/session.rs b/src/core/session.rs index 4e426489..ff971b35 100644 --- a/src/core/session.rs +++ b/src/core/session.rs @@ -300,7 +300,7 @@ impl Workspaces { /// `{active, tabs}` session, which migrates to a single open workspace so /// upgrading users keep their tabs (and their attached daemon panes). pub fn decode(text: &str) -> Result { - let value: serde_json::Value = serde_json::from_str(text)?; + let value: serde_json::Value = serde_json::from_str(crate::core::config::strip_bom(text))?; if value.get("workspaces").is_some() { return serde_json::from_value(value); } @@ -759,6 +759,25 @@ mod tests { )); } + #[test] + fn a_utf8_bom_does_not_discard_the_session() { + // `Session::load` treats a parse error as "no session", so a BOM on a + // hand-edited `session.json` doesn't warn — it drops every workspace + // and opens on the home page as if nothing had been saved. + // Legacy `{active, tabs}` shape, so this also covers the migration path. + let decoded = Workspaces::decode( + "\u{FEFF}{\"active\": 0, \"tabs\": [{\"pane\": {\"Leaf\": {\"cwd\": \"/work\"}}}]}", + ) + .expect("a BOM'd session still decodes"); + let tabs = &decoded + .workspaces + .first() + .expect("migrated workspace") + .session + .tabs; + assert_eq!(tabs.len(), 1); + } + #[test] fn session_defaults_fill_missing_fields() { // An empty object → default (active 0, no tabs). diff --git a/src/ui/presets.rs b/src/ui/presets.rs index 3f951cf2..80e94963 100644 --- a/src/ui/presets.rs +++ b/src/ui/presets.rs @@ -942,7 +942,8 @@ fn default_image_opacity() -> f32 { fn load_yaml_theme(path: &std::path::Path) -> Result { let text = std::fs::read_to_string(path).map_err(|e| e.to_string())?; - let file: ThemeFile = serde_yaml::from_str(&text).map_err(|e| e.to_string())?; + let file: ThemeFile = + serde_yaml::from_str(crate::core::config::strip_bom(&text)).map_err(|e| e.to_string())?; let (id, derived_name) = id_and_name(path); let background = file.background.into_fill()?; From b139568ca3ef6df0abc086e91ca1151890cf5b7d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:38:47 +0800 Subject: [PATCH 2/3] fix(daemon): report every prompt-boundary crossing in one chunk The OSC sniffer folded a whole chunk's 133 marks into a single shell state and `apply_signals` sent one `Prompt` frame for it. That loses information the client depends on: it counts `at_prompt` false -> true edges as prompt *cycles*, which is how it tells a fresh prompt from a same-prompt redraw (a prompt framework re-emitting the PS1-embedded `133;B` on reset-prompt bumps the report seq but must not count). A chunk carrying a whole command cycle (`C` ... `D`) therefore reported only the final at-prompt state, and the edge vanished. Over SSH that is routine rather than exotic: a fast command's `C`, its output and its `D` leave the remote together and arrive in one read. Report one state per crossing instead. Marks on the same side of the boundary still fold into their latest state, so the ordinary `D`/`A`/`B` prompt draw costs exactly one frame as before. --- src/daemon/pane.rs | 159 +++++++++++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 47 deletions(-) diff --git a/src/daemon/pane.rs b/src/daemon/pane.rs index 1b3b512d..3f4ba9a6 100644 --- a/src/daemon/pane.rs +++ b/src/daemon/pane.rs @@ -1100,12 +1100,14 @@ impl DaemonPane { // 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() { + if signals.shell.iter().any(|s| s.at_prompt) && foreground_running() { + for s in signals.shell.iter_mut() { s.at_prompt = false; } + // Clearing the flag can leave neighbours identical; + // they no longer describe a crossing, so don't spend + // a frame on each. + signals.shell.dedup(); } // SSH-context + coding-agent detection are process-table @@ -1869,7 +1871,9 @@ fn apply_signals(st: &mut PaneState, signals: SniffSignals) { st.cwd = Some(cwd); } } - if let Some(shell) = signals.shell { + // One frame per entry: the client needs every prompt-boundary crossing the + // chunk carried, not just where it ended up (see [`SniffSignals::shell`]). + for shell in signals.shell { // Windows: agent identity rides the C mark's command capture — ConPTY // has no foreground process group for the Unix 0.5 s poll to read an // argv from. `C;` detects, `D` cleared `command` so it applies @@ -2195,7 +2199,20 @@ struct ShellState { #[derive(Default)] struct SniffSignals { cwd: Option, - shell: Option, + /// Shell states completed in this chunk, in stream order — one entry per + /// `at_prompt` *transition*, not one per marker: a run of marks on the same + /// side of the prompt boundary folds into its latest state, so the ordinary + /// `D`/`A`/`B` chunk still yields the single entry it always did. + /// + /// Only the last state describes "what the shell is doing now", but the + /// client keys its prompt *cycle* off the false→true edge + /// (`terminal::remote::ShellState::cycle` — what releases a Tab handoff, see + /// `TerminalView::editor_handoff`). Collapsing to the last state alone hides + /// that edge whenever a whole command cycle (`C` … `D`) lands in one read — + /// routine over SSH, where a fast command's output arrives in a single + /// packet — and the handed-off prompt would then never come back to tty7's + /// line editor. + shell: Vec, /// Sentinel agent events completed in this chunk, in stream order — each /// one is a state-machine step, so unlike cwd/shell they must *all* apply /// (a `stop` directly after a `notification` still means "done"). @@ -2223,9 +2240,9 @@ impl OscSniffer { } } - /// Feed a chunk; return any cwd / shell-state change completed within it. (If a - /// chunk completes several markers, the last cwd / last shell state wins, which - /// is the only state worth reporting.) + /// Feed a chunk; return any cwd / shell-state change completed within it. (If + /// a chunk completes several cwd markers the last one wins; shell states keep + /// every prompt-boundary crossing — see [`SniffSignals::shell`].) fn feed(&mut self, bytes: &[u8]) -> SniffSignals { let mut signals = SniffSignals::default(); let shell = &mut self.shell; @@ -2234,7 +2251,15 @@ impl OscSniffer { signals.cwd = Some(path); } else if let Some(rest) = payload.strip_prefix(b"133;") { if handle_osc133(shell, rest) { - signals.shell = Some(shell.clone()); + match signals.shell.last_mut() { + // Still on the same side of the prompt boundary: fold in, + // latest wins (it carries the freshest exit code / command + // capture). Only a crossing earns its own entry. + Some(last) if last.at_prompt == shell.at_prompt => { + *last = shell.clone(); + } + _ => signals.shell.push(shell.clone()), + } } } else if let Some(event) = crate::core::cli_agent::parse_agent_event(payload) { signals.agent_events.push(event); @@ -2974,17 +2999,53 @@ mod tests { fn sniff_osc133_prompt() { let mut s = OscSniffer::new(); let b = s.feed(b"\x1b]133;B\x07"); - assert!(b.shell.as_ref().unwrap().active); - assert!(b.shell.as_ref().unwrap().at_prompt); + assert!(b.shell.last().unwrap().active); + assert!(b.shell.last().unwrap().at_prompt); let c = s.feed(b"\x1b]133;C\x07"); - assert!(!c.shell.as_ref().unwrap().at_prompt); + assert!(!c.shell.last().unwrap().at_prompt); // D (command finished) means no command is running, so we're back at the // prompt: at_prompt is true again (it also carries the exit code). let d = s.feed(b"\x1b]133;D;130\x07"); - assert!(d.shell.as_ref().unwrap().at_prompt); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, Some(130)); + assert!(d.shell.last().unwrap().at_prompt); + assert_eq!(d.shell.last().unwrap().last_exit_code, Some(130)); + } + + /// A whole command cycle inside ONE chunk still reports the prompt-boundary + /// crossing. The client counts `at_prompt` false→true edges to tell a fresh + /// prompt from a same-prompt redraw, and a Tab handoff only returns the line + /// to tty7's editor on that edge (`TerminalView::editor_handoff`). Reporting + /// just the chunk's final state hid the edge whenever `C` … `D` arrived + /// together — the norm over SSH, where a fast command's whole output lands in + /// one read — so one Tab in an ssh pane disabled the local editor for good. + #[test] + fn a_full_command_cycle_in_one_chunk_still_reports_leaving_the_prompt() { + let mut s = OscSniffer::new(); + s.feed(b"\x1b]133;A\x07\x1b]133;B\x07"); // sitting at the prompt + + // Enter → command → output → done → next prompt, all in one read. + let sig = + s.feed(b"\x1b]133;C;echo%20hi\x07hi\r\n\x1b]133;D;0\x07\x1b]133;A\x07\x1b]133;B\x07"); + let states: Vec = sig.shell.iter().map(|s| s.at_prompt).collect(); + assert_eq!( + states, + vec![false, true], + "the chunk must report leaving the prompt and coming back, not just the end state" + ); + assert_eq!(sig.shell.last().unwrap().last_exit_code, Some(0)); + assert_eq!(sig.shell.last().unwrap().command, None); + } + + /// The flip side: marks that stay on one side of the boundary fold into a + /// single state, so the ordinary prompt draw still costs exactly one frame. + #[test] + fn marks_on_the_same_side_of_the_prompt_boundary_fold_into_one_state() { + let mut s = OscSniffer::new(); + let sig = s.feed(b"\x1b]133;D;3\x07\x1b]133;A\x07\x1b]133;B\x07"); + assert_eq!(sig.shell.len(), 1, "D/A/B is one at-prompt state"); + assert!(sig.shell[0].at_prompt); + assert_eq!(sig.shell[0].last_exit_code, Some(3)); } /// The C mark's command capture (tty7 extension, PowerShell integration) — @@ -2999,7 +3060,7 @@ mod tests { // A submitted `claude --help` (space percent-encoded, as the // PowerShell body emits it). let c = s.feed(b"\x1b]133;C;claude%20--help\x07"); - let shell = c.shell.as_ref().unwrap(); + let shell = c.shell.last().unwrap(); assert!(!shell.at_prompt); assert_eq!(shell.command.as_deref(), Some("claude --help")); assert_eq!( @@ -3009,19 +3070,19 @@ mod tests { // The command finishing (D) clears the capture → the agent clears. let d = s.feed(b"\x1b]133;D;0\x07"); - let shell = d.shell.as_ref().unwrap(); + let shell = d.shell.last().unwrap(); assert_eq!(shell.command, None); assert_eq!(agent_from_shell_mark(shell, &custom), None); // A non-agent command sets the capture but detects nothing. let c = s.feed(b"\x1b]133;C;git%20status\x07"); - let shell = c.shell.as_ref().unwrap(); + let shell = c.shell.last().unwrap(); assert_eq!(shell.command.as_deref(), Some("git status")); assert_eq!(agent_from_shell_mark(shell, &custom), None); // A bare `C` (a foreign shell integration) leaves no capture. let c = s.feed(b"\x1b]133;C\x07"); - assert_eq!(c.shell.as_ref().unwrap().command, None); + assert_eq!(c.shell.last().unwrap().command, None); // A stray A/B mid-command (a nested/remote shell drawing its own // prompt) must NOT wipe the capture — only D (command finished) does. @@ -3029,21 +3090,21 @@ mod tests { // what keeps the agent chip alive while the agent runs. let _ = s.feed(b"\x1b]133;C;codex\x07"); let a = s.feed(b"\x1b]133;A\x1b]133;B\x07"); - assert_eq!(a.shell.as_ref().unwrap().command.as_deref(), Some("codex")); + assert_eq!(a.shell.last().unwrap().command.as_deref(), Some("codex")); let d = s.feed(b"\x1b]133;D;0\x07"); - assert_eq!(d.shell.as_ref().unwrap().command, None); + assert_eq!(d.shell.last().unwrap().command, None); // A multi-line command arrives %0A-joined (fish re-joins the split // list with it) and decodes back to real newlines. let c = s.feed(b"\x1b]133;C;echo%20a%0Aecho%20b\x07"); assert_eq!( - c.shell.as_ref().unwrap().command.as_deref(), + c.shell.last().unwrap().command.as_deref(), Some("echo a\necho b") ); // An all-whitespace payload is no capture, like a bare `C`. let c = s.feed(b"\x1b]133;C;%20%20\x07"); - assert_eq!(c.shell.as_ref().unwrap().command, None); + assert_eq!(c.shell.last().unwrap().command, None); } /// The Windows apply gate ([`shell_mark_capture_changed`]): detection @@ -3057,7 +3118,7 @@ mod tests { // A wrapper launch: capture set, but detection has no answer. let mut prev = ShellState::default(); - let c = s.feed(b"\x1b]133;C;.%5Cdev.ps1\x07").shell.unwrap(); + let c = s.feed(b"\x1b]133;C;.%5Cdev.ps1\x07").shell.pop().unwrap(); assert!(shell_mark_capture_changed(&prev, &c)); assert_eq!( agent_from_shell_mark(&c, &std::collections::HashMap::new()), @@ -3067,13 +3128,13 @@ mod tests { // Stray foreign prompt marks mid-command: same capture, no re-apply — // an agent branded by sentinel events keeps its chip. - let ab = s.feed(b"\x1b]133;A\x1b]133;B\x07").shell.unwrap(); + let ab = s.feed(b"\x1b]133;A\x1b]133;B\x07").shell.pop().unwrap(); assert!(!shell_mark_capture_changed(&prev, &ab)); prev = ab; // The command finishing clears the capture: that change applies (its // `None` is what clears the chip at the prompt). - let d = s.feed(b"\x1b]133;D;0\x07").shell.unwrap(); + let d = s.feed(b"\x1b]133;D;0\x07").shell.pop().unwrap(); assert!(shell_mark_capture_changed(&prev, &d)); } @@ -3082,13 +3143,13 @@ mod tests { let mut s = OscSniffer::new(); let sig = s.feed(b"\x1b]133;V;1\x07"); assert!( - sig.shell.is_none(), + sig.shell.is_empty(), "edit-mode metadata must not bump prompt state or prompt sequence" ); let b = s.feed(b"\x1b]133;B\x07"); - assert!(b.shell.as_ref().unwrap().active); - assert!(b.shell.as_ref().unwrap().at_prompt); + assert!(b.shell.last().unwrap().active); + assert!(b.shell.last().unwrap().at_prompt); } /// The foreground-command predicate: only a process group *other* than the @@ -3117,18 +3178,20 @@ mod tests { // 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, + signals.shell.last().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; + if signals.shell.iter().any(|st| st.at_prompt) && ssh_running { + for st in signals.shell.iter_mut() { + st.at_prompt = false; + } } assert!( - !signals.shell.as_ref().unwrap().at_prompt, + !signals.shell.last().unwrap().at_prompt, "a foreground program's prompt marks must not engage the local editor" ); @@ -3136,10 +3199,12 @@ mod tests { // 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; + if local.shell.iter().any(|st| st.at_prompt) && shell_idle { + for st in local.shell.iter_mut() { + st.at_prompt = false; + } } - assert!(local.shell.as_ref().unwrap().at_prompt); + assert!(local.shell.last().unwrap().at_prompt); } /// Regression: a well-formed OSC marker directly following an *unterminated* @@ -3158,7 +3223,7 @@ mod tests { let mut s = OscSniffer::new(); let sig = s.feed(b"\x1b]133;A\x1b]133;B\x07"); assert!( - sig.shell.as_ref().map(|sh| sh.at_prompt).unwrap_or(false), + sig.shell.last().is_some_and(|sh| sh.at_prompt), "OSC 133;B after an unterminated 133;A was dropped (no resync on `]`)" ); @@ -3188,13 +3253,13 @@ mod tests { let mut s = OscSniffer::new(); // A command was running… - assert!(!s.feed(b"\x1b]133;C\x07").shell.as_ref().unwrap().at_prompt); + assert!(!s.feed(b"\x1b]133;C\x07").shell.last().unwrap().at_prompt); // …then finishes: D (in its own chunk, before any prompt text) already // marks us back at the prompt. let d = s.feed(b"\x1b]133;D;0\x07"); assert!( - d.shell.as_ref().unwrap().at_prompt, + d.shell.last().unwrap().at_prompt, "D should mark us back at the prompt before the prompt text is drawn" ); @@ -3206,12 +3271,12 @@ mod tests { b"\x1b]133;A\x07\x1b]7;file://host/repo/tty7\x07\r\ntty7 git:(main) \xe2\x9e\x9c ", ); assert!( - chunk.shell.as_ref().unwrap().at_prompt, + chunk.shell.last().unwrap().at_prompt, "prompt visible but at_prompt=false — the mis-routing window is still open" ); // The trailing B finally arrives and keeps it true. - assert!(s.feed(b"\x1b]133;B\x07").shell.as_ref().unwrap().at_prompt); + assert!(s.feed(b"\x1b]133;B\x07").shell.last().unwrap().at_prompt); } /// `pty_size` never reports a zero dimension (a 0×0 window would make the @@ -3342,16 +3407,16 @@ mod tests { let mut s = OscSniffer::new(); // D with no code. let d = s.feed(b"\x1b]133;D\x07"); - assert!(d.shell.as_ref().unwrap().at_prompt); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, None); + assert!(d.shell.last().unwrap().at_prompt); + assert_eq!(d.shell.last().unwrap().last_exit_code, None); // D with a non-numeric code stays None. let d = s.feed(b"\x1b]133;D;oops\x07"); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, None); + assert_eq!(d.shell.last().unwrap().last_exit_code, None); // A negative exit code parses. let d = s.feed(b"\x1b]133;D;-1\x07"); - assert_eq!(d.shell.as_ref().unwrap().last_exit_code, Some(-1)); + assert_eq!(d.shell.last().unwrap().last_exit_code, Some(-1)); } /// A fresh `PaneState` for the PTY-less state-machine tests. @@ -3809,12 +3874,12 @@ mod tests { apply_signals( &mut st, SniffSignals { - shell: Some(ShellState { + shell: vec![ShellState { active: true, at_prompt: true, last_exit_code: Some(0), command: None, - }), + }], ..SniffSignals::default() }, ); From 4bcc4066d771f9a39bcfe6089550306f15eb3303 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:38:59 +0800 Subject: [PATCH 3/3] feat(completion): complete remote paths over the pane's SSH connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tab at a path position in a native-SSH pane had nothing to offer: path candidates come off the local filesystem, and a remote pane deliberately passes no local cwd, so `complete` returned `None`. A no-match hands the line to the shell (#136), which costs the user the inline editor until the next prompt — so in practice every path Tab in an SSH session dropped them back to the raw shell for the rest of that line. Ask the far side instead. The pane already holds an authenticated russh connection, and `SftpList` — the same request the SFTP panel browses with — lists a remote directory over it. No shell involvement: nothing is echoed into the scrollback, no prompt hooks need suppressing, and there is no background process to cancel when the user submits. The listing is a network round-trip, so it can't answer the keystroke synchronously. Results land on the main thread and only then behave as a local Tab would; a line that moved on in the meantime drops the answer, and an empty one falls back to the shell handoff, so a directory we cannot read is no worse off than before. completion.rs stays pure: it splits the word into a request and turns the answer into candidates, both unit-tested. Candidate rules mirror the local path engine exactly — hidden entries need a dot prefix, `cd` takes directories only, symlinks follow to their target's kind, same ordering and cap. Positions a listing cannot help (command position, `~` without the remote's $HOME, no absolute remote cwd) decline and let the shell have the Tab. Only native-SSH panes qualify: a foreground `ssh` typed into a local shell and a WSL pane have no tty7-owned connection to ask. --- src/terminal/completion.rs | 271 ++++++++++++++++++++++++++++++++++++- src/terminal/view.rs | 215 +++++++++++++++++++++++++---- 2 files changed, 455 insertions(+), 31 deletions(-) diff --git a/src/terminal/completion.rs b/src/terminal/completion.rs index 737a97c4..1e81ecc9 100644 --- a/src/terminal/completion.rs +++ b/src/terminal/completion.rs @@ -1,10 +1,15 @@ //! A small, self-contained completion engine for the command editor — tty7's own //! engine, not the shell's `compsys`. //! -//! It offers two sources, each candidate carrying the exact char range it +//! It offers three sources, each candidate carrying the exact char range it //! replaces: //! - **command** — builtins + `$PATH` executables, in command position; -//! - **path** — files / directories, elsewhere (replace just the word). +//! - **path** — files / directories, elsewhere (replace just the word); +//! - **remote path** — the same, for a pane whose filesystem is on the far +//! end of an SSH connection. The listing itself is a network round-trip the +//! view owns, so this module only splits the word into a request +//! ([`remote_path_request`]) and turns the answer into candidates +//! ([`remote_path_candidates`]) — both pure, both unit-tested. //! //! History deliberately does *not* feed the menu: //! whole-line recall belongs to the inline ghost text (frecency-ranked, cwd @@ -280,6 +285,150 @@ fn sort_candidates_by_closeness(cands: &mut [Candidate]) { }); } +/// What a path-position Tab in a remote pane needs listed on the *far side*, +/// produced by [`remote_path_request`] and consumed by +/// [`remote_path_candidates`] once the listing comes back. +/// +/// Split in two because the listing is a network round-trip: nothing here +/// touches a filesystem, so both halves stay pure and testable while the view +/// owns the async middle. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemotePathRequest { + /// Absolute directory to list on the remote. + pub dir: String, + /// What an entry's name must start with to be offered. + pub prefix: String, + /// The typed text up to and including the last `/`, re-prepended to every + /// candidate so the path the user typed is preserved (as [`complete_path`] + /// does locally). + pub dir_part: String, + /// Char range in the line the candidates replace. + pub word_start: usize, + pub cursor: usize, + /// Drop file entries — the command only takes directories. + pub dirs_only: bool, +} + +/// One entry of a remote directory listing, reduced to what completion cares +/// about. Keeps this module free of the daemon's SFTP protocol types; the view +/// converts (and is where "a symlink to a directory counts as a directory" +/// gets decided, since only the protocol knows the link target). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemoteEntry { + pub name: String, + pub is_dir: bool, +} + +/// The remote directory a path-position Tab wants listed, or `None` when the +/// caret isn't somewhere a remote path listing could help. +/// +/// `remote_cwd` is the pane's cwd *in the remote's namespace* — the caller must +/// have established that the pane really is remote. Declines: +/// - **command position** (a bare first word): those complete from `$PATH`, +/// and this machine's `$PATH` is the wrong answer for a remote anyway — +/// that's [`complete_command`]'s call, not a filesystem question. +/// - **`~`-prefixed words**: expanding one needs the remote's `$HOME`, which +/// no OSC reports. Declining hands the Tab to the remote shell, which can +/// expand it. +/// - a **relative `remote_cwd`**: nothing to resolve against. +/// +/// Separators are `/` only — deliberately not [`std::path::is_separator`], +/// which also accepts `\` on Windows. A Windows host talking to a POSIX remote +/// must not treat a backslash in the *remote's* path as a separator. +pub fn remote_path_request( + line: &str, + cursor: usize, + remote_cwd: &str, +) -> Option { + if !remote_cwd.starts_with('/') { + return None; + } + let chars: Vec = line.chars().collect(); + let cursor = cursor.min(chars.len()); + let mut word_start = cursor; + while word_start > 0 && !chars[word_start - 1].is_whitespace() { + word_start -= 1; + } + let word: String = chars[word_start..cursor].iter().collect(); + + let is_command = chars[..word_start].iter().all(|c| c.is_whitespace()); + if is_command && !word.contains('/') { + return None; + } + if word.starts_with('~') { + return None; + } + + let (dir_part, prefix) = match word.rfind('/') { + Some(i) => (&word[..=i], &word[i + 1..]), + None => ("", word.as_str()), + }; + // An absolute `dir_part` stands alone; anything else resolves against the + // remote cwd. `.`/`..` inside the path are left for the far side to + // resolve — an SFTP server handles them, and we have no remote filesystem + // to normalize against here. + let dir = if dir_part.starts_with('/') { + dir_part.to_string() + } else if dir_part.is_empty() { + remote_cwd.to_string() + } else { + format!("{}/{dir_part}", remote_cwd.trim_end_matches('/')) + }; + + Some(RemotePathRequest { + dir, + prefix: prefix.to_string(), + dir_part: dir_part.to_string(), + word_start, + cursor, + dirs_only: current_command(&chars, word_start) + .is_some_and(|c| DIR_ONLY_COMMANDS.contains(&c.as_str())), + }) +} + +/// Turn a remote directory listing into candidates for `req`. Mirrors +/// [`complete_path`]'s rules exactly — hidden entries only when the prefix asks +/// for them, `dirs_only` filtering, closeness ordering, the same cap — so a +/// remote Tab behaves like a local one. +/// +/// `.` and `..` are dropped: a local `read_dir` never yields them, and offering +/// them here would make the two panes feel different. +pub fn remote_path_candidates(req: &RemotePathRequest, entries: &[RemoteEntry]) -> Vec { + let mut out: Vec = Vec::new(); + for entry in entries { + let name = entry.name.as_str(); + if name == "." || name == ".." { + continue; + } + if name.starts_with('.') && !req.prefix.starts_with('.') { + continue; + } + if !name.starts_with(&req.prefix) { + continue; + } + if req.dirs_only && !entry.is_dir { + continue; + } + out.push(Candidate { + text: format!("{}{name}", req.dir_part), + kind: if entry.is_dir { + CandidateKind::Dir + } else { + CandidateKind::File + }, + start: req.word_start, + end: req.cursor, + description: None, + icon: None, + }); + if out.len() >= MAX_CANDIDATES { + break; + } + } + sort_candidates_by_closeness(&mut out); + out +} + /// Filesystem path completion. Splits `word` into the directory part (kept /// verbatim in each candidate so the typed path prefix is preserved) and the /// final-segment prefix to match in that directory. Ordered by closeness. @@ -1158,6 +1307,124 @@ mod tests { } } + /// The word under the caret, split and resolved against the *remote* cwd. + /// This is the request the pane's SSH connection is asked to list. + #[test] + fn remote_path_request_splits_the_word_and_resolves_against_the_remote_cwd() { + // Bare word: list the cwd itself, nothing to re-prepend. + let r = remote_path_request("cat fi", 6, "/home/me").unwrap(); + assert_eq!( + (r.dir.as_str(), r.prefix.as_str(), r.dir_part.as_str()), + ("/home/me", "fi", "") + ); + assert_eq!((r.word_start, r.cursor), (4, 6)); + assert!(!r.dirs_only); + + // Relative subdirectory: resolved against the cwd, typed text preserved. + let r = remote_path_request("cat sub/fi", 10, "/home/me").unwrap(); + assert_eq!(r.dir, "/home/me/sub/"); + assert_eq!((r.prefix.as_str(), r.dir_part.as_str()), ("fi", "sub/")); + + // Absolute: stands alone, the cwd is irrelevant. + let r = remote_path_request("cat /etc/pa", 11, "/home/me").unwrap(); + assert_eq!(r.dir, "/etc/"); + assert_eq!(r.prefix, "pa"); + + // A trailing separator on the cwd must not double up. + let r = remote_path_request("cat sub/", 8, "/").unwrap(); + assert_eq!(r.dir, "/sub/"); + + // `cd` takes directories only — same rule as the local engine. + assert!( + remote_path_request("cd pro", 6, "/home/me") + .unwrap() + .dirs_only + ); + + // A backslash is a filename character on a POSIX remote, not a + // separator — even when tty7 itself runs on Windows. + let r = remote_path_request(r"cat a\b", 7, "/home/me").unwrap(); + assert_eq!((r.dir.as_str(), r.prefix.as_str()), ("/home/me", r"a\b")); + } + + /// Positions where a remote listing is the wrong answer: the caller falls + /// back to the shell handoff for these rather than guessing. + #[test] + fn remote_path_request_declines_where_a_listing_cannot_help() { + // Command position: `$PATH`, not a directory listing. + assert!(remote_path_request("ls", 2, "/home/me").is_none()); + assert!(remote_path_request("", 0, "/home/me").is_none()); + // ...unless the "command" is itself a path, which is a real listing. + assert!(remote_path_request("./scr", 5, "/home/me").is_some()); + + // `~` needs the remote's $HOME, which no OSC reports. The remote shell + // can expand it; we can't, so we decline and let it have the Tab. + assert!(remote_path_request("cat ~/pro", 9, "/home/me").is_none()); + + // No absolute cwd to resolve against (the remote shell hasn't reported + // one yet, or reported something unusable). + assert!(remote_path_request("cat fi", 6, "").is_none()); + assert!(remote_path_request("cat fi", 6, "relative/dir").is_none()); + } + + /// A remote listing becomes candidates under exactly the local rules — + /// hidden entries stay hidden, the typed directory prefix is preserved, and + /// the ordering is the shared closeness sort. + #[test] + fn remote_path_candidates_mirror_the_local_path_rules() { + let entries = |names: &[(&str, bool)]| -> Vec { + names + .iter() + .map(|(n, d)| RemoteEntry { + name: (*n).to_string(), + is_dir: *d, + }) + .collect() + }; + let all = entries(&[ + (".", true), + ("..", true), + (".hidden", false), + ("src", true), + ("setup.py", false), + ("s", false), + ("other", false), + ]); + let texts = + |cands: Vec| -> Vec { cands.into_iter().map(|c| c.text).collect() }; + + let req = remote_path_request("cat s", 5, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!( + got, + vec!["s", "src", "setup.py"], + "prefix-matched, shortest first; `.`/`..`/hidden/non-matching dropped" + ); + + // The directory kind survives, so the menu can mark it and the insert + // can add the trailing separator. + let cands = remote_path_candidates(&req, &all); + assert!(cands.iter().find(|c| c.text == "src").unwrap().is_dir()); + assert!(!cands.iter().find(|c| c.text == "s").unwrap().is_dir()); + + // The typed directory part is re-prepended to every candidate, and the + // replacement range covers the whole word. + let req = remote_path_request("cat sub/s", 9, "/home/me").unwrap(); + let c = &remote_path_candidates(&req, &all)[0]; + assert_eq!(c.text, "sub/s"); + assert_eq!((c.start, c.end), (4, 9)); + + // A dot prefix opts into hidden entries, as it does locally. + let req = remote_path_request("cat .h", 6, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!(got, vec![".hidden"]); + + // `cd` drops the files. + let req = remote_path_request("cd s", 4, "/home/me").unwrap(); + let got = texts(remote_path_candidates(&req, &all)); + assert_eq!(got, vec!["src"]); + } + #[test] fn no_candidates_returns_none() { let dir = temp_tree("empty", &[("zzz", false)]); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 36b78b74..705c2f88 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -336,6 +336,11 @@ pub struct TerminalView { /// when it opened. Typing/Backspace re-filter it in place; it closes on /// accept, on Escape, or once the edited word no longer matches anything. completion: Option, + /// Whether a remote directory listing for completion is on the wire (see + /// [`Self::spawn_remote_path_completion`]). Holding Tab down would + /// otherwise dial the daemon once per repeat while the first answer is + /// still travelling. + remote_completion_inflight: bool, /// Monotonic tag bumped every time a completion session opens or closes. /// Dynamic generators run on background threads and land their results here /// via `cx.spawn`; each task captures the generation it was spawned under and @@ -1139,6 +1144,7 @@ impl TerminalView { completion: None, completion_generation: 0, editor_handoff: None, + remote_completion_inflight: false, reverse_search: None, integration_notice: None, integration_notice_shown: false, @@ -3750,6 +3756,12 @@ impl TerminalView { let line = self.cmd.text(); let cursor = self.cmd.cursor(); let Some(comp) = super::completion::complete(&line, cursor, cwd.as_deref()) else { + // Nothing *locally*. A native-SSH pane can still answer for the + // remote filesystem over its own connection — ask before giving up + // the line (see `spawn_remote_path_completion`). + if self.spawn_remote_path_completion(&line, cursor, forward, cx) { + return; + } // Nothing to offer. Don't swallow the keypress (#136) — hand the // line to the shell and let its completion have the Tab. self.handoff_tab_to_shell(!forward, cx); @@ -3763,15 +3775,6 @@ impl TerminalView { // classic behavior byte-for-byte. let has_pending = !comp.pending.is_empty(); - if !has_pending && comp.candidates.len() == 1 { - // Unique match: accept it outright. - let c = comp.candidates[0].clone(); - self.completion_insert(&c, c.start); - self.cursor_visible = true; - cx.notify(); - return; - } - // The word range is carried by any candidate; with none (pure-generator // slot) derive it from the caret so the session still knows what it // replaces. @@ -3779,26 +3782,18 @@ impl TerminalView { Some(c) => (c.start, c.end), None => (word_start_of(&line, cursor), cursor), }; - let word: String = line - .chars() - .skip(word_start) - .take(word_end - word_start) - .collect(); - let s = CompletionSession::new(word_start, word.clone(), comp.candidates); - if !has_pending - && let Some(lcp) = s.common_prefix() - && lcp.chars().count() > word.chars().count() - { - // Static-only: fill the longest common prefix when it extends the - // typed word. All candidates share it, so the fill never invalidates - // the set. With generators pending we skip this — the eventual set - // may share a shorter prefix, and mutating the line before results - // arrive would be jarring. - self.apply_candidate(&line, word_start, word_end, &lcp); - } - let generation = self.open_completion(s); - self.cursor_visible = true; - cx.notify(); + let Some(generation) = self.offer_candidates( + &line, + word_start, + word_end, + comp.candidates, + has_pending, + cx, + ) else { + // A unique match was accepted outright; nothing is open to merge into, + // and a static-only slot has no generators anyway. + return; + }; // Kick off each generator on the background executor and merge results // back on the main thread, tagged with this session's generation. @@ -3824,6 +3819,168 @@ impl TerminalView { } } + /// Put `cands` in front of the user: accept a unique match outright, else + /// open the menu over them (filling the longest common prefix first). + /// Returns the opened session's generation, or `None` when a unique match + /// was accepted and no menu exists. + /// + /// `has_pending` means more candidates are still inbound, which disables + /// both shortcuts: a result landing a moment later could add to or change + /// the pick, and mutating the line before then would be jarring. + fn offer_candidates( + &mut self, + line: &str, + word_start: usize, + word_end: usize, + cands: Vec, + has_pending: bool, + cx: &mut Context, + ) -> Option { + if !has_pending && cands.len() == 1 { + let c = cands[0].clone(); + self.completion_insert(&c, c.start); + self.cursor_visible = true; + cx.notify(); + return None; + } + let word: String = line + .chars() + .skip(word_start) + .take(word_end - word_start) + .collect(); + let s = CompletionSession::new(word_start, word.clone(), cands); + if !has_pending + && let Some(lcp) = s.common_prefix() + && lcp.chars().count() > word.chars().count() + { + // Fill the longest common prefix when it extends the typed word. + // All candidates share it, so the fill never invalidates the set. + self.apply_candidate(line, word_start, word_end, &lcp); + } + let generation = self.open_completion(s); + self.cursor_visible = true; + cx.notify(); + Some(generation) + } + + /// The pane's cwd as a path on the *remote* — `Some` only for a native-SSH + /// pane whose remote shell has reported an absolute cwd (OSC 7). Those are + /// the panes tty7 itself dialled, so the daemon holds an authenticated + /// connection we can ask about that filesystem; every other pane kind + /// (a foreground `ssh`, WSL, plain local) answers `None`. + fn remote_ssh_cwd(&self) -> Option { + let remote = self.terminal.remote_context()?; + if remote.kind != crate::daemon::protocol::RemoteKind::NativeSsh { + return None; + } + let cwd = self.cwd()?.to_string_lossy().into_owned(); + cwd.starts_with('/').then_some(cwd) + } + + /// Complete a path against the *remote* filesystem, over the pane's own SSH + /// connection. Returns whether a request went out — the caller then leaves + /// the Tab to us instead of handing the line to the shell. + /// + /// The listing is a daemon round-trip (`SftpList` on the pane's existing + /// authenticated connection — the same channel the SFTP panel browses + /// with), so it cannot answer this keystroke synchronously. Results land on + /// the main thread and only *then* behave as a local Tab would; see + /// [`Self::remote_path_results`] for what happens to a stale or empty one. + /// + /// Out-of-band deliberately. The other way to read a remote directory is to + /// inject a listing command into the live shell and scrape it back out of + /// the PTY — the only option for a terminal that merely *bootstrapped into* + /// a session someone else dialled. tty7 opened this connection itself, so it + /// can just ask: nothing is echoed into the scrollback, no prompt hooks need + /// suppressing, and there's no stray background process to cancel when the + /// user hits Enter. The in-band route stays the answer for the pane kinds + /// that have no tty7-owned connection (a foreground `ssh`, WSL), which is + /// why those decline in [`Self::remote_ssh_cwd`] rather than pretend. + fn spawn_remote_path_completion( + &mut self, + line: &str, + cursor: usize, + forward: bool, + cx: &mut Context, + ) -> bool { + let Some(cwd) = self.remote_ssh_cwd() else { + return false; + }; + let Some(req) = completion::remote_path_request(line, cursor, &cwd) else { + return false; + }; + // A listing for this same keystroke is already on the wire. Swallow the + // repeat rather than dialling again — the answer is about to arrive and + // will open the menu. + if self.remote_completion_inflight { + return true; + } + self.remote_completion_inflight = true; + let pane_id = self.pane_id; + let dir = req.dir.clone(); + let line = line.to_string(); + cx.spawn(async move |this, cx| { + let listed = cx + .background_spawn(async move { RemoteTerminal::sftp_list(pane_id, &dir) }) + .await; + let _ = this.update(cx, |view, cx| { + view.remote_completion_inflight = false; + view.remote_path_results( + req, + &line, + cursor, + listed.unwrap_or_default(), + forward, + cx, + ) + }); + }) + .detach(); + true + } + + /// Land a remote directory listing as completion candidates. + /// + /// Three outcomes, in the order they're checked: + /// - **the line moved on** while the network answered: drop it. The + /// answer describes a word the user is no longer typing, and the Tab + /// that asked for it is long past. + /// - **nothing matched**: fall back to the shell handoff, exactly as a + /// local no-match does (#136). A directory we couldn't read, or a + /// prefix with no entries, is then no worse off than before this + /// existed — the remote's own completion still gets its shot. + /// - **candidates**: offer them like any local Tab. + fn remote_path_results( + &mut self, + req: completion::RemotePathRequest, + line: &str, + cursor: usize, + listed: Vec, + forward: bool, + cx: &mut Context, + ) { + if self.cmd.text() != line || self.cmd.cursor() != cursor { + return; + } + let entries: Vec = listed + .into_iter() + .map(|e| completion::RemoteEntry { + // Follow symlinks when classifying, as the local path engine + // does: a link to a directory takes the trailing `/` and + // survives a dirs-only filter (`cd` into one is routine). The + // daemon resolved the target for us. + is_dir: e.kind == crate::daemon::protocol::SftpEntryKind::Dir || e.target_is_dir, + name: e.name, + }) + .collect(); + let cands = completion::remote_path_candidates(&req, &entries); + if cands.is_empty() { + self.handoff_tab_to_shell(!forward, cx); + return; + } + self.offer_candidates(line, req.word_start, req.cursor, cands, false, cx); + } + /// Open a completion menu and bump the generation tag, returning it so a /// caller spawning generators can stamp their in-flight results. Every open /// gets a fresh generation, so a slow generator from a prior session can't be