fix: reattach the last workspace closed, and resupervise a reopened remote one (#267)

* fix(session): come back to the last workspace closed, not the home page

Launch only ever restored a workspace that still had a window at quit, so
closing them one by one and relaunching came up on the empty home page —
with no hint that four workspaces were sitting there detached.

Closing a window here is a detach: its panes keep running in the daemon,
which makes that workspace every bit as much "where you left off" as one
that still had a window on screen. `workspace_to_restore` now falls back
to the most recently active workspace of any kind, and since
`close_window` touches it on the way out, that is the one closed last. An
open workspace still outranks a more recently touched detached one, so a
background agent cannot steal the restore from the window that was
actually on screen. Deleting a workspace still drops it from the file —
that is the one gesture meaning "done with this".

`None` therefore means a genuine first run only, which retires the
`FreshStart::HomePage` launch path along with the enum that threaded it
through `windows::open` and `Tty7App::for_workspace`.

* fix(remote): reopening a remote workspace starts the supervisor again

The supervisor stops — and clears every MachineLink with it — as soon as
no open workspace is on a remote machine, which closing the last remote
window does. The connection itself stays: a closed window is a detach,
so HostLinks keeps the socket for whatever opens next.

Reopening that workspace then found a live connection with no link
behind it. `reopen_remote_at_startup` read the surviving HostLinks entry
as "another window got there first" and returned before starting the
pump, so nothing ever put a MachineLink back and `status_of` answered
Disconnected for good: a "Not connected to <machine>" strip and a dead
keyboard over panes that were visibly still running on the far side.

An existing HostLinks entry is not a reason to skip the supervisor. It
answers "is there a socket"; what the window renders from is `machines`.
Both now go through RemoteLinks::supervise, which is a no-op for a local
workspace and an unconditional ensure_running for a remote one —
idempotent, so a machine already supervised costs a flag check, and the
first tick over a live socket marks it Attached without opening a second
SSH session.

Fixes the same hole on the switcher's path, which never told the
supervisor anything at all: picking a remote workspace from it goes
through `switch_workspace`, not the launch path.

---------

Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
This commit is contained in:
l0ng-ai
2026-07-30 21:28:44 +08:00
committed by GitHub
co-authored by l0ng-ai
parent 1e5106de59
commit ac8968643d
7 changed files with 229 additions and 97 deletions
+10
View File
@@ -231,6 +231,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- **Closing every window before quitting no longer loses your place** — launch
only ever restored a workspace that still had a window at quit, so closing them
one by one and relaunching came up on the empty home page, with no hint that
four workspaces were sitting there. Closing a window here is a *detach*: its
panes keep running in the daemon, which makes that workspace every bit as much
"where you left off" as one that still had a window on screen. Launch now falls
back to the workspace closed last, and the only launch that comes up on a fresh
workspace is a genuine first run. Deleting a workspace still means deleting it —
that is the one gesture that drops it from the file.
- **An unsubscribed directory watch stops delivering immediately** — dropping a
local watch handle now closes its delivery channel rather than only asking the
OS backend to stand down. Tearing that backend down is not instantaneous, and
+79 -16
View File
@@ -555,20 +555,41 @@ impl WindowViews {
/// others are not lost by any measure that matters: their panes never
/// stopped running in the daemon, and the switcher lists them a click away.
///
/// [`active`](Self::active) is the answer whenever it is still open, since
/// it is written on every focus change and so names the window that had the
/// user's attention last. `last_active` is the fallback for a store written
/// by a build that did not track focus, or one whose active workspace was
/// closed before quitting.
/// Three answers, in order:
///
/// 1. [`active`](Self::active) while it is still open — written on every
/// focus change, so it names the window that had the user's attention
/// last.
/// 2. the most recently active *open* workspace, for a store written by a
/// build that did not track focus, or one whose active workspace was
/// closed before quitting.
/// 3. the most recently active workspace of any kind, open or not.
///
/// That last one is why closing every window and quitting still comes back
/// somewhere. Closing a window here is a *detach*: the panes keep running in
/// the daemon, so the workspace behind them is every bit as much "where the
/// user left off" as one that still had a window — and `close_window`
/// touches it on the way out, which makes the most recent of them the one
/// closed last. Only the explicit *Close Workspace* drops an entry from the
/// file, and that is the one gesture that means "I am done with this".
///
/// `None` therefore means one thing: no workspaces at all, i.e. a first run.
pub fn workspace_to_restore(&self) -> Option<WorkspaceId> {
let focused = self
.active
.filter(|id| self.get(*id).is_some_and(|w| w.open));
focused.or_else(|| {
self.open_views()
.max_by_key(|w| w.last_active)
.map(|w| w.id)
})
focused
.or_else(|| {
self.open_views()
.max_by_key(|w| w.last_active)
.map(|w| w.id)
})
.or_else(|| {
self.views
.iter()
.max_by_key(|w| w.last_active)
.map(|w| w.id)
})
}
/// Persist as JSON, creating the parent directory if needed. Any
@@ -938,14 +959,56 @@ mod tests {
};
assert_eq!(all.workspace_to_restore(), Some(open_id));
// Nothing open at all: launch has no workspace to come up on and shows
// the home page instead of inventing one.
let mut none_open = view();
none_open.open = false;
// Nothing open at all — the user closed every window before quitting.
// Launch still comes back to the one closed last, because a detached
// workspace's panes are still running and `close_window` touches it on
// the way out.
let mut first_closed = view();
first_closed.open = false;
first_closed.last_active = 100;
let mut closed_last = view();
closed_last.open = false;
closed_last.last_active = 900;
let closed_last_id = closed_last.id;
let all = WindowViews {
active: None,
views: vec![none_open],
views: vec![first_closed, closed_last],
};
assert_eq!(all.workspace_to_restore(), None);
assert_eq!(all.workspace_to_restore(), Some(closed_last_id));
// A stale `active` naming a workspace that is gone from the file does
// not stop the fallback from answering.
let all = WindowViews {
active: Some(WorkspaceId::new()),
..all
};
assert_eq!(all.workspace_to_restore(), Some(closed_last_id));
// The only `None` left is a genuine first run.
assert_eq!(WindowViews::default().workspace_to_restore(), None);
}
/// An open workspace outranks a detached one even when the detached one saw
/// activity more recently — the fallback is for when *nothing* is open, not
/// a recency race across the two states.
///
/// Without this, a background agent touching a detached workspace after the
/// user's last keystroke would have launch reopen that one instead of the
/// window that was actually on screen at quit.
#[test]
fn an_open_workspace_outranks_a_more_recently_touched_detached_one() {
let mut open_one = view();
open_one.open = true;
open_one.last_active = 100;
let open_id = open_one.id;
let mut detached = view();
detached.open = false;
detached.last_active = 900;
let all = WindowViews {
active: None,
views: vec![open_one, detached],
};
assert_eq!(all.workspace_to_restore(), Some(open_id));
}
}
+8 -3
View File
@@ -159,11 +159,14 @@ impl WorkspaceStore {
/// Their panes are untouched — this is exactly the state
/// [`close_window`](Self::close_window) leaves behind, reached in bulk.
///
/// Returns `None` when nothing was open, which launch reads as "come up on
/// the home page".
/// The workspace kept need not have been open at all: quitting with every
/// window closed comes back to the one closed last (see
/// [`WindowViews::workspace_to_restore`]). `None` means there are no saved
/// workspaces whatsoever — a first run.
pub fn restore_one(cx: &mut gpui::App) -> Option<WorkspaceId> {
let store = Self::try_store(cx)?;
let keep = store.views.workspace_to_restore()?;
let reattaching = store.views.get(keep).is_some_and(|view| !view.open);
let mut detached = 0usize;
for view in &mut store.views.views {
if view.open && view.id != keep {
@@ -173,7 +176,9 @@ impl WorkspaceStore {
}
store.views.active = Some(keep);
store.views.save();
if detached > 0 {
if reattaching {
log::info!("launch: no window was open at quit; reattaching the last one closed");
} else if detached > 0 {
log::info!("launch: restoring 1 workspace, left {detached} detached");
}
Some(keep)
+8 -21
View File
@@ -426,28 +426,15 @@ fn main() {
// quit: see `WindowViews::workspace_to_restore` for why, and
// `WorkspaceStore::restore_one` for what happens to the others (they
// are detached, not forgotten — panes keep running and the switcher
// lists them). Quitting with every window closed — or a first run —
// opens a single window on a fresh workspace.
let any_saved = {
let store = crate::core::session::WorkspaceStore::all(cx);
!store.views.is_empty()
};
// lists them). Closing every window before quitting is *not* a
// reason to come up empty: those workspaces still hold running
// panes, so launch reattaches the one closed last.
//
// `None` is therefore a first run only, and it opens a single window
// on a fresh workspace holding one terminal — exactly as every
// pre-multi-window build did.
let reopen = crate::core::session::WorkspaceStore::restore_one(cx);
// With nothing to reopen, what that one window should hold depends on
// whether there is anything to come back to: workspaces the user
// detached are listed by the home page's picker, so leave it empty
// for them. A genuine first run has no picker to show and no reason
// to greet the user with a blank page — it opens a terminal, exactly
// as every pre-multi-window build did.
let fresh = if any_saved {
crate::ui::windows::FreshStart::HomePage
} else {
crate::ui::windows::FreshStart::Shell
};
match reopen {
Some(id) => crate::ui::windows::open(cx, Some(id)),
None => crate::ui::windows::open_with(cx, None, fresh),
}
crate::ui::windows::open(cx, reopen);
});
}
+12 -14
View File
@@ -937,7 +937,6 @@ impl Tty7App {
/// that is no longer on file.
pub fn for_workspace(
id: Option<WorkspaceId>,
fresh: crate::ui::windows::FreshStart,
window: &mut Window,
cx: &mut Context<Self>,
) -> Self {
@@ -960,19 +959,12 @@ impl Tty7App {
// A remote workspace hydrates even with restore off: its panes are
// running sessions on another machine, not a saved layout.
let hydrate = known && (restore || is_remote);
// What the window opens holding is the caller's call for a *brand-new*
// workspace: `None` takes the first-run path in `with_session`,
// spawning a single default terminal — what `New Workspace` and a
// first run both want — while an empty session lands on the home page,
// for the launch that exists to show the workspace picker. A known
// workspace opens empty (the hydration fills it), or on a fresh shell
// when the user turned restore off.
let session = match (known, fresh) {
(true, _) if hydrate => Some(Session::default()),
(true, _) => None,
(false, crate::ui::windows::FreshStart::Shell) => None,
(false, crate::ui::windows::FreshStart::HomePage) => Some(Session::default()),
};
// A brand-new workspace takes the first-run path in `with_session`
// (`None`), spawning a single default terminal — what `New Workspace`
// and a first run both want. A known workspace opens on an empty
// session that the hydration fills, or on a fresh shell when the user
// turned restore off.
let session = hydrate.then(Session::default);
let app = Self::with_session(Some(workspace), session, window, cx);
if hydrate {
// No immediate save: the window is deliberately empty, and racing
@@ -1701,6 +1693,12 @@ impl Tty7App {
let claimed = WorkspaceStore::claim(cx, Some(id));
crate::ui::windows::WindowRegistry::rebind(cx, previous, claimed);
// A pick from the switcher is one of the ways a remote workspace comes
// back, so it owes the supervisor the same call the launch path makes —
// see `RemoteLinks::supervise` for what skipping it leaves on screen.
// The outgoing workspace needs no counterpart: `pump_tick` drops a
// machine the moment its last open workspace goes.
crate::ui::remote_workspace::RemoteLinks::supervise(cx, claimed);
// The machine's tree is the layout's only home now, so an explicit
// pick from the switcher always hydrates — restore-off governs what
// *launch* comes back to, not what a deliberate open shows. The window
+109 -21
View File
@@ -757,28 +757,11 @@ impl Tty7App {
/// its window already built and its layout the last one this client pulled.
/// What M6 adds is the connect itself plus the auth queue that keeps ten
/// windows from raising ten password sheets at once.
///
/// Nothing but a call to [`RemoteLinks::supervise`], and local workspaces
/// pass straight through it — the launch path stays ignorant of hosts.
pub(crate) fn reopen_remote_at_startup(&self, cx: &mut Context<Self>) {
let Some(host) = WorkspaceStore::remote_ref(cx, self.workspace) else {
return;
};
remote_connect::register(cx);
if remote_connect::HostLinks::get(cx, host.host_id()).is_some() {
// Another window on the same machine got there first. One connection
// per machine is the point — D7's "connect immediately" is about the
// *machine*, and a second link to it would be a second SSH session
// for no reason.
return;
}
log::info!("reconnecting to {} at startup", host.target);
// No per-window connect call: the supervisor already knows which
// machines have open workspaces, so starting it *is* the reconnect, and
// ten windows on one box produce one attempt rather than ten.
//
// Nothing here classifies the host as needing authentication or not
// (D7): every machine is attempted in parallel, and the ones that turn
// out to need a human queue for the sheet at the moment they ask — see
// [`AuthSheetQueue`].
RemoteLinks::ensure_running(cx);
RemoteLinks::supervise(cx, self.workspace);
}
// ----- prompts -----------------------------------------------------------
@@ -1341,6 +1324,45 @@ impl RemoteLinks {
.detach();
}
/// Put `workspace`'s machine under the supervisor, if it has one. A local
/// workspace is a no-op, which is what lets every "a window took over a
/// workspace" path call this without first asking whether it is remote.
///
/// **Every such path must.** The supervisor is not a one-shot at start-up:
/// [`pump_tick`] stops it — and clears every [`MachineLink`] with it — as
/// soon as no *open* workspace is on a remote machine, which closing the
/// last remote window does. What that leaves behind is a live connection
/// with no link behind it, because a closed window is a detach and
/// [`remote_connect::HostLinks`] outlives it by design. Reopening the
/// workspace then reads as [`RemoteStatus::Disconnected`] — a "Not
/// connected" strip and a dead keyboard over panes that are visibly still
/// running (#issue: reopened remote workspace stays "not connected").
///
/// So an existing `HostLinks` entry is **not** a reason to skip this: it
/// answers "is there a socket", and the state the window renders from is
/// `machines`. `ensure_running` is idempotent, so the machine that really
/// is already supervised costs a flag check, and the first tick over a live
/// socket marks it `Attached` without opening a second SSH session.
pub(crate) fn supervise(cx: &mut gpui::App, workspace: WorkspaceId) {
let Some(host) = WorkspaceStore::remote_ref(cx, workspace) else {
return;
};
remote_connect::register(cx);
log::info!(
"supervising {} for a workspace that just opened",
host.target
);
// No per-window connect call: the supervisor already knows which
// machines have open workspaces, so starting it *is* the reconnect, and
// ten windows on one box produce one attempt rather than ten.
//
// Nothing here classifies the host as needing authentication or not
// (D7): every machine is attempted in parallel, and the ones that turn
// out to need a human queue for the sheet at the moment they ask — see
// [`AuthSheetQueue`].
RemoteLinks::ensure_running(cx);
}
/// This workspace's state, or `None` when it is a local one.
pub(crate) fn status_of(cx: &gpui::App, workspace: WorkspaceId) -> Option<RemoteStatus> {
let host = WorkspaceStore::remote_ref(cx, workspace)?;
@@ -2205,6 +2227,72 @@ mod tests {
});
}
/// **Stopping the supervisor is not a terminal state.** It stops whenever no
/// open workspace is remote — closing the last remote window does it — and a
/// workspace reopened afterwards has to start it again, or it sits under a
/// "Not connected" strip with a dead keyboard for ever while its panes run
/// on the far side.
///
/// What this pins is that [`RemoteLinks::supervise`] is that restart, from a
/// pump that has genuinely stopped. It cannot reproduce the original bug in
/// full — that needed a live `HostLinks` entry, which takes a real control
/// connection to build — so the other half of the rule lives in
/// `supervise`'s own doc: never gate the `ensure_running` call on one.
#[gpui::test]
fn a_stopped_supervisor_restarts_when_a_remote_workspace_comes_back(
cx: &mut gpui::TestAppContext,
) {
let id = cx.update(|cx| {
cx.set_global(crate::core::config::Config::default());
// Nothing remote on file yet, so the first tick has no machine to
// supervise and shuts the pump down — the state a closed remote
// window leaves behind.
crate::core::session::WorkspaceStore::install_for_test(
cx,
crate::core::session::WindowViews::default(),
);
RemoteLinks::ensure_running(cx);
assert!(cx.default_global::<RemoteLinks>().pumping);
WorkspaceId::new()
});
// The tick runs and returns `false` without ever reaching its timer, so
// this needs no clock of its own.
cx.background_executor.run_until_parked();
cx.update(|cx| {
assert!(
!cx.default_global::<RemoteLinks>().pumping,
"with no remote workspace open the pump is expected to stop"
);
// The workspace comes back — reopened from the switcher, or the
// launch path's window landing on it.
let host = RemoteRef::new(
RemoteTarget::Alias {
alias: "build-box".into(),
},
WorkspaceId::new(),
);
crate::core::session::WorkspaceStore::install_for_test(
cx,
crate::core::session::WindowViews {
views: vec![crate::core::session::WindowView {
id,
host: Some(host),
open: true,
..Default::default()
}],
active: Some(id),
},
);
RemoteLinks::supervise(cx, id);
assert!(
cx.default_global::<RemoteLinks>().pumping,
"reopening a remote workspace has to start the supervisor again"
);
});
}
/// A plain reconnect (never preempted) must not be marked for a rebuild —
/// its panes are alive and re-attachable, and a Replace would tear down
/// views the relink was about to reuse.
+3 -22
View File
@@ -178,34 +178,15 @@ impl WindowRegistry {
}
}
/// What a *brand-new* workspace's window starts with. Only consulted when the
/// window is opening on a freshly minted workspace — a known one opens empty
/// and is filled from its machine's tree.
#[derive(Clone, Copy, PartialEq, Eq)]
pub enum FreshStart {
/// A single default terminal, the way every previous launch of tty7 came
/// up. What `New Workspace` and a genuine first run want: a window whose
/// workspace has nothing in it yet is a window you asked for to work in.
Shell,
/// No tabs — the home page. Used at launch when there *are* saved
/// workspaces but none were open at quit: the picker listing them is the
/// whole point of that window, and a shell in front of it would bury it.
HomePage,
}
/// Open a window on `workspace` — or on a brand-new workspace when `None`,
/// which starts with a single terminal (see [`open_with`] for the other case).
/// which starts with a single terminal. A known workspace opens empty and is
/// filled from its machine's tree.
///
/// When that workspace already has a window, this focuses it instead of
/// opening a second one: two windows on one workspace would both attach the
/// same daemon panes, and the daemon's single-subscriber model means the
/// second attach silently kills the first window's terminal.
pub fn open(cx: &mut App, workspace: Option<WorkspaceId>) {
open_with(cx, workspace, FreshStart::Shell);
}
/// [`open`], with a say in what a brand-new workspace comes up holding.
pub fn open_with(cx: &mut App, workspace: Option<WorkspaceId>, fresh: FreshStart) {
if let Some(id) = workspace
&& let Some(handle) = WindowRegistry::window_for(cx, id)
{
@@ -218,7 +199,7 @@ pub fn open_with(cx: &mut App, workspace: Option<WorkspaceId>, fresh: FreshStart
// only the root view — so capture it on the way past.
let mut created: Option<gpui::Entity<Tty7App>> = None;
let opened = cx.open_window(options, |window, cx| {
let app = cx.new(|cx| Tty7App::for_workspace(workspace, fresh, window, cx));
let app = cx.new(|cx| Tty7App::for_workspace(workspace, window, cx));
created = Some(app.clone());
// Root's own background is fully transparent: `Tty7App`'s root div is
// the single owner of the window background (solid / gradient / image,