From 7b611de21146b0b5d0b4601f5444d5b992fad982 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Mon, 23 Mar 2026 13:19:31 -0700 Subject: [PATCH] Update PLAN.md with current status and remaining work --- PLAN.md | 152 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 75 insertions(+), 77 deletions(-) diff --git a/PLAN.md b/PLAN.md index d743988ff0fecccd95224f4d73e8464f82d1c654..94f313c93ab5190bb5b8068d2f8b1808e3413c9f 100644 --- a/PLAN.md +++ b/PLAN.md @@ -6,111 +6,116 @@ Threads in the sidebar are grouped by their `folder_paths` (a `PathList` stored in the thread metadata database). When a thread is created from a git worktree checkout (e.g. `/Users/eric/repo/worktrees/zed/lasalle-lceljoj7/zed`), its `folder_paths` records the worktree path. But the sidebar computes workspace -groups from `visible_worktrees().abs_path()`, which returns the root repo path -(e.g. `/Users/eric/repo/zed`). Since `entries_for_path` did exact `PathList` -equality, threads from worktree checkouts were invisible in the sidebar. +groups from `visible_worktrees().abs_path()`, which returns the checkout path. +Threads from different checkouts of the same repos (different branches) have +different raw paths and don't match. ## What we've done -### 1. `PathList` equality fix (PR #52052 — ready to merge) +### 1. `PathList` equality fix (PR #52052 — merged) **File:** `crates/util/src/path_list.rs` -`PathList` derived `PartialEq`/`Eq`/`Hash` which included the `order` field -(display ordering of paths). Two `PathList` values with the same paths in -different order were considered unequal. This caused thread matching to break -after worktree reordering in the project panel. +Manual `PartialEq`/`Eq`/`Hash` impls that only compare the sorted `paths` +field, ignoring display order. -**Fix:** Manual `PartialEq`/`Eq`/`Hash` impls that only compare the sorted -`paths` field. +### 2. Worktree path canonicalization + historical groups (this branch) -### 2. Worktree path canonicalization (on this branch, not yet PR'd) +**Files:** `crates/sidebar/src/sidebar.rs`, `crates/agent_ui/src/thread_metadata_store.rs` -**File:** `crates/sidebar/src/sidebar.rs` +#### Core changes: -Added two functions: -- `build_worktree_root_mapping()` — iterates all repo snapshots from all open - workspaces and builds a `HashMap>` mapping every known - worktree checkout path to its root repo path (using `original_repo_abs_path` - and `linked_worktrees` from `RepositorySnapshot`). -- `canonicalize_path_list()` — maps each path in a `PathList` through the - worktree root mapping, producing a canonical `PathList` keyed by root repo - paths. +- **`build_worktree_root_mapping()`** — iterates ALL repos from all workspaces + (not just root repos) to build a `HashMap>` mapping every + known worktree checkout path to its root repo path. Robust against snapshot + timing where linked-worktree lists may be temporarily incomplete. -In `rebuild_contents`, instead of querying `entries_for_path(&path_list)` with -the workspace's literal path list, we now: -1. Build the worktree→root mapping once at the top -2. Iterate all thread entries and index them by their canonicalized `folder_paths` -3. Query that canonical index when populating each workspace's thread list +- **`canonicalize_path_list()`** — maps each path in a `PathList` through the + worktree root mapping. -Also applied the same canonicalization to `find_current_workspace_for_path_list` -and `find_open_workspace_for_path_list` (used by archive thread restore). +- **`rebuild_contents()` three-tier thread lookup:** + 1. **Raw lookup** (`entries_for_path`) — exact match by workspace's raw paths + 2. **Linked worktree loop** (canonical lookup per repo) — finds threads from + absorbed worktree checkouts, assigns correct worktree chips + 3. **Canonical lookup** — catches threads from different checkouts of the same + repos (e.g. thread saved in branch-a, workspace is branch-b) -**Status:** The core grouping works — threads from worktree checkouts now appear -under the root repo's sidebar header. But there are remaining issues with the -archive restore flow and workspace absorption. +- **Historical groups** — after the workspace loop, iterates all unclaimed + threads (tracked via `claimed_session_ids`) and creates `Closed` project + group sections. These appear at the bottom of the sidebar. + +- **`ProjectHeader.workspace`** is now `Option>` to support + closed historical group headers. + +- **`find_current_workspace_for_path_list` / `find_open_workspace_for_path_list`** + — canonicalize both sides (thread path and workspace path) before comparing. + +- **`activate_archived_thread`** — when no matching workspace is found, saves + metadata and sets `focused_thread` instead of opening a new workspace (which + would get absorbed via `find_existing_workspace`). + +- **`prune_stale_worktree_workspaces`** — doesn't prune a worktree workspace + when its main repo workspace is still open (linked-worktree list may be + temporarily incomplete during re-scans). + +- **`thread_entry_from_metadata`** — extracted helper for building ThreadEntry + from ThreadMetadata. + +- **`SidebarThreadMetadataStore::all_entries()`** — new method returning + `&[ThreadMetadata]` for reference-based iteration. ## Remaining issues -### Archive thread restore doesn't route correctly +### Canonical lookup assigns threads to wrong workspace (next up) -When restoring a thread from the archive, `activate_archived_thread` tries to -find a matching workspace via `find_current_workspace_for_path_list`. If the -thread's `folder_paths` is a single worktree path (e.g. `[zed/meteco/zed]`), -canonicalization maps it to `[/Users/eric/repo/zed]`. But if the current window -only has an `[ex, zed]` workspace, the canonical `[zed]` doesn't match `[ex, -zed]` — they're different path sets. So it falls through to -`open_workspace_and_activate_thread`, which opens the correct worktree but: -- The new workspace gets **absorbed** under the `ex, zed` header (no separate - "zed" header appears) -- The thread activation may not route to the correct agent panel +When multiple workspaces share the same canonical path (e.g. main repo + worktree +checkout of the same repos), the canonical lookup assigns threads to whichever +workspace processes first in the loop. This causes threads to open in the wrong +workspace context. -This needs investigation into how absorption interacts with the restore flow, -and possibly the creation of a dedicated "zed" workspace (without ex) for -threads that were created in a zed-only context. +**Fix needed:** Two-pass approach in `rebuild_contents`: +- **Pass 1:** Raw lookups across all workspaces (priority claims, correct + workspace assignment) +- **Pass 2:** Canonical lookups only for threads not claimed in pass 1 + +### Click-to-open from Closed groups bypasses `find_existing_workspace` + +When a user clicks a thread under a `Closed` historical group header, +`open_workspace_and_activate_thread` goes through `open_paths` → +`find_existing_workspace`, which routes to an existing workspace that contains +the path instead of creating a new workspace tab. Need to either: +- Pass `open_new_workspace: Some(true)` through the call chain +- Or use a direct workspace creation path ### Path set mutation (adding/removing folders) When you add a folder to a project (e.g. adding `ex` to a `zed` workspace), existing threads saved with `[zed]` don't match the new `[ex, zed]` path list. -Similarly, removing `ex` leaves threads saved with `[ex, zed]` orphaned. - -This is a **design decision** the team is still discussing. Options include: -- Treat adding/removing a folder as mutating the project group (update all - thread `folder_paths` to match) -- Show threads under the closest matching workspace -- Show "historical" groups for path lists that have threads but no open workspace +This is a design decision still being discussed. -### Absorption suppresses workspace headers +### Pre-existing test failure -When a worktree workspace is absorbed under a main repo workspace, it doesn't -get its own sidebar header. This is by design for the common case (you don't -want `zed` and `zed/meteor-36zvf3d7` as separate headers). But it means that a -thread from a single-path worktree workspace like `[zed/meteco/zed]` has no -header to appear under if the main workspace is `[ex, zed]` (different path -count). +`test_two_worktree_workspaces_absorbed_when_main_added` fails on `origin/main` +before our changes. Root cause is a git snapshot timing issue where linked +worktrees temporarily disappear during re-scans, causing the prune function +to remove workspaces prematurely. ## Key code locations - **Thread metadata storage:** `crates/agent_ui/src/thread_metadata_store.rs` - `SidebarThreadMetadataStore` — in-memory cache + SQLite DB - `threads_by_paths: HashMap>` — index by literal paths - - DB location: `~/Library/Application Support/Zed/db/0-{channel}/db.sqlite` table `sidebar_threads` -- **Old thread storage:** `crates/agent/src/db.rs` - - `ThreadsDatabase` — the original thread DB (being migrated from) - - DB location: `~/Library/Application Support/Zed/threads/threads.db` - **Sidebar rebuild:** `crates/sidebar/src/sidebar.rs` - - `rebuild_contents()` — the main function that assembles sidebar entries - - `build_worktree_root_mapping()` — new: builds worktree→root path map - - `canonicalize_path_list()` — new: maps a PathList through the root mapping - - Absorption logic starts around "Identify absorbed workspaces" - - Linked worktree query starts around "Load threads from linked git worktrees" + - `rebuild_contents()` — three-tier lookup + historical groups + - `build_worktree_root_mapping()` — worktree→root path map + - `canonicalize_path_list()` — maps a PathList through the root mapping + - `thread_entry_from_metadata()` — helper for building ThreadEntry - **Thread saving:** `crates/agent/src/agent.rs` - - `NativeAgent::save_thread()` — snapshots `folder_paths` from `project.visible_worktrees()` on every save + - `NativeAgent::save_thread()` — snapshots `folder_paths` from visible worktrees - **PathList:** `crates/util/src/path_list.rs` - - Equality now compares only sorted paths, not display order + - Equality compares only sorted paths, not display order - **Archive restore:** `crates/sidebar/src/sidebar.rs` - - `activate_archived_thread()` → `find_current_workspace_for_path_list()` → `open_workspace_and_activate_thread()` + - `activate_archived_thread()` — saves metadata + focuses thread (no workspace open) ## Useful debugging queries @@ -119,14 +124,7 @@ count). sqlite3 ~/Library/Application\ Support/Zed/db/0-nightly/db.sqlite \ "SELECT folder_paths, COUNT(*) FROM sidebar_threads GROUP BY folder_paths ORDER BY COUNT(*) DESC" --- All distinct folder_paths in the old thread store -sqlite3 ~/Library/Application\ Support/Zed/threads/threads.db \ - "SELECT folder_paths, COUNT(*) FROM threads WHERE parent_id IS NULL GROUP BY folder_paths ORDER BY COUNT(*) DESC" - -- Find a specific thread sqlite3 ~/Library/Application\ Support/Zed/db/0-nightly/db.sqlite \ "SELECT session_id, title, folder_paths FROM sidebar_threads WHERE title LIKE '%search term%'" - --- List all git worktrees for a repo -git -C /Users/eric/repo/zed worktree list --porcelain ```