fix(tree-sync): let a window arriving at a workspace speak for nothing in it (#716) (#728)

* fix(tree-sync): let a window arriving at a workspace speak for nothing in it (#716)

A remote client connected to a machine holding nineteen panes and came
back to one tab. The workspace kept its id and its shells kept running —
`pane ls --all` listed them live, owned by the workspace, held by no tab
— but its tab tree was gone, and every GUI on that machine lost the
layout at once.

The tabs were closed by the window that arrived. `switch_workspace`
claims the workspace, then hands `adopt_workspace` an empty session to
put up while the real one is pulled — and `adopt_workspace` saves what
it put up. That save syncs: a window showing no tabs at all, against
whatever the last visit to that workspace left in the tree-sync map.
Left Primed and informed, the diff runs at `SyncScope::Full`, where
every mirror tab the window is not showing is a tab the user closed. It
queued nineteen `TabClose`s and pumped them before `hydrate_window_with_tabs`
on the next line had ordered the pull that would have populated the
window. `tab_close` removes the tab and the pane records under it and
returns the orphaned ids for the caller to hang up, which the CLI does
and the GUI does not — hence shells still running under no tab.

The workspace is now forgotten on the way in as well as on the way out.
An unprimed state has no mirror to diff against, so the empty session
goes up, is saved, and closes nothing; the pull lands, the rebuild puts
the real tabs up, and `settle_rebuild` hands back the licence to a
window that has actually seen what it is speaking for.

That the licence outlived the arrival was the whole vulnerability, and
it is what the test holds: the closes it authorises are queued and
pumped inside `adopt_workspace`, and the hydrate on the next line clears
the queue, so the ops are gone by the time a test can look at them
either way.

This is the local half. A remote client also renames on arrival and the
reported workspace came back under the other machine's user name, which
`settle_chosen_name` will fire at whatever workspace the window landed
on when a parked name differs from the machine's — it cannot tell a
name it created a workspace with from one it adopted. Left alone here;
it loses a name, not a layout.

Reported by xAlisher in #716, with the daemon state that identified it.

* fix(test): gate the arrival test on unix, like the harness it uses

harness_with_pane is #[cfg(unix)], so the new test broke the Windows
build. Its sibling above already carries the same gate.

---------

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

src/ui/app.rs

@@ -1571,6 +1571,14 @@ impl Tty7App {
         let claimed = WorkspaceStore::claim(cx, id);
         crate::ui::windows::WindowRegistry::rebind(cx, previous, claimed);
         crate::ui::remote_workspace::RemoteLinks::supervise(cx, claimed);
+        // Forgotten on the way in as well as on the way out. `adopt_workspace`
+        // puts the empty session up before the pull below orders the real one,
+        // and it saves what it put up: a window showing nothing, syncing
+        // against whatever this workspace was left Primed and informed with
+        // the last time it was visited, which is a Full diff that closes every
+        // tab on the machine (#716). Arriving speaks for nothing until a pull
+        // says otherwise.
+        crate::ui::tree_sync::forget(cx, claimed);
         self.adopt_workspace(claimed, Session::default(), window, cx);
         // This method runs under the app's own update lease, so the tabs the
         // pull must see are the ones just adopted here — reading the app back

src/ui/tree_sync.rs

@@ -3764,6 +3764,85 @@ mod tests {
         });
     }
 
+    /// #716: switching into a workspace put its empty session up and saved
+    /// it — a window showing nothing, syncing at Full scope against the
+    /// mirror and licence the last visit left behind, which closes every tab
+    /// on the machine before the pull that would have populated the window
+    /// has even been ordered. Arriving must speak for nothing.
+    ///
+    /// The licence is what the assertion holds. The closes it authorises are
+    /// queued and pumped inside `adopt_workspace`, and the hydrate ordered
+    /// straight after clears the queue, so by the time a test can look the
+    /// ops are gone either way — while `informed` outliving the arrival is
+    /// both durable and the thing that made them possible.
+    #[cfg(unix)]
+    #[gpui::test]
+    fn arriving_at_a_workspace_does_not_prune_what_is_already_in_it(cx: &mut gpui::TestAppContext) {
+        let (app, mut vcx, _pane_stream) = crate::ui::app::test_window::harness_with_pane(cx);
+        let theirs = (TabId::new(), TabId::new());
+        app.update_in(&mut vcx, |app, window, cx| {
+            crate::ui::windows::WindowRegistry::init(cx);
+            let here = crate::core::session::WindowView::default();
+            let there = crate::core::session::WindowView::default();
+            let (here_id, there_id) = (here.id, there.id);
+            WorkspaceStore::install_for_test(
+                cx,
+                crate::core::session::WindowViews {
+                    views: vec![here, there],
+                    active: Some(here_id),
+                },
+            );
+            app.workspace = here_id;
+
+            // The workspace being switched to, as an earlier visit left it:
+            // primed with the tabs it holds, and licensed to prune them.
+            {
+                let state = cx
+                    .default_global::<TreeSync>()
+                    .windows
+                    .entry(there_id)
+                    .or_default();
+                state.sync = SyncPhase::Primed(WsMirror {
+                    tabs: vec![
+                        TreeTab {
+                            id: theirs.0,
+                            name: None,
+                            sidebar_group: None,
+                            root: PaneNode::Leaf { pane: 11 },
+                        },
+                        TreeTab {
+                            id: theirs.1,
+                            name: None,
+                            sidebar_group: None,
+                            root: PaneNode::Leaf { pane: 12 },
+                        },
+                    ],
+                    active: Some(theirs.0),
+                });
+                state.informed = true;
+                // Keeps whatever the switch queues where the test can read it.
+                state.inflight = true;
+            }
+
+            app.switch_workspace(Some(there_id), window, cx);
+
+            let state = &cx.default_global::<TreeSync>().windows[&there_id];
+            assert!(
+                !state.informed,
+                "a window that has just arrived speaks for nothing in the workspace \
+                 until its own pull lands — least of all that it is empty"
+            );
+            assert!(
+                !state
+                    .queue
+                    .iter()
+                    .any(|op| matches!(op, ControlRequest::TabClose { .. })),
+                "and it closes nothing it never showed: {:?}",
+                state.queue
+            );
+        });
+    }
+
     #[test]
     fn a_ratio_delta_is_clamped_to_the_servers_band_not_a_narrower_one() {
         let mut pane = Pane::split_node(gpui::Axis::Horizontal, 0.5, Pane::Empty, Pane::Empty);