fix(ui): resync local windows when the daemon comes back as a new process (#553) (#569)

* fix(ui): resync local windows when the daemon comes back as a new process (#553)

From the client's side a killed daemon is indistinguishable from one
whose shells all exited at once — its DeathReporter says nothing while
it shuts down, and a taskkill says nothing ever — so the window kept
showing every pane with its last title, and the reconnect then pushed
that dead layout back up as the new daemon's truth. On Windows a
force-killed daemon can even leave the shells themselves alive but
permanently unreachable.

The control handshake already carries an instance id per server
process, and the remote path already compared it (server_restarted);
the local link never did. Now LocalLink remembers the hello instance —
across invalidate(), deliberately, or the restart path's own first move
would blind the comparison — and a changed instance on reconnect routes
every local window through resync_after_local_daemon_change instead of
on_link_up: the dead link is dropped first, then hydrate rebuilds from
the machine tree and each pane comes back from its scrollback snapshot
with the "new shell" banner.

note_instance moves to tree_sync as the shared comparison (empty
instance = server predates the field, never a restart, never
overwrites), with the remote callers and their tests repointed. The
restart-server action now uses the same helper instead of open-coding
invalidate + resync, which also fixes it rebuilding only the current
window when several local windows are open.

Per the review, the title wording ("process exited" while the link was
down) and the missing Respawn menu item are a separate PR: the current
wording is pinned by a_dropped_link_does_not_claim_the_process_exited
and changing it is its own decision.

* docs(changelog): entry for the local-daemon reconnect resync (#553)

* fix(ui): keep the reconnect's own link when the daemon came back new

The reconnect installs the new daemon's client and then, on a changed
instance, called the helper that starts by dropping it. The link the
handshake had just proved good was thrown away, every window's pull went
out with nothing to send it on, and the layout had to wait for the next
tick to connect a second time.

That is not only wasted work. `hydrate` gives the link fifteen seconds to
come back and then owes the window a `Replace` — and a `Replace` is
abandoned the moment the window has tabs, which in this scenario it
always does: the dead ones still on screen are the whole bug. So a slow
second connect turned the resync into nothing at all, silently.

Split the helper: `resync_after_local_daemon_change` still invalidates
first, for the restart-server path whose link really does point at a
server it killed, and `resync_local_windows_from_tree` is the half the
reconnect wants, with the fresh link left in place.

The two note_instance tests that moved here with the function now say
what tree_sync's own tests say; the one that is this module's — the slot
is per host — stays.

---------

Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>

CHANGELOG.md

@@ -18,6 +18,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **A local daemon that dies and comes back no longer leaves a window of dead
+  panes looking live** — from the client's side a killed daemon is
+  indistinguishable from one whose shells all exited at once, so the window
+  kept showing every pane with its last title, and the reconnect then pushed
+  that dead layout back up as the new daemon's truth. The control handshake's
+  instance id is now compared on every local reconnect — the same check the
+  remote path already made — and a changed instance rebuilds each local
+  window from the machine tree instead: tabs come back, and each pane is
+  restored from its scrollback snapshot with the "this is a new shell" banner
+  rather than left frozen mid-lie. The restart-server action uses the same
+  rebuild path (#553).
 - **A file link in a remote pane no longer opens this machine's copy.** An
   absolute path was checked against the local filesystem whatever the pane was
   connected to, so `/etc/nginx/nginx.conf` on a server opened the one on your

src/ui/app.rs

@@ -1519,11 +1519,13 @@ impl Tty7App {
             let _ = this.update_in(cx, |this, window, cx| {
                 match &restarted {
                     Ok(()) => {
-                        // The link we held pointed at the server we just killed. Drop it
-                        // before asking for the tree, or the pull goes out on a dead
-                        // socket and the window is left empty on the home page.
-                        crate::ui::local_link::LocalLink::invalidate(cx);
-                        crate::ui::tree_sync::resync_window_from_tree(cx, this.workspace);
+                        // The link we held pointed at the server we just killed;
+                        // the reconnect finds a new process whose registry knows
+                        // nothing about these panes. The helper drops the dead
+                        // link first — a pull sent down it dies on a dead socket
+                        // before the reader notices — and rebuilds every local
+                        // window from the tree.
+                        crate::ui::tree_sync::resync_after_local_daemon_change(cx);
                     }
                     Err(e) => {
                         // The user asked for this and lands on an empty home

src/ui/local_link.rs

@@ -8,6 +8,12 @@ use crate::ui::remote_workspace::Backoff;
 #[derive(Default)]
 pub struct LocalLink {
     client: Option<Arc<ControlClient>>,
+    /// The daemon process the current link is talking to, by its hello
+    /// instance — empty until the first connect records one. Survives
+    /// `invalidate` on purpose: forgetting it there would make every
+    /// reconnect a first sighting, and a daemon that came back as a
+    /// *different* process would never be noticed (#553).
+    instance: String,
     backoff: Backoff,
     next_attempt: Option<std::time::Instant>,
     attempting: bool,
@@ -49,6 +55,11 @@ impl LocalLink {
     /// for a moment after we kill the daemon ourselves the dead link still
     /// hands itself out and every call on it fails. Callers that know the far
     /// end is gone say so here, and the next tick reconnects.
+    ///
+    /// The remembered hello `instance` deliberately survives: the reconnect
+    /// compares against it to tell "same daemon, link hiccuped" from "new
+    /// daemon process" (#553), and forgetting it here — the restart path's
+    /// own first move — would blind exactly that comparison.
     pub fn invalidate(cx: &mut App) {
         let link = cx.default_global::<LocalLink>();
         if link.client.take().is_some() {
@@ -95,6 +106,26 @@ impl LocalLink {
                 match connected {
                     Ok(client) => {
                         log::info!("control link to the local daemon is up");
+                        // A daemon that died and came back is a *new process*
+                        // whose registry knows nothing about the panes this
+                        // window is showing — and from the client's side a
+                        // killed daemon is indistinguishable from one whose
+                        // shells all exited at once (its DeathReporter says
+                        // nothing while it shuts down, and a kill says nothing
+                        // ever), so the panes on screen are probably lying
+                        // about being alive. The instance id in the hello is
+                        // the only way to tell "same daemon, link hiccuped"
+                        // from "new daemon": compare before syncing, or
+                        // `on_link_up` would push the window of dead panes up
+                        // as the new daemon's truth (#553).
+                        let restarted = {
+                            let link = cx.default_global::<LocalLink>();
+                            crate::ui::tree_sync::note_instance(
+                                &mut link.instance,
+                                &client.hello().instance,
+                            )
+                        };
+                        let link = cx.default_global::<LocalLink>();
                         link.client = Some(client);
                         link.backoff.reset();
                         link.next_attempt = None;
@@ -102,7 +133,19 @@ impl LocalLink {
                             cx,
                             tty7_core::host::HostId::LOCAL,
                         );
-                        crate::ui::tree_sync::on_link_up(cx, tty7_core::host::HostId::LOCAL);
+                        if restarted {
+                            // The link installed just above is this new
+                            // daemon's own and answers right now, so the pull
+                            // goes out on it. Dropping it first — which is what
+                            // a caller that killed the daemon itself has to do —
+                            // would leave every window waiting out another
+                            // connect, and a pull that runs out its fifteen
+                            // seconds waiting owes a `Replace` a window with
+                            // tabs on screen never claims back.
+                            crate::ui::tree_sync::resync_local_windows_from_tree(cx);
+                        } else {
+                            crate::ui::tree_sync::on_link_up(cx, tty7_core::host::HostId::LOCAL);
+                        }
                     }
                     Err(e) => match dialect_refusal(&e) {
                         // Retrying will not talk this server round, and the

src/ui/remote_workspace.rs

@@ -1580,27 +1580,7 @@ fn relink_panes(cx: &mut gpui::App, workspace: WorkspaceId) {
 fn server_restarted(cx: &mut gpui::App, host: HostId, peer: &RemoteHost) -> bool {
     let instance = peer.peer().instance.clone();
     let seen = &mut cx.default_global::<RemoteLinks>().instances;
-    note_instance(seen, host, &instance)
-}
-
-fn note_instance(
-    seen: &mut std::collections::HashMap<HostId, String>,
-    host: HostId,
-    instance: &str,
-) -> bool {
-    if instance.is_empty() {
-        return false;
-    }
-    match seen.insert(host, instance.to_string()) {
-        Some(before) if before != instance => {
-            log::info!(
-                "the tty7-server on this machine is a new process ({before} → {instance}); \
-                 its panes are gone"
-            );
-            true
-        }
-        _ => false,
-    }
+    crate::ui::tree_sync::note_instance(seen.entry(host).or_default(), &instance)
 }
 
 fn refresh_window_shells(cx: &mut gpui::App, workspace: WorkspaceId) {
@@ -1893,41 +1873,30 @@ mod tests {
         assert_eq!(flow.choice(), Some(&choice));
     }
 
-    #[test]
-    fn only_a_changed_instance_counts_as_a_restart() {
-        let mut seen = std::collections::HashMap::new();
-        let host = HostId::from_connection_key("ssh:build-box");
-
-        assert!(!note_instance(&mut seen, host, "abc"));
-        assert!(!note_instance(&mut seen, host, "abc"));
-        assert!(note_instance(&mut seen, host, "def"));
-        assert!(!note_instance(&mut seen, host, "def"));
-    }
-
-    #[test]
-    fn an_unknown_instance_is_not_a_restart_and_is_not_remembered() {
-        let mut seen = std::collections::HashMap::new();
-        let host = HostId::from_connection_key("ssh:build-box");
-
-        assert!(!note_instance(&mut seen, host, ""));
-        assert!(seen.is_empty(), "an unknown instance must not be recorded");
-        assert!(
-            !note_instance(&mut seen, host, "abc"),
-            "the first real instance is a first sighting, not a restart"
-        );
-    }
-
+    /// What the comparison itself answers is pinned where it now lives, in
+    /// `tree_sync`. What is this module's own is the slot it reads: one per
+    /// host, keyed the way `server_restarted` keys it, so a machine that
+    /// restarted says nothing about the one next to it.
     #[test]
     fn instances_are_per_machine() {
         let mut seen = std::collections::HashMap::new();
         let a = HostId::from_connection_key("ssh:box-a");
         let b = HostId::from_connection_key("ssh:box-b");
 
-        assert!(!note_instance(&mut seen, a, "a1"));
-        assert!(!note_instance(&mut seen, b, "b1"));
-        assert!(note_instance(&mut seen, a, "a2"));
+        assert!(!crate::ui::tree_sync::note_instance(
+            seen.entry(a).or_default(),
+            "a1"
+        ));
+        assert!(!crate::ui::tree_sync::note_instance(
+            seen.entry(b).or_default(),
+            "b1"
+        ));
+        assert!(crate::ui::tree_sync::note_instance(
+            seen.entry(a).or_default(),
+            "a2"
+        ));
         assert!(
-            !note_instance(&mut seen, b, "b1"),
+            !crate::ui::tree_sync::note_instance(seen.entry(b).or_default(), "b1"),
             "box-b never changed; box-a restarting is not its business"
         );
     }

src/ui/tree_sync.rs

@@ -1906,6 +1906,68 @@ pub(crate) fn resync_window_from_tree(cx: &mut App, client_ws: WorkspaceId) {
     hydrate(cx, client_ws, Adopt::Replace);
 }
 
+/// Records the daemon process a link is talking to, answering "did the server
+/// behind this link just become a *different* process?".
+///
+/// `seen` is the instance last recorded for this link, empty when unknown. A
+/// first sighting is not a restart (there is nothing on screen to be wrong
+/// about yet), and an empty instance means the server predates the field —
+/// it neither reports nor overwrites what was seen before. Shared by the
+/// remote path (`remote_workspace::server_restarted`, keyed per host) and the
+/// local link (single daemon), which needs the same comparison to notice the
+/// daemon it lost came back as another process (#553).
+pub(crate) fn note_instance(seen: &mut String, instance: &str) -> bool {
+    if instance.is_empty() {
+        return false;
+    }
+    let before = std::mem::replace(seen, instance.to_string());
+    if !before.is_empty() && before != instance {
+        log::info!(
+            "the tty7-server on this machine is a new process ({before} → {instance}); \
+             its panes are gone"
+        );
+        return true;
+    }
+    false
+}
+
+/// Drops the link to the local daemon and rebuilds every local window from the
+/// machine tree, for a caller that killed that daemon itself (#553).
+///
+/// The invalidate half comes first on purpose: the link still holds the client
+/// that pointed at the server that is now gone, and a pull sent down it dies on
+/// a dead socket before the reader notices. With it dropped, `hydrate` waits for
+/// the reconnect `LocalLink::tick` is already driving and pulls the layout the
+/// daemon actually has.
+///
+/// Only for the caller that has no live link left. One that just handshaked a
+/// *new* daemon calls [`resync_local_windows_from_tree`] with that link in hand:
+/// dropping it there would throw away a working link and make every window wait
+/// out another connect for no reason.
+pub(crate) fn resync_after_local_daemon_change(cx: &mut App) {
+    crate::ui::local_link::LocalLink::invalidate(cx);
+    resync_local_windows_from_tree(cx);
+}
+
+/// Rebuilds every local window from the machine tree, because the daemon behind
+/// the local link became a different process — it died and the reconnect found a
+/// new one, whose registry knows nothing about the panes on screen (#553).
+///
+/// Remote windows are left alone: their own link says when their machine's
+/// server changed, and this one speaks for this computer only.
+///
+/// When no daemon answers the pull, the windows owe a rehydration instead
+/// (`owe_rehydration`) — which is also the guard that keeps a window emptied by
+/// a failed pull from being pushed back up as "close every tab".
+pub(crate) fn resync_local_windows_from_tree(cx: &mut App) {
+    for (workspace, _) in crate::ui::windows::WindowRegistry::open_windows(cx) {
+        if WorkspaceStore::host_of(cx, workspace) != HostId::LOCAL {
+            continue;
+        }
+        resync_window_from_tree(cx, workspace);
+    }
+}
+
 impl Tty7App {
     pub(crate) fn apply_layout_delta(
         &mut self,
@@ -2136,6 +2198,40 @@ fn set_gui_ratio(pane: &mut Pane, path: &[Side], ratio: f32) -> bool {
 mod tests {
     use super::*;
 
+    #[test]
+    fn note_instance_reports_only_a_real_change() {
+        let mut seen = String::new();
+
+        assert!(
+            !note_instance(&mut seen, "abc"),
+            "a first sighting is not a restart"
+        );
+        assert!(
+            !note_instance(&mut seen, "abc"),
+            "the same process is not a restart"
+        );
+        assert!(note_instance(&mut seen, "def"), "a new process is");
+        assert!(!note_instance(&mut seen, "def"));
+    }
+
+    #[test]
+    fn note_instance_ignores_a_server_that_predates_the_field() {
+        let mut seen = String::from("abc");
+
+        assert!(
+            !note_instance(&mut seen, ""),
+            "an unknown instance is never a restart"
+        );
+        assert_eq!(seen, "abc", "and it must not overwrite what was seen");
+
+        let mut fresh = String::new();
+        assert!(!note_instance(&mut fresh, ""));
+        assert!(
+            !note_instance(&mut fresh, "abc"),
+            "the first real instance after an unknown one is a first sighting"
+        );
+    }
+
     #[cfg(unix)]
     #[test]
     fn a_peer_without_the_machine_tree_bit_classifies_as_unserved() {