From 3b5df0b6b54ca44b77e46c26fd55fd5b458166ec Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sat, 22 Aug 2026 22:10:05 +0800 Subject: [PATCH] fix(tree-sync): adopting a workspace is not creating one (#716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `chosen_name` is the name a user typed for a workspace a window is about to create. It travels with the create rather than following it, because a rename sent before the workspace exists is answered `NotFound`. When the create came back without that name, `settle_chosen_name` sent it as a rename. Its own comment gave two reasons the create might not have run — the other create of this window's pair won the race, or the workspace was already there — and treated them the same. They are not the same. The first is this window finishing its own job. The second is renaming somebody else's workspace. #716 is the second one from the far end: a client opened a workspace on another machine that already held nineteen live panes, and the workspace came back named after the connecting client's local user, because that side spent a codename it had rolled for a workspace it thought it was making. A workspace this window's sibling create just made is empty, so the two cases separate on whether the workspace holds tabs — and the existing arbitration tests all pull an empty mirror, so they are exactly the case that still renames. The name is still consumed when the rename is declined. It was owed once, and adopting the workspace is how it stops being owed; parking it would only fire the rename at the next pull. This is the naming half of that report. The tab tree it also lost is not addressed here. --- src/ui/tree_sync.rs | 72 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index b5282442..2cdaf07a 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -1375,13 +1375,24 @@ pub(crate) fn fresh_workspace_name(cx: &App, host: HostId) -> String { /// /// `answered` is the machine's answer. A chosen name it read back was spent by /// the create that carried it, and there is nothing left to do. One it did not -/// means the create never ran — the workspace was already there, or the other -/// create won the race with a stale idea of the name — so it goes out as the -/// rename it has become. Either way the name is owed only once. +/// means the create never ran, and there are two ways that happens: the other +/// create of this window's own pair won the race with a stale idea of the +/// name, or the workspace was simply already there. Either way the name is +/// owed only once — `take` runs whatever is decided below. +/// +/// `holds_tabs` is what separates those two. A workspace this window's sibling +/// create just made is empty, so renaming it is finishing this window's own +/// job. A workspace that already holds tabs was made by somebody else and is +/// somebody else's: a client opening a populated workspace on another machine +/// used to rename it to the codename *its* side had rolled, so a workspace +/// full of another user's running panes took the connecting client's name +/// (#716). Adopting a workspace is not creating one, and only the create was +/// ever owed a name. fn settle_chosen_name( cx: &mut App, client_ws: WorkspaceId, answered: Option, + holds_tabs: bool, ) -> Option { let chosen = cx .default_global::() @@ -1390,6 +1401,8 @@ fn settle_chosen_name( .and_then(|state| state.chosen_name.take()); match chosen { Some(chosen) if answered.as_deref() == Some(chosen.as_str()) => answered, + // Adopted, not created. The machine's name stands. + Some(_) if holds_tabs => answered, Some(chosen) => { rename_workspace(cx, client_ws, Some(chosen.clone())); Some(chosen) @@ -1478,12 +1491,13 @@ fn finish_prime( }; let host = WorkspaceStore::host_of(cx, client_ws); let machine_ws = tree_workspace_id(cx, client_ws); + let holds_tabs = !landed.0.is_empty(); crate::ui::machine_mirror::MachineMirrors::note_synced_workspace( cx, host, machine_ws, landed.0, landed.1, ); // The pull above is the only place this window will hear the workspace's // name — it is left out of the deltas its own create raises (#604). - let name = settle_chosen_name(cx, client_ws, landed.2); + let name = settle_chosen_name(cx, client_ws, landed.2, holds_tabs); crate::ui::machine_mirror::MachineMirrors::note_workspace_name(cx, host, machine_ws, name); if !was_dirty { return; @@ -2352,7 +2366,7 @@ fn settle_hydration( .find(|w| w.id == machine_ws) .and_then(|w| w.name.clone()); crate::ui::machine_mirror::MachineMirrors::install(cx, host, machine); - let name = settle_chosen_name(cx, client_ws, answered); + let name = settle_chosen_name(cx, client_ws, answered, !mirror.tabs.is_empty()); crate::ui::machine_mirror::MachineMirrors::note_workspace_name(cx, host, machine_ws, name); let machine_was_empty = mirror.tabs.is_empty(); let was_dirty = { @@ -3250,6 +3264,54 @@ mod tests { }); } + /// #716: opening a workspace that another client already filled must not + /// rename it. + /// + /// The typed-name arbitration above exists for a create that never ran + /// because this window's *own* sibling create won the race. A workspace + /// that already holds tabs did not come from either of them — it belongs + /// to whoever built it — and the report is what that looked like from the + /// other end: a machine's workspace of nineteen live panes came back named + /// after the connecting client's local user, because the connecting side + /// spent a codename it had rolled for a workspace it thought it was + /// making. + /// + /// The name is still consumed. It was owed once, and adopting a workspace + /// is how it stops being owed — leaving it parked would only fire the + /// rename at the next pull instead. + #[gpui::test] + fn a_populated_workspace_keeps_the_name_its_own_machine_gave_it(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + let (ws, view) = primed_window(cx, Some("sher1")); + let epoch = cx.default_global::().windows[&ws].epoch; + + finish_prime( + cx, + ws, + epoch, + Ok(( + WsMirror { + tabs: vec![TreeTab::leaf(7), TreeTab::leaf(11)], + active: None, + }, + Some("alisher-work".into()), + )), + ); + + assert_eq!( + crate::ui::machine_mirror::display_name(cx, &view).as_deref(), + Some("alisher-work"), + "a workspace with tabs in it was not created here, so it keeps its own name" + ); + assert!( + cx.default_global::().windows[&ws] + .chosen_name + .is_none(), + "and the name stops being owed either way, so no later pull fires the rename" + ); + }); + } + /// A window with no name owed reads whatever the machine says, which is /// the whole of #604 and must survive the arbitration above. #[gpui::test]