diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index 7b7a6e75..a5577612 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -994,9 +994,11 @@ fn pane_ls_all(backend: &mut dyn Backend) -> Result { }) }) .collect(); + // The same test `orphan_panes` makes, so the count and the reaper cannot + // disagree — the line names `pane close --orphans` as the fix for it. let orphans = running .iter() - .filter(|info| holder(info.pane_id).is_none()) + .filter(|info| !info.attached && holder(info.pane_id).is_none()) .count(); let mut human = output::registry_table(&running, &|pane| holder(pane).map(|ws| ws.to_string())); if orphans > 0 { @@ -1135,6 +1137,24 @@ fn pane_close( /// that predates it simply omits it — but it is a wire change, and until it /// happens the honest thing is that the docs say plainly that `--orphans` will /// kill a `run` started from another shell. +/// The panes nothing holds and nobody is watching. +/// +/// "No workspace holds it" is not enough on its own, and the gap is not +/// theoretical: during a restore the window spawns a pane, attaches to it, and +/// only then files it into the tree. Polled through a cold start, seven live +/// panes of a seven-tab window each reported as held by no workspace — every +/// one of them about to be adopted. A reaper run at that moment takes the +/// session it was meant to clean up after. +/// +/// So attachment is the other half. It is the daemon's own fact rather than a +/// guess about timing: `attach` takes the seat and a pane connection closing +/// calls `detach`, which clears it. A pane a window is adopting is attached; a +/// pane whose layout was thrown away had its view dropped, which closed the +/// connection, which emptied the seat. That is the whole difference between +/// the two, and nothing else in the registry shows it. +/// +/// An older server that does not send the field reads as unattached, which is +/// what this command did before. fn orphan_panes(machine: &Machine, backend: &mut dyn Backend) -> Result> { let held: Vec = machine .workspaces @@ -1145,6 +1165,7 @@ fn orphan_panes(machine: &Machine, backend: &mut dyn Backend) -> Result Ok(backend .list_panes()? .iter() + .filter(|info| !info.attached) .map(|info| info.pane_id) .filter(|pane| !held.contains(pane)) .collect()) @@ -2237,10 +2258,23 @@ mod tests { title: "sh".into(), osc_title: None, alive: true, + attached: false, owner: owner.map(str::to_string), } } + /// The same pane with a client watching it — what a window adopting one + /// looks like from the registry. + fn attached_pane_info( + pane_id: u64, + owner: Option<&str>, + ) -> tty7_core::daemon::protocol::PaneInfo { + tty7_core::daemon::protocol::PaneInfo { + attached: true, + ..pane_info(pane_id, owner) + } + } + /// One condition, one sentence. /// /// `capture %999` used to answer with the wire request's name — "daemon @@ -2896,6 +2930,81 @@ mod tests { ); } + /// A pane a client is watching is never reaped, however unheld it looks. + /// + /// Reproduced before this existed: polling `pane ls --all` through a cold + /// start of a seven-tab window reported seven live panes as held by no + /// workspace, one after another, because the window spawns and attaches + /// before it files them. `--orphans` at that moment takes the session. + /// + /// The second half matters as much: a stray really is still reaped. It is + /// the same pane with nobody attached, which is exactly what a dropped + /// view leaves behind, and the reaper must still take it or the recovery + /// tool has been quietly turned off. + #[test] + fn pane_close_orphans_spares_a_pane_a_client_is_still_watching() { + let mut backend = mock(); + backend.registry = vec![ + pane_info(1, None), + // Unheld and attached: a window is adopting it. + attached_pane_info(77, Some("tty7-app")), + // Unheld and unwatched: a view was dropped and never came back. + pane_info(78, Some("tty7-app")), + ]; + + let json = json_of(run_cli( + &["tty7", "pane", "close", "--orphans"], + &Context::default(), + &mut backend, + )); + assert_eq!( + json["closed"], + serde_json::json!([78]), + "only the pane nobody is watching" + ); + assert_eq!(backend.killed, vec![78], "and %77 was left running"); + } + + /// The count in `pane ls --all` names the set `--orphans` would take. + /// + /// That line tells the reader to run the reaper, so a number arrived at + /// some other way sends them after panes it will not touch — or worse, + /// reads zero while the reaper is about to take something. + #[test] + fn the_orphan_count_and_the_reaper_agree() { + let mut backend = mock(); + backend.registry = vec![ + pane_info(1, None), + attached_pane_info(77, Some("tty7-app")), + pane_info(78, Some("tty7-app")), + ]; + let counted = json_of(run_cli( + &["tty7", "pane", "ls", "--all"], + &Context::default(), + &mut backend, + ))["orphans"] + .as_u64() + .expect("a count"); + + let mut backend = mock(); + backend.registry = vec![ + pane_info(1, None), + attached_pane_info(77, Some("tty7-app")), + pane_info(78, Some("tty7-app")), + ]; + let reaped = json_of(run_cli( + &["tty7", "pane", "close", "--orphans"], + &Context::default(), + &mut backend, + ))["closed"] + .as_array() + .expect("a list") + .len() as u64; + + assert_eq!(counted, reaped, "the count is the set the reaper takes"); + assert_eq!(counted, 1, "and it is the unwatched one"); + } + /// The CLI is what creates orphans, so it should be able to clear them. /// `--orphans` closes exactly the panes the registry holds and the tab /// trees do not — panes that *are* held must survive it untouched. diff --git a/crates/tty7-cli/src/output.rs b/crates/tty7-cli/src/output.rs index 8e400e87..693ca7d2 100644 --- a/crates/tty7-cli/src/output.rs +++ b/crates/tty7-cli/src/output.rs @@ -464,6 +464,7 @@ mod tests { let held = "9fd8072f-465c-4016-9a81-8143bff1240c"; let elsewhere = "76698a44-3f13-4961-8fed-90d0b3defff1"; let pane = |id: u64, owner: Option<&str>| PaneInfo { + attached: false, pane_id: id, cwd: None, title: "zsh".into(), diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 8107f869..2075203e 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -2154,9 +2154,14 @@ impl DaemonPane { } pub fn info(&self) -> PaneInfo { - let (cwd, osc_title, alive) = { + let (cwd, osc_title, alive, attached) = { let st = self.state.locked(); - (st.cwd.clone(), st.osc_title.clone(), st.alive) + ( + st.cwd.clone(), + st.osc_title.clone(), + st.alive, + st.subscriber.is_some(), + ) }; PaneInfo { pane_id: self.id, @@ -2164,6 +2169,7 @@ impl DaemonPane { title: self.foreground_title(), osc_title, alive, + attached, owner: self.owner.clone(), } } diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 91901d1f..20c82f20 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -144,6 +144,25 @@ pub struct PaneInfo { #[serde(default, skip_serializing_if = "Option::is_none")] pub osc_title: Option, pub alive: bool, + /// Whether a client is attached to this pane right now. + /// + /// The daemon's own answer to "is anybody watching this", kept by + /// `PaneHandle::attach`/`detach` — a pane's connection closing clears the + /// seat, so a view that has been dropped stops counting immediately. + /// + /// It is what tells a pane the tree has not filed *yet* from one it will + /// never file. Both are live and unheld, and nothing else distinguishes + /// them: a window adopting a pane is attached to it, and a pane whose + /// layout was thrown away is not. `orphan_panes` spends it so that + /// `tty7 pane close --orphans` cannot reap a session mid-restore. + /// + /// `#[serde(default)]`, and deliberately not a `PROTOCOL_VERSION` bump, + /// for the same reason as `AuthPromptKind::KeyPassphrase::rejected`: an + /// older peer on either side never sets it and serde ignores fields it + /// does not know, so a peer that cannot answer reads as unattached — which + /// is what this command did before the field existed. + #[serde(default)] + pub attached: bool, #[serde(default, skip_serializing_if = "Option::is_none")] pub owner: Option, } @@ -1813,6 +1832,7 @@ mod tests { DaemonMsg::Exited { code: None }, DaemonMsg::PaneList(vec![ PaneInfo { + attached: false, pane_id: 3, cwd: Some(PathBuf::from("/x")), title: "zsh".into(), @@ -1821,6 +1841,7 @@ mod tests { owner: None, }, PaneInfo { + attached: false, pane_id: 4, cwd: None, title: String::new(), diff --git a/crates/tty7-core/src/host/server.rs b/crates/tty7-core/src/host/server.rs index 0ad785de..2007c828 100644 --- a/crates/tty7-core/src/host/server.rs +++ b/crates/tty7-core/src/host/server.rs @@ -1762,6 +1762,7 @@ mod aggregate_tests { attachments: Arc::new(AttachRegistry::default()), panes: Some(Arc::new(ThreePanesOneAgent { panes: vec![PaneInfo { + attached: false, pane_id: 7, cwd: Some(PathBuf::from("/repo/tty7")), title: "nvim".into(), diff --git a/src/ui/switcher.rs b/src/ui/switcher.rs index 6d6bbd19..cda77913 100644 --- a/src/ui/switcher.rs +++ b/src/ui/switcher.rs @@ -3454,6 +3454,7 @@ mod tests { let listed: Vec = shown .iter() .map(|&pane_id| PaneInfo { + attached: false, pane_id, cwd: None, title: "zsh".to_string(), @@ -3484,6 +3485,7 @@ mod tests { fn info(pane_id: u64, alive: bool) -> PaneInfo { PaneInfo { + attached: false, pane_id, cwd: Some(std::path::PathBuf::from("/tmp/x")), title: "zsh".to_string(),