From 7b384c5015f028dab105a9a33817d518d6b07202 Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:18:13 +0000 Subject: [PATCH] sidebar: Fix threads disappearing when stored main paths go stale (#54382) (cherry-pick to preview) (#54434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of #54382 to preview ---- Fixes a user-reported bug where a thread could be missing from the sidebar even though it was still present in the metadata store and still visible in Thread History. The thread reappears in the sidebar only after the user sends a message. ### Scenario A single multi-root workspace whose roots are `[/cloud, /worktrees/zed/wt_a/zed]`, where: - `/cloud` is a standalone git repo (main == folder). - `/worktrees/zed/wt_a/zed` is a linked worktree of a separate `/zed` repo. The project group normalizes to `main_worktree_paths = [/cloud, /zed]`. A thread created in this workspace is written with `main=[/cloud, /zed], folder=[/cloud, /worktrees/zed/wt_a/zed]` and the sidebar finds it via `entries_for_main_worktree_path`. If the thread's stored `main_worktree_paths` ever drifts from the group key — e.g. a stale row loaded from the store on startup, a legacy write, or a row persisted with `main == folder` — all three existing lookups in `Sidebar::rebuild_contents` miss: 1. `entries_for_main_worktree_path([/cloud, /zed])` — the thread's stored main doesn't equal the group key. 2. `entries_for_path([/cloud, /zed])` — the thread's folder paths don't equal the group key either. 3. The linked-worktree fallback iterates the group workspaces' `linked_worktrees()` snapshots. Those yield *sibling* linked worktrees of the repo, not the workspace's own roots, so `/worktrees/zed/wt_a/zed` doesn't match. The row falls out of the sidebar entirely even though the metadata is intact and the thread's folder paths exactly equal the open workspace's roots. The store heals the stored row on the next `RootThreadUpdated` event, which is why sending a message makes the row reappear — but until then the sidebar misrepresents the state. ### Fix Add a fourth lookup to `Sidebar::rebuild_contents`: for each open workspace in the group, query the store by the workspace's own root paths. Any thread whose `folder_paths` matches an open workspace's roots belongs under that group, regardless of what its `main_worktree_paths` say. This covers the gap between stale-row load and store self-heal, matches the principle that the sidebar should reflect state that exists in a reasonable way, and is symmetric with the existing lookups (same store API, one more iterator). ### Commits 1. `sidebar: Add failing repro for thread disappearing from sidebar` — adds `test_sidebar_keeps_multi_root_thread_with_stale_main_paths` which reproduces the bug. Sets up the multi-root + linked-worktree layout, persists a thread in the stale shape, and asserts the row is still visible in the sidebar. 2. `sidebar: Show threads whose folder paths match an open workspace` — the fourth-lookup fix. 31 lines in `crates/sidebar/src/sidebar.rs`, no deletions. ### Verification - `cargo test -p sidebar`: 103 passed, 0 failed (the new test was failing before commit 2 and is passing after). - `./script/clippy -p sidebar`: clean. ### Follow-ups The three existing lookups in `rebuild_contents` are each covering a different historical shape the store can be in. A real cleanup — do a one-shot migration on reload that fills in `main_worktree_paths` for any row missing it, then retire the two legacy-shape lookups — is worth doing as a separate PR, but out of scope here. Release Notes: - Fixed an issue where agent threads could go missing from the sidebar. Co-authored-by: Eric Holk --- crates/sidebar/src/sidebar.rs | 31 +++++ crates/sidebar/src/sidebar_tests.rs | 184 ++++++++++++++++++++++++++++ 2 files changed, 215 insertions(+) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 21d0c49cddc348071bcba04754cfb7e4edcf26be..289a24619636e906d2e065ee18f57b45a01fb5a0 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -1152,6 +1152,37 @@ impl Sidebar { threads.push(make_thread_entry(row, workspace)); } + // Also surface any thread whose `folder_paths` equals + // one of this group's open workspaces' root paths. + // The three lookups above can all miss when the + // thread's stored `main_worktree_paths` disagree with + // the group key (for example, a stale row whose main + // paths equal its folder paths for a linked-worktree + // workspace). The thread will be rewritten into the + // correct shape the next time `handle_conversation_event` + // fires, but until then the sidebar should still show + // it under the group whose workspace it actually + // belongs to. + for ws in group_workspaces { + let ws_paths = workspace_path_list(ws, cx); + if ws_paths.paths().is_empty() { + continue; + } + for row in thread_store + .read(cx) + .entries_for_path(&ws_paths, group_host.as_ref()) + .cloned() + { + if !seen_thread_ids.insert(row.thread_id) { + continue; + } + threads.push(make_thread_entry( + row, + ThreadEntryWorkspace::Open(ws.clone()), + )); + } + } + // Load any legacy threads for any single linked wortree of this project group. let mut linked_worktree_paths = HashSet::new(); for workspace in group_workspaces { diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 5e34bb1c8425963883a068015e0e2c129df045f2..7600793dff500cde8d8e54d86186e26e3816a923 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -3983,6 +3983,190 @@ async fn test_clicking_absorbed_worktree_thread_activates_worktree_workspace( ); } +// Reproduces the core of the user-reported bug: a thread belonging to +// a multi-root workspace that mixes a standalone project and a linked +// git worktree can become invisible in the sidebar when its stored +// `main_worktree_paths` don't match the workspace's project group +// key. The metadata still exists and Thread History still shows it, +// but the sidebar rebuild's lookups all miss. +// +// Real-world setup: a single multi-root workspace whose roots are +// `[/cloud, /worktrees/zed/wt_a/zed]`, where: +// - `/cloud` is a standalone git repo (main == folder). +// - `/worktrees/zed/wt_a/zed` is a linked worktree of `/zed`. +// +// Once git scans complete the project group key is +// `[/cloud, /zed]` — the main paths of the two roots. A thread +// created in this workspace is written with +// `main=[/cloud, /zed], folder=[/cloud, /worktrees/zed/wt_a/zed]` +// and the sidebar finds it via `entries_for_main_worktree_path`. +// +// If some other code path (stale data on reload, a path-less archive +// restored via the project picker, a legacy write …) persists the +// thread with `main == folder` instead, the stored +// `main_worktree_paths` is +// `[/cloud, /worktrees/zed/wt_a/zed]` ≠ `[/cloud, /zed]`. The three +// lookups in `rebuild_contents` all miss: +// +// 1. `entries_for_main_worktree_path([/cloud, /zed])` — the +// thread's stored main doesn't equal the group key. +// 2. `entries_for_path([/cloud, /zed])` — the thread's folder paths +// don't equal the group key either. +// 3. The linked-worktree fallback iterates the group's workspaces' +// `linked_worktrees()` snapshots. Those yield *sibling* linked +// worktrees of the repo, not the workspace's own roots, so the +// thread's folder `/worktrees/zed/wt_a/zed` doesn't match. +// +// The row falls out of the sidebar entirely — matching the user's +// symptom of a thread visible in the agent panel but missing from +// the sidebar. It only reappears once something re-writes the +// thread's metadata in the good shape (e.g. `handle_conversation_event` +// firing after the user sends a message). +// +// We directly persist the bad shape via `store.save(...)` rather +// than trying to reproduce the original writer. The bug is +// ultimately about the sidebar's tolerance for any stale row whose +// folder paths correspond to an open workspace's roots, regardless +// of how that row came to be in the store. +#[gpui::test] +async fn test_sidebar_keeps_multi_root_thread_with_stale_main_paths(cx: &mut TestAppContext) { + agent_ui::test_support::init_test(cx); + cx.update(|cx| { + cx.set_global(agent_ui::MaxIdleRetainedThreads(1)); + ThreadStore::init_global(cx); + ThreadMetadataStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + prompt_store::init(cx); + }); + + let fs = FakeFs::new(cx.executor()); + + // Standalone repo — one of the workspace's two roots, main + // worktree of its own .git. + fs.insert_tree( + "/cloud", + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + + // Separate /zed repo whose linked worktree will form the second + // workspace root. /zed itself is NOT opened as a workspace root. + fs.insert_tree( + "/zed", + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + fs.insert_tree( + "/worktrees/zed/wt_a/zed", + serde_json::json!({ + ".git": "gitdir: /zed/.git/worktrees/wt_a", + "src": {}, + }), + ) + .await; + fs.add_linked_worktree_for_repo( + Path::new("/zed/.git"), + false, + git::repository::Worktree { + path: std::path::PathBuf::from("/worktrees/zed/wt_a/zed"), + ref_name: Some("refs/heads/wt_a".into()), + sha: "aaa".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + // Single multi-root project with both /cloud and the linked + // worktree of /zed. + let project = project::Project::test( + fs.clone(), + ["/cloud".as_ref(), "/worktrees/zed/wt_a/zed".as_ref()], + cx, + ) + .await; + project.update(cx, |p, cx| p.git_scans_complete(cx)).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().unwrap().clone()); + let _panel = add_agent_panel(&workspace, cx); + cx.run_until_parked(); + + // Sanity-check the shapes the rest of the test depends on. + let group_key = workspace.read_with(cx, |ws, cx| ws.project_group_key(cx)); + let expected_main_paths = PathList::new(&[PathBuf::from("/cloud"), PathBuf::from("/zed")]); + assert_eq!( + group_key.path_list(), + &expected_main_paths, + "expected the multi-root workspace's project group key to normalize to \ + [/cloud, /zed] (main of the standalone repo + main of the linked worktree)" + ); + + let folder_paths = PathList::new(&[ + PathBuf::from("/cloud"), + PathBuf::from("/worktrees/zed/wt_a/zed"), + ]); + let workspace_root_paths = workspace.read_with(cx, |ws, cx| PathList::new(&ws.root_paths(cx))); + assert_eq!( + workspace_root_paths, folder_paths, + "expected the workspace's root paths to equal [/cloud, /worktrees/zed/wt_a/zed]" + ); + + let session_id = acp::SessionId::new(Arc::from("multi-root-stale-paths")); + let thread_id = ThreadId::new(); + + // Persist the thread in the "bad" shape that the bug manifests as: + // main == folder for every root. Any stale row where + // `main_worktree_paths` no longer equals the group key produces + // the same user-visible symptom; this is the concrete shape + // produced by `WorktreePaths::from_folder_paths` on the workspace + // roots. + cx.update(|_, cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.save( + ThreadMetadata { + thread_id, + session_id: Some(session_id.clone()), + agent_id: agent::ZED_AGENT_ID.clone(), + title: Some("Stale Multi-Root Thread".into()), + updated_at: Utc::now(), + created_at: None, + interacted_at: None, + worktree_paths: WorktreePaths::from_folder_paths(&folder_paths), + archived: false, + remote_connection: None, + }, + cx, + ) + }); + }); + cx.run_until_parked(); + + let entries = visible_entries_as_strings(&sidebar, cx); + let visible = sidebar.read_with(cx, |sidebar, _cx| has_thread_entry(sidebar, &session_id)); + + // If this assert fails, we've reproduced the bug: the sidebar's + // rebuild queries can't locate the thread under the current + // project group, even though the metadata is intact and the + // thread's folder paths exactly equal the open workspace's roots. + assert!( + visible, + "thread disappeared from the sidebar when its main_worktree_paths \ + ({folder_paths:?}) diverged from the project group key ({expected_main_paths:?}); \ + sidebar entries: {entries:?}" + ); +} + #[gpui::test] async fn test_activate_archived_thread_with_saved_paths_activates_matching_workspace( cx: &mut TestAppContext,