1# Plan: Fix sidebar flicker when remote workspace is added
2
3## Context
4
5Read `summary.md` for all changes made so far. This plan covers the remaining flicker bug.
6
7## The Bug
8
9When a remote workspace is added to the sidebar, the project group briefly flickers (appears as a separate group for 1-2 frames). This happens because:
10
111. **Server-side `set_snapshot`** in `zed/crates/worktree/src/worktree.rs` (~line 1205) unconditionally recomputes `root_repo_common_dir` from `git_repositories`:
12
13 ```rust
14 new_snapshot.root_repo_common_dir = new_snapshot
15 .local_repo_for_work_directory_path(RelPath::empty())
16 .map(|repo| SanitizedPath::from_arc(repo.common_dir_abs_path.clone()));
17 ```
18
19 During early scan passes, `.git` hasn't been discovered yet, so this overwrites the correct value (set by `Worktree::local()` during creation) with `None`.
20
212. The server sends an `UpdateWorktree` message with `root_repo_common_dir = None`.
22
233. The client's `apply_remote_update` in `zed/crates/worktree/src/worktree.rs` (~line 2437) currently has a partial fix that only updates when `Some`:
24 ```rust
25 if let Some(dir) = update.root_repo_common_dir.map(...) {
26 self.root_repo_common_dir = Some(dir);
27 }
28 ```
29 This prevents the client from clearing it, but the real fix should be server-side.
30
31## What To Do
32
33### Step 1: Add flicker detection to the existing test
34
35Extend `test_clicking_closed_remote_thread_opens_remote_workspace` in `zed/crates/sidebar/src/sidebar_tests.rs` to catch transient flicker. Use the `observe_self` pattern from `test_clicking_worktree_thread_does_not_briefly_render_as_separate_project` (line ~3326-3397), which installs an observer that fires on **every notification** and panics if more than one project header ever appears:
36
37```rust
38sidebar
39 .update(cx, |_, cx| cx.observe_self(assert_sidebar_state))
40 .detach();
41```
42
43Add this observer BEFORE the stale key injection / workspace addition steps. The callback should assert that there is never more than one project group header at any point during the test. This catches the case where an `UpdateWorktree` message with `root_repo_common_dir = None` temporarily creates a wrong project group key.
44
45Since the full remote mock connection is hard to set up for a second connection, an alternative approach: simulate the `UpdateWorktree` message arriving with `root_repo_common_dir = None` by directly calling the worktree's update mechanism on the existing project. Or, test at a lower level by verifying that `set_snapshot` doesn't clear `root_repo_common_dir`.
46
47### Step 2: Fix the server-side root cause
48
49In `zed/crates/worktree/src/worktree.rs`, find `set_snapshot` (~line 1200-1210). Change the `root_repo_common_dir` recomputation to not downgrade once set:
50
51```rust
52// Before (overwrites unconditionally):
53new_snapshot.root_repo_common_dir = new_snapshot
54 .local_repo_for_work_directory_path(RelPath::empty())
55 .map(|repo| SanitizedPath::from_arc(repo.common_dir_abs_path.clone()));
56
57// After (preserve existing value if scan hasn't discovered repo yet):
58new_snapshot.root_repo_common_dir = new_snapshot
59 .local_repo_for_work_directory_path(RelPath::empty())
60 .map(|repo| SanitizedPath::from_arc(repo.common_dir_abs_path.clone()))
61 .or(self.snapshot.root_repo_common_dir.clone());
62```
63
64This ensures the value discovered by `Worktree::local()` during creation is preserved until the scanner finds the repo and confirms/updates it.
65
66### Step 3: Verify the client-side guard is still useful
67
68The `apply_remote_update` change (only update when `Some`) is a defense-in-depth measure. With the server fix, the server should never send `None` after having the correct value. But keeping the client guard is good practice. Verify the test passes with both fixes.
69
70### Step 4: Update `summary.md`
71
72Add the flicker fix to the summary of changes.
73
74## Important Notes
75
76- Use sub-agents for research tasks to keep context manageable
77- The key test pattern is `cx.observe_self(callback)` which fires on every `cx.notify()` — this catches transient states that `run_until_parked` would miss
78- Read `test_clicking_worktree_thread_does_not_briefly_render_as_separate_project` (~line 3262-3397) for the full example of this testing pattern
79- After all changes, run `cargo check` on all affected packages and run the sidebar + agent_ui tests