From 48d712143ea47db7d313d62030584c2137b0393f Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:54:46 +0800 Subject: [PATCH] fix(cli): answer `wait --until free` on remote and SSH panes (#840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pane_is_free` read the pane's local process tree and folded two very different answers into one `false`. On a pane that is only the near end of a connection that tree describes the tunnel: a pane routed to a remote daemon has no local pty at all, so `DaemonPane::procs` returned `Default::default()` and the tree came back empty; a pane whose shell is running `ssh` has a tree whose depth-1 process is the `ssh` itself, busy for exactly as long as you are logged in. Either way `free` was unreachable structurally, `seen_busy` was set on every poll — which suppressed the one hint that would have pointed at the gap — and the wait rode the whole `--timeout` before recommending `--until free`, the flag that had just failed. Freeness is now three-valued: free, busy, or "nothing here can answer", and the last one carries its reason. On a remote pane freeness is the far shell's own OSC 133 prompt marks. That is sound because the near shell cannot be at a prompt while the connection owns its pty, so a prompt mark on such a pane can only have come from the far side. Two things had to change for the daemon to be able to say it. The reader suppresses relayed prompt marks so a foreground program cannot engage the local line editor — right for the editor, and precisely wrong here — so `ShellState` now keeps the mark's own unsuppressed reading beside the editor's. And "not at a prompt" on a remote pane means nothing until the far shell has proved it reports at all, since the newest mark is otherwise the near shell's own "I started `ssh`", which nothing will ever supersede; a latch records the first prompt mark that arrives while the pane is remote, and is cleared on every hop. `PaneProcs` carries all of this to the CLI in a new optional `context` — remote target, whether this machine holds the pty, the mark's reading, the latch — so the answer still costs one request per poll and an older server, which omits the field, keeps today's tree-only behaviour. When the far host has no shell integration the honest answer is that this machine cannot tell an idle remote prompt from a running remote command. `wait` says so — exit 1, `status: unknown`, a `free_unknown` string naming the host — after one poll of grace for a handshake still in flight, and only when `free` was the only state that could still answer, so `--until done,free` keeps waiting on `done`. Without that it would hang forever on a wait with no `--timeout`. The `no-agent` timeout hint now fires only for a caller who did not already pass `--until free`, and the reason freeness never resolved is printed and put in the JSON in its place. Deliberately left alone: on a local pane the process tree still holds the verdict. A prompt mark can only turn a busy tree into free — which is what fixes a plain `ssh` pane on Windows, where the daemon has no way to name the pane as remote — never the other way round, so no pane that reads busy today can start reading free because an integration went quiet. `free` therefore now means "will take input" rather than strictly "back to the bare shell": a pane sitting at a nested shell's prompt is free, and the reference says so. The handoff record is unchanged, so a pane mid-`ssh` that survives an exec comes back reporting "cannot determine" until the far side's next prompt rather than guessing. Claude-Session: https://claude.ai/code/session_01UUyWQXzcBAoBzaSX8pc7nU --- crates/tty7-cli/src/cli.rs | 11 + crates/tty7-cli/src/commands.rs | 508 ++++++++++++++++++++++-- crates/tty7-cli/src/output.rs | 1 + crates/tty7-cli/tests/cli_e2e.rs | 37 ++ crates/tty7-core/src/daemon/pane.rs | 215 +++++++++- crates/tty7-core/src/daemon/procinfo.rs | 8 +- crates/tty7-core/src/daemon/protocol.rs | 40 ++ crates/tty7-core/src/host/server.rs | 1 + docs/cli/reference.mdx | 34 +- 9 files changed, 809 insertions(+), 46 deletions(-) diff --git a/crates/tty7-cli/src/cli.rs b/crates/tty7-cli/src/cli.rs index bc216a6d..dc57d7d9 100644 --- a/crates/tty7-cli/src/cli.rs +++ b/crates/tty7-cli/src/cli.rs @@ -247,6 +247,16 @@ pub enum WaitState { /// Costs one extra request per poll, so it is only checked when asked for. Free, Exit, + /// Freeness could not be determined for this pane at all — the usual cause + /// is a remote or SSH pane whose far shell sends no prompt marks, where + /// the local process tree only describes this end of the connection. + /// + /// Reported, never awaited: `#[value(skip)]` keeps it out of `--until`, + /// because "wait until I cannot tell" is not a thing to wait for. It + /// exists so `--json` has a `status` to name the one outcome that is + /// neither an answer nor a timeout. + #[value(skip)] + Unknown, } impl WaitState { @@ -262,6 +272,7 @@ impl WaitState { WaitState::NoAgent => "no-agent", WaitState::Free => "free", WaitState::Exit => "exit", + WaitState::Unknown => "unknown", } } } diff --git a/crates/tty7-cli/src/commands.rs b/crates/tty7-cli/src/commands.rs index c31f2c1d..db84d410 100644 --- a/crates/tty7-cli/src/commands.rs +++ b/crates/tty7-cli/src/commands.rs @@ -1014,6 +1014,17 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result; + /// How many polls in a row must fail to determine freeness before the wait + /// calls it structural and stops. + /// + /// One is not enough: a pane whose `ssh` handshake is still in flight has + /// no far-side prompt mark yet and no local tree worth reading, and that + /// window is shorter than a poll. Two consecutive misses is one `--interval` + /// of grace — long enough for the transient, short enough that a wait with + /// no `--timeout` at all still ends rather than hanging on a question this + /// machine cannot answer. + const UNKNOWN_POLLS: u32 = 2; + let pane = address::pane_or_context(args.target.as_deref(), ctx)?; // `checked_add` rather than `+`: an absurd `--timeout` must not panic. let deadline = args @@ -1021,10 +1032,25 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result = None; // Sticky: has the pane been seen running something since the wait began? // This is `--changed`'s edge for `free` — see the flag's own comment. let mut seen_busy = false; + // Why freeness could not be read, and for how many polls running. Cleared + // the moment a verdict does arrive: a wait that timed out on a pane it + // could read perfectly well by then must not blame the handshake it + // watched go by. + let mut unknown: Option = None; + let mut unknown_polls: u32 = 0; let mut polls: u32 = 0; loop { let states = match backend.control(ControlRequest::AgentStates)? { @@ -1055,18 +1081,30 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result { + current = WaitState::Free; + changed = seen_busy; + (unknown, unknown_polls) = (None, 0); + } + Freeness::Busy => { + seen_busy = true; + (unknown, unknown_polls) = (None, 0); + } + // Not `seen_busy`: "we could not look" is not "something was + // running", and letting it set that flag was what suppressed + // the one hint that would have pointed at the gap (#840). + Freeness::Unknown(why) => { + unknown_polls += 1; + unknown = Some(why); + } } } @@ -1131,6 +1169,38 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result= UNKNOWN_POLLS { + let why = unknown.as_deref().unwrap_or("no reason recorded"); + let human = format!("pane %{pane}: cannot determine whether it is free — {why}"); + eprintln!("tty7: pane %{pane}: cannot determine whether it is free"); + let session = entry.as_ref().map(|e| &e.state); + return Ok(Outcome::Exit( + 1, + Report { + human, + // The success path's shape, so a consumer written against + // it does not find its fields missing on the one branch it + // wrote error handling for (#589) — plus the reason, which + // is the only thing this branch actually knows. + json: json!({ + "pane": pane, + "status": WaitState::Unknown.name(), + "matched": false, + "stale": !changed, + "free_unknown": why, + "activity": session.map(|s| s.activity), + "message": session.and_then(|s| s.message.clone()), + "session_id": session.and_then(|s| s.session_id.clone()), + }), + }, + )); + } + if deadline.is_some_and(|d| Instant::now() >= d) { // 124 = the `timeout(1)` convention: "gave up", distinct from // both success and error, so orchestration scripts can branch. @@ -1139,13 +1209,27 @@ fn wait(args: WaitArgs, ctx: &Context, backend: &mut dyn Backend) -> Result Result Result Result { - let procs = backend.procs(pane)?.procs; - Ok(!procs.is_empty() && procs.iter().all(|p| p.depth == 0)) +/// Two sources, and which one leads depends on where the pane's session is. +/// +/// **The process tree**, for a pane whose pty is on this machine. Depth, not +/// count: the pane's own shell sits at depth 0 and everything it launched hangs +/// below, so "nothing deeper than the shell" holds however many shells the pane +/// ended up with, and does not have to guess at process names. It is also the +/// portable question — Windows has no foreground process group to ask about, so +/// `ProcEntry::foreground` is never true there. What it cannot see, both by +/// construction: a pane whose depth-0 process *is* the command — which is what +/// `tty7 run` spawns — reads free for as long as it runs, and a backgrounded +/// job keeps a pane busy after the foreground command is long gone. +/// +/// **The shell's own OSC 133 marks**, which are the only thing that can speak +/// for a pane that is the near end of a connection. There the local tree +/// describes the tunnel: a native-SSH pane has no local tree at all, and a pane +/// running `ssh` has one whose depth-1 process is the `ssh` itself, busy for as +/// long as you are logged in. A prompt mark on such a pane can only have come +/// from the far shell — the near one cannot be at a prompt while the connection +/// owns its pty — so it is both trustworthy and the only evidence available. +/// +/// Where they disagree on a local pane, a prompt mark outranks a deeper +/// process, because the process drawing that prompt is a shell: a `sudo -i`, a +/// nested `bash`, an `ssh` on a platform where the daemon cannot name it as +/// remote. It does not work the other way round — the absence of a mark proves +/// nothing, and the tree keeps the verdict there. +fn pane_freeness(backend: &mut dyn Backend, pane: u64) -> Result { + let answer = backend.procs(pane)?; + let bare_tree = !answer.procs.is_empty() && answer.procs.iter().all(|p| p.depth == 0); + let Some(ctx) = &answer.context else { + // A daemon from before the field existed. It can only be answering for + // a pty of its own, so the tree is all there was and all there is. + return Ok(match (bare_tree, answer.procs.is_empty()) { + (true, _) => Freeness::Free, + (false, false) => Freeness::Busy, + (false, true) => Freeness::Unknown( + "this server is too old to say where the pane's session lives, and its \ + process tree came back empty" + .into(), + ), + }); + }; + + let remote = ctx.remote.as_ref(); + if remote.is_some() || !ctx.local_pty { + let whereabouts = match remote { + Some(r) => format!("connected to {}", r.target), + // A pane with no pty here and no context to name: the daemon knows + // the session is elsewhere without knowing where. + None => "not backed by a pty on this machine".to_string(), + }; + return Ok(match ctx.at_prompt { + // Only the far shell can be at a prompt while the near end is busy + // holding the connection open. + Some(true) => Freeness::Free, + // The connection is over and the near shell is bare again — the + // remote context just has not been re-probed yet. + _ if ctx.local_pty && bare_tree => Freeness::Free, + // Once the far side has proved it reports, its silence means work. + Some(false) if ctx.remote_prompt_seen => Freeness::Busy, + _ => Freeness::Unknown(format!( + "this pane is {whereabouts}, so its local process tree describes this end of \ + the connection, not what is running on the far one — and the far shell has \ + sent no prompt mark, so nothing here can tell an idle remote prompt from a \ + running remote command. Install tty7's shell integration on the remote host, \ + or wait on something observable from here (`--until exit`, or an agent status)" + )), + }); + } + + // A local pane. The tree leads; a prompt mark only ever adds to it. + if bare_tree || ctx.at_prompt == Some(true) { + return Ok(Freeness::Free); + } + if !answer.procs.is_empty() || ctx.at_prompt == Some(false) { + return Ok(Freeness::Busy); + } + Ok(Freeness::Unknown( + "the pane's process tree came back empty and its shell has sent no prompt marks, so \ + there is nothing here to read freeness off — check `tty7 procs` on this pane" + .into(), + )) } /// Whether the daemon still has a live pane behind this id. Absent from the @@ -2881,6 +3050,7 @@ mod tests { tty7_core::daemon::protocol::PaneProcs { procs: vec![proc_entry(100, "zsh", 0, true)], ports: Vec::new(), + context: Some(local_context(None)), } } @@ -2892,6 +3062,64 @@ mod tests { proc_entry(101, "cargo", 1, true), ], ports: Vec::new(), + context: Some(local_context(None)), + } + } + + /// A pane whose pty is on this machine, optionally with a shell that is + /// reporting prompt marks. + fn local_context(at_prompt: Option) -> tty7_core::daemon::protocol::PaneContext { + tty7_core::daemon::protocol::PaneContext { + remote: None, + local_pty: true, + at_prompt, + remote_prompt_seen: false, + } + } + + /// A pane whose shell is running `ssh`: the local tree has the connection + /// in it, and everything that matters is on the far end. + fn ssh_procs( + at_prompt: Option, + remote_prompt_seen: bool, + ) -> tty7_core::daemon::protocol::PaneProcs { + use tty7_core::daemon::protocol::{RemoteContext, RemoteKind}; + tty7_core::daemon::protocol::PaneProcs { + procs: vec![ + proc_entry(100, "zsh", 0, false), + proc_entry(101, "ssh", 1, true), + ], + ports: Vec::new(), + context: Some(tty7_core::daemon::protocol::PaneContext { + remote: Some(RemoteContext { + kind: RemoteKind::Ssh, + argv: vec!["ssh".into(), "build-box".into()], + target: "build-box".into(), + }), + local_pty: true, + at_prompt, + remote_prompt_seen, + }), + } + } + + /// A pane routed to a remote tty7 daemon: no pty on this machine at all, + /// so the process list is empty by construction rather than by failure. + fn routed_procs(at_prompt: Option) -> tty7_core::daemon::protocol::PaneProcs { + use tty7_core::daemon::protocol::{RemoteContext, RemoteKind}; + tty7_core::daemon::protocol::PaneProcs { + procs: Vec::new(), + ports: Vec::new(), + context: Some(tty7_core::daemon::protocol::PaneContext { + remote: Some(RemoteContext { + kind: RemoteKind::NativeSsh, + argv: Vec::new(), + target: "me@build-box".into(), + }), + local_pty: false, + at_prompt, + remote_prompt_seen: at_prompt == Some(true), + }), } } @@ -3310,6 +3538,10 @@ mod tests { /// An unreadable process tree is not an idle one. Answering `free` on an /// empty reply would be the same false success `no-agent` was added to /// remove, one layer down. + /// + /// A bare `PaneProcs` is also what a server from before `context` existed + /// sends, so this pins the fallback: no context means the tree was all + /// there ever was, and an empty one is still "we could not look". #[test] fn wait_free_does_not_read_an_empty_process_tree_as_finished() { let mut backend = mock(); @@ -3326,6 +3558,230 @@ mod tests { matches!(out, Outcome::Exit(124, _)), "nothing was seen, so nothing can be claimed" ); + + // Without a deadline to hide behind it says so rather than spinning. + let mut backend = mock(); + for _ in 0..4 { + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + } + backend.procs_reply = tty7_core::daemon::protocol::PaneProcs::default(); + let out = execute( + cli(&["tty7", "wait", "%3", "--until", "free", "--interval", "50"]), + &Context::default(), + &mut backend, + ) + .expect("an undeterminable pane is an exit code, not an error"); + let Outcome::Exit(1, r) = out else { + panic!("expected exit 1 with a reason, got {out:?}"); + }; + assert_eq!(r.json["status"], "unknown"); + assert!( + r.json["free_unknown"] + .as_str() + .is_some_and(|w| w.contains("too old")), + "an old server's silence is named as such: {:?}", + r.json["free_unknown"] + ); + } + + /// The heart of #840. A pane sitting at an idle prompt over `ssh` has a + /// local process tree that is busy for as long as you are logged in — the + /// `ssh` itself — so the tree can never report the pane free. The far + /// shell's own prompt mark can, and it is trustworthy precisely because + /// the near shell cannot be at a prompt while the connection holds its pty. + #[test] + fn wait_free_answers_from_the_far_shells_prompt_on_an_ssh_pane() { + let mut backend = mock(); + for _ in 0..3 { + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + } + // The tree is identical on all three polls — `ssh` at depth 1 — and + // only the marks move. + backend + .procs_replies + .push_back(ssh_procs(Some(false), true)); + backend + .procs_replies + .push_back(ssh_procs(Some(false), true)); + backend.procs_replies.push_back(ssh_procs(Some(true), true)); + + let json = json_of(run_cli( + &["tty7", "wait", "%3", "--until", "free", "--interval", "50"], + &Context::default(), + &mut backend, + )); + assert_eq!(json["status"], "free"); + assert_eq!(json["matched"], true); + assert_eq!(json["stale"], false, "we watched the remote command finish"); + } + + /// A pane routed to a remote daemon has no local tree at all. Its far + /// shell's prompt mark is the entire answer, and an empty `procs` beside + /// it must not read as either "free" or "busy". + #[test] + fn wait_free_reads_a_routed_panes_prompt_mark() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + backend.procs_reply = routed_procs(Some(true)); + + let json = json_of(run_cli( + &["tty7", "wait", "%3", "--until", "free"], + &Context::default(), + &mut backend, + )); + assert_eq!(json["status"], "free"); + assert_eq!(json["matched"], true); + } + + /// The other half of #840: when the far shell sends no marks there is + /// nothing on this machine that can tell an idle remote prompt from a + /// running remote command. Saying so — and saying it without a `--timeout` + /// to hide behind — beats polling until the caller gives up. + #[test] + fn wait_free_refuses_to_guess_on_a_remote_pane_with_no_integration() { + let mut backend = mock(); + for _ in 0..4 { + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + } + backend.procs_reply = ssh_procs(Some(false), false); + + // Deliberately no `--timeout`: the old code would have polled here + // until something killed it. + let out = execute( + cli(&["tty7", "wait", "%3", "--until", "free", "--interval", "50"]), + &Context::default(), + &mut backend, + ) + .expect("an undeterminable pane is an exit code, not an error"); + let Outcome::Exit(1, r) = out else { + panic!("expected exit 1 with a reason, got {out:?}"); + }; + assert_eq!(r.json["status"], "unknown"); + assert_eq!(r.json["matched"], false); + let why = r.json["free_unknown"].as_str().expect("a recorded reason"); + assert!( + why.contains("build-box") && why.contains("shell integration"), + "the reason names the host and the thing that is missing: {why}" + ); + assert_eq!( + backend.procs_calls.len(), + 2, + "one poll of grace for a handshake in flight, then it stops" + ); + } + + /// A structurally undeterminable `free` must not cancel a wait that has + /// another state still able to answer — the agent ladder is read from a + /// different source and knows nothing about the far shell. + #[test] + fn wait_unknown_free_does_not_cancel_a_wait_on_an_agent_state() { + use tty7_core::core::cli_agent::AgentStatus; + let mut backend = mock(); + for _ in 0..3 { + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + } + backend + .replies + .push_back(ReplyOk::AgentStates(vec![agent_state( + 3, + AgentStatus::Done, + )])); + backend.procs_reply = routed_procs(None); + + let json = json_of(run_cli( + &[ + "tty7", + "wait", + "%3", + "--until", + "done,free", + "--interval", + "50", + ], + &Context::default(), + &mut backend, + )); + assert_eq!( + json["status"], "done", + "the ladder answered while `free` could not" + ); + } + + /// The insult on top of the injury: the timeout hint used to send a caller + /// to `--until free` when `--until free` was what had just timed out. + #[test] + fn wait_timeout_does_not_recommend_the_flag_that_just_failed() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + backend.procs_reply = ssh_procs(Some(false), false); + + let out = execute( + cli(&["tty7", "wait", "%3", "--until", "free", "--timeout", "0"]), + &Context::default(), + &mut backend, + ) + .expect("a timeout is an exit code, not an error"); + let Outcome::Exit(124, r) = out else { + panic!("expected exit 124, got {out:?}"); + }; + assert!( + !r.human.contains("--until free"), + "it must not recommend the flag it was given: {}", + r.human + ); + assert!( + r.human + .contains("could not determine whether this pane is free"), + "it must say what it could not determine: {}", + r.human + ); + assert_eq!( + r.json["free_unknown"] + .as_str() + .map(|s| s.contains("build-box")), + Some(true), + "the JSON carries the reason too" + ); + } + + /// `no-agent` still points at `--until free` for the caller who has not + /// tried it — that hint is the right one, it was only wrong when repeated + /// back at someone who already used it. + #[test] + fn wait_timeout_still_points_an_agentless_wait_at_free() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + + let out = execute( + cli(&["tty7", "wait", "%3", "--until", "done", "--timeout", "0"]), + &Context::default(), + &mut backend, + ) + .expect("a timeout is an exit code, not an error"); + let Outcome::Exit(124, r) = out else { + panic!("expected exit 124, got {out:?}"); + }; + assert!(r.human.contains("--until free"), "{}", r.human); + } + + /// A prompt mark outranks a deeper process on a local pane too: whatever + /// drew that prompt is a shell, not work. This is the shape a plain `ssh` + /// takes on a platform where the daemon cannot name the pane as remote — + /// Windows has no foreground process group to read the invocation from. + #[test] + fn wait_free_lets_a_prompt_mark_outrank_a_deeper_process() { + let mut backend = mock(); + backend.replies.push_back(ReplyOk::AgentStates(Vec::new())); + let mut procs = busy_procs(); + procs.context = Some(local_context(Some(true))); + backend.procs_reply = procs; + + let json = json_of(run_cli( + &["tty7", "wait", "%3", "--until", "free"], + &Context::default(), + &mut backend, + )); + assert_eq!(json["status"], "free"); } /// Watching `free` must not cost anything for callers who did not ask: diff --git a/crates/tty7-cli/src/output.rs b/crates/tty7-cli/src/output.rs index f6381c6e..8c657e9c 100644 --- a/crates/tty7-cli/src/output.rs +++ b/crates/tty7-cli/src/output.rs @@ -551,6 +551,7 @@ mod tests { addr: "*".into(), name: "node".into(), }], + context: None, }; let rendered = procs_tables(&procs); assert!( diff --git a/crates/tty7-cli/tests/cli_e2e.rs b/crates/tty7-cli/tests/cli_e2e.rs index 7283b928..22350890 100644 --- a/crates/tty7-cli/tests/cli_e2e.rs +++ b/crates/tty7-cli/tests/cli_e2e.rs @@ -75,6 +75,10 @@ fn main() { "capture_plain_returns_text_not_escapes", capture_plain_returns_text_not_escapes, ), + ( + "procs_says_where_the_panes_session_lives", + procs_says_where_the_panes_session_lives, + ), ]; let mut failed = 0; @@ -216,6 +220,14 @@ impl Daemon { .env(DAEMON_ENV, "1") .env("TTY7_CONFIG_DIR", dir.path()) .env("TTY7_DATA_DIR", dir.path()) + // The shell integration's re-entrancy guard. A test run started + // from inside a tty7 pane would otherwise hand it to every pane + // this daemon spawns, and each of them would skip its own setup — + // no prompt marks anywhere, and any assertion about them green for + // the wrong reason. The injection blanks it per pane too; this is + // the belt to that's braces, and it also covers the panes the + // injection declines to touch. + .env_remove("TTY7_SHELL_INTEGRATION") .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -398,6 +410,31 @@ fn new_builds_a_workspace_with_a_live_pane(daemon: &Daemon) { assert!(panes.contains(&format!("%{pane}")), "{panes}"); } +/// `wait --until free` reads freeness off this object, and its whole point is +/// that a process list alone cannot say whether it covers the pane. A real +/// daemon, a real pty: the context has to come back filled, and say this +/// machine holds the pane (#840). +fn procs_says_where_the_panes_session_lives(daemon: &Daemon) { + let created = daemon.run_json(&["new", &workdir()]); + let pane = created["pane"].as_u64().expect("new prints the pane id"); + + let procs = daemon.run_json(&["procs", &format!("%{pane}")]); + let context = &procs["context"]; + assert!( + context.is_object(), + "a current server always answers with a context: {procs}" + ); + assert_eq!( + context["local_pty"].as_bool(), + Some(true), + "a pane this daemon spawned itself is backed by a pty here: {procs}" + ); + assert!( + context.get("remote").is_none(), + "and it is not the near end of anything: {procs}" + ); +} + 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"); diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 3954a7e0..08461a67 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -693,6 +693,16 @@ struct PaneState { /// record. See [`crate::core::machine::PaneRecord::osc_title`]. osc_title: Option, shell: ShellState, + /// Whether a prompt mark has arrived since `remote` was last set. + /// + /// The near shell cannot be at a prompt while a connection owns its pty, + /// so a mark that says "at a prompt" on a remote pane can only be the far + /// shell's — and that is the proof that the far side runs tty7's shell + /// integration. Until it lands, the newest mark on the pane is the near + /// shell's own "I started `ssh`", which says nothing about the far side + /// and will never be superseded. Cleared whenever `remote` changes, so a + /// second hop is proved on its own terms. + remote_prompt_seen: bool, /// What this pane is running, for the machine tree to record. Distinct from /// `shell` above, which is the shell-integration state. shell_spec: Option, @@ -1461,6 +1471,7 @@ impl DaemonPane { cwd: spawn.initial_cwd, osc_title: restored_title, shell: ShellState::default(), + remote_prompt_seen: false, shell_spec: spawn.shell.clone(), remote: spawn.remote.clone(), agent: None, @@ -1682,7 +1693,14 @@ impl DaemonPane { at_prompt: carried.at_prompt, last_exit_code: carried.last_exit, command: None, + // Not carried: the handoff record is a wire format shared + // with older images, and a pane mid-`ssh` that comes back + // claiming a prompt it cannot vouch for would be worse + // than one that says it does not know. It re-latches on + // the far side's next prompt. + mark_at_prompt: false, }, + remote_prompt_seen: false, remote: carried.remote, agent: carried.agent, agent_session: carried.agent_session, @@ -1735,6 +1753,7 @@ impl DaemonPane { cwd: None, osc_title: None, shell: ShellState::default(), + remote_prompt_seen: false, remote: Some(remote), agent: None, agent_session: None, @@ -1982,11 +2001,10 @@ impl DaemonPane { // cwd/prompt change to emit while we hold the lock. let mut signals = sniffer.feed(bytes); + // `any` first: `foreground_running` is a syscall, + // and most reads carry no prompt mark at all. if signals.shell.iter().any(|s| s.at_prompt) && foreground_running() { - for s in signals.shell.iter_mut() { - s.at_prompt = false; - } - signals.shell.dedup(); + suppress_relayed_prompt_marks(&mut signals.shell); } // A prompt mark that survived the suppression above @@ -2033,6 +2051,10 @@ impl DaemonPane { || remote.is_some() || agent.is_some() || probed_cwd.is_some(); + // Read off `signals` before `apply_signals` consumes + // it; spent after the hop below — see the function. + let saw_prompt_mark = + signals.shell.iter().any(|s| s.mark_at_prompt); let mut st = state.lock().unwrap(); let facts_before = may_change_facts.then(|| observed_facts(&st)); st.ring.append(bytes); @@ -2041,6 +2063,7 @@ impl DaemonPane { if let Some(remote) = remote { apply_remote_context(&mut st, remote); } + latch_remote_prompt(&mut st, saw_prompt_mark); // Keep kitty file/shm transfer gated on the pane's // *current* locality: an `ssh` that just took the PTY // must stop us honoring host-local object names. Cheap @@ -2147,13 +2170,35 @@ impl DaemonPane { } pub fn procs(&self) -> crate::daemon::protocol::PaneProcs { - let Some(pty) = self.pty() else { - return Default::default(); + let mut out = match self.pty().and_then(|pty| { + pty.shell_pid + .map(|pid| crate::daemon::procinfo::snapshot(pid, pty_foreground_pgid(&pty.master))) + }) { + Some(procs) => procs, + None => Default::default(), }; - let Some(shell_pid) = pty.shell_pid else { - return Default::default(); - }; - crate::daemon::procinfo::snapshot(shell_pid, pty_foreground_pgid(&pty.master)) + out.context = Some(self.context()); + out + } + + /// What the pane knows about itself beyond its process list. + /// + /// Always filled, including on the branches above that have no tree to + /// walk — a native-SSH pane's empty `procs` is exactly the answer this has + /// to qualify, and returning `Default::default()` there would have said + /// "nothing is running" in the same shape as "we could not look". + fn context(&self) -> crate::daemon::protocol::PaneContext { + let st = self.state.lock().unwrap(); + crate::daemon::protocol::PaneContext { + // The cached value only. `remote_context()` falls back to probing + // the pty, and this is answered on a poll: paying a `/proc` walk + // per tick for a fact the reader already refreshes on every prompt + // would put the cost on the wrong side. + remote: st.remote.clone(), + local_pty: matches!(self.backend, PaneBackend::Pty(_)), + at_prompt: st.shell.active.then_some(st.shell.mark_at_prompt), + remote_prompt_seen: st.remote_prompt_seen, + } } fn pty(&self) -> Option<&PtyBackend> { @@ -2823,15 +2868,57 @@ fn same_dir(a: &Path, b: &Path) -> bool { } } +/// Take a batch of prompt marks out of the local line editor's reach. +/// +/// Called when a foreground program owns the pty, which means the marks are +/// being relayed — an `ssh` passing the far shell's prompt through, a nested +/// shell drawing its own. The local editor must not engage on those, so +/// `at_prompt` is cleared across the whole batch. +/// +/// [`ShellState::mark_at_prompt`] is deliberately left standing: it is the +/// same marks read for a different question, and on a remote pane it is the +/// only thing this machine knows about the far shell (#840). Because of it, +/// dedup has to compare what is actually emitted rather than the whole struct +/// — two marks that used to collapse into one `Prompt` message must still +/// collapse when only their unsuppressed twin tells them apart. +fn suppress_relayed_prompt_marks(shell: &mut Vec) { + for s in shell.iter_mut() { + s.at_prompt = false; + } + shell.dedup_by(|a, b| { + a.active == b.active + && a.at_prompt == b.at_prompt + && a.last_exit_code == b.last_exit_code + && a.command == b.command + }); +} + fn apply_remote_context(st: &mut PaneState, remote: Option) { if st.remote == remote { return; } st.cwd = None; + // A new far side has to prove its own shell integration. The mark that is + // standing right now belongs to whatever the pane was before this hop — + // the near shell's "I started `ssh`", or the previous host's prompt. + st.remote_prompt_seen = false; notify(st, DaemonMsg::RemoteContext(remote.clone())); st.remote = remote; } +/// Record that this read carried a prompt mark while the pane was remote. +/// +/// Called *after* [`apply_remote_context`], not with the signals: the read that +/// first sees a pane as remote is very often the one carrying the far shell's +/// opening prompt, and latching before the hop was applied would throw exactly +/// that mark away — the reset above would clear it a line later. Ordering it +/// here means the far side's first prompt counts, which is what makes a later +/// "not at a prompt" mean "the remote is busy" rather than "we never heard +/// from it" (#840). +fn latch_remote_prompt(st: &mut PaneState, saw_prompt_mark: bool) { + st.remote_prompt_seen |= saw_prompt_mark && st.remote.is_some(); +} + fn apply_agent( st: &mut PaneState, detected: Option<(crate::core::cli_agent::CLIAgent, Vec)>, @@ -3014,6 +3101,17 @@ fn foreground_agent( struct ShellState { active: bool, at_prompt: bool, + /// `at_prompt` as the marks themselves read it, kept out of the + /// suppression the reader applies when a foreground program is running. + /// + /// Two different questions share one pair of marks. "Should the local line + /// editor engage?" must say no while `ssh` owns the pty, whoever drew the + /// prompt — that is `at_prompt`. "Is anything running in this pane?" wants + /// the opposite: the prompt an `ssh` is relaying belongs to the shell that + /// is actually driving the pane, and it is the only thing this side can + /// read about the far one. Keeping both means neither answer has to be + /// derived from the other's. + mark_at_prompt: bool, last_exit_code: Option, command: Option, } @@ -3093,6 +3191,10 @@ fn handle_osc133(shell: &mut ShellState, rest: &[u8]) -> bool { } _ => return false, } + // The unsuppressed twin, set here so every arm gets it and no future arm + // can forget to. Suppression happens later, on the reader thread, and only + // ever touches `at_prompt`. + shell.mark_at_prompt = shell.at_prompt; true } @@ -4193,25 +4295,104 @@ mod tests { let ssh_running = is_foreground_command(Some(2000), Some(1000)); if signals.shell.iter().any(|st| st.at_prompt) && ssh_running { - for st in signals.shell.iter_mut() { - st.at_prompt = false; - } + suppress_relayed_prompt_marks(&mut signals.shell); } assert!( !signals.shell.last().unwrap().at_prompt, "a foreground program's prompt marks must not engage the local editor" ); + assert!( + signals.shell.last().unwrap().mark_at_prompt, + "the mark's own reading survives: over `ssh` it is the far shell speaking, and \ + the only thing this side knows about whether it is busy (#840)" + ); let mut local = s.feed(b"\x1b]133;A\x1b]133;B\x07"); let shell_idle = is_foreground_command(Some(1000), Some(1000)); if local.shell.iter().any(|st| st.at_prompt) && shell_idle { - for st in local.shell.iter_mut() { - st.at_prompt = false; - } + suppress_relayed_prompt_marks(&mut local.shell); } assert!(local.shell.last().unwrap().at_prompt); } + /// A prompt mark that arrives while the pane is pointed at a remote host + /// can only be the far shell's — the near one cannot be at a prompt while + /// the connection owns its pty. That is what proves the far side runs the + /// shell integration, and it is the difference between "the remote is + /// busy" and "we cannot tell" (#840). + #[test] + fn a_remote_panes_prompt_mark_latches_and_a_new_hop_clears_it() { + let mark = |mark_at_prompt: bool, command: Option<&str>| ShellState { + active: true, + at_prompt: false, + mark_at_prompt, + last_exit_code: None, + command: command.map(str::to_string), + }; + let hop = |target: &str| RemoteContext { + kind: RemoteKind::Ssh, + argv: vec!["ssh".into(), target.into()], + target: target.into(), + }; + /// One pass of the reader's ordering: signals, then the hop the same + /// read detected, then the latch. + fn read(st: &mut PaneState, shell: Vec, remote: Option>) { + let saw_prompt_mark = shell.iter().any(|s| s.mark_at_prompt); + apply_signals( + st, + SniffSignals { + shell, + ..SniffSignals::default() + }, + ); + if let Some(remote) = remote { + apply_remote_context(st, remote); + } + latch_remote_prompt(st, saw_prompt_mark); + } + + // Local: the same mark proves nothing about any far side. + let mut st = test_state(true); + read(&mut st, vec![mark(true, None)], None); + assert!(!st.remote_prompt_seen); + + // The near shell's own "I started `ssh`" — no prompt in it — must not + // latch, even though the hop lands in the same read. + read( + &mut st, + vec![mark(false, Some("ssh build-box"))], + Some(Some(hop("build-box"))), + ); + assert!( + !st.remote_prompt_seen, + "starting the connection is not evidence about what is behind it" + ); + + // The far shell's opening prompt does — including when it arrives in + // the very read that first sees the pane as remote, which on a unix + // host is the common case: the relayed prompt is suppressed, so it + // cannot trigger the immediate re-probe that would have split the two. + let mut fresh = test_state(true); + read( + &mut fresh, + vec![mark(true, None)], + Some(Some(hop("build-box"))), + ); + assert!(fresh.remote_prompt_seen); + read(&mut st, vec![mark(true, None)], None); + assert!(st.remote_prompt_seen); + + // A second hop has to prove itself over again. + read(&mut st, Vec::new(), Some(Some(hop("other-box")))); + assert!(!st.remote_prompt_seen); + + // And coming back to the local shell clears it too. + read(&mut st, vec![mark(true, None)], None); + assert!(st.remote_prompt_seen); + read(&mut st, Vec::new(), Some(None)); + assert!(!st.remote_prompt_seen); + } + #[test] fn sniff_resyncs_on_new_osc_after_an_unterminated_one() { let mut s = OscSniffer::new(); @@ -4376,6 +4557,7 @@ mod tests { cwd: None, osc_title: None, shell: ShellState::default(), + remote_prompt_seen: false, remote: None, agent: None, agent_session: None, @@ -6335,6 +6517,7 @@ mod tests { shell: vec![ShellState { active: true, at_prompt: true, + mark_at_prompt: true, last_exit_code: Some(0), command: None, }], diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index 30a8fe85..54ee4780 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -10,7 +10,13 @@ pub fn snapshot(shell_pid: u32, fg_pgid: Option) -> PaneProcs { let table = process_table(); let procs = walk(&table, shell_pid, fg_pgid); let ports = listening_ports(&procs); - PaneProcs { procs, ports } + // The caller fills `context`: only the pane knows where its session lives, + // and this module only ever walks *this* machine's table. + PaneProcs { + procs, + ports, + context: None, + } } struct Row { diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 0eaa807e..234720ee 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -468,6 +468,46 @@ impl PortEntry { pub struct PaneProcs { pub procs: Vec, pub ports: Vec, + /// What the pane can say about itself that the process list cannot — see + /// [`PaneContext`]. `None` from a daemon built before the field existed, + /// which reads as "this daemon cannot say", never as a set of falses. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub context: Option, +} + +/// Where a pane's session actually lives, and what its shell says about itself. +/// +/// The process list beside it is always *this* machine's: it starts at the +/// pty's own child and walks down. For a pane that is only the near end of a +/// connection — an `ssh` the shell is running, a native-SSH pane whose pty is +/// on another host — that list describes the tunnel, not the work. This is the +/// part of the answer that can still speak for the far side. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct PaneContext { + /// The host the pane is pointed at, when it is not this one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub remote: Option, + /// Whether this machine holds the pane's pty at all. `false` for a + /// native-SSH pane, whose `procs` is empty because there is nothing here + /// to walk — not because the walk failed. + pub local_pty: bool, + /// What the pane's shell integration last said, `None` until it emits its + /// first OSC 133 mark. + /// + /// This is the mark's own reading, taken before the suppression that keeps + /// a foreground program's prompt marks from engaging the local line editor. + /// That suppression is right for the editor and wrong here: on a pane + /// running `ssh` the marks are the *far* shell's, and they are the only + /// thing on this side that knows whether the far shell is busy. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub at_prompt: Option, + /// Whether a prompt mark has arrived while the pane was pointed at + /// `remote`. Only the far shell can be at a prompt while the near one is + /// occupied by the connection, so this is the proof that the far side's + /// shell integration is loaded and reporting. Without it, "not at a + /// prompt" on a remote pane means nothing: the newest mark is then the + /// near shell's own "I started `ssh`", and it will never be replaced. + pub remote_prompt_seen: bool, } fn default_term() -> String { diff --git a/crates/tty7-core/src/host/server.rs b/crates/tty7-core/src/host/server.rs index 1f2fa862..b59f5631 100644 --- a/crates/tty7-core/src/host/server.rs +++ b/crates/tty7-core/src/host/server.rs @@ -1711,6 +1711,7 @@ mod aggregate_tests { name: "node".into(), addr: "*".into(), }], + context: None, } } diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index a7cf88b5..c64ac16d 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -152,10 +152,18 @@ The process tree inside the pane, indented by depth, `*` on the foreground process — then a second table of ports those processes are listening on. Prints `nothing running in this pane` when both are empty. -JSON: `{"procs":[{"pid","name","depth","foreground"}],"ports":[{"port","pid","name","addr"}]}` — +JSON: `{"procs":[{"pid","name","depth","foreground"}],"ports":[{"port","pid","name","addr"}],"context":{...}}` — `addr` is the address the socket is bound to (`*`, `0.0.0.0`, `127.0.0.1`, `[::1]`, or a specific interface). +`context` says how much of the pane the process list actually covers: +`{"remote","local_pty","at_prompt","remote_prompt_seen"}`. The tree is always +*this* machine's, so on a pane that is the near end of a connection it lists the +tunnel rather than the work — `local_pty` is `false` for a pane routed to a +remote daemon (nothing here to walk), and `remote` names the host otherwise. +`at_prompt` is the pane's own shell integration talking, absent until it emits +its first mark. Older servers omit `context` entirely. + ### `tty7 agents` Every pane running a recognised coding agent. Table: @@ -182,8 +190,9 @@ by its [hooks](/agents/status); the last three are facts about the pane: |---|---| | `idle` `working` `waiting` `done` | The agent's status | | `no-agent` | Nothing is reporting status here — a plain shell, or an agent whose hooks are not installed | -| `free` | The foreground command has exited; the pane is back to its bare shell | +| `free` | Nothing is running in front of the pane's shell — it will take input now | | `exit` | The pane itself is gone. Ends every wait whether it was asked for or not | +| `unknown` | Only ever reported, never awaited: freeness could not be determined at all. See below | `free` is how you wait for a **command** rather than an agent, and it is the one state that costs a second request per poll — so it is only checked when you @@ -192,13 +201,32 @@ With `--changed` it means "something ran and then finished", which is what you want directly after a `send`; a command quick enough to finish inside one `--interval` is never seen running, and the timeout says so. +Two things answer it. On a pane whose shell is on this machine it is the process +tree: nothing deeper than the shell means free. On a **remote or SSH pane** that +tree describes the near end of the connection — the `ssh` itself, busy for as +long as you are logged in, or nothing at all for a pane routed to a remote +daemon — so there it is the far shell's own +[prompt marks](/reference/shell-integration) that decide. A prompt mark on such a +pane can only have come from the far side, because the near shell cannot be at a +prompt while the connection holds its pty. Marks outrank a deeper process on a +local pane too, which is why `free` means "will take input" rather than strictly +"back to the bare shell": a pane sitting at a nested shell's prompt is free. + +When the far host has no shell integration loaded there is nothing on this side +that can tell an idle remote prompt from a running remote command. `wait` says +so — `status: unknown`, exit `1`, and a `free_unknown` string naming the host — +rather than polling to the end of `--timeout`. It gives one poll of grace first, +for a handshake still in flight, and it only gives up when `free` was the only +state that could still answer: `--until done,free` keeps waiting on `done`. + The reply carries the agent's message and native session id. The JSON's `stale` flag says whether the answer might belong to the previous turn. JSON: `{"pane","status","matched","stale","activity","message","session_id"}`. A timeout exits `124` with the same object plus `"timed_out": true` — `matched` is `false` there, and `stale` still says whether the pane moved -while you watched. +while you watched, and `free_unknown` carries the reason when `free` was asked +for and never resolved. [Orchestration →](/agents/orchestration) ### `tty7 events`