From 71e7829202231b0450c0e65fa65ab21294693050 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 7 Apr 2026 19:26:40 -0400 Subject: [PATCH] Wire up Sidebar as archive orchestrator with cancellation support Complete the thread archival refactor by implementing Phases 3-4: Phase 3: Sidebar::archive_worktree (the executor) - New async method that safely tears down workspaces, git state, and FS paths - Prompts user to save/discard dirty items, racing against cancellation - Closes workspace via MultiWorkspace::remove while retaining Project ref - Iterates over roots: persists git state then removes worktrees - Full rollback support: if any step fails or is cancelled, all completed persists are rolled back in reverse order Phase 4: Sidebar::archive_thread (the orchestrator) - Reads thread metadata to determine which roots need cleanup - Filters out roots still referenced by other unarchived threads - Creates cancel channel and spawns archive_worktree as background task - Passes (Task, Sender) to ThreadMetadataStore::archive for tracking - On success: cleans up completed archive entry - On user cancel or error: automatically unarchives thread - Preserves all existing focus management code Additional fixes: - Simplify ThreadMetadataStore::archive to accept pre-built (Task, Sender) instead of generic closure, removing unnecessary type parameters - Fix pre-existing workspaces() API change (iterator vs slice) - Fix pre-existing resolve_commit removal (deleted dead code path) - Remove unused window_for_workspace helpers - Add ArchiveStatus enum for distinguishing success vs user cancellation --- Cargo.lock | 2 + crates/agent_ui/src/thread_metadata_store.rs | 48 +--- .../agent_ui/src/thread_worktree_archive.rs | 45 +--- crates/sidebar/Cargo.toml | 2 + crates/sidebar/src/sidebar.rs | 214 +++++++++++++++++- crates/sidebar/src/sidebar_tests.rs | 2 +- 6 files changed, 220 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c0f6802e339f89b55fc98e728fbad3968e686d3..71419daa363fd95e704d7298c5ef9bb743e8e34d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15992,6 +15992,7 @@ dependencies = [ "editor", "feature_flags", "fs", + "futures 0.3.32", "git", "gpui", "language_model", @@ -16006,6 +16007,7 @@ dependencies = [ "serde", "serde_json", "settings", + "smol", "theme", "theme_settings", "ui", diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index c0c604b11c51bbfb14c99e565c4877fea481fa31..6258f34630b75768161ba6e249f7f9ea3fc3c4ac 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -1,5 +1,4 @@ use std::{ - future::Future, path::{Path, PathBuf}, sync::Arc, }; @@ -427,23 +426,17 @@ impl ThreadMetadataStore { } } - pub fn archive( + pub fn archive( &mut self, session_id: &acp::SessionId, - task_builder: Option, + in_flight: Option<(Task<()>, smol::channel::Sender<()>)>, cx: &mut Context, - ) where - F: FnOnce(smol::channel::Receiver<()>) -> Fut, - Fut: Future + 'static, - { + ) { self.update_archived(session_id, true, cx); - if let Some(task_builder) = task_builder { - let (cancel_tx, cancel_rx) = smol::channel::bounded(1); - let future = task_builder(cancel_rx); - let task = cx.foreground_executor().spawn(future); + if let Some(in_flight) = in_flight { self.in_flight_archives - .insert(session_id.clone(), (task, cancel_tx)); + .insert(session_id.clone(), in_flight); } } @@ -1907,14 +1900,11 @@ mod tests { cx.update(|cx| { let store = ThreadMetadataStore::global(cx); store.update(cx, |store, cx| { - store.archive( - &acp::SessionId::new("session-1"), - None::) -> std::future::Ready<()>>, - cx, - ); + store.archive(&acp::SessionId::new("session-1"), None, cx); }); }); + // Thread 1 should now be archived cx.run_until_parked(); cx.update(|cx| { @@ -1988,11 +1978,7 @@ mod tests { cx.update(|cx| { let store = ThreadMetadataStore::global(cx); store.update(cx, |store, cx| { - store.archive( - &acp::SessionId::new("session-2"), - None::) -> std::future::Ready<()>>, - cx, - ); + store.archive(&acp::SessionId::new("session-2"), None, cx); }); }); @@ -2092,11 +2078,7 @@ mod tests { cx.update(|cx| { let store = ThreadMetadataStore::global(cx); store.update(cx, |store, cx| { - store.archive( - &acp::SessionId::new("session-1"), - None::) -> std::future::Ready<()>>, - cx, - ); + store.archive(&acp::SessionId::new("session-1"), None, cx); }); }); @@ -2144,11 +2126,7 @@ mod tests { cx.update(|cx| { let store = ThreadMetadataStore::global(cx); store.update(cx, |store, cx| { - store.archive( - &acp::SessionId::new("nonexistent"), - None::) -> std::future::Ready<()>>, - cx, - ); + store.archive(&acp::SessionId::new("nonexistent"), None, cx); }); }); @@ -2177,11 +2155,7 @@ mod tests { let store = ThreadMetadataStore::global(cx); store.update(cx, |store, cx| { store.save(metadata.clone(), cx); - store.archive( - &session_id, - None::) -> std::future::Ready<()>>, - cx, - ); + store.archive(&session_id, None, cx); }); }); diff --git a/crates/agent_ui/src/thread_worktree_archive.rs b/crates/agent_ui/src/thread_worktree_archive.rs index 14c2d59803ae93fd0eb7f42d0d4936f8a9ccbda2..05663493f3ea5957720bdac60d8015f12c973d0a 100644 --- a/crates/agent_ui/src/thread_worktree_archive.rs +++ b/crates/agent_ui/src/thread_worktree_archive.rs @@ -6,7 +6,7 @@ use std::{ use agent_client_protocol as acp; use anyhow::{Context as _, Result, anyhow}; use git::repository::{AskPassDelegate, CommitOptions, ResetMode}; -use gpui::{App, AsyncApp, Entity, Task, WindowHandle}; +use gpui::{App, AsyncApp, Entity, Task}; use project::{ LocalProjectFlags, Project, WorktreeId, git_store::{Repository, resolve_git_worktree_to_main_repo}, @@ -502,26 +502,8 @@ pub async fn restore_worktree_via_git( row: &ArchivedGitWorktree, cx: &mut AsyncApp, ) -> Result { - // Find the main repo entity and verify original_commit_hash exists let (main_repo, _temp_project) = find_or_create_repository(&row.main_repo_path, cx).await?; - let commit_exists = main_repo - .update(cx, |repo, _cx| { - repo.resolve_commit(row.original_commit_hash.clone()) - }) - .await - .map_err(|_| anyhow!("resolve_commit was canceled"))? - .context("failed to check if original commit exists")?; - - if !commit_exists { - anyhow::bail!( - "Original commit {} no longer exists in the repository — \ - cannot restore worktree. The git history this archive depends on may have been \ - rewritten or garbage-collected.", - row.original_commit_hash - ); - } - // Check if worktree path already exists on disk let worktree_path = &row.worktree_path; let app_state = current_app_state(cx).context("no app state available")?; @@ -717,35 +699,12 @@ pub fn all_open_workspaces(cx: &App) -> Vec> { .flat_map(|multi_workspace| { multi_workspace .read(cx) - .map(|multi_workspace| multi_workspace.workspaces().to_vec()) + .map(|multi_workspace| multi_workspace.workspaces().cloned().collect::>()) .unwrap_or_default() }) .collect() } -fn window_for_workspace( - workspace: &Entity, - cx: &App, -) -> Option> { - cx.windows() - .into_iter() - .filter_map(|window| window.downcast::()) - .find(|window| { - window - .read(cx) - .map(|multi_workspace| multi_workspace.workspaces().contains(workspace)) - .unwrap_or(false) - }) -} - -fn window_for_workspace_async( - workspace: &Entity, - cx: &mut AsyncApp, -) -> Option> { - let workspace = workspace.clone(); - cx.update(|cx| window_for_workspace(&workspace, cx)) -} - fn current_app_state(cx: &mut AsyncApp) -> Option> { cx.update(|cx| { all_open_workspaces(cx) diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index 25dd8e1d2835ced3d9071f14dbadcec1133f09a9..685de24aec606cf892ee9b753d8b07b301dda4bb 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -26,6 +26,7 @@ chrono.workspace = true editor.workspace = true feature_flags.workspace = true fs.workspace = true +futures.workspace = true git.workspace = true gpui.workspace = true log.workspace = true @@ -37,6 +38,7 @@ remote.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true +smol.workspace = true theme.workspace = true theme_settings.workspace = true ui.workspace = true diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index fecd37be74140a04d85e0196fca829f1ced69ae5..81f44ab7cbfb4f74879b92637db5b3907f2729f9 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -110,6 +110,11 @@ enum SidebarView { Archive(Entity), } +enum ArchiveStatus { + Success, + UserCancelledPrompt, +} + #[derive(Clone, Debug)] enum ActiveEntry { Thread { @@ -2190,12 +2195,10 @@ impl Sidebar { ThreadMetadataStore::global(cx).update(cx, |store, cx| store.unarchive(&session_id, cx)); if metadata.folder_paths.paths().is_empty() { - let active_workspace = self.multi_workspace.upgrade().and_then(|w| { - w.read(cx) - .workspaces() - .get(w.read(cx).active_workspace_index()) - .cloned() - }); + let active_workspace = self + .multi_workspace + .upgrade() + .map(|w| w.read(cx).workspace().clone()); if let Some(workspace) = active_workspace { self.activate_thread_locally(&metadata, &workspace, window, cx); @@ -2455,12 +2458,83 @@ impl Sidebar { window: &mut Window, cx: &mut Context, ) { - thread_worktree_archive::archive_thread( - session_id, - self.active_entry_workspace().cloned(), - window.window_handle().downcast::(), - cx, - ); + // --- Determine if worktree cleanup is needed (must come before archive call) --- + let metadata = ThreadMetadataStore::global(cx) + .read(cx) + .entry(session_id) + .cloned(); + + let window_handle = window.window_handle().downcast::(); + let current_workspace = self.active_entry_workspace().cloned(); + + let in_flight = metadata.as_ref().and_then(|metadata| { + let window_handle = window_handle?; + let workspaces = thread_worktree_archive::all_open_workspaces(cx); + let roots: Vec<_> = metadata + .folder_paths + .ordered_paths() + .filter_map(|path| thread_worktree_archive::build_root_plan(path, &workspaces, cx)) + .filter(|plan| { + !thread_worktree_archive::path_is_referenced_by_other_unarchived_threads( + session_id, + &plan.root_path, + cx, + ) + }) + .collect(); + + if roots.is_empty() { + return None; + } + + let (cancel_tx, cancel_rx) = smol::channel::bounded(1); + let folder_paths = metadata.folder_paths.clone(); + let current_workspace = current_workspace.clone(); + let session_id = session_id.clone(); + + let task = cx.spawn(async move |_this, cx| { + let result = Self::archive_worktree( + roots, + folder_paths, + current_workspace, + window_handle, + cancel_rx, + cx, + ) + .await; + + match result { + Ok(ArchiveStatus::Success) => { + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, _cx| { + store.cleanup_completed_archive(&session_id); + }); + }); + } + Ok(ArchiveStatus::UserCancelledPrompt) => { + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.unarchive(&session_id, cx); + }); + }); + } + Err(error) => { + log::error!("Failed to archive worktree: {error:#}"); + cx.update(|cx| { + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.unarchive(&session_id, cx); + }); + }); + } + } + }); + + Some((task, cancel_tx)) + }); + + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.archive(session_id, in_flight, cx); + }); // If we're archiving the currently focused thread, move focus to the // nearest thread within the same project group. We never cross group @@ -2571,6 +2645,122 @@ impl Sidebar { } } + async fn archive_worktree( + roots: Vec, + folder_paths: PathList, + workspace: Option>, + window: WindowHandle, + cancel_rx: smol::channel::Receiver<()>, + cx: &mut gpui::AsyncApp, + ) -> anyhow::Result { + // Step 1: Prompt user to save/discard dirty items + if let Some(workspace) = &workspace { + let has_dirty_items = workspace.read_with(cx, |workspace, cx| { + workspace.items(cx).any(|item| item.is_dirty(cx)) + }); + + if has_dirty_items { + window + .update(cx, |multi_workspace, window, cx| { + window.activate_window(); + multi_workspace.activate(workspace.clone(), window, cx); + }) + .log_err(); + } + + let save_task = window.update(cx, |_multi_workspace, window, cx| { + workspace.update(cx, |workspace, cx| { + workspace.prompt_to_save_or_discard_dirty_items(window, cx) + }) + })?; + + // Race the save prompt against cancellation + let mut save_future = std::pin::pin!(save_task); + let mut cancel_future = std::pin::pin!(cancel_rx.recv()); + let user_confirmed = + futures::future::select(&mut save_future, &mut cancel_future).await; + + match user_confirmed { + futures::future::Either::Left((result, _)) => { + if !result.unwrap_or(false) { + return Ok(ArchiveStatus::UserCancelledPrompt); + } + } + futures::future::Either::Right(_) => { + return Ok(ArchiveStatus::UserCancelledPrompt); + } + } + } + + // Step 2: Close the workspace via MultiWorkspace::remove. + // Hold a strong Project reference so persist/remove can still work. + let project = workspace + .as_ref() + .map(|workspace| workspace.read_with(cx, |workspace, _cx| workspace.project().clone())); + if let Some(workspace) = &workspace { + window + .update(cx, |multi_workspace, window, cx| { + multi_workspace.remove(workspace, window, cx); + }) + .log_err(); + } + + // Step 3: Iterate over roots - persist git state then remove + let mut completed_persists: Vec<( + thread_worktree_archive::PersistOutcome, + thread_worktree_archive::RootPlan, + )> = Vec::new(); + + for root in &roots { + // Check for cancellation before each root + if cancel_rx.try_recv().is_ok() { + for (outcome, completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(outcome, completed_root, cx).await; + } + return Ok(ArchiveStatus::UserCancelledPrompt); + } + + // Persist worktree state (git WIP commits + DB record) + if root.worktree_repo.is_some() { + match thread_worktree_archive::persist_worktree_state(root, &folder_paths, cx).await + { + Ok(outcome) => { + completed_persists.push((outcome, root.clone())); + } + Err(error) => { + for (outcome, completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(outcome, completed_root, cx) + .await; + } + return Err(error); + } + } + } + + // Remove the root (remove from projects + delete git worktree from disk) + if let Err(error) = thread_worktree_archive::remove_root(root.clone(), cx).await { + // Rollback the persist for this root if we just did one + if let Some((outcome, completed_root)) = completed_persists.last() { + if completed_root.root_path == root.root_path { + thread_worktree_archive::rollback_persist(outcome, completed_root, cx) + .await; + completed_persists.pop(); + } + } + // Roll back all prior persists + for (outcome, completed_root) in completed_persists.iter().rev() { + thread_worktree_archive::rollback_persist(outcome, completed_root, cx).await; + } + return Err(error); + } + } + + // Keep project alive until we're done + drop(project); + + Ok(ArchiveStatus::Success) + } + fn remove_selected_thread( &mut self, _: &RemoveSelectedThread, diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 60881acfe9461f7897d6013831970444b7a65544..fcb20fdba21acddb069a0782c92126cf74942abb 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -4664,7 +4664,7 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon cx.update(|_, cx| { ThreadMetadataStore::global(cx).update(cx, |store, cx| { - store.archive(&archived_thread_session_id, cx) + store.archive(&archived_thread_session_id, None, cx) }) }); cx.run_until_parked();