From b89496b9d493f8d801389c8a2ebdce7f86610f6a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:45:20 +0800 Subject: [PATCH] fix(cli): a refused split left a live pane nothing was holding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tty7 split` spawns the shell and only then asks the tree to hold it, so any refusal in between leaves a pane running that nothing references — `pane ls --all` shows it, the tree does not, and `pane close --orphans` is the only way to be rid of it. Two attempts at `--ratio nan` left two. NaN is the case that gets there, because it cannot be serialised onto the control connection at all. The link drops mid-request, so the daemon's own "a split ratio must be a finite number" never comes back: the caller sees "control connection lost (request 2)" instead, which says nothing about the ratio, and the pane is already running. Refuse a non-finite ratio before the spawn, in the daemon's own terms. Its clamp to a usable range is deliberate and stays its business — 0, 1 and 5.0 still land on the clamp, as intended; this only refuses what the daemon would refuse anyway. Then take the pane back down if the split fails for any other reason too. Nobody asked for a pane that no tree holds, and leaving it for the user to find with `--orphans` is not a refusal, it is a mess. Verified against a live server: two refused splits now leave the pane count where it was, and a valid `--ratio 0.3` split still works. Swept the rest of the value-taking flags while here, all sound: `-m` on an unknown machine, `--key` on an unknown key (which lists the real ones), `--timeout 0` (exits 124, the conventional code) and a negative timeout are each refused with the right message. --- crates/tty7-cli/src/commands.rs | 46 +++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index 4524ce58..5f108b37 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -519,8 +519,17 @@ fn pane_split(args: SplitArgs, ctx: &Context, backend: &mut dyn Backend) -> Resu } else { Axis::Vertical }; + // Before the spawn, because the pane exists before the tree hears about + // it. `--ratio nan` cannot even be put on the wire — serialising it breaks + // the control connection, so the daemon's own "a split ratio must be a + // finite number" never arrives — and by then a shell is already running + // that no tree holds. The daemon's clamp to a usable range is deliberate + // and stays its business; this only refuses what it would refuse too. + if !args.ratio.is_finite() { + bail!("--ratio {} is not a finite number", args.ratio); + } let new = backend.spawn_shell(workspace, cwd.clone())?; - backend.control(ControlRequest::PaneSplit { + let split = backend.control(ControlRequest::PaneSplit { workspace, pane, axis, @@ -533,7 +542,15 @@ fn pane_split(args: SplitArgs, ctx: &Context, backend: &mut dyn Backend) -> Resu shell: None, }, first: false, - })?; + }); + if let Err(e) = split { + // Whatever the refusal was, the pane spawned above is now running with + // nothing referencing it: `pane ls --all` shows it, the tree does not, + // and it takes `pane close --orphans` to be rid of it. Nobody asked for + // that pane, so take it back down rather than leave it for the user. + let _ = backend.kill_pane(new); + return Err(e); + } report(format!("%{new}"), json!({ "pane": new })) } @@ -3012,6 +3029,31 @@ mod tests { assert_eq!(backend.runs.len(), 1, "the routed run must still happen"); } + /// A ratio the split cannot use is refused before a pane is spawned. + /// + /// `split` spawns the shell and only then asks the tree to hold it, so any + /// refusal in between leaves a pane running that nothing references — + /// visible to `pane ls --all`, absent from the tree, and removable only by + /// `pane close --orphans`. `--ratio nan` did exactly that twice over, + /// because a NaN cannot be serialised onto the control connection at all: + /// the link dropped, so the daemon's own "must be a finite number" never + /// came back, and two orphans were left behind. + #[test] + fn a_ratio_that_cannot_be_used_spawns_nothing() { + let mut backend = mock(); + let err = execute( + cli(&["tty7", "split", "%1", "--horizontal", "--ratio", "nan"]), + &Context::default(), + &mut backend, + ) + .expect_err("a split needs a real ratio"); + assert!(err.to_string().contains("finite"), "{err}"); + assert!( + backend.spawned.is_empty(), + "the refusal has to come before the spawn, or it leaves an orphan" + ); + } + /// `tty7 new ` refuses a path it cannot root the workspace at, and /// refuses it before creating anything. ///