diff --git a/crates/tty7-cli/src/backend.rs b/crates/tty7-cli/src/backend.rs index 5733cf39..ccf249ff 100644 --- a/crates/tty7-cli/src/backend.rs +++ b/crates/tty7-cli/src/backend.rs @@ -85,6 +85,7 @@ pub mod mock { pub procs_reply: PaneProcs, pub registry: Vec, pub killed: Vec, + pub kill_failures: Vec, pub runs: Vec, pub run_exit: Option, pub events: Vec, @@ -105,6 +106,7 @@ pub mod mock { procs_reply: PaneProcs::default(), registry: Vec::new(), killed: Vec::new(), + kill_failures: Vec::new(), runs: Vec::new(), run_exit: Some(0), events: Vec::new(), @@ -171,6 +173,9 @@ pub mod mock { fn kill_pane(&mut self, pane: u64) -> Result<()> { self.killed.push(pane); + if self.kill_failures.contains(&pane) { + anyhow::bail!("mock hangup failure for pane %{pane}"); + } Ok(()) } diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index 6fb51cea..00bb1ff8 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -294,7 +294,8 @@ fn ws_rename(ws: &str, name: String, backend: &mut dyn Backend) -> Result Result { let machine = fetch_machine(backend)?; let id = resolve::workspace(&machine, &address::parse_workspace(ws))?.id; - backend.control(ControlRequest::WorkspaceRemove { workspace: id })?; + let reply = backend.control(ControlRequest::WorkspaceRemove { workspace: id })?; + hang_up_removed_panes("WorkspaceRemove", reply, backend)?; report("", json!({ "removed": id.to_string() })) } @@ -560,10 +561,32 @@ fn tab_close(tab: &str, backend: &mut dyn Backend) -> Result { let addr = address::parse_tab(tab)?; let machine = fetch_machine(backend)?; let (workspace, tab) = resolve::tab(&machine, &addr)?; - backend.control(ControlRequest::TabClose { workspace, tab })?; + let reply = backend.control(ControlRequest::TabClose { workspace, tab })?; + hang_up_removed_panes("TabClose", reply, backend)?; report("", json!({ "closed": tab.to_string() })) } +fn hang_up_removed_panes(request: &str, reply: ReplyOk, backend: &mut dyn Backend) -> Result<()> { + let panes = match reply { + ReplyOk::Panes(panes) => panes, + other => bail!("the server answered {request} with {other:?}"), + }; + let mut failures = Vec::new(); + for pane in panes { + if let Err(error) = backend.kill_pane(pane) { + failures.push(format!("%{pane}: {error:#}")); + } + } + if !failures.is_empty() { + bail!( + "failed to hang up {} pane(s) removed by {request}: {}", + failures.len(), + failures.join("; ") + ); + } + Ok(()) +} + fn tab_rename(tab: &str, name: String, backend: &mut dyn Backend) -> Result { let addr = address::parse_tab(tab)?; let machine = fetch_machine(backend)?; @@ -669,7 +692,8 @@ fn pane_close(target: Option<&str>, ctx: &Context, backend: &mut dyn Backend) -> match resolve::workspace_of_pane(&machine, pane) { Ok(ws) => { let workspace = ws.id; - backend.control(ControlRequest::PaneClose { workspace, pane })?; + let reply = backend.control(ControlRequest::PaneClose { workspace, pane })?; + hang_up_removed_panes("PaneClose", reply, backend)?; } // No workspace holds it, so PaneClose has nothing to route through. // Hang it up directly instead of refusing — this is exactly the orphan @@ -1101,15 +1125,13 @@ mod tests { // A pane the tree does hold still goes through PaneClose. let mut backend = mock(); + backend.replies.push_back(ReplyOk::Panes(vec![1])); run_cli( &["tty7", "pane", "close", "%1"], &Context::default(), &mut backend, ); - assert!( - backend.killed.is_empty(), - "a filed pane is closed, not killed" - ); + assert_eq!(backend.killed, vec![1], "the removed pane is hung up"); assert!( backend .control_calls @@ -1239,11 +1261,17 @@ mod tests { ); backend.control_calls.clear(); + backend.replies.push_back(ReplyOk::Panes(vec![3, 4])); run_cli(&["tty7", "ws", "rm", "web"], &ctx, &mut backend); assert_eq!( backend.control_calls[1], ControlRequest::WorkspaceRemove { workspace: web } ); + assert_eq!( + backend.killed, + vec![3, 4], + "removing a workspace must hang up the panes it held" + ); backend.control_calls.clear(); backend.replies.push_back(ReplyOk::Attached { @@ -1274,6 +1302,7 @@ mod tests { let mut backend = mock(); let web = backend.machine.workspaces[1].clone(); + backend.replies.push_back(ReplyOk::Panes(Vec::new())); run_cli(&["tty7", "tab", "close", "@3"], &ctx, &mut backend); assert_eq!( backend.control_calls, @@ -1314,6 +1343,63 @@ mod tests { ); } + #[test] + fn tab_close_hangs_up_every_pane_the_server_removed() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::Panes(vec![2, 3])); + + run_cli( + &["tty7", "tab", "close", "@2"], + &Context::default(), + &mut backend, + ); + + assert_eq!( + backend.killed, + vec![2, 3], + "every pane removed with the tab must be hung up" + ); + } + + #[test] + fn tab_close_attempts_every_hangup_before_reporting_failures() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::Panes(vec![2, 3])); + backend.kill_failures.push(2); + + let error = execute( + cli(&["tty7", "tab", "close", "@2"]), + &Context::default(), + &mut backend, + ) + .expect_err("a failed pane hangup must fail tab close"); + + assert_eq!( + backend.killed, + vec![2, 3], + "a failed hangup must not skip the remaining panes" + ); + assert!(error.to_string().contains("%2"), "{error:#}"); + } + + #[test] + fn tab_close_rejects_an_unexpected_server_reply() { + let mut backend = mock(); + let error = execute( + cli(&["tty7", "tab", "close", "@2"]), + &Context::default(), + &mut backend, + ) + .expect_err("TabClose must return the panes it removed"); + + assert!( + error + .to_string() + .contains("the server answered TabClose with Unit"), + "{error:#}" + ); + } + #[test] fn tab_new_uses_the_workspace_from_the_environment() { let mut backend = mock(); @@ -1414,6 +1500,7 @@ mod tests { #[test] fn pane_close_traces_the_pane_to_its_workspace() { let mut backend = mock(); + backend.replies.push_back(ReplyOk::Panes(vec![5])); run_cli( &["tty7", "pane", "close", "%5"], &Context::default(), @@ -1430,6 +1517,11 @@ mod tests { }, ] ); + assert_eq!( + backend.killed, + vec![5], + "a pane removed from its workspace must also be hung up" + ); } #[test] diff --git a/crates/tty7-cli/tests/cli_e2e.rs b/crates/tty7-cli/tests/cli_e2e.rs index 7c98fa21..a17826f9 100644 --- a/crates/tty7-cli/tests/cli_e2e.rs +++ b/crates/tty7-cli/tests/cli_e2e.rs @@ -13,6 +13,7 @@ const PASTE_AWARE_TEXT: &str = "tty7 paste aware input"; const PASTE_BURST_WINDOW: Duration = Duration::from_millis(120); const READY_WITHIN: Duration = Duration::from_secs(30); const SETTLE_WITHIN: Duration = Duration::from_secs(60); +const CLOSE_WITHIN: Duration = Duration::from_secs(5); fn main() { if std::env::args().any(|arg| arg == PASTE_AWARE_FIXTURE_ARG) { @@ -33,6 +34,10 @@ fn main() { "new_builds_a_workspace_with_a_live_pane", new_builds_a_workspace_with_a_live_pane, ), + ( + "tab_close_terminates_every_pane_in_the_tab", + tab_close_terminates_every_pane_in_the_tab, + ), ( "run_streams_output_and_passes_the_exit_code", run_streams_output_and_passes_the_exit_code, @@ -389,6 +394,42 @@ fn new_builds_a_workspace_with_a_live_pane(daemon: &Daemon) { assert!(panes.contains(&format!("%{pane}")), "{panes}"); } +fn tab_close_terminates_every_pane_in_the_tab(daemon: &Daemon) { + let created = daemon.run_json(&["new", &workdir()]); + let ws_id = created["id"].as_str().expect("new prints the workspace id"); + let tab = daemon.run_json(&["tab", "new", ws_id, "--cwd", &workdir()]); + let tab_id = tab["tab"].as_str().expect("tab new prints the tab id"); + let first = tab["pane"].as_u64().expect("tab new prints the pane id"); + let first_addr = format!("%{first}"); + let split = daemon.run_json(&["split", &first_addr, "--horizontal"]); + let second = split["pane"] + .as_u64() + .expect("split prints the new pane id"); + + let tab_addr = format!("@{tab_id}"); + daemon.run_ok(&["tab", "close", &tab_addr]); + + let deadline = Instant::now() + CLOSE_WITHIN; + loop { + let listed = daemon.run_json(&["pane", "ls", "--all"]); + let running = listed["panes"] + .as_array() + .expect("pane ls --all prints the daemon registry"); + let closed_are_gone = running + .iter() + .all(|pane| !matches!(pane["pane"].as_u64(), Some(id) if id == first || id == second)); + if closed_are_gone { + assert_eq!(listed["orphans"].as_u64(), Some(0), "{listed}"); + return; + } + assert!( + Instant::now() < deadline, + "tab close left one of panes %{first} and %{second} live: {listed}" + ); + std::thread::sleep(Duration::from_millis(50)); + } +} + fn run_streams_output_and_passes_the_exit_code(daemon: &Daemon) { let echo = one_shot("echo tty7_e2e_run_marker"); let mut args: Vec<&str> = vec!["run", "--"];