mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-10-03 08:02:02 +00:00
fix(ssh): never restore an SSH tab as a local shell; show a reattached one as connected
After the server restarted, a native SSH tab's old pane id was attached to, the attach failed, and the fallback spawned a local shell in its place: the tab that had been another machine was now this one, still titled and grouped like the remote. An SSH leaf is now only reattached when the server lists its pane; otherwise the host is dialled again. A window reattaching to a live SSH pane also never learned its phase, since status frames only went to whoever was attached when they were sent. The pane keeps the last one and replays it, so the tab keeps its connected dot and warn-before-closing still applies.
This commit is contained in:
@@ -729,6 +729,11 @@ struct PaneState {
|
||||
/// and will never be superseded. Cleared whenever `remote` changes, so a
|
||||
/// second hop is proved on its own terms.
|
||||
remote_prompt_seen: bool,
|
||||
/// A native SSH pane's connection phase, as last sent. Status frames go
|
||||
/// only to whoever is attached at the time, so a window reattaching to a
|
||||
/// live session learned nothing and drew it as an unknown remote — no
|
||||
/// "connected" dot, and no warning before closing it.
|
||||
ssh_phase: Option<crate::daemon::protocol::SshPhase>,
|
||||
/// The private modes the pane's output has switched on — the alternate
|
||||
/// screen and mouse reporting above all. Folded from the same bytes the
|
||||
/// ring gets, because the ring cannot be trusted to still hold them: a
|
||||
@@ -1767,6 +1772,7 @@ impl DaemonPane {
|
||||
osc_title: restored_title,
|
||||
shell: ShellState::default(),
|
||||
remote_prompt_seen: false,
|
||||
ssh_phase: None,
|
||||
modes: TerminalModes::default(),
|
||||
shell_spec: spawn.shell.clone(),
|
||||
remote: spawn.remote.clone(),
|
||||
@@ -2002,6 +2008,7 @@ impl DaemonPane {
|
||||
mark_at_prompt: false,
|
||||
},
|
||||
remote_prompt_seen: false,
|
||||
ssh_phase: None,
|
||||
modes: TerminalModes::default(),
|
||||
remote: carried.remote,
|
||||
agent: carried.agent,
|
||||
@@ -2059,6 +2066,7 @@ impl DaemonPane {
|
||||
osc_title: None,
|
||||
shell: ShellState::default(),
|
||||
remote_prompt_seen: false,
|
||||
ssh_phase: None,
|
||||
modes: TerminalModes::default(),
|
||||
remote: Some(remote),
|
||||
agent: None,
|
||||
@@ -2074,7 +2082,11 @@ impl DaemonPane {
|
||||
let broker = {
|
||||
let state = state.clone();
|
||||
crate::daemon::ssh::PromptBroker::new(Box::new(move |msg: DaemonMsg| {
|
||||
match &state.lock().unwrap().subscriber {
|
||||
let mut st = state.lock().unwrap();
|
||||
if let DaemonMsg::SshStatus { phase } = &msg {
|
||||
st.ssh_phase = Some(phase.clone());
|
||||
}
|
||||
match &st.subscriber {
|
||||
Some(sub) => sub.send(msg).is_ok(),
|
||||
None => false,
|
||||
}
|
||||
@@ -3117,6 +3129,11 @@ fn replay_state(st: &PaneState, subscriber: &Sender<DaemonMsg>, foreground_comma
|
||||
if st.remote.is_some() {
|
||||
let _ = subscriber.send(DaemonMsg::RemoteContext(st.remote.clone()));
|
||||
}
|
||||
if let Some(phase) = &st.ssh_phase {
|
||||
let _ = subscriber.send(DaemonMsg::SshStatus {
|
||||
phase: phase.clone(),
|
||||
});
|
||||
}
|
||||
if st.agent.is_some() {
|
||||
let _ = subscriber.send(DaemonMsg::Agent(st.agent));
|
||||
}
|
||||
@@ -4904,6 +4921,25 @@ mod tests {
|
||||
assert!(sig.shell.last().unwrap().at_prompt);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_window_reattaching_to_an_ssh_pane_learns_it_is_connected() {
|
||||
use crate::daemon::protocol::SshPhase;
|
||||
let mut st = test_state(true);
|
||||
st.ssh_phase = Some(SshPhase::Connected);
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
replay_state(&st, &tx, false);
|
||||
drop(tx);
|
||||
assert!(
|
||||
rx.iter().any(|m| matches!(
|
||||
m,
|
||||
DaemonMsg::SshStatus {
|
||||
phase: SshPhase::Connected
|
||||
}
|
||||
)),
|
||||
"the replay must carry the connection phase"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_title_is_kept_until_it_changes_and_a_reset_clears_it() {
|
||||
let mut st = test_state(true);
|
||||
@@ -5489,6 +5525,7 @@ mod tests {
|
||||
osc_title: None,
|
||||
shell: ShellState::default(),
|
||||
remote_prompt_seen: false,
|
||||
ssh_phase: None,
|
||||
modes: TerminalModes::default(),
|
||||
remote: None,
|
||||
agent: None,
|
||||
|
||||
+37
-4
@@ -10117,6 +10117,19 @@ fn redial_native_ssh(layout: &SessionPane) -> SessionPane {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a native SSH pane can be attached to by its old id: only when the
|
||||
/// server lists it. A failed attach falls through to a fresh *local* shell,
|
||||
/// and after a server restart that put the user's own machine behind a tab
|
||||
/// that had been — and still looked like — a session on another one. Without
|
||||
/// a listing the host is dialled again: a second connection is recoverable,
|
||||
/// typing into the wrong machine is not.
|
||||
fn native_ssh_pane_alive(
|
||||
alive: Option<&std::collections::HashMap<u64, Option<String>>>,
|
||||
id: u64,
|
||||
) -> bool {
|
||||
alive.is_some_and(|alive| alive.contains_key(&id))
|
||||
}
|
||||
|
||||
fn leaf_shares_the_window_daemon(window_is_remote: bool, leaf_is_native_ssh: bool) -> bool {
|
||||
!(window_is_remote && leaf_is_native_ssh)
|
||||
}
|
||||
@@ -10147,7 +10160,11 @@ fn session_to_pane(
|
||||
// Not `pane_attachable`: a dead pane's id is what the restore
|
||||
// is keyed on, so it has to survive being dead. The attach is
|
||||
// still attempted first and still gives way to a fresh spawn.
|
||||
false => (*pane_id).filter(|id| same_daemon && pane_free_for(alive, *id, owner)),
|
||||
false => (*pane_id).filter(|id| {
|
||||
same_daemon
|
||||
&& pane_free_for(alive, *id, owner)
|
||||
&& (ssh_spec.is_none() || native_ssh_pane_alive(alive, *id))
|
||||
}),
|
||||
};
|
||||
if restore.is_none() {
|
||||
if let Some(spec) = ssh_spec.clone() {
|
||||
@@ -10970,9 +10987,10 @@ mod tests {
|
||||
use super::{
|
||||
CloseReason, DOCUMENT_MIN_W, Dir, Pane, Rename, TERMINAL_MIN_W, TITLE_BAR_HEIGHT, Tab,
|
||||
TabAgentSession, clear_window_override_values, close_prompt, document_column_px,
|
||||
join_shell_args, leaf_shares_the_window_daemon, mru_order, one_slot_move, pane_free_for,
|
||||
parse_ssh_connect_input, parse_ssh_option_words, rename_outcome, side_panel_max,
|
||||
split_shell_args, step_in_order, strip_band, wd_path_saveable,
|
||||
join_shell_args, leaf_shares_the_window_daemon, mru_order, native_ssh_pane_alive,
|
||||
one_slot_move, pane_free_for, parse_ssh_connect_input, parse_ssh_option_words,
|
||||
rename_outcome, side_panel_max, split_shell_args, step_in_order, strip_band,
|
||||
wd_path_saveable,
|
||||
};
|
||||
use gpui::{Edges, point, px, size};
|
||||
|
||||
@@ -11463,6 +11481,21 @@ mod tests {
|
||||
assert!(leaf_shares_the_window_daemon(false, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_native_ssh_pane_the_server_no_longer_has_is_dialled_again() {
|
||||
let mut alive = std::collections::HashMap::new();
|
||||
alive.insert(4u64, None);
|
||||
assert!(native_ssh_pane_alive(Some(&alive), 4));
|
||||
assert!(
|
||||
!native_ssh_pane_alive(Some(&alive), 9),
|
||||
"gone: dial, don't attach"
|
||||
);
|
||||
assert!(
|
||||
!native_ssh_pane_alive(None, 4),
|
||||
"no listing: dial, don't guess"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_fork_needs_a_command_an_id_and_a_local_pane() {
|
||||
let session = |fork_label, session_id: Option<&str>, remote| TabAgentSession {
|
||||
|
||||
@@ -770,7 +770,8 @@ impl Item {
|
||||
Item::localized(L10nKey::CmdSshAddConnection, SearchHosts),
|
||||
Item::localized(L10nKey::CmdSshManageProfiles, OpenSshProfiles),
|
||||
Item::localized(L10nKey::CmdSshReconnect, RestartSshSession),
|
||||
Item::localized(L10nKey::CmdSshRemoteFiles, ToggleSftp),
|
||||
// The panel is an SFTP browser, and that is the word people search.
|
||||
Item::localized(L10nKey::CmdSshRemoteFiles, ToggleSftp).with_alias("SFTP"),
|
||||
Item::localized(L10nKey::CmdSshPortForwarding, ShowSshForwards),
|
||||
];
|
||||
|
||||
|
||||
@@ -194,6 +194,13 @@ mod tests {
|
||||
assert!(title_hit > subtitle_hit);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_remote_files_panel_is_found_as_sftp() {
|
||||
let cmd =
|
||||
Item::localized(L10nKey::CmdSshRemoteFiles, CommandKind::ToggleSftp).with_alias("SFTP");
|
||||
assert!(item_score("sftp", &cmd).is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn letters_strewn_across_a_description_do_not_match_it() {
|
||||
let cmd = Item::new("Git: Discard All Changes", CommandKind::NewTab)
|
||||
|
||||
Reference in New Issue
Block a user