starred/tty7
1
0
Fork 0
mirror of https://github.com/l0ng-ai/tty7.git synced 2026-09-22 00:02:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(remote): keep a remote workspace's layout across reconnects

Browse Source 
A remote workspace came back empty every time. The layout was thrown away
on both sides of design §10's storage split: `record_session` blanked a
remote entry's `session` before anything could push it, and
`claimable_session` blanked it again on the way out, so the record pulled
from the machine on connect was discarded the moment the window opened
from it. Nothing ever called `push_remote_layout` outside workspace
creation either, which left the machine's own `workspaces.json` holding a
record with no tabs in it.

The entry's `session` is now what it was always meant to be — this
client's cache of a record the machine owns — and the split is enforced
by reachability instead of by erasure:

- `claim` / `record` ask `machine_is_connected` first. A connected remote
  window stores and reopens its layout like any other; an unreachable one
  opens empty and, the half that matters, leaves the cached layout alone
  so there is still something to rebuild from.
- `save_session` pushes to the machine that owns the record, so the
  remote's copy tracks every structural change rather than only the
  workspace's creation.
- `finish_attempt` hydrates a window that came up before its machine did.
  Only an empty one: a window with tabs is one the user is working in.

The invariant the erasure was protecting still holds, and holds where it
belongs — every leaf routes through `pane_workspace_for`, so a remote
workspace's panes attach or spawn over there, and an unreachable machine
fails the spawn rather than falling back to a local shell.

Also stop matching a native-SSH leaf's `pane_id` against a remote
workspace's alive set. That pane lives in this client's daemon however
the window is bound, and pane ids are unique only within one daemon, so a
collision would have swapped the user's SSH tab for whatever the machine
happened to be running under the same number.
This commit is contained in:
l0ng-ai
2026-07-28 11:46:08 +08:00
parent 4814d94b81
commit d534bdbb75
3 changed files with 249 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/session.rs
+148 -59
View File
@@ -79,6 +79,11 @@ impl WorkspaceStore {
/// when `id` is `None` / no longer on file (the "New Workspace" path). Marks it
/// open and returns its id plus the tabs the window should rebuild.
pub fn claim(cx: &mut gpui::App, id: Option<WorkspaceId>) -> (WorkspaceId, Session) {
// Read before the store is borrowed: whether the layout may be rebuilt
// depends on another global (the connection table), and a remote
// workspace whose machine is unreachable must open empty. See
// [`claimable_session`].
let reachable = id.is_none_or(|id| Self::machine_is_connected(cx, id));
let Some(store) = Self::try_store(cx) else {
// No store (tests): hand back a detached identity so the window
// still builds, but nothing is persisted.
@@ -94,7 +99,7 @@ impl WorkspaceStore {
};
workspace.open = true;
workspace.touch();
let claimed = (workspace.id, claimable_session(workspace));
let claimed = (workspace.id, claimable_session(workspace, reachable));
store.workspaces.active = Some(claimed.0);
store.workspaces.save();
claimed
@@ -108,6 +113,11 @@ impl WorkspaceStore {
session: Session,
window: Option<crate::core::window_state::WindowState>,
) {
// Same reason as in [`claim`]: read the connection table before the
// store is borrowed. A window whose machine is unreachable is not
// describing that machine's layout, so it does not get to overwrite the
// copy we have of it — see [`record_session`].
let reachable = Self::machine_is_connected(cx, id);
let Some(store) = Self::try_store(cx) else {
return;
};
@@ -116,7 +126,7 @@ impl WorkspaceStore {
// tearing down); nothing to record.
return;
};
record_session(workspace, session);
record_session(workspace, session, reachable);
if let Some(window) = window {
workspace.window = Some(window);
}
@@ -189,6 +199,20 @@ impl WorkspaceStore {
Self::all(cx).get(id).and_then(|w| w.host.clone())
}
/// Whether this client can reach the machine `id`'s panes are on *right
/// now* — the predicate both halves of the layout cache turn on
/// ([`claimable_session`], [`record_session`]).
///
/// A local workspace is always reachable: its daemon is this machine's, and
/// a gate that could answer otherwise for a local window would stop it
/// saving its own tabs.
pub fn machine_is_connected(cx: &mut gpui::App, id: WorkspaceId) -> bool {
let Some(host) = Self::remote_ref(cx, id) else {
return true;
};
crate::ui::remote_connect::RemoteConnections::get(cx, host.host_id()).is_some()
}
/// The client-side entry for `host` — the existing one if this machine has
/// seen that workspace before, a fresh one otherwise.
///
@@ -268,15 +292,6 @@ impl WorkspaceStore {
}
}
/// Write a window's layout onto its workspace entry, honouring design §10's
/// storage split.
///
/// **A remote workspace's entry never holds a layout on this client.** The
/// machine's own `workspaces.json` is the authority for it, and the client entry
/// is a pointer plus this machine's view state. That is not just tidiness: a
/// remote entry carrying local `SessionPane`s is exactly the shape "one window,
/// two hosts" would take on disk, and clearing it here is what makes the
/// invariant survive a restart rather than only holding while the app runs.
/// The machine a window showing `id` is bound to.
///
/// The whole of "one window, one machine" reduces to this being a *function*: a
@@ -305,30 +320,42 @@ pub(crate) fn crosses_machines(previous: HostId, current: HostId) -> bool {
/// The layout a window opening on `workspace` may rebuild — the read-side twin
/// of [`record_session`].
///
/// Both halves are needed, and it took a real launch to notice: the write guard
/// stops this client *creating* a remote entry with a local layout, but it says
/// nothing about one that arrived some other way — a hand-edited `session.json`,
/// a file written before the split existed, a sync tool. Restoring such an entry
/// would rebuild local shells inside a window bound to another machine, which is
/// design §3's "never do this" arriving through the back door.
/// A remote entry's `session` is this client's copy of a record the machine
/// owns: pulled on connect, refreshed on every `WorkspaceChanged`, pushed back
/// on every structural change. Rebuilding from it is the whole of "reconnecting
/// gets my tabs back", and it is safe to do because every pane it names is
/// routed by [`crate::ui::remote_workspace::pane_workspace_for`] — a leaf in a
/// remote workspace attaches or spawns *over there*, and a machine that cannot
/// be reached fails the spawn rather than falling back to a local shell.
///
/// So a remote workspace always opens empty here, and the entry is scrubbed on
/// the way past so the bad layout does not survive to be tried again. The real
/// layout is the remote's `workspaces.json`, pulled on connect.
fn claimable_session(workspace: &mut Workspace) -> Session {
if workspace.is_remote() {
workspace.session = Session::default();
/// `reachable` is what keeps that guarantee from being theoretical. With the
/// link down, `List` answers nothing, so every leaf would miss its live pane and
/// try to spawn a fresh one — either failing (an empty window, having thrown the
/// layout away) or, worse, landing a second shell next to the one still running
/// over there. So an unreachable remote workspace opens empty **without
/// touching the cached layout**, and
/// [`crate::ui::remote_workspace`]'s connect path rebuilds the window the moment
/// the machine answers.
fn claimable_session(workspace: &mut Workspace, reachable: bool) -> Session {
if workspace.is_remote() && !reachable {
return Session::default();
}
workspace.session.clone()
}
fn record_session(workspace: &mut Workspace, session: Session) {
workspace.session = if workspace.is_remote() {
Session::default()
} else {
session
};
/// Write a window's layout onto its entry — the write-side twin of
/// [`claimable_session`].
///
/// A window that cannot reach its machine is not describing that machine's
/// layout (its panes failed to restore, or are sitting there disconnected), so
/// it records nothing rather than replacing the copy we have with the wreckage.
/// The remote's own `workspaces.json` is still the authority; this entry is the
/// cache the next launch opens from.
fn record_session(workspace: &mut Workspace, session: Session, reachable: bool) {
if workspace.is_remote() && !reachable {
return;
}
workspace.session = session;
}
#[cfg(test)]
@@ -370,22 +397,25 @@ mod tests {
#[test]
fn a_local_workspace_stores_its_own_layout() {
let mut workspace = Workspace::default();
record_session(&mut workspace, local_layout());
record_session(&mut workspace, local_layout(), true);
assert_eq!(workspace.session.tabs.len(), 1);
assert_eq!(workspace.pane_ids(), vec![7]);
}
/// The one that matters: a remote entry must never end up holding panes
/// from this machine. This is the on-disk half of "a window is one machine"
/// — if a local layout could be written onto a remote entry, the next launch
/// would restore local shells into a window bound to a remote host, which is
/// design §3's "never do this".
/// The point of the whole cache: a connected remote window's layout is
/// kept, so the next launch has something to open from and
/// `remote_payload` has something to push. Without this, reconnecting to a
/// machine gives an empty window every time.
#[test]
fn a_remote_workspace_never_stores_a_local_layout() {
fn a_connected_remote_workspace_stores_its_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
record_session(&mut workspace, local_layout());
assert!(workspace.session.tabs.is_empty());
assert!(workspace.pane_ids().is_empty());
record_session(&mut workspace, local_layout(), true);
assert_eq!(workspace.session.tabs.len(), 1);
assert_eq!(
workspace.pane_ids(),
vec![7],
"the pane ids are the remote daemon's, and are what a reconnect re-attaches"
);
}
/// A local workspace opens on the layout it saved.
@@ -395,42 +425,101 @@ mod tests {
session: local_layout(),
..Workspace::default()
};
let claimed = claimable_session(&mut workspace);
let claimed = claimable_session(&mut workspace, true);
assert_eq!(claimed.tabs.len(), 1);
// And the entry is left alone.
assert_eq!(workspace.session.tabs.len(), 1);
}
/// The regression a real launch caught: a remote entry that arrived holding
/// a local layout — a hand-edited `session.json`, or a file written before
/// the storage split — would otherwise rebuild local shells inside a window
/// bound to another machine on the next start.
///
/// It must open empty *and* be scrubbed, so a layout that got in somehow
/// cannot sit there being retried on every launch.
/// A connected remote workspace reopens on the layout its machine last
/// reported — the read half of "reconnecting gets my tabs back".
#[test]
fn a_remote_workspace_never_reopens_a_local_layout() {
fn a_connected_remote_workspace_reopens_its_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
let claimed = claimable_session(&mut workspace, true);
assert_eq!(claimed.tabs.len(), 1);
assert_eq!(workspace.session.tabs.len(), 1);
}
/// With the machine unreachable, `List` answers nothing, so every leaf
/// would miss its live pane and try to spawn a fresh one beside it. The
/// window opens empty instead — and, the half that took a real launch to
/// get right, **the cached layout survives**: it is what the connect path
/// rebuilds the window from a moment later.
#[test]
fn an_unreachable_remote_workspace_opens_empty_but_keeps_its_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
let claimed = claimable_session(&mut workspace);
let claimed = claimable_session(&mut workspace, false);
assert!(claimed.tabs.is_empty(), "the window must open with no tabs");
assert!(
workspace.session.tabs.is_empty(),
"and the bad layout must not survive to be tried again"
assert_eq!(
workspace.session.tabs.len(),
1,
"and the layout must still be there for the connect to rebuild from"
);
}
/// And a remote entry that somehow *arrived* holding a layout (a
/// hand-edited `session.json`, a record from a build that predates the
/// split) is cleaned out the first time the window records itself, rather
/// than being left to restore later.
/// The write-side twin: a window that could not restore its panes is not
/// describing the machine's layout, so its empty tab list must not replace
/// the copy we have of it.
#[test]
fn recording_clears_a_layout_a_remote_entry_should_never_have_had() {
fn an_unreachable_remote_window_does_not_overwrite_the_cached_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
record_session(&mut workspace, Session::default());
assert!(workspace.session.tabs.is_empty());
record_session(&mut workspace, Session::default(), false);
assert_eq!(workspace.session.tabs.len(), 1);
}
/// The launch path end to end, with the store's own reachability lookup
/// rather than a hand-passed flag: nothing has ever connected to that
/// machine in this process, so the window opens empty and the layout it
/// will be rebuilt from is still on file afterwards.
///
/// The config dir is pinned first because `claim` and `record` both persist
/// — without it this test would rewrite the developer's real
/// `session.json`.
#[gpui::test]
fn an_unconnected_machine_keeps_its_workspace_layout_across_a_claim(
cx: &mut gpui::TestAppContext,
) {
cx.update(|cx| {
// The same path every other config-pinning test in this process
// uses: `set_config_dir` is first-call-wins, so a test that pinned
// a *different* scratch would silently redirect whichever tests
// lost the race away from the directory they then read back.
let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id()));
std::fs::create_dir_all(&dir).ok();
crate::core::config::set_config_dir(dir);
let mut entry = Workspace::on_remote(remote_ref());
entry.session = local_layout();
let id = entry.id;
WorkspaceStore::install_for_test(
cx,
Workspaces {
workspaces: vec![entry],
active: None,
},
);
let (claimed, session) = WorkspaceStore::claim(cx, Some(id));
assert_eq!(claimed, id);
assert!(
session.tabs.is_empty(),
"an unreachable machine's window opens empty"
);
// …and the window recording that emptiness does not erase what the
// machine still has.
WorkspaceStore::record(cx, id, Session::default(), None);
assert_eq!(
WorkspaceStore::all(cx).get(id).unwrap().session.tabs.len(),
1,
"the cached layout must survive for the connect to rebuild from"
);
});
}
/// The remote-bound payload travels under the *remote's* id, so a record
src/ui/app.rs
+48 -2
View File
@@ -1239,6 +1239,13 @@ impl Tty7App {
session,
Some(WindowState::from_bounds(self.window_bounds)),
);
// …and for a remote workspace the machine that owns the layout has to
// hear about it, or `session.json` is the only place it exists and any
// other client (or a fresh install) opens the workspace empty. No-ops
// for a local workspace and for a machine we are not connected to —
// the latter is also what keeps a window that failed to restore from
// pushing its emptiness over a good record.
self.push_remote_layout(self.workspace, cx);
}
/// This window is going away: capture its final state (a plain `cd` may
@@ -6349,6 +6356,23 @@ fn tabs_from_session(
(tabs, active)
}
/// Whether a restored leaf's saved `pane_id` names a pane in the same daemon
/// the caller read its `alive` set from — the window's daemon.
///
/// Pane ids are unique only *within* a daemon, so the question is not academic:
/// looking one up in the wrong set is how a saved id silently matches somebody
/// else's live pane and the restore attaches to it.
///
/// A native-SSH leaf is the one case where a pane does not live in its window's
/// daemon. Its russh session is spawned by **this client's** daemon however the
/// window is bound, so in a remote workspace it belongs to a different machine
/// than every other leaf around it — and its id must not be matched against the
/// remote's pane list. It reconnects from its saved spec instead, which is what
/// it does for any id that is no longer live.
fn leaf_shares_the_window_daemon(window_is_remote: bool, leaf_is_native_ssh: bool) -> bool {
!(window_is_remote && leaf_is_native_ssh)
}
/// Rebuild a live `Pane` tree from a saved `SessionPane`. A leaf whose saved
/// `pane_id` is still alive in the daemon re-`attach`es (process + scrollback
/// intact); otherwise it spawns a fresh shell in the saved cwd. `alive` is the
@@ -6376,7 +6400,12 @@ fn session_to_pane(
} => {
// Only restore the pane id when the daemon confirms it's still live;
// a stale id (daemon restarted, pane killed) falls back to a spawn.
let restore = (*pane_id).filter(|id| alive.contains(id));
//
// …and `alive` is *one* daemon's pane set, so a leaf whose pane
// lives in a different one must not be looked up in it.
let same_daemon =
leaf_shares_the_window_daemon(workspace.is_some(), ssh_spec.is_some());
let restore = (*pane_id).filter(|id| same_daemon && alive.contains(id));
// A *dead* native-SSH leaf (spec persisted, pane no longer alive)
// reconnects rather than dropping back to a local shell (FR-C2/E4):
// re-resolve secrets from the profile when it names one, else reuse
@@ -6721,7 +6750,24 @@ fn apply_ssh_o_option(
#[cfg(test)]
mod tests {
use super::{parse_ssh_connect_input, parse_ssh_option_words};
use super::{leaf_shares_the_window_daemon, parse_ssh_connect_input, parse_ssh_option_words};
/// A remote window's saved layout can hold a native-SSH pane, whose russh
/// session runs in *this* client's daemon rather than the machine's. Its
/// saved id must not be matched against the remote's pane list: the two
/// daemons number panes independently, so `1` over there is a different
/// pane, and restoring it would swap the user's SSH tab for whatever the
/// remote happens to be running.
#[test]
fn a_native_ssh_leaf_in_a_remote_window_is_not_looked_up_in_the_remote_daemon() {
assert!(!leaf_shares_the_window_daemon(true, true));
// Everything else is the window's own daemon: a shell in a remote
// window is a pane over there, and in a local window both kinds are
// panes here.
assert!(leaf_shares_the_window_daemon(true, false));
assert!(leaf_shares_the_window_daemon(false, true));
assert!(leaf_shares_the_window_daemon(false, false));
}
#[test]
fn parses_ssh_option_words_with_quotes() {
src/ui/remote_workspace.rs
+53
View File
@@ -1513,6 +1513,10 @@ fn finish_attempt(
}
cx.default_global::<RemoteLinks>().preempted.remove(&id);
relink_panes(cx, id);
// A window that came up before its machine did has no panes to
// relink — it opened empty because there was nothing to route
// to. Now there is.
hydrate_window(cx, id);
}
RemoteLinks::mark(cx, host, |link| {
link.state = LinkState::Attached;
@@ -1603,6 +1607,55 @@ fn relink_panes(cx: &mut gpui::App, workspace: WorkspaceId) {
}
}
/// Build the tabs of a window that opened before its machine was reachable.
///
/// This is the other end of [`crate::core::session::WorkspaceStore::claim`]'s
/// reachability rule. A remote workspace reopened at launch has nowhere to route
/// to yet — the link is still being built — so it opens empty rather than
/// spawning a second set of shells beside the ones still running over there.
/// The layout it *would* have opened from is the entry's cached session, which
/// [`finish_attempt`] has just refreshed from the machine itself, so by the time
/// this runs the window is rebuilding from the authority.
///
/// # What it will not do
///
/// **Only an empty window is touched.** A window with tabs is one the user is
/// working in; rearranging it because a link came back is the same fight
/// [`refresh_remote_workspace`] refuses to pick. That also makes this safe to
/// call on every reconnect — the second one through finds tabs and leaves.
fn hydrate_window(cx: &mut gpui::App, workspace: WorkspaceId) {
let session = match WorkspaceStore::all(cx).get(workspace) {
Some(entry) if entry.is_remote() => entry.session.clone(),
// Local, or an entry that went away while the connect was in flight.
_ => return,
};
if session.tabs.is_empty() {
// Nothing to restore: a workspace that was quit from the home page, or
// a brand-new one. Its window is right as it is.
return;
}
let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, workspace) else {
return;
};
let Some(app) =
crate::ui::windows::WindowRegistry::app_for(cx, workspace).and_then(|app| app.upgrade())
else {
return;
};
if !app.read(cx).tabs.is_empty() {
return;
}
log::info!(
"rebuilding {} tab(s) of workspace {workspace} now its machine is reachable",
session.tabs.len()
);
let _ = handle.update(cx, move |_, window, cx| {
app.update(cx, |app, cx| {
app.adopt_workspace(workspace, session, window, cx)
});
});
}
/// Design §10's takeover, on this client's side: stop holding a stream to a
/// workspace somebody else is now typing in.
///