From 6edfb282008c69c46c2cd35e99f5c48d563ec099 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 27 Jul 2026 08:21:36 +0800 Subject: [PATCH] fix(terminal): stop a stale hover cell from indexing a shrunken grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hovering a pane records the cell under the pointer so ⌘-hover can underline links. Nothing invalidated that cell when the grid shrank under it — a vertical split, un-maximizing a pane, a smaller window — so the remembered row could name a line the grid no longer has. The next modifier press then walked into it: ModifiersChanged → set_link_modifier → every pane in every tab → link_span_at, which indexed the grid with the stale row. Grid's Index only debug_asserts the bound, so a release build underflows the storage index and panics on the slice check. That panic happens inside gpui's extern "C" input callback, where it cannot unwind — the process aborts, and the OS crash report keeps the abort rather than the panic. Validate the row against the grid before indexing (one choke point, shared by hover and click), and drop the remembered cell on resize. Two more ways the same class of crash reached users: - `new_terminal` unwrapped a failed daemon spawn with `expect`, so a daemon that died took the app down with the next ⌘D/⌘T instead of reporting the failure. It returns a Result now; split, new tab, worktree tab and reopen-closed-tab surface a notification, and session restore drops the leaves it can't rebuild. - A spawn that fails because nothing is listening on the socket now starts a daemon and retries once, the way a disconnect mid-Spawn already did. `connect()` had to stop interpolating the io::Error into a string for that classification to see it. And so the next one is diagnosable at all: a panic hook writes the message, location and backtrace to `crash.log` in the config dir. The GUI has no logger and no stderr when launched from Finder, and the crash report loses the message to the abort. Fixes #201 --- src/core/crash.rs | 139 +++++++++++++++++++++++++++++++++++++++++ src/core/mod.rs | 1 + src/main.rs | 11 ++++ src/terminal/remote.rs | 68 +++++++++++++++++++- src/terminal/view.rs | 115 +++++++++++++++++++++++++++++----- src/ui/app.rs | 105 ++++++++++++++++++++++++------- 6 files changed, 397 insertions(+), 42 deletions(-) create mode 100644 src/core/crash.rs diff --git a/src/core/crash.rs b/src/core/crash.rs new file mode 100644 index 00000000..77948777 --- /dev/null +++ b/src/core/crash.rs @@ -0,0 +1,139 @@ +//! Crash log — the panic message the OS crash reporter throws away. +//! +//! Most tty7 panics happen inside a gpui input callback, and those callbacks are +//! `extern "C"`: the panic can't unwind across them, so the runtime aborts. What +//! macOS then records is the *abort* — `panic_cannot_unwind` on top of +//! `handle_key_event` — with no message, no `file:line`, and the original frames +//! already unwound away. Reports like that are undiagnosable, and the GUI has no +//! logger and no terminal to print to. +//! +//! So we write the two lines that matter (message + location, plus a backtrace) +//! to `crash.log` in the config dir before the process goes down. + +use std::fmt::Write as _; +use std::path::PathBuf; + +/// Rewrite the log once it passes this, so a panic loop can't grow it forever. +const MAX_BYTES: u64 = 256 * 1024; + +/// Install the panic hook for this process. `role` labels the records, since the +/// GUI and the daemon it spawns share one config dir. Chains to the previously +/// installed hook, so the usual stderr output still happens when there's a +/// terminal to see it. +pub fn install(role: &'static str) { + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + record(role, info); + previous(info); + })); +} + +/// Append one record. Every step is best-effort: a panic handler that panics +/// (or fails loudly) is worse than one that loses a log line. +fn record(role: &str, info: &std::panic::PanicHookInfo<'_>) { + let Some(path) = log_path() else { + return; + }; + let thread = std::thread::current(); + let mut record = String::new(); + // `info` renders as "panicked at :\n" — the exact + // pair the crash report is missing. + let _ = write!( + record, + "\n=== {} {} v{} pid {} thread {:?}\n{info}\n{}\n", + utc_timestamp(), + role, + env!("CARGO_PKG_VERSION"), + std::process::id(), + thread.name().unwrap_or(""), + std::backtrace::Backtrace::force_capture(), + ); + append(&path, &record); +} + +fn append(path: &PathBuf, record: &str) { + use std::io::Write as _; + let truncate = std::fs::metadata(path).is_ok_and(|m| m.len() > MAX_BYTES); + let mut file = match std::fs::OpenOptions::new() + .create(true) + .append(!truncate) + .write(true) + .truncate(truncate) + .open(path) + { + Ok(f) => f, + Err(_) => return, + }; + let _ = file.write_all(record.as_bytes()); + let _ = file.flush(); +} + +fn log_path() -> Option { + crate::core::config::config_path("crash.log") +} + +/// `YYYY-MM-DD HH:MM:SS UTC` from the epoch seconds, so a record can be lined up +/// against an OS crash report without pulling in a date crate. +fn utc_timestamp() -> String { + let secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let (days, rem) = (secs / 86_400, secs % 86_400); + let (y, m, d) = civil_from_days(days as i64); + format!( + "{y:04}-{m:02}-{d:02} {:02}:{:02}:{:02} UTC", + rem / 3600, + (rem % 3600) / 60, + rem % 60 + ) +} + +/// Howard Hinnant's `civil_from_days`: days since the Unix epoch → (y, m, d). +fn civil_from_days(z: i64) -> (i64, u32, u32) { + let z = z + 719_468; + let era = z.div_euclid(146_097); + let doe = z.rem_euclid(146_097); + let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; + let y = yoe + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let d = (doy - (153 * mp + 2) / 5 + 1) as u32; + let m = if mp < 10 { mp + 3 } else { mp - 9 } as u32; + (if m <= 2 { y + 1 } else { y }, m, d) +} + +#[cfg(test)] +mod tests { + use super::{civil_from_days, install, log_path}; + + /// The whole point of the hook: after a panic, the message and its location + /// are on disk. `catch_unwind` stands in for the abort — the hook runs + /// before either outcome. + #[test] + fn a_panic_lands_in_the_crash_log() { + // Same pinned temp dir the config tests use (set-once, first call wins). + let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id())); + std::fs::create_dir_all(&dir).ok(); + crate::core::config::set_config_dir(dir); + let path = log_path().expect("a pinned config dir resolves a log path"); + let _ = std::fs::remove_file(&path); + + install("test"); + let _ = std::panic::catch_unwind(|| panic!("crash-log probe")); + + let body = std::fs::read_to_string(&path).expect("the hook wrote a record"); + assert!(body.contains("crash-log probe"), "message: {body}"); + assert!(body.contains("src/core/crash.rs"), "location: {body}"); + assert!(body.contains("test v"), "role + version: {body}"); + } + + #[test] + fn civil_from_days_matches_known_dates() { + assert_eq!(civil_from_days(0), (1970, 1, 1)); + assert_eq!(civil_from_days(20_660), (2026, 7, 26)); + // Leap day, and the day after it. + assert_eq!(civil_from_days(19_782), (2024, 2, 29)); + assert_eq!(civil_from_days(19_783), (2024, 3, 1)); + } +} diff --git a/src/core/mod.rs b/src/core/mod.rs index aa190f83..34583dfe 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -12,6 +12,7 @@ pub mod agent_hooks; pub mod agent_prompt; pub mod cli_agent; pub mod config; +pub mod crash; // SSH connection-manager data layer (WS1). Its public API is consumed by the // daemon-session, auth, forwarding, and UI workstreams, which land separately — // so parts of it read as dead code until those merge. diff --git a/src/main.rs b/src/main.rs index c7bb923d..51efefe7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -298,6 +298,17 @@ fn main() { // under this dir too, so the order matters). apply_config_dir_arg(); + // Panics inside gpui's `extern "C"` input callbacks abort instead of + // unwinding, and the OS crash report then holds the abort rather than the + // panic — no message, no location. Record those to `crash.log` in the config + // dir. Installed here, right after the config dir resolves, so both the GUI + // and the daemon below are covered from their first line of real work. + crate::core::crash::install(if std::env::args().any(|a| a == "--daemon") { + "daemon" + } else { + "gui" + }); + // Daemon mode: when launched with `--daemon` we run the headless persistent // terminal server and never open a window. This is the backing process the GUI // auto-spawns and reconnects to; it owns all PTYs + child shells and outlives diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 5971763a..d16b3db0 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -236,6 +236,24 @@ impl RemoteTerminal { let retry_shell = shell.clone(); match Self::spawn_once(size, cell_w, cell_h, cwd, shell) { Ok(term) => Ok(term), + Err(first_err) if daemon_not_listening(&first_err) => { + // Nothing is on the socket: the daemon died (crash, OOM, a stray + // `kill`) since the last pane was opened. Every later spawn would + // fail the same way, so bring one back up and retry rather than + // leaving the window unable to open another terminal. + if let Err(start_err) = crate::daemon::spawn::ensure_running() { + return Err(anyhow::anyhow!( + "daemon not running ({first_err}); starting one failed: {start_err}" + )); + } + Self::spawn_once(size, cell_w, cell_h, retry_cwd, retry_shell).map_err( + |second_err| { + anyhow::anyhow!( + "daemon not running ({first_err}); started one but Spawn still failed: {second_err}" + ) + }, + ) + } Err(first_err) if daemon_disconnected_before_spawn_reply(&first_err) => { // A live-but-old daemon can accept the connection, panic while // handling Spawn, and close before replying. Restart once so an @@ -1430,6 +1448,21 @@ fn record_mark(term: &Term, marks: &crate::terminal::marks::Marks, e } } +/// Whether the failure is "nothing is listening on the socket" — the daemon is +/// gone, as opposed to alive but unhappy. On Unix a dead daemon leaves the +/// socket file behind (`ConnectionRefused`) or removed it on the way out +/// (`NotFound`); on Windows the named pipe simply isn't there (`NotFound`). +fn daemon_not_listening(err: &anyhow::Error) -> bool { + err.chain().any(|cause| { + cause.downcast_ref::().is_some_and(|io| { + matches!( + io.kind(), + std::io::ErrorKind::ConnectionRefused | std::io::ErrorKind::NotFound + ) + }) + }) +} + fn daemon_disconnected_before_spawn_reply(err: &anyhow::Error) -> bool { err.chain().any(|cause| { cause.downcast_ref::().is_some_and(|io| { @@ -1604,10 +1637,14 @@ fn parse_osc_notification(payload: &[u8]) -> Option<(Option, String)> { /// isolation (dev vs. real config dir), exactly like every other config-dir file. fn connect() -> anyhow::Result { transport::connect().map_err(|e| { - anyhow::anyhow!( - "connect to daemon at {}: {e}", + // `context`, not a formatted `anyhow!`: callers classify the failure by + // downcasting to `io::Error` (see `daemon_not_listening`), and + // interpolating the cause into a string would leave the chain with + // nothing to find. + anyhow::Error::new(e).context(format!( + "connect to daemon at {}", transport::endpoint_display() - ) + )) }) } @@ -1742,6 +1779,31 @@ mod tests { assert!(!daemon_disconnected_before_spawn_reply(&refused)); } + /// A dead daemon is the one failure the client can fix by itself, and it + /// must be told apart from a live daemon saying no — restarting on *that* + /// would kill every running pane over a bad shell setting. + #[test] + fn only_a_dead_daemon_is_worth_starting_one_for() { + let connect_failed = |kind| -> anyhow::Error { + anyhow::Error::new(std::io::Error::new(kind, "no listener")) + .context("connect to daemon at /tmp/tty7.sock") + }; + assert!(daemon_not_listening(&connect_failed( + std::io::ErrorKind::ConnectionRefused + ))); + assert!(daemon_not_listening(&connect_failed( + std::io::ErrorKind::NotFound + ))); + + // A daemon that answered and refused, and one that hung up mid-Spawn: + // neither is "not running", and each has its own recovery. + let refused = anyhow::anyhow!("daemon refused Spawn: configured shell missing"); + assert!(!daemon_not_listening(&refused)); + let eof: anyhow::Error = + std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "closed").into(); + assert!(!daemon_not_listening(&eof)); + } + /// Without a real daemon, drive the reader path directly: a `UnixStream::pair` /// stands in for the connection. We hand `RemoteTerminal` one half (as if it /// were the attach'd socket) and push framed `DaemonMsg`s down the other, then diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 36b78b74..cff8f9a7 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -93,6 +93,18 @@ pub struct NativeSshParts { persist: Box, } +/// An established shell daemon pane (fresh spawn or re-attach), ready to be +/// wrapped in a view: the output of the fallible +/// [`TerminalView::spawn_shell_terminal`], consumed by the infallible +/// [`TerminalView::from_shell_parts`]. +pub struct ShellParts { + terminal: RemoteTerminal, + pane_id: u64, + /// The explicit shell pick this pane was spawned with, if any; `None` for a + /// re-attached pane (the pick isn't persisted). + shell_spec: Option, +} + /// See `TerminalView::drag_scroll`. #[derive(Clone, Copy)] struct DragScroll { @@ -816,19 +828,25 @@ fn fallback_chain(family: &str, configured: &[String]) -> Vec { } impl TerminalView { - pub fn new( + /// The fallible half of a shell-backed view: establish the daemon pane + /// *before* the view is built, so a refused spawn (daemon down, spawn + /// error) comes back as an `Err` the caller can report. Splitting it out + /// matters beyond tidiness: the view is constructed inside `cx.new`, deep + /// under gpui's `extern "C"` input callbacks, where a panic can't unwind + /// and aborts the process instead. Mirrors + /// [`Self::spawn_native_ssh_terminal`]. + /// + /// Provisional size; corrected on the first prepaint once we can measure. + /// The PTY lives in the daemon now. On session restore (`restore_pane`), + /// re-`attach` to the still-running pane so its process + scrollback come + /// back intact; otherwise `spawn` a fresh pane (with the caller's shell + /// pick, if any). The caller only passes a `restore_pane` it has already + /// confirmed alive, so we trust it here. + pub fn spawn_shell_terminal( working_directory: Option, restore_pane: Option, shell: Option, - window: &mut Window, - cx: &mut Context, - ) -> anyhow::Result { - // Provisional size; corrected on the first prepaint once we can measure. - // The PTY lives in the daemon now. On session restore (`restore_pane`), - // re-`attach` to the still-running pane so its process + scrollback come - // back intact; otherwise `spawn` a fresh pane (with the caller's shell - // pick, if any). The caller only passes a `restore_pane` it has already - // confirmed alive, so we trust it here. + ) -> anyhow::Result { let (terminal, pane_id, shell_spec) = match restore_pane { Some(id) => ( RemoteTerminal::attach(TermSize::new(80, 24), 8, 17, id)?, @@ -848,9 +866,23 @@ impl TerminalView { (terminal, id, shell) } }; - let mut view = Self::with_terminal(terminal, pane_id, window, cx); - view.shell_spec = shell_spec; - Ok(view) + Ok(ShellParts { + terminal, + pane_id, + shell_spec, + }) + } + + /// Wrap an established shell pane (from [`Self::spawn_shell_terminal`]) in + /// a view. Infallible by construction — see that function. + pub fn from_shell_parts( + parts: ShellParts, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let mut view = Self::with_terminal(parts.terminal, parts.pane_id, window, cx); + view.shell_spec = parts.shell_spec; + view } /// Spawn a native (russh) SSH pane for `spec` and build the view around it @@ -1160,6 +1192,14 @@ impl TerminalView { cell_width: Pixels, line_height: Pixels, ) { + // A remembered hover cell describes the *old* geometry: after a resize + // its row may not exist any more, and the pointer sits over a different + // cell regardless. Forget it — the next mouse move records a fresh one. + // (`grid_line` also refuses a stale row, so this is about not underlining + // the wrong cell, not about safety.) + if (cols, rows) != (self.terminal.size().cols, self.terminal.size().rows) { + self.last_hover_cell = None; + } self.cell_width = cell_width; self.line_height = line_height; self.terminal.resize( @@ -4335,6 +4375,23 @@ impl TerminalView { } } + /// The grid line a screen `row` currently maps to, or `None` when that row + /// is outside the grid. + /// + /// Mandatory before indexing: `Grid`'s `Index` only `debug_assert`s + /// the bound, so a release build walks off the storage and panics on the + /// slice check instead. A remembered cell goes stale whenever the grid + /// shrinks under it — split a pane, drag the window smaller — and the + /// callers here run inside gpui's `extern "C"` input callbacks, where that + /// panic can't unwind and aborts the process. + fn grid_line( + term: &alacritty_terminal::Term, + row: usize, + ) -> Option { + let line = Line(row as i32 - term.grid().display_offset() as i32); + (line >= term.topmost_line() && line <= term.bottommost_line()).then_some(line) + } + /// Open the link under the given cell, if any (OSC 8 hyperlink, plain URL or /// existing file or directory path detected in the row text). Returns true if one opened. pub fn open_link_at(&self, col: usize, row: usize, cx: &mut Context) -> bool { @@ -4342,8 +4399,9 @@ impl TerminalView { return false; } let term = self.terminal.term.lock(); - let display_offset = term.grid().display_offset() as i32; - let line = Line(row as i32 - display_offset); + let Some(line) = Self::grid_line(&term, row) else { + return false; + }; let cols = term.columns(); if col >= cols { return false; @@ -4490,8 +4548,7 @@ impl TerminalView { include_loopback: bool, ) -> Option { let term = self.terminal.term.lock(); - let display_offset = term.grid().display_offset() as i32; - let line = Line(row as i32 - display_offset); + let line = Self::grid_line(&term, row)?; let cols = term.columns(); if col >= cols { return None; @@ -6790,6 +6847,30 @@ mod gpui_tests { (window, daemon_side) } + /// A hover cell remembered while the pane was tall names a row the grid no + /// longer has once the pane shrinks (a vertical split, a smaller window). + /// Resolving it must decline rather than index the grid — this path runs + /// from `ModifiersChanged` (the ⌘ of the very ⌘⇧D that split the pane), an + /// `extern "C"` callback where the panic can't unwind and aborts the app. + #[gpui::test] + fn a_stale_hover_row_does_not_index_the_shrunken_grid(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, _, cx| { + // Hover the last row of the 24-row grid, then shrink to 8 rows. + view.hover_link_at(0, 23, true, cx); + view.terminal.resize(TermSize::new(80, 8), 8, 17); + // `set_grid_size` drops the stale cell in the real app; pin it + // here so the guard inside the lookup is what's under test. + view.last_hover_cell = Some((0, 23)); + assert!( + !view.refresh_link_hover(true, cx), + "a row outside the grid can't hold a link" + ); + }) + .unwrap(); + } + #[gpui::test] fn title_events_drive_the_tab_title(cx: &mut TestAppContext) { let (window, _daemon) = harness(cx); diff --git a/src/ui/app.rs b/src/ui/app.rs index 6772af2c..5bd6fa3b 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -843,10 +843,16 @@ impl Tty7App { // First run (no session file): the very first terminal has no // predecessor to inherit from, so start in the app's current // directory (None → default behavior). - None => { - let first = new_terminal(font_size, None, None, None, window, cx); - (vec![Tab::new(Pane::leaf(first))], 0) - } + None => match new_terminal(font_size, None, None, None, window, cx) { + Ok(first) => (vec![Tab::new(Pane::leaf(first))], 0), + // The daemon we just tried to start isn't answering. A window + // with no tabs is a legal state (it shows the home page), and + // far better than taking the launch down over it. + Err(e) => { + log::error!("first terminal failed to start: {e}"); + (Vec::new(), 0) + } + }, // A saved session (with tabs, or an empty home-page state): rebuild it // the same way a daemon restart does. some => tabs_from_session(some, font_size, window, cx), @@ -1265,7 +1271,13 @@ impl Tty7App { return; }; let alive = alive_panes(); - let pane = session_to_pane(&st.pane, &alive, self.font_size, window, cx); + let Some(pane) = session_to_pane(&st.pane, &alive, self.font_size, window, cx) else { + // Nothing came back (an unreachable daemon). Put the entry back so + // the tab is still reopenable once the daemon is up again. + window.push_notification("Could not reopen the tab: no terminal started", cx); + self.closed.push(st); + return; + }; // Leaving the current tab for the reopened one; snapshot its focused // pane so switching back restores it (same as `activate`). self.remember_active_pane(window, cx); @@ -2712,7 +2724,14 @@ impl Tty7App { .focused_or_first(window, cx) .and_then(|leaf| leaf.read(cx).local_cwd()) }); - let tab = new_terminal(self.font_size, cwd, None, shell, window, cx); + let tab = match new_terminal(self.font_size, cwd, None, shell, window, cx) { + Ok(view) => view, + Err(e) => { + log::error!("new tab spawn failed: {e}"); + window.push_notification(format!("Could not open a terminal: {e}"), cx); + return; + } + }; // Leaving the current tab for the new one; snapshot its focused pane // so switching back restores it (same as `activate`). self.remember_active_pane(window, cx); @@ -2826,7 +2845,14 @@ impl Tty7App { } } else { let shell = target.read(cx).shell_spec(); - new_terminal(self.font_size, cwd, None, shell, window, cx) + match new_terminal(self.font_size, cwd, None, shell, window, cx) { + Ok(view) => view, + Err(e) => { + log::error!("split spawn failed: {e}"); + window.push_notification(format!("Could not split the pane: {e}"), cx); + return; + } + } }; if let Some(tab) = self.tabs.get_mut(self.active) { if tab.pane.split_leaf(&target, axis, new.clone()) { @@ -3368,7 +3394,14 @@ impl Tty7App { window: &mut Window, cx: &mut Context, ) { - let view = new_terminal(self.font_size, Some(wt.path), None, None, window, cx); + let view = match new_terminal(self.font_size, Some(wt.path), None, None, window, cx) { + Ok(view) => view, + Err(e) => { + log::error!("worktree tab spawn failed: {e}"); + window.push_notification(format!("Could not open a terminal: {e}"), cx); + return; + } + }; self.remember_active_pane(window, cx); self.maximized = None; let insert_at = self.new_tab_insert_at(cx); @@ -5693,7 +5726,12 @@ fn tabs_from_session( let alive = alive_panes(); let mut tabs: Vec = Vec::with_capacity(session.tabs.len()); for st in &session.tabs { - let pane = session_to_pane(&st.pane, &alive, font_size, window, cx); + // A tab whose every leaf failed to come back has nothing to show; drop + // it rather than restore an empty frame (or, worse, abort the launch). + let Some(pane) = session_to_pane(&st.pane, &alive, font_size, window, cx) else { + log::error!("dropping a restored tab: no pane in it could be started"); + continue; + }; tabs.push(Tab { pane, name: st.name.clone(), @@ -5707,8 +5745,9 @@ fn tabs_from_session( sidebar_group: std::cell::RefCell::new(st.sidebar_group.clone()), }); } - // Clamp the saved active index into the rebuilt range. - let active = session.active.min(tabs.len() - 1); + // Clamp the saved active index into the rebuilt range (which can be empty + // when nothing restored). + let active = session.active.min(tabs.len().saturating_sub(1)); (tabs, active) } @@ -5716,13 +5755,17 @@ fn tabs_from_session( /// `pane_id` is still alive in the daemon re-`attach`es (process + scrollback /// intact); otherwise it spawns a fresh shell in the saved cwd. `alive` is the /// daemon's current pane set, computed once by the caller. +/// +/// `None` when nothing under this node could be started (an unreachable +/// daemon): restore drops what it can't rebuild instead of leaving `Empty` +/// nodes — which every tree operation ignores — in a live tab. fn session_to_pane( sp: &SessionPane, alive: &std::collections::HashSet, font_size: f32, window: &mut Window, cx: &mut Context, -) -> Pane { +) -> Option { match sp { SessionPane::Leaf { cwd, @@ -5743,7 +5786,7 @@ fn session_to_pane( if let Some(spec) = ssh_spec.clone() { let resolved = crate::ui::ssh_connect::resolve_persisted_ssh_spec(spec, cx); match new_terminal_native(font_size, cwd.clone(), resolved, window, cx) { - Ok(view) => return Pane::leaf(view), + Ok(view) => return Some(Pane::leaf(view)), // Keep restore alive: fall through to a local shell in // this slot rather than aborting startup. Err(e) => log::error!("restoring native SSH pane failed: {e}"), @@ -5752,7 +5795,13 @@ fn session_to_pane( } // A shell pick isn't persisted in the session, so a stale pane that // must respawn comes back on the default shell. - let view = new_terminal(font_size, cwd.clone(), restore, None, window, cx); + let view = match new_terminal(font_size, cwd.clone(), restore, None, window, cx) { + Ok(view) => view, + Err(e) => { + log::error!("restoring pane failed: {e}"); + return None; + } + }; // A pane that could NOT re-attach lost its running agent with the // daemon; when we captured that agent's native session id, hand // the fresh shell its resume command so the conversation picks up @@ -5766,20 +5815,32 @@ fn session_to_pane( { view.read(cx).run_command_line(&cmd); } - Pane::leaf(view) + Some(Pane::leaf(view)) } SessionPane::Split { axis, ratio, a, b } => { let axis = match axis { SessionAxis::Horizontal => Axis::Horizontal, SessionAxis::Vertical => Axis::Vertical, }; - let a = session_to_pane(a, alive, font_size, window, cx); - let b = session_to_pane(b, alive, font_size, window, cx); - Pane::split_node(axis, *ratio, a, b) + // One side failing collapses the split onto the survivor, exactly + // as closing that pane by hand would. + match ( + session_to_pane(a, alive, font_size, window, cx), + session_to_pane(b, alive, font_size, window, cx), + ) { + (Some(a), Some(b)) => Some(Pane::split_node(axis, *ratio, a, b)), + (Some(only), None) | (None, Some(only)) => Some(only), + (None, None) => None, + } } } } +/// Build a shell-backed terminal view, wiring the per-pane subscriptions every +/// pane needs. Fallible: the daemon can refuse the spawn (it died, it's +/// wedged, the shell doesn't exist), and every caller here runs inside a gpui +/// input callback, where a panic can't unwind and would abort the app instead +/// of surfacing the failure. Report it, don't `expect` it. fn new_terminal( font_size: f32, working_directory: Option, @@ -5787,10 +5848,10 @@ fn new_terminal( shell: Option, window: &mut Window, cx: &mut Context, -) -> Entity { +) -> anyhow::Result> { + let parts = TerminalView::spawn_shell_terminal(working_directory, restore_pane, shell)?; let view = cx.new(|cx| { - let mut view = TerminalView::new(working_directory, restore_pane, shell, window, cx) - .expect("failed to start terminal"); + let mut view = TerminalView::from_shell_parts(parts, window, cx); // Inherit the current global font size so new panes match existing ones. view.font_size = px(font_size); view @@ -5816,7 +5877,7 @@ fn new_terminal( ) .detach(); watch_pane_focus(&view, window, cx); - view + Ok(view) } /// Re-render the app whenever `view` takes focus. Nothing else does this: a