diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index 797c5ec0..8daa3486 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -346,6 +346,7 @@ fn new_workspace(path: Option, open: bool, backend: &mut dyn Backend) -> cwd: path, ssh_spec: None, agent: None, + shell: None, }, tab: None, })?; @@ -418,6 +419,7 @@ fn run(args: RunArgs, ctx: &Context, backend: &mut dyn Backend) -> Result Resu cwd, ssh_spec: None, agent: None, + shell: None, }, first: false, })?; @@ -595,6 +598,7 @@ fn tab_new( cwd, ssh_spec: None, agent: None, + shell: None, }, tab: None, })? { @@ -1397,6 +1401,7 @@ mod tests { cwd: Some("C:\\newproj".into()), ssh_spec: None, agent: None, + shell: None, }, tab: None, }, @@ -1635,6 +1640,7 @@ mod tests { cwd: Some("C:\\elsewhere".into()), ssh_spec: None, agent: None, + shell: None, }, tab: None, } @@ -1668,6 +1674,7 @@ mod tests { cwd: Some("C:\\proj".into()), ssh_spec: None, agent: None, + shell: None, }, first: false, }, @@ -1898,6 +1905,7 @@ mod tests { cwd: Some("C:\\proj".into()), ssh_spec: None, agent: None, + shell: None, }, tab: None, }, diff --git a/crates/tty7-cli/src/testbed.rs b/crates/tty7-cli/src/testbed.rs index 71894e13..adb7d3c6 100644 --- a/crates/tty7-cli/src/testbed.rs +++ b/crates/tty7-cli/src/testbed.rs @@ -47,6 +47,7 @@ pub fn two_workspace_machine() -> Machine { title: String::new(), ssh_spec: None, agent: None, + shell: None, live: true, }; Machine { diff --git a/crates/tty7-core/src/core/machine.rs b/crates/tty7-core/src/core/machine.rs index db0ca399..67c2526e 100644 --- a/crates/tty7-core/src/core/machine.rs +++ b/crates/tty7-core/src/core/machine.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use crate::core::cli_agent::CLIAgent; use crate::core::session::WorkspaceId; -use crate::daemon::protocol::NativeSshSpec; +use crate::daemon::protocol::{NativeSshSpec, ShellSpec}; pub const MACHINE_FILE: &str = "machine.json"; @@ -276,6 +276,15 @@ pub struct PaneRecord { pub ssh_spec: Option>, #[serde(default)] pub agent: Option, + /// What the pane is actually running, resolved: the spawn's override if it + /// had one, otherwise the shell the config named at the time. + /// + /// Without it a pane rebuilt from this tree comes back on whatever the + /// default shell is now, so a daemon restart silently turns a bash pane + /// into a PowerShell one. The rest of the record describes where a pane is + /// and what is running in it; this is the part that says what it *is*. + #[serde(default)] + pub shell: Option, #[serde(default)] pub live: bool, } @@ -288,6 +297,7 @@ impl PaneRecord { title: String::new(), ssh_spec: None, agent: None, + shell: None, live: false, } } @@ -313,6 +323,11 @@ pub struct PaneSeed { pub ssh_spec: Option>, #[serde(default)] pub agent: Option, + /// See [`PaneRecord::shell`]. Carried here too so a pane that reaches the + /// tree as a seed — a split, a `tty7` CLI call — names its shell from the + /// start rather than only once the daemon has observed it. + #[serde(default)] + pub shell: Option, } impl PaneSeed { @@ -322,6 +337,7 @@ impl PaneSeed { cwd: None, ssh_spec: None, agent: None, + shell: None, } } @@ -332,6 +348,7 @@ impl PaneSeed { title: String::new(), ssh_spec: self.ssh_spec.map(|s| Box::new(s.without_secrets())), agent: self.agent, + shell: self.shell, live, } } @@ -1364,6 +1381,7 @@ mod tests { cwd: Some(cwd.to_string()), ssh_spec: None, agent: None, + shell: None, } } diff --git a/crates/tty7-core/src/core/session.rs b/crates/tty7-core/src/core/session.rs index 0118858c..3c0e5046 100644 --- a/crates/tty7-core/src/core/session.rs +++ b/crates/tty7-core/src/core/session.rs @@ -17,6 +17,12 @@ pub enum SessionPane { cwd: Option, #[serde(default)] pane_id: Option, + /// The shell this pane was running. Without it a pane that has to be + /// respawned — its daemon restarted, or this is a cold start — comes + /// back on whatever the default shell is, which is how a bash pane + /// turns into a PowerShell one. + #[serde(default)] + shell: Option, #[serde(default)] ssh_spec: Option>, #[serde(default)] diff --git a/crates/tty7-core/src/daemon/handoff.rs b/crates/tty7-core/src/daemon/handoff.rs index b608dd9f..e82db450 100644 --- a/crates/tty7-core/src/daemon/handoff.rs +++ b/crates/tty7-core/src/daemon/handoff.rs @@ -83,6 +83,8 @@ struct PaneRecord { integration_dir: Option, size: WinSize, cwd: Option, + #[serde(default)] + shell: Option, shell_active: bool, at_prompt: bool, last_exit: Option, @@ -227,6 +229,7 @@ fn stage(panes: &[Carried], next_pane_id: u64) -> std::io::Result integration_dir: pane.integration_dir.clone(), size: pane.size, cwd: pane.cwd.clone(), + shell: pane.shell_spec.clone(), shell_active: pane.shell_active, at_prompt: pane.at_prompt, last_exit: pane.last_exit, @@ -358,6 +361,7 @@ pub fn adopt(fd: RawFd) -> Option { size: record.size, ring, cwd: record.cwd, + shell_spec: record.shell, shell_active: record.shell_active, at_prompt: record.at_prompt, last_exit: record.last_exit, @@ -400,6 +404,7 @@ mod tests { bytes: output.to_vec(), }], cwd: Some(PathBuf::from("/work")), + shell_spec: None, shell_active: true, at_prompt: true, last_exit: Some(0), diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 2e37489d..e0d4b252 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -111,6 +111,10 @@ struct SpawnConfig { initial_cwd: Option, integration_dir: Option, remote: Option, + /// The shell this pane actually got, after the override and the config have + /// been resolved against each other. Recorded so the machine tree can name + /// it — see [`crate::core::machine::PaneRecord::shell`]. + shell: Option, } /// Why a configured shell cannot be run, in one sentence, before anything @@ -156,12 +160,21 @@ fn build_spawn_config( anyhow::bail!(problem); } let remote = wsl_remote_context(configured.as_ref()); + // Taken before the chosen shell is consumed by the command builder: what + // goes in the tree is what was resolved here, not the possibly-empty + // override the caller sent. + let shell = configured.as_ref().map(|c| ShellSpec { + program: c.program.clone(), + args: c.args.clone(), + args_are_tty7_defaults: c.args_are_tty7_defaults, + }); let (cmd, integration_dir) = build_shell_command(configured, &initial_cwd, pane, workspace)?; Ok(SpawnConfig { cmd, initial_cwd, integration_dir, remote, + shell, }) } @@ -630,6 +643,9 @@ struct PaneState { observer_seq: u64, cwd: Option, shell: ShellState, + /// What this pane is running, for the machine tree to record. Distinct from + /// `shell` above, which is the shell-integration state. + shell_spec: Option, remote: Option, agent: Option, agent_argv: Option>, @@ -954,6 +970,11 @@ pub struct Carried { pub size: WinSize, pub ring: Vec, pub cwd: Option, + /// What the pane is running. Nothing on the other side of the exec can work + /// it out again — the command line belongs to a child this image never + /// spawned — so a handoff that dropped it would leave the tree naming no + /// shell for a pane that plainly has one. + pub shell_spec: Option, pub shell_active: bool, pub at_prompt: bool, pub last_exit: Option, @@ -1268,6 +1289,7 @@ impl DaemonPane { observer_seq: 0, cwd: spawn.initial_cwd, shell: ShellState::default(), + shell_spec: spawn.shell.clone(), remote: spawn.remote.clone(), agent: None, agent_session: None, @@ -1415,6 +1437,7 @@ impl DaemonPane { size: st.ring.tail_size(), ring: st.ring.snapshot(), cwd: st.cwd.clone(), + shell_spec: st.shell_spec.clone(), shell_active: st.shell.active, at_prompt: st.shell.at_prompt, last_exit: st.shell.last_exit_code, @@ -1477,6 +1500,7 @@ impl DaemonPane { observers: Vec::new(), observer_seq: 0, cwd: carried.cwd, + shell_spec: carried.shell_spec, shell: ShellState { active: carried.shell_active, at_prompt: carried.at_prompt, @@ -1526,6 +1550,9 @@ impl DaemonPane { subscriber_epoch: 0, observers: Vec::new(), observer_seq: 0, + // A native ssh pane is not running a shell of this machine's; what + // it is, `ssh_spec` already says. + shell_spec: None, cwd: None, shell: ShellState::default(), remote: Some(remote), @@ -1814,12 +1841,15 @@ impl DaemonPane { && let (Some(before), Some(after)) = (facts_before, facts_after) && facts_changed(&before, &after) { - let (cwd, agent) = after; + let (cwd, agent, shell) = after; crate::core::machine::observe_pane(pane, |p| { if cwd.is_some() { p.cwd = cwd; } p.agent = agent; + if shell.is_some() { + p.shell = shell; + } if alive { p.live = true; } @@ -2373,7 +2403,13 @@ fn agent_state_snapshot(st: &PaneState) -> Option (Option, Option) { +type ObservedFacts = ( + Option, + Option, + Option, +); + +fn observed_facts(st: &PaneState) -> ObservedFacts { let cwd = st.cwd.as_ref().map(|p| p.to_string_lossy().into_owned()); let agent = st.agent.map(|agent| crate::core::machine::AgentFacts { agent, @@ -2385,14 +2421,13 @@ fn observed_facts(st: &PaneState) -> (Option, Option, Option), - after: &(Option, Option), -) -> bool { - before.0 != after.0 || agent_facts_changed(before.1.as_ref(), after.1.as_ref()) +fn facts_changed(before: &ObservedFacts, after: &ObservedFacts) -> bool { + before.0 != after.0 + || agent_facts_changed(before.1.as_ref(), after.1.as_ref()) + || before.2 != after.2 } fn agent_facts_changed( @@ -3923,6 +3958,7 @@ mod tests { subscriber_epoch: 0, observers: Vec::new(), observer_seq: 0, + shell_spec: None, cwd: None, shell: ShellState::default(), remote: None, @@ -3939,7 +3975,7 @@ mod tests { use crate::core::cli_agent::{AgentSessionState, AgentStatus, CLIAgent}; let mut st = test_state(true); - assert_eq!(observed_facts(&st), (None, None)); + assert_eq!(observed_facts(&st), (None, None, None)); st.cwd = Some(PathBuf::from("/work/api")); st.agent = Some(CLIAgent::Claude); @@ -3951,7 +3987,7 @@ mod tests { ..Default::default() }); - let (cwd, agent) = observed_facts(&st); + let (cwd, agent, _shell) = observed_facts(&st); assert_eq!(cwd.as_deref(), Some("/work/api")); let agent = agent.expect("an agent in the foreground is a fact"); assert_eq!(agent.agent, CLIAgent::Claude); @@ -3964,7 +4000,7 @@ mod tests { assert_eq!(agent.status, Some(AgentStatus::Working)); st.agent_session = None; - let (_, agent) = observed_facts(&st); + let (_, agent, _) = observed_facts(&st); assert_eq!( agent.unwrap().launch_argv.as_deref(), Some(&["claude".to_string()][..]) @@ -3998,6 +4034,7 @@ mod tests { launch_argv: Some(vec!["claude".to_string()]), status: None, }), + shell: None, }, None, None, diff --git a/crates/tty7-core/src/daemon/server.rs b/crates/tty7-core/src/daemon/server.rs index 2ff1dee8..97b6fd4c 100644 --- a/crates/tty7-core/src/daemon/server.rs +++ b/crates/tty7-core/src/daemon/server.rs @@ -159,14 +159,21 @@ fn restorable_pane_ids(registry: &Registry) -> std::collections::HashSet { let mut ids: std::collections::HashSet = registry.list().into_iter().map(|p| p.pane_id).collect(); if let Some(store) = crate::core::machine::observed_store() { + let machine = store.machine(); ids.extend( - store - .machine() + machine .workspaces .iter() .flat_map(|w| w.tabs.iter()) .flat_map(|t| t.root.pane_ids()), ); + // And the pane list, not only the panes some tab currently stands on. + // The two disagree while a window is between layouts — a pane whose + // tab has been taken down and not yet put back is still a pane the + // tree knows about — and the cost of being wrong is asymmetric: an + // extra file is swept a tick later, a missing one is somebody's + // terminal. + ids.extend(machine.panes.iter().map(|p| p.id)); } ids } @@ -243,10 +250,13 @@ fn restored_screen( return None; } let segments = crate::daemon::scrollback::load(request.pane_id)?; - crate::daemon::scrollback::forget(request.pane_id); if segments.is_empty() { return None; } + // After the emptiness check, not before it: dropping the file is how a + // screen that *was* handed out stops being handed out twice, and a + // snapshot that turned out to hold nothing was never handed out at all. + crate::daemon::scrollback::forget(request.pane_id); log::info!( "pane {} is gone; its last screen is restored into a fresh pane", request.pane_id @@ -559,14 +569,24 @@ fn run_with(registry: Arc) -> anyhow::Result<()> { } spawn_orphan_sweep(registry.clone()); - // Before the writer starts: a daemon that has just come up owns no panes, - // so everything on disk belongs to panes the machine tree either still - // names — those are the ones a window is about to ask to restore — or has - // forgotten, and the latter are nobody's to restore any more. let restorable = restorable_pane_ids(®istry); - if crate::daemon::scrollback::enabled() { - crate::daemon::scrollback::sweep(&restorable); - } else { + // Deliberately not swept here while the setting is on. Startup is the one + // moment this process knows least: it owns no panes yet, and the windows + // that know which screens are still wanted cannot say so until the + // endpoint below is listening. Answering "is anyone going to ask for + // this?" here answers it when nobody can — and the answer deletes. A tree + // that failed to parse makes it worse, because `read_machine` quarantines + // it and hands back an empty `Machine`, so one bad file would take every + // pane's screen with it. + // + // The periodic sweep asks the same question a tick later, with the + // registry filled in and the tree caught up, and that is soon enough: + // nothing here is serving a request in the meantime. + // + // Off is not the same question. Then nothing on disk is worth keeping and + // deleting it promptly is the setting's whole promise, so that one still + // happens before anything else runs. + if !crate::daemon::scrollback::enabled() { crate::daemon::scrollback::sweep(&std::collections::HashSet::new()); } // A daemon that was killed outright never retired anything, so the files of diff --git a/crates/tty7-core/src/host/server.rs b/crates/tty7-core/src/host/server.rs index 9ad1846f..71864287 100644 --- a/crates/tty7-core/src/host/server.rs +++ b/crates/tty7-core/src/host/server.rs @@ -1716,6 +1716,7 @@ mod aggregate_tests { cwd: Some("/repo/tty7".into()), ssh_spec: None, agent: None, + shell: None, }, None, None, diff --git a/crates/tty7-server/tests/client_lib.rs b/crates/tty7-server/tests/client_lib.rs index 26896704..b17a18a0 100644 --- a/crates/tty7-server/tests/client_lib.rs +++ b/crates/tty7-server/tests/client_lib.rs @@ -137,6 +137,7 @@ fn seed(pane: u64) -> PaneSeed { cwd: Some("/home/me/proj".into()), ssh_spec: None, agent: None, + shell: None, } } diff --git a/crates/tty7-server/tests/scrollback_restore.rs b/crates/tty7-server/tests/scrollback_restore.rs new file mode 100644 index 00000000..2664a477 --- /dev/null +++ b/crates/tty7-server/tests/scrollback_restore.rs @@ -0,0 +1,402 @@ +//! A pane's screen survives the daemon being stopped and started. +//! +//! The unit tests under `daemon::scrollback` cover the file: it round-trips, it +//! is trimmed from the front, the sweep keeps what it is told to keep. None of +//! that answers the question the feature exists for, which is whether a window +//! that comes back to a restarted daemon is shown what its panes had on them. +//! That answer involves a real daemon writing a real snapshot, dying, and a +//! second daemon handing the bytes to the client that asks — so this runs all +//! of it and reads the wire. +//! +//! The ordering is the whole difficulty. A client cannot ask for a restore +//! until the daemon is listening, so anything the daemon throws away *while +//! starting up* is thrown away before the only party who knows what is still +//! wanted has been able to say so. + +use std::io::Write as _; +use std::path::PathBuf; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use tty7_core::client::{ControlClient, PaneClient}; +use tty7_core::core::machine::PaneSeed; +use tty7_core::daemon::control::{ControlHello, ControlRequest, ReplyOk}; +use tty7_core::daemon::protocol::{ClientMsg, DaemonMsg, RestoreFrom, ShellSpec, WinSize}; +use tty7_core::daemon::transport; + +const READY_WITHIN: Duration = Duration::from_secs(30); +const STREAM_WITHIN: Duration = Duration::from_secs(30); +const STOP_WITHIN: Duration = Duration::from_secs(30); + +const MARKER: &[u8] = b"tty7_screen_kept"; + +/// One instance's directories, outliving the daemons that serve them — the +/// point of the test is what is on disk between two of them. +struct Instance { + dir: tempfile::TempDir, +} + +impl Instance { + fn new() -> Instance { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::write( + dir.path().join("config.json"), + r#"{"persist_scrollback": true}"#, + ) + .unwrap(); + Instance { dir } + } + + fn path(&self) -> &std::path::Path { + self.dir.path() + } + + /// What the daemon writes down for clients to find it by: a socket on unix, + /// a port-and-token file on Windows. + fn endpoint(&self) -> PathBuf { + #[cfg(unix)] + let name = "daemon.sock"; + #[cfg(windows)] + let name = "daemon.port"; + self.dir.path().join(name) + } + + fn panes(&self) -> PaneClient { + PaneClient::at(self.endpoint()) + } + + fn control_endpoint(&self) -> PathBuf { + #[cfg(unix)] + let name = "control.sock"; + #[cfg(windows)] + let name = "control.port"; + self.dir.path().join(name) + } + + fn snapshot_of(&self, pane_id: u64) -> Option> { + std::fs::read( + self.dir + .path() + .join("scrollback") + .join(format!("{pane_id}.bin")), + ) + .ok() + } + + /// The tree a window would have left behind: one workspace, one tab, and + /// that tab standing on the pane whose screen we want back. + fn record_tab_on(&self, pane_id: u64) { + let tree = format!( + r#"{{ + "workspaces": [ + {{ + "id": "11111111-2222-3333-4444-555555555555", + "name": null, + "last_active": 1786343761, + "tabs": [ + {{ + "id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + "name": null, + "sidebar_group": null, + "root": {{ "Leaf": {{ "pane": {pane_id} }} }} + }} + ], + "active_tab": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + }} + ], + "panes": [ + {{ "id": {pane_id}, "cwd": null, "title": "", "ssh_spec": null, "agent": null, "live": false }} + ] +}}"# + ); + std::fs::write(self.dir.path().join("machine.json"), tree).unwrap(); + } + + fn start(&self) -> Running { + let child = Command::new(env!("CARGO_BIN_EXE_tty7-server")) + .arg("--daemon") + .arg("--config-dir") + .arg(self.path()) + .env("TTY7_DATA_DIR", self.path()) + .env("TTY7_CONTROL_SOCK", self.path().join("control.sock")) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("start tty7-server --daemon"); + let running = Running { + child, + stopped: false, + }; + let deadline = Instant::now() + READY_WITHIN; + loop { + if self.panes().version().is_ok() { + return running; + } + assert!( + Instant::now() < deadline, + "the daemon did not open its pane endpoint within {READY_WITHIN:?}" + ); + std::thread::sleep(Duration::from_millis(50)); + } + } + + /// Ask the daemon to go, the way the restart in Settings asks. This is the + /// path that takes each pane's last snapshot on the way out, so a test that + /// killed the process instead would be testing the periodic writer. + fn stop(&self, mut running: Running) { + let mut stream = + transport::connect_endpoint_at(&self.endpoint()).expect("connect to ask for shutdown"); + ClientMsg::Shutdown + .encode(&mut stream) + .expect("send Shutdown"); + stream.flush().ok(); + drop(stream); + + let deadline = Instant::now() + STOP_WITHIN; + loop { + match running.child.try_wait() { + Ok(Some(_)) => { + running.stopped = true; + return; + } + Ok(None) if Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(50)); + } + Ok(None) => panic!("the daemon did not exit within {STOP_WITHIN:?}"), + Err(e) => panic!("waiting for the daemon failed: {e}"), + } + } + } + + /// The `Spawn` a window sends for a pane whose `Attach` found nothing: a + /// new shell, asked to open showing what the dead one had. + fn spawn_restoring(&self, dead: u64) -> (u64, Vec) { + let mut stream = + transport::connect_endpoint_at(&self.endpoint()).expect("connect to spawn"); + ClientMsg::Spawn { + cwd: None, + size: size(), + shell: Some(interactive_shell()), + owner: None, + workspace: None, + restore: Some(RestoreFrom { + pane_id: dead, + banner: Some("the shell below is new".to_string()), + }), + } + .encode(&mut stream) + .expect("send Spawn"); + + let pane_id = match DaemonMsg::read(&mut stream) { + Ok(DaemonMsg::Spawned { pane_id }) => pane_id, + other => panic!("expected Spawned, got {other:?}"), + }; + + // Everything the daemon replays sits in the socket ahead of whatever + // the new shell writes, so a bounded drain is enough: the replay is + // already queued by the time `Spawned` is read. + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .expect("bound the drain"); + let mut replayed = Vec::new(); + while let Ok(msg) = DaemonMsg::read(&mut stream) { + match msg { + DaemonMsg::Snapshot(bytes) | DaemonMsg::Output(bytes) => { + replayed.extend_from_slice(&bytes) + } + _ => {} + } + } + (pane_id, replayed) + } +} + +struct Running { + child: Child, + stopped: bool, +} + +impl Drop for Running { + fn drop(&mut self) { + if !self.stopped { + let _ = self.child.kill(); + let _ = self.child.wait(); + } + } +} + +fn size() -> WinSize { + WinSize { + cols: 100, + rows: 30, + cell_w: 8, + cell_h: 16, + } +} + +fn interactive_shell() -> ShellSpec { + #[cfg(unix)] + let program = "/bin/sh"; + #[cfg(windows)] + let program = "cmd.exe"; + ShellSpec { + program: program.into(), + args: Vec::new(), + args_are_tty7_defaults: false, + } +} + +fn windows_contain(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} + +fn collect_until(session: &mut tty7_core::client::PaneSession, marker: &[u8]) -> Vec { + let mut seen: Vec = Vec::new(); + loop { + match session.recv() { + Ok(DaemonMsg::Output(bytes)) | Ok(DaemonMsg::Snapshot(bytes)) => { + seen.extend_from_slice(&bytes); + if windows_contain(&seen, marker) { + return seen; + } + } + Ok(DaemonMsg::Exited { code }) => panic!( + "the pane exited ({code:?}) before {:?} appeared; saw {:?}", + String::from_utf8_lossy(marker), + String::from_utf8_lossy(&seen) + ), + Ok(_) => {} + Err(e) => panic!( + "the pane stream ended early: {e}; saw {:?}", + String::from_utf8_lossy(&seen) + ), + } + } +} + +/// Put a marker on a pane's screen and give the daemon back the pane's id. +fn pane_showing_the_marker(instance: &Instance) -> u64 { + let mut session = instance + .panes() + .spawn(None, size(), Some(interactive_shell()), None, None) + .expect("spawn a pane"); + session + .set_recv_timeout(Some(STREAM_WITHIN)) + .expect("bound the stream reads"); + let pane_id = session.pane_id(); + session + .input(format!("echo {}\r", String::from_utf8_lossy(MARKER)).as_bytes()) + .expect("the shell takes input"); + collect_until(&mut session, MARKER); + // Detach rather than kill: the pane outlives this connection, which is the + // state a daemon restart finds its panes in. + session.detach().expect("detach"); + pane_id +} + +/// The case the feature is for: nothing has told the new daemon anything yet, +/// because the window that would tell it is still waiting for it to listen. +#[test] +fn a_restarted_daemon_still_has_the_screen_when_the_window_asks() { + let instance = Instance::new(); + let running = instance.start(); + let dead = pane_showing_the_marker(&instance); + instance.stop(running); + + let stored = instance + .snapshot_of(dead) + .expect("the shutdown wrote the pane's screen"); + assert!( + windows_contain(&stored, MARKER), + "the snapshot on disk does not hold the pane's screen" + ); + + let _restarted = instance.start(); + let (_new_pane, replayed) = instance.spawn_restoring(dead); + assert!( + windows_contain(&replayed, MARKER), + "the restarted daemon did not give the pane back its screen; \ + the client received {:?}", + String::from_utf8_lossy(&replayed) + ); +} + +/// What a pane is running has to reach the tree, because that is the only +/// place a window rebuilding the pane can read it from. Without it the rebuild +/// falls back to the default shell, which is how a restart turns a bash pane +/// into a PowerShell one. +#[test] +fn the_tree_records_the_shell_a_pane_is_running() { + let instance = Instance::new(); + let _running = instance.start(); + let pane = pane_showing_the_marker(&instance); + + // A pane reaches the tree by being put in a tab, which is what the window + // does right after it spawns one — carrying the same seed it spawned with. + // Until then the daemon has nothing to record its facts against, so the + // seed is the pane's first and only chance to say what it is running. + let mut control = ControlClient::connect_at( + &instance.control_endpoint(), + &ControlHello::host_rpc("probe", "probe"), + ) + .expect("control handshake"); + let ws = match control + .request(ControlRequest::WorkspaceCreate { + name: Some("probe".into()), + workspace: None, + }) + .expect("create a workspace") + { + ReplyOk::WorkspaceTree(ws) => *ws, + other => panic!("expected WorkspaceTree, got {other:?}"), + }; + control + .request(ControlRequest::TabCreate { + workspace: ws.id, + at: None, + pane: PaneSeed { + shell: Some(interactive_shell()), + ..PaneSeed::bare(pane) + }, + tab: None, + }) + .expect("put the pane in a tab"); + + let wanted = interactive_shell().program; + let deadline = Instant::now() + STREAM_WITHIN; + loop { + let tree = std::fs::read_to_string(instance.path().join("machine.json")).unwrap_or_default(); + // The record is keyed by pane id and the program is a plain string in + // it; matching on both together is enough to say this pane's shell — + // not some other pane's — reached the tree. + if tree.contains(&format!("\"id\": {pane}")) && tree.contains(&wanted) { + return; + } + assert!( + Instant::now() < deadline, + "the tree never recorded pane {pane}'s shell ({wanted}); it holds: {tree}" + ); + std::thread::sleep(Duration::from_millis(100)); + } +} + +/// The same thing for an instance whose tree still names the pane. This one +/// passes on its own merits today; it is here so a fix for the case above +/// cannot be one that quietly stops honouring the tree. +#[test] +fn a_screen_comes_back_when_the_tree_still_names_its_pane() { + let instance = Instance::new(); + let running = instance.start(); + let dead = pane_showing_the_marker(&instance); + instance.stop(running); + instance.record_tab_on(dead); + + let _restarted = instance.start(); + let (_new_pane, replayed) = instance.spawn_restoring(dead); + assert!( + windows_contain(&replayed, MARKER), + "a pane the tree still names came back blank; the client received {:?}", + String::from_utf8_lossy(&replayed) + ); +} diff --git a/src/ui/app.rs b/src/ui/app.rs index 70d8aa89..eed97af6 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -6566,6 +6566,7 @@ fn pane_to_session(pane: &Pane, cx: &App) -> SessionPane { SessionPane::Leaf { cwd: spawn.working_directory.clone(), pane_id: spawn.restore_pane, + shell: spawn.shell.clone(), ssh_spec: None, agent: spawn.agent, agent_session_id: spawn.agent_session_id.clone(), @@ -6577,6 +6578,11 @@ fn pane_to_session(pane: &Pane, cx: &App) -> SessionPane { SessionPane::Leaf { cwd: view.spawnable_cwd(), pane_id: Some(view.pane_id), + // `None` for a pane this window attached to rather than + // spawned: it never knew what was on the other end. The tree + // does — the daemon records it — and that is what a restore + // reads, so the gap here costs nothing it can see. + shell: view.shell_spec(), ssh_spec: view.ssh_spec(), agent: view.agent(), agent_session_id: view.agent_session().and_then(|s| s.session_id), @@ -6597,6 +6603,7 @@ fn pane_to_session(pane: &Pane, cx: &App) -> SessionPane { Pane::Empty => SessionPane::Leaf { cwd: None, pane_id: None, + shell: None, ssh_spec: None, agent: None, agent_session_id: None, @@ -6736,6 +6743,7 @@ fn session_to_pane( SessionPane::Leaf { cwd, pane_id, + shell, ssh_spec, agent, agent_session_id, @@ -6762,7 +6770,7 @@ fn session_to_pane( font_size, cwd.clone(), restore, - None, + shell.clone(), window, cx, ) { diff --git a/src/ui/home.rs b/src/ui/home.rs index cf70bd7a..9e06cf7a 100644 --- a/src/ui/home.rs +++ b/src/ui/home.rs @@ -293,6 +293,7 @@ mod tests { fn leaf(cwd: Option<&str>) -> SessionPane { SessionPane::Leaf { + shell: None, cwd: cwd.map(PathBuf::from), pane_id: None, ssh_spec: None, diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index b0829d79..05c3a2e6 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -179,6 +179,11 @@ fn desired_node(pane: &Pane, remote_window: bool, cx: &App) -> Option Option SessionPane match node { PaneNode::Leaf { pane } => { let record = panes.iter().find(|p| p.id == *pane); - let (cwd, ssh_spec, agent) = match record { + let (cwd, ssh_spec, agent, shell) = match record { Some(r) => ( r.cwd.clone().map(std::path::PathBuf::from), r.ssh_spec.clone(), r.agent.clone(), + r.shell.clone(), ), - None => (None, None, None), + None => (None, None, None, None), }; SessionPane::Leaf { cwd, @@ -1210,6 +1217,7 @@ fn session_pane_from_node(node: &PaneNode, panes: &[PaneRecord]) -> SessionPane // trying to guess — except it is right. `live` stays a hint for // what to show, never the judge of what to destroy. pane_id: Some(*pane), + shell, ssh_spec, agent: agent.as_ref().map(|a| a.agent), agent_session_id: agent.as_ref().and_then(|a| a.session_id.clone()), @@ -2259,6 +2267,7 @@ mod tests { cwd: Some(format!("/work/{pane}")), ssh_spec: None, agent: None, + shell: None, } }