diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a9095d5..60343854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Typing `exit` (or Ctrl-D) left a dead "process exited" pane behind instead + of closing it. A pane whose shell genuinely ends now closes itself — + collapsing its split, or closing the tab when it was the only pane (the + last tab falls back to the home page), like every other terminal. A pane + that merely *lost its daemon connection* still stays visible: auto-closing + those would silently discard — and kill — sessions that may still be alive + daemon-side. Panes that died while detached clean themselves up on the next + attach the same way. + - A full-screen TUI dying without restoring the terminal — the canonical case being an ssh session dropping mid-`htop`/`vim` — left the pane stranded on the alt screen with a hidden cursor and live mouse reporting: a visible diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 77758b18..a13b2c8f 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -86,6 +86,17 @@ struct ShellState { last_exit: Option, } +/// The shared handles the reader thread writes into as daemon frames arrive; +/// `RemoteTerminal` keeps the other ends for the view to read. Bundled so +/// `spawn_reader`'s signature stays readable as signals accrue. +struct ReaderSignals { + cwd: Arc>>, + shell: Arc>, + exited: Arc, + child_exited: Arc, + zle_reading: Arc, +} + /// A terminal whose PTY lives in the daemon. Mirrors `backend::Terminal`'s public /// surface so the view can treat the two interchangeably. pub struct RemoteTerminal { @@ -117,6 +128,14 @@ pub struct RemoteTerminal { /// Set true by the reader thread once the child exits or the daemon /// disconnects. `poll_exited()` copies this into the `exited` field. exited_flag: Arc, + /// Set true only on a *genuine* child exit (`DaemonMsg::Exited` — the + /// shell ended: `exit`, Ctrl-D, a crash), never on a daemon disconnect or + /// protocol desync, which also flip `exited_flag`. The distinction gates + /// pane auto-close: a pane whose shell ended closes itself, while a pane + /// that merely lost its connection stays visible (auto-closing it would + /// silently discard — and `close_tab` would try to kill — a session that + /// may still be alive daemon-side). + child_exited: Arc, /// Whether zle is reading the keyboard right now, sniffed client-side from /// *live* OSC 133 marks: `B` (prompt end — zle takes over immediately /// after) arms it, any other mark disarms it, and Snapshot replays never @@ -202,16 +221,20 @@ impl RemoteTerminal { let cwd: Arc>> = Arc::new(Mutex::new(None)); let shell_state: Arc> = Arc::new(Mutex::new(ShellState::default())); let exited_flag = Arc::new(AtomicBool::new(false)); + let child_exited = Arc::new(AtomicBool::new(false)); let zle_reading = Arc::new(AtomicBool::new(false)); let reader_thread = Self::spawn_reader( term.clone(), proxy, read_half, - cwd.clone(), - shell_state.clone(), - exited_flag.clone(), - zle_reading.clone(), + ReaderSignals { + cwd: cwd.clone(), + shell: shell_state.clone(), + exited: exited_flag.clone(), + child_exited: child_exited.clone(), + zle_reading: zle_reading.clone(), + }, ); Ok(Self { @@ -225,6 +248,7 @@ impl RemoteTerminal { cwd, shell_state, exited_flag, + child_exited, zle_reading, reader_thread: Some(reader_thread), }) @@ -246,14 +270,18 @@ impl RemoteTerminal { term: Arc>>, proxy: EventProxy, read_half: Stream, - cwd: Arc>>, - shell: Arc>, - exited_flag: Arc, - zle_reading: Arc, + signals: ReaderSignals, ) -> JoinHandle<()> { std::thread::Builder::new() .name("tty7-remote-reader".to_string()) .spawn(move || { + let ReaderSignals { + cwd, + shell, + exited: exited_flag, + child_exited, + zle_reading, + } = signals; // The client end of the visible-output path: keep it off the // efficiency cores (see `core::threads`). crate::core::threads::promote_to_user_interactive(); @@ -480,7 +508,13 @@ impl RemoteTerminal { // Child gone: apply what it printed last, then // mark the emulator exited and flip the shared // flag so the next `poll_exited()` surfaces it. + // This is the one exit path where the child + // *really* ended (vs the connection dying), so + // record that before the teardown's events fire + // — the view reads it to decide whether the + // pane should close itself. flush_batch!(); + child_exited.store(true, Ordering::SeqCst); teardown(); break 'main; } @@ -581,6 +615,12 @@ impl RemoteTerminal { } } + /// Whether the pane's child process genuinely exited (as opposed to the + /// daemon connection dropping — see the `child_exited` field docs). + pub fn child_exited(&self) -> bool { + self.child_exited.load(Ordering::SeqCst) + } + /// Send raw bytes (keyboard input, pasted text, query replies) to the pane as /// a `ClientMsg::Input` frame. Mirrors `Terminal::write`'s signature exactly. pub fn write>>(&self, bytes: B) { @@ -965,6 +1005,48 @@ mod tests { assert!(term.exited_flag.load(Ordering::SeqCst)); } + /// A `DaemonMsg::Exited` frame (the child really ended) must set + /// `child_exited`; a bare daemon disconnect (EOF) must not — both flip + /// `exited_flag`. The distinction is what keeps pane auto-close from + /// firing on a lost connection and destroying a session that may still be + /// alive daemon-side. + #[test] + fn child_exit_is_distinguished_from_daemon_disconnect() { + // A genuine child exit: the daemon reports it explicitly. + let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); + DaemonMsg::Exited { code: Some(0) } + .encode(&mut daemon_side) + .unwrap(); + for _ in 0..200 { + if term.exited_flag.load(Ordering::SeqCst) { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(term.exited_flag.load(Ordering::SeqCst)); + assert!( + term.child_exited(), + "an Exited frame is a genuine child exit" + ); + + // A daemon disconnect: the socket just closes. + let (client_side, daemon_side) = UnixStream::pair().unwrap(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); + drop(daemon_side); + for _ in 0..200 { + if term.exited_flag.load(Ordering::SeqCst) { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(term.exited_flag.load(Ordering::SeqCst)); + assert!( + !term.child_exited(), + "a disconnect is not a child exit — auto-close must not fire" + ); + } + /// `stale_mode_resets` maps each residue bit to its reset — and nothing /// more. The guards matter as much as the resets: `?1049l` on a grid that /// is *not* on the alt screen performs a cursor restore, so a clean (or diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 1d6e06c2..f84d26f7 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -43,6 +43,15 @@ actions!( ] ); +/// Emitted when the pane's child process has genuinely exited (`exit`, +/// Ctrl-D, a crashed shell) — as opposed to the daemon connection dropping, +/// which keeps the dead pane visible. `Tty7App` subscribes (see +/// `new_terminal`) and closes the pane in response: collapsing its split, or +/// closing the tab when it was the only pane. +pub struct ChildExited; + +impl gpui::EventEmitter for TerminalView {} + pub struct TerminalView { pub terminal: RemoteTerminal, /// Daemon-assigned id of the pane this view mirrors. Persisted in the session @@ -619,6 +628,14 @@ impl TerminalView { AlacEvent::ChildExit(_) | AlacEvent::Exit => { self.terminal.exited = true; self.title = "tty7 — process exited".to_string(); + // A genuine child exit closes the pane (the app subscribes and + // collapses the split / closes the tab). A daemon disconnect + // reaches this same arm but must NOT auto-close: the session + // may still be alive daemon-side, and closing would both hide + // the failure and kill the pane. + if self.terminal.child_exited() { + cx.emit(ChildExited); + } cx.notify(); } AlacEvent::ClipboardStore(_, text) => { @@ -3968,6 +3985,60 @@ mod gpui_tests { ); } + /// A genuine child exit (`DaemonMsg::Exited`) must surface as a + /// `ChildExited` gpui event — the app's cue to close the pane/tab (the + /// "typing `exit` leaves a dead pane behind" bug). A daemon disconnect + /// marks the view exited through the same `AlacEvent::Exit` arm but must + /// emit nothing: auto-closing on a lost connection would silently discard + /// (and kill) a pane that may still be alive daemon-side. + #[gpui::test] + fn child_exit_emits_the_close_event_but_disconnect_does_not(cx: &mut TestAppContext) { + use std::cell::Cell; + use std::rc::Rc; + + let subscribe = |window: &gpui::WindowHandle, cx: &mut TestAppContext| { + let got = Rc::new(Cell::new(false)); + let seen = got.clone(); + window + .update(cx, |_, _, cx| { + let this = cx.entity(); + cx.subscribe(&this, move |_, _, _: &ChildExited, _| seen.set(true)) + .detach(); + }) + .unwrap(); + got + }; + let wait_exited = |window: &gpui::WindowHandle, cx: &mut TestAppContext| { + for _ in 0..400 { + cx.run_until_parked(); + let exited = window + .update(cx, |view, _, _| view.terminal.exited) + .unwrap(); + if exited { + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + panic!("the view never noticed the exit"); + }; + + // The child really exits: the daemon says so. + let (window, mut daemon) = harness(cx); + let got = subscribe(&window, cx); + DaemonMsg::Exited { code: Some(0) } + .encode(&mut daemon) + .unwrap(); + wait_exited(&window, cx); + assert!(got.get(), "a genuine child exit must emit ChildExited"); + + // The connection just drops. + let (window, daemon) = harness(cx); + let got = subscribe(&window, cx); + drop(daemon); + wait_exited(&window, cx); + assert!(!got.get(), "a daemon disconnect must not emit ChildExited"); + } + /// Regression for the "cursor vanishes after an ssh session dies mid-TUI" /// bug. Over ssh, a remote full-screen TUI entered the alt screen and hid /// the cursor (`\e[?1049h\e[?25l`). The network then drops: the restore diff --git a/src/ui/app.rs b/src/ui/app.rs index e9983972..ca155bd5 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -14,7 +14,7 @@ use gpui_component::{ActiveTheme as _, IndexPath, TitleBar}; use crate::core::actions::*; use crate::core::config::{Config, NewTabPosition, ShellConfig, color_or, hsla_to_hex6}; use crate::core::session::{Session, SessionAxis, SessionPane, SessionTab}; -use crate::terminal::view::TerminalView; +use crate::terminal::view::{ChildExited, TerminalView}; use crate::ui::palette::{Command, CommandKind, PaletteEvent, PaletteView}; use crate::ui::pane::{CloseOutcome, Pane}; use crate::ui::settings::{ColorKey, SettingsSection, SettingsState}; @@ -746,6 +746,46 @@ impl Tty7App { } } + /// Close the pane whose shell just exited on its own (`ChildExited` from + /// the view — `exit`, Ctrl-D, a crashed shell): collapse its split, or + /// close its tab when it was the only pane. Unlike `close_pane` this + /// targets the *emitting* leaf, not the focused one — the exit can happen + /// in a background tab. The daemon pane is killed even though its child is + /// already dead: the daemon still lists it for reattach, and killing is + /// what drops it from the session. + fn on_child_exited( + &mut self, + view: Entity, + window: &mut Window, + cx: &mut Context, + ) { + let id = view.entity_id(); + let Some(index) = self + .tabs + .iter() + .position(|tab| tab.pane.leaves().iter().any(|l| l.entity_id() == id)) + else { + return; // already closed (e.g. by the user racing the exit) + }; + match self.tabs[index].pane.close_leaf(&view) { + // The exited pane was the tab's only leaf: close the whole tab + // (which snapshots it for reopen and kills its daemon panes). + CloseOutcome::RemoveSelf => self.close_tab(index, window, cx), + // Unreachable — containment was just checked — but never close a + // tab we failed to locate the leaf in. + CloseOutcome::NotFound => {} + CloseOutcome::Collapsed => { + crate::terminal::RemoteTerminal::kill_pane(view.read(cx).pane_id); + if index == self.active { + self.maximized = None; + self.focus_active(window, cx); + } + self.save_session(cx); + cx.notify(); + } + } + } + /// Cycle focus among the panes of the active tab. fn cycle_pane(&mut self, forward: bool, window: &mut Window, cx: &mut Context) { // `leaves()` returns owned clones, so the immutable borrow of `self.tabs` @@ -1813,11 +1853,21 @@ fn new_terminal( window: &mut Window, cx: &mut Context, ) -> Entity { - cx.new(|cx| { + let view = cx.new(|cx| { let mut view = TerminalView::new(working_directory, restore_pane, window, cx) .expect("failed to start terminal"); // Inherit the current global font size so new panes match existing ones. view.font_size = px(font_size); view + }); + // A pane whose shell exits on its own (`exit`, Ctrl-D, a crash) closes + // itself, like every other terminal. This is the single place all panes + // are built — new tab, split, session restore — so the subscription + // covers them all; restore even cleans up panes that died while no + // client was attached (the daemon replays their exit on reattach). + cx.subscribe_in(&view, window, |app, view, _: &ChildExited, window, cx| { + app.on_child_exited(view.clone(), window, cx); }) + .detach(); + view } diff --git a/src/ui/pane.rs b/src/ui/pane.rs index d601fcae..631c16b9 100644 --- a/src/ui/pane.rs +++ b/src/ui/pane.rs @@ -213,6 +213,14 @@ impl Pane> { self.close_leaf_where(&|v| v.read(cx).focus_handle.contains_focused(window, cx)) } + /// Remove a specific leaf (matched by entity identity), collapsing its + /// parent split into the sibling. Used when a pane closes for a reason + /// other than user focus — its child exited on its own — so the leaf to + /// remove is the exited one, wherever focus happens to be. + pub fn close_leaf(&mut self, target: &Entity) -> CloseOutcome { + self.close_leaf_where(&|v| v.entity_id() == target.entity_id()) + } + /// Render the subtree. `show_focus` draws a focus ring on the active leaf /// (suppressed when the tab has a single pane). pub fn render(&self, show_focus: bool, window: &mut Window, cx: &mut App) -> gpui::AnyElement {