diff --git a/crates/tty7-core/src/daemon/shell_integration.rs b/crates/tty7-core/src/daemon/shell_integration.rs index 6f4aeea3..9b0664c7 100644 --- a/crates/tty7-core/src/daemon/shell_integration.rs +++ b/crates/tty7-core/src/daemon/shell_integration.rs @@ -1296,6 +1296,26 @@ pub mod remote { } } + /// What the probe *said*, kept apart from its having said nothing. + /// + /// `None` means the far side never answered: the channel or the command + /// failed, or the timeout cut the read off before the marker and the line + /// it introduces both arrived. That is a different fact from "answered, + /// and logs in with a shell there is no bootstrap for", which is + /// `Some(None)`. Only the second is worth remembering — the first is a + /// slow link or a busy server, and the next pane deserves a fresh ask. + pub(crate) fn probe_answer(output: &str) -> Option> { + // Read exactly as far as `parse_probe` does before it can form an + // opinion, so the two cannot disagree about whether there was one. + let mut lines = output + .lines() + .map(|l| l.trim_end_matches('\r').trim()) + .skip_while(|l| *l != PROBE_MARKER); + lines.next()?; + lines.find(|l| !l.is_empty())?; + Some(parse_probe(output)) + } + pub(crate) fn parse_probe(output: &str) -> Option<(RemoteShell, String)> { let mut lines = output .lines() @@ -1436,6 +1456,39 @@ fi assert_eq!(parse_probe("/bin/zsh\n"), None); } + #[test] + fn a_probe_that_never_answered_is_told_apart_from_one_that_said_no() { + // Answered, with a shell there is a bootstrap for. + assert_eq!( + probe_answer("__tty7_shell\n/bin/zsh\n"), + Some(Some((RemoteShell::Zsh, "/bin/zsh".to_string()))) + ); + // Answered, and the answer is one there is no bootstrap for. Worth + // remembering: asking again gets the same `/bin/ksh` every time. + assert_eq!(probe_answer("__tty7_shell\n/bin/ksh\n"), Some(None)); + assert_eq!(probe_answer("__tty7_shell\n$SHELL\n"), Some(None)); + + // Never answered. The channel or the exec failed, or the read timed + // out — none of which says anything about the remote's shell, and + // all of which used to be cached as if it did. + assert_eq!(probe_answer(""), None, "nothing came back at all"); + assert_eq!( + probe_answer("Welcome to prod!\n"), + None, + "a banner, and then the read was cut off" + ); + assert_eq!( + probe_answer("__tty7_shell\n"), + None, + "the marker arrived and the line it introduces did not" + ); + assert_eq!( + probe_answer("__tty7_shell\n\n"), + None, + "and a blank line is not that line" + ); + } + #[test] fn zsh_bootstrap_gates_zdotdir_on_every_redirector_landing() { let script = bootstrap_command(RemoteShell::Zsh, "/bin/zsh"); diff --git a/crates/tty7-core/src/daemon/ssh/mod.rs b/crates/tty7-core/src/daemon/ssh/mod.rs index 50d33fe9..e69a28f5 100644 --- a/crates/tty7-core/src/daemon/ssh/mod.rs +++ b/crates/tty7-core/src/daemon/ssh/mod.rs @@ -449,17 +449,28 @@ impl SshManager { let cached = { self.probes.lock().unwrap().get(&key).cloned() }; let probed = match cached { Some(hit) => hit, - None => { - let probed = probe_remote_shell(conn).await; - match &probed { - Some((shell, path)) => { - log::debug!("ssh {key:?}: remote shell {shell:?} at {path}") + // Only an answer is remembered. This map is on the process-wide + // `SshManager` and nothing ever evicts from it, so caching a probe + // that failed would spend the rest of the daemon's life claiming a + // host has no shell integration because one channel open, one + // exec, or one five-second read went badly. Reconnecting would not + // clear it either. + None => match probe_remote_shell(conn).await { + Some(answer) => { + match &answer { + Some((shell, path)) => { + log::debug!("ssh {key:?}: remote shell {shell:?} at {path}") + } + None => log::debug!("ssh {key:?}: no remote shell integration"), } - None => log::debug!("ssh {key:?}: no remote shell integration"), + self.probes.lock().unwrap().insert(key, answer.clone()); + answer } - self.probes.lock().unwrap().insert(key, probed.clone()); - probed - } + None => { + log::debug!("ssh {key:?}: shell probe did not answer; will ask again"); + None + } + }, }; probed.map(|(shell, path)| remote::bootstrap_command(shell, &path)) } @@ -612,7 +623,9 @@ const PROBE_TIMEOUT: Duration = Duration::from_secs(5); const PROBE_OUTPUT_LIMIT: usize = 8 * 1024; -async fn probe_remote_shell(conn: &SshConnection) -> Option<(remote::RemoteShell, String)> { +/// `None` when the remote never answered, so the caller knows not to remember +/// it. See [`remote::probe_answer`]. +async fn probe_remote_shell(conn: &SshConnection) -> Option> { let mut channel = conn.open_session_channel().await.ok()?; channel.exec(true, remote::PROBE_COMMAND).await.ok()?; @@ -633,7 +646,7 @@ async fn probe_remote_shell(conn: &SshConnection) -> Option<(remote::RemoteShell }; let _ = tokio::time::timeout(PROBE_TIMEOUT, collect).await; - remote::parse_probe(&String::from_utf8_lossy(&out)) + remote::probe_answer(&String::from_utf8_lossy(&out)) } async fn probe_remote_env(conn: &SshConnection) -> Option {