plan.md

 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