mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
Recreate archived worktrees from scratch on restore
Restoring an archived agent thread always runs git read-tree --reset -u, which clobbers anything currently in the worktree directory. Previously, when the worktree directory existed but wasn't a valid linked worktree (which can happen on Windows if archival's git worktree remove --force succeeded in unregistering but failed to fully delete files because they were locked by another process, like node-pty/conpty), the restore code only ran git worktree repair, which is a no-op when the main repo's registration is also gone. The user ended up with a disconnected directory and no clear way to recover. This PR replaces the restore flow with a recreate-from-scratch approach that collapses every messy intermediate state (missing dir, stale registration, leftover content from a partial Windows archive, fully valid worktree) into one path: rename any pre-existing content into a sibling zed-restore-backup-<uuid> directory, drop any stale registration, then git worktree add --detach. Any pre-existing content at the worktree path is moved aside rather than deleted up-front. If any destructive step fails, rollback_backup restores the backup so the user's content is preserved. On success, the backup is deleted asynchronously. The backup directory is always created as a sibling so the rename stays same-volume and can't fail with EXDEV. Adds NotAWorktreeError to GitRepository::remove_worktree so the restore code can treat 'nothing to remove' as success without substring-matching git's localized stderr. The path comparison uses a new util::paths::paths_resolve_to_same_location that walks up to the nearest existing ancestor before canonicalizing, so it handles missing leaves (e.g. macOS /var/... vs the registered /private/var/... when the directory is already gone). The DB commit (path replacements + unarchive) runs via AsyncApp so it survives the user closing the window between the destructive restore pass and the commit; only the workspace-activation UI is window-bound and best-effort. The per-window re-entry guard prevents double-clicks from spawning racing tasks. Release Notes: - Fixed a bug where restoring an archived agent thread on Windows could leave the worktree disconnected from git.
This commit is contained in:
Richard Feldman
parent
5e36ec8256
commit
c83b6f68f3
7 changed files with 1981 additions and 189 deletions
File diff suppressed because it is too large
Load diff
|
|
@ -13,8 +13,8 @@ use git::{
|
|||
repository::{
|
||||
AskPassDelegate, Branch, CommitData, CommitDataReader, CommitDetails, CommitOptions,
|
||||
CreateWorktreeTarget, FetchOptions, GRAPH_CHUNK_SIZE, GitRepository,
|
||||
GitRepositoryCheckpoint, InitialGraphCommitData, LogOrder, LogSource, PushOptions, RefEdit,
|
||||
Remote, RepoPath, ResetMode, SearchCommitArgs, Worktree,
|
||||
GitRepositoryCheckpoint, InitialGraphCommitData, LogOrder, LogSource, NotAWorktreeError,
|
||||
PushOptions, RefEdit, Remote, RepoPath, ResetMode, SearchCommitArgs, Worktree,
|
||||
},
|
||||
stash::GitStash,
|
||||
status::{
|
||||
|
|
@ -769,9 +769,17 @@ impl GitRepository for FakeGitRepository {
|
|||
.trim();
|
||||
PathBuf::from(gitdir)
|
||||
} else {
|
||||
self.find_worktree_entry_dir_by_path(&path)
|
||||
.await
|
||||
.with_context(|| format!("no worktree found at path: {}", path.display()))?
|
||||
match self.find_worktree_entry_dir_by_path(&path).await {
|
||||
Some(dir) => dir,
|
||||
None => {
|
||||
// Match the real backend's behavior: surface a
|
||||
// structured `NotAWorktreeError` so callers can
|
||||
// treat "nothing to remove" as success.
|
||||
return Err(anyhow::Error::new(NotAWorktreeError {
|
||||
path: path.clone(),
|
||||
}));
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
// Remove the worktree checkout directory if it still exists.
|
||||
|
|
|
|||
|
|
@ -823,6 +823,13 @@ pub trait GitRepository: Send + Sync {
|
|||
create: bool,
|
||||
) -> BoxFuture<'_, Result<()>>;
|
||||
|
||||
/// Removes a git worktree.
|
||||
///
|
||||
/// When the path has no entry in the main repo's worktree registry
|
||||
/// (because it was never registered, or its registration was already
|
||||
/// pruned), implementations return [`NotAWorktreeError`] as the error
|
||||
/// payload so callers can downcast and treat the "nothing to remove"
|
||||
/// case as a no-op.
|
||||
fn remove_worktree(&self, path: PathBuf, force: bool) -> BoxFuture<'_, Result<()>>;
|
||||
|
||||
fn rename_worktree(&self, old_path: PathBuf, new_path: PathBuf) -> BoxFuture<'_, Result<()>>;
|
||||
|
|
@ -1930,6 +1937,35 @@ impl GitRepository for RealGitRepository {
|
|||
|
||||
self.executor
|
||||
.spawn(async move {
|
||||
// Pre-check the worktree registry directly rather than
|
||||
// matching `git worktree remove`'s stderr. Git localizes its
|
||||
// error messages (`LC_MESSAGES`), and Zed makes no attempt to
|
||||
// pin the locale on its git invocations, so substring-matching
|
||||
// an English phrase would silently miss the "not registered"
|
||||
// case on non-English machines.
|
||||
let list_output = git
|
||||
.build_command(&["worktree", "list", "--porcelain"])
|
||||
.output()
|
||||
.await?;
|
||||
if !list_output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&list_output.stderr);
|
||||
anyhow::bail!("git worktree list failed: {stderr}");
|
||||
}
|
||||
let stdout = String::from_utf8_lossy(&list_output.stdout);
|
||||
let worktrees = parse_worktrees_from_str(&stdout, None);
|
||||
// Cheap literal-equality pass first so the common case
|
||||
// (caller path matches the registered form byte-for-byte)
|
||||
// doesn't pay for an `fs::canonicalize` per registered
|
||||
// worktree. Only fall back to the symlink-resolving
|
||||
// comparison when literal equality misses.
|
||||
let matches_registration = worktrees.iter().any(|wt| wt.path == path)
|
||||
|| worktrees
|
||||
.iter()
|
||||
.any(|wt| util::paths::paths_resolve_to_same_location(&wt.path, &path));
|
||||
if !matches_registration {
|
||||
return Err(anyhow::Error::new(NotAWorktreeError { path }));
|
||||
}
|
||||
|
||||
let mut args: Vec<OsString> = vec!["worktree".into(), "remove".into()];
|
||||
if force {
|
||||
args.push("--force".into());
|
||||
|
|
@ -3451,6 +3487,17 @@ struct GitBinaryCommandError {
|
|||
status: ExitStatus,
|
||||
}
|
||||
|
||||
/// Returned by [`GitRepository::remove_worktree`] implementations when the
|
||||
/// target path is not a registered worktree (e.g. its registration was
|
||||
/// already pruned, or it was never `git worktree add`-ed). Callers that
|
||||
/// want to treat "nothing to remove" as success can match on this with
|
||||
/// `error.downcast_ref::<NotAWorktreeError>()`.
|
||||
#[derive(Error, Debug)]
|
||||
#[error("'{}' is not a registered git worktree", path.display())]
|
||||
pub struct NotAWorktreeError {
|
||||
pub path: PathBuf,
|
||||
}
|
||||
|
||||
async fn run_git_command(
|
||||
env: Arc<HashMap<String, String>>,
|
||||
ask_pass: AskPassDelegate,
|
||||
|
|
@ -4450,6 +4497,133 @@ mod tests {
|
|||
assert!(!worktree_path.exists());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_remove_worktree_returns_not_a_worktree_error_for_unregistered_path(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
disable_git_global_config();
|
||||
cx.executor().allow_parking();
|
||||
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let repo_dir = temp_dir.path().join("repo");
|
||||
git2::Repository::init(&repo_dir).unwrap();
|
||||
|
||||
let repo = RealGitRepository::new(
|
||||
&repo_dir.join(".git"),
|
||||
None,
|
||||
Some("git".into()),
|
||||
cx.executor(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
smol::fs::write(repo_dir.join("file.txt"), "content")
|
||||
.await
|
||||
.unwrap();
|
||||
repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default()))
|
||||
.await
|
||||
.unwrap();
|
||||
repo.commit(
|
||||
"Initial commit".into(),
|
||||
None,
|
||||
CommitOptions::default(),
|
||||
AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}),
|
||||
Arc::new(checkpoint_author_envs()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let bogus_path = temp_dir.path().join("never-registered");
|
||||
let error = repo
|
||||
.remove_worktree(bogus_path.clone(), true)
|
||||
.await
|
||||
.expect_err("removing a path with no worktree registration must fail");
|
||||
|
||||
assert!(
|
||||
error
|
||||
.downcast_ref::<NotAWorktreeError>()
|
||||
.is_some_and(|e| e.path == bogus_path),
|
||||
"error must downcast to NotAWorktreeError with the requested path, got: {error:#}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression: a worktree whose directory has been deleted from disk
|
||||
/// must still be removable when the caller passes a path that doesn't
|
||||
/// canonicalize the same way git's registry does. Naive
|
||||
/// `std::fs::canonicalize` on the caller-passed path returns `Err`
|
||||
/// (path missing), so the literal/path-equality check would miss the
|
||||
/// match on systems where `temp_dir()` and its canonical form differ
|
||||
/// (e.g. macOS, where `/var` is a symlink to `/private/var`). We
|
||||
/// instead canonicalize the longest existing ancestor and compare
|
||||
/// that, which must succeed here.
|
||||
#[gpui::test]
|
||||
async fn test_remove_worktree_succeeds_when_directory_was_deleted(cx: &mut TestAppContext) {
|
||||
disable_git_global_config();
|
||||
cx.executor().allow_parking();
|
||||
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let repo_dir = temp_dir.path().join("repo");
|
||||
let worktrees_dir = temp_dir.path().join("worktrees");
|
||||
git2::Repository::init(&repo_dir).unwrap();
|
||||
|
||||
let repo = RealGitRepository::new(
|
||||
&repo_dir.join(".git"),
|
||||
None,
|
||||
Some("git".into()),
|
||||
cx.executor(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
smol::fs::write(repo_dir.join("file.txt"), "content")
|
||||
.await
|
||||
.unwrap();
|
||||
repo.stage_paths(vec![repo_path("file.txt")], Arc::new(HashMap::default()))
|
||||
.await
|
||||
.unwrap();
|
||||
repo.commit(
|
||||
"Initial commit".into(),
|
||||
None,
|
||||
CommitOptions::default(),
|
||||
AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}),
|
||||
Arc::new(checkpoint_author_envs()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let worktree_path = worktrees_dir.join("to-be-deleted");
|
||||
repo.create_worktree(
|
||||
CreateWorktreeTarget::NewBranch {
|
||||
branch_name: "to-be-deleted".to_string(),
|
||||
base_sha: Some("HEAD".to_string()),
|
||||
},
|
||||
worktree_path.clone(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(worktree_path.exists(), "precondition: worktree dir exists");
|
||||
|
||||
// Simulate the original Windows file-locks scenario where the
|
||||
// working directory is gone but the main-repo registration still
|
||||
// points at it.
|
||||
std::fs::remove_dir_all(&worktree_path).unwrap();
|
||||
assert!(
|
||||
!worktree_path.exists(),
|
||||
"precondition: worktree dir is gone"
|
||||
);
|
||||
|
||||
// Must NOT return `NotAWorktreeError` — the registration is still
|
||||
// present, and git can still clean it up via
|
||||
// `git worktree remove --force`.
|
||||
repo.remove_worktree(worktree_path.clone(), true)
|
||||
.await
|
||||
.expect("remove_worktree on a missing-but-registered path must succeed");
|
||||
|
||||
let worktrees = repo.worktrees().await.unwrap();
|
||||
assert!(
|
||||
worktrees.iter().all(|w| w.path != worktree_path),
|
||||
"registration must be gone after remove_worktree"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_rename_worktree(cx: &mut TestAppContext) {
|
||||
disable_git_global_config();
|
||||
|
|
|
|||
|
|
@ -7140,6 +7140,21 @@ impl Repository {
|
|||
)
|
||||
}
|
||||
|
||||
/// Removes a worktree by path.
|
||||
///
|
||||
/// Note on error shape: the local backend translates git's "path is
|
||||
/// not a working tree" failure into a structured
|
||||
/// [`git::repository::NotAWorktreeError`] (see
|
||||
/// [`git::repository::GitRepository::remove_worktree`]). The remote arm
|
||||
/// here does **not** preserve that type — the RPC boundary collapses
|
||||
/// the error into a generic `anyhow::Error` whose `Display` carries
|
||||
/// the original message. Callers that need to react to the "nothing to
|
||||
/// remove" case structurally must either route through the local
|
||||
/// backend or pre-check with `Self::worktrees()`. As of writing the
|
||||
/// only such caller (archived-thread restore in `agent_ui`) takes the
|
||||
/// pre-check path and rejects remote restores at its entry point, so
|
||||
/// this asymmetry has no live consumer; document it here so the next
|
||||
/// caller doesn't get surprised.
|
||||
pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver<Result<()>> {
|
||||
let id = self.id;
|
||||
let repository_anchor_path: Arc<Path> = self
|
||||
|
|
|
|||
|
|
@ -24,9 +24,10 @@ use feature_flags::{
|
|||
AgentThreadWorktreeLabel, AgentThreadWorktreeLabelFlag, FeatureFlag, FeatureFlagAppExt as _,
|
||||
};
|
||||
use gpui::{
|
||||
Action as _, AnyElement, App, ClickEvent, Context, DismissEvent, Entity, EntityId, FocusHandle,
|
||||
Focusable, KeyContext, ListState, Modifiers, Pixels, Render, SharedString, Task, TaskExt,
|
||||
WeakEntity, Window, WindowHandle, linear_color_stop, linear_gradient, list, prelude::*, px,
|
||||
Action as _, AnyElement, App, AsyncApp, ClickEvent, Context, DismissEvent, Entity, EntityId,
|
||||
FocusHandle, Focusable, KeyContext, ListState, Modifiers, Pixels, Render, SharedString, Task,
|
||||
TaskExt, WeakEntity, Window, WindowHandle, linear_color_stop, linear_gradient, list,
|
||||
prelude::*, px,
|
||||
};
|
||||
use itertools::Itertools;
|
||||
use menu::{
|
||||
|
|
@ -432,6 +433,50 @@ fn workspace_contains_worktree_path(
|
|||
.any(|worktree| worktree.read(cx).abs_path().as_ref() == worktree_path)
|
||||
}
|
||||
|
||||
/// Commits the DB-visible state changes for a successful restore:
|
||||
/// rewrites worktree paths on the thread metadata, unarchives the
|
||||
/// thread, and returns the updated metadata.
|
||||
///
|
||||
/// Uses [`AsyncApp`] (not the window-bound context) so this commit
|
||||
/// succeeds even if the user closed the window between the destructive
|
||||
/// restore pass and now. That ordering matters for correctness: the
|
||||
/// on-disk worktrees are already restored at this point, and if the DB
|
||||
/// is left out-of-sync the user would re-open Zed to a thread marked
|
||||
/// archived whose worktrees are physically present, and clicking
|
||||
/// Restore would prompt them to overwrite their own freshly-restored
|
||||
/// files.
|
||||
///
|
||||
/// Returns `None` when the thread has been removed from the store mid-
|
||||
/// restore (e.g. another window deleted it); the caller distinguishes
|
||||
/// this case so the spinner gets cleared and the user gets a log line.
|
||||
async fn commit_db_state_after_restore(
|
||||
store: &Entity<ThreadMetadataStore>,
|
||||
thread_id: ThreadId,
|
||||
path_replacements: &[(PathBuf, PathBuf)],
|
||||
cx: &mut AsyncApp,
|
||||
) -> Option<ThreadMetadata> {
|
||||
if path_replacements.is_empty() {
|
||||
return None;
|
||||
}
|
||||
// `AsyncApp::update` panics if the App is fully dropped (process
|
||||
// shutdown); for our purposes that's fine, since at that point the
|
||||
// task is being torn down with the rest of the process.
|
||||
cx.update(|cx| {
|
||||
store.update(cx, |store, cx| {
|
||||
store.update_restored_worktree_paths(thread_id, path_replacements, cx);
|
||||
});
|
||||
});
|
||||
let updated_metadata = cx.update(|cx| store.read(cx).entry(thread_id).cloned());
|
||||
if updated_metadata.is_some() {
|
||||
cx.update(|cx| {
|
||||
store.update(cx, |store, cx| {
|
||||
store.unarchive(thread_id, cx);
|
||||
});
|
||||
});
|
||||
}
|
||||
updated_metadata
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct WorkspaceMenuWorktreeLabel {
|
||||
icon: Option<IconName>,
|
||||
|
|
@ -2938,6 +2983,62 @@ impl Sidebar {
|
|||
})
|
||||
}
|
||||
|
||||
fn finish_restore_ui(
|
||||
&mut self,
|
||||
thread_id: agent_ui::ThreadId,
|
||||
weak_archive_view: &Option<WeakEntity<ThreadsArchiveView>>,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
self.restoring_tasks.remove(&thread_id);
|
||||
if let Some(weak_archive_view) = weak_archive_view {
|
||||
weak_archive_view
|
||||
.update(cx, |view, cx| {
|
||||
view.clear_restoring(&thread_id, cx);
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
}
|
||||
|
||||
/// Shows a toast in the current window's active workspace. Used for the
|
||||
/// transient "restore" status messages so each callsite doesn't repeat
|
||||
/// the `multi_workspace.upgrade()` ladder. Silently no-ops if the
|
||||
/// multi-workspace has been torn down.
|
||||
///
|
||||
/// Takes `&mut self` for consistency with [`Self::finish_restore_ui`]
|
||||
/// and [`Self::fail_restore_with_toast`], even though the body only
|
||||
/// reads `self`; sibling helpers are commonly called as a pair and
|
||||
/// matching receivers keeps borrow patterns at the call sites uniform.
|
||||
fn show_restore_toast(
|
||||
&mut self,
|
||||
notification_id: NotificationId,
|
||||
message: impl Into<String>,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let Some(multi_workspace) = self.multi_workspace.upgrade() else {
|
||||
return;
|
||||
};
|
||||
let workspace = multi_workspace.read(cx).workspace().clone();
|
||||
let message = message.into();
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
workspace.show_toast(Toast::new(notification_id, message).autohide(), cx);
|
||||
});
|
||||
}
|
||||
|
||||
/// Combined UI cleanup helper for the failure paths of
|
||||
/// [`Self::open_thread_from_archive`]: clears the spinner / restoring
|
||||
/// state and shows a toast with the given message.
|
||||
fn fail_restore_with_toast(
|
||||
&mut self,
|
||||
thread_id: agent_ui::ThreadId,
|
||||
weak_archive_view: &Option<WeakEntity<ThreadsArchiveView>>,
|
||||
notification_id: NotificationId,
|
||||
message: impl Into<String>,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
self.finish_restore_ui(thread_id, weak_archive_view, cx);
|
||||
self.show_restore_toast(notification_id, message, cx);
|
||||
}
|
||||
|
||||
fn open_thread_from_archive(
|
||||
&mut self,
|
||||
metadata: ThreadMetadata,
|
||||
|
|
@ -2945,6 +3046,15 @@ impl Sidebar {
|
|||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let thread_id = metadata.thread_id;
|
||||
// Re-entry guard: if a restore is already in flight for this
|
||||
// thread (e.g. the user double-clicked Restore while the
|
||||
// previous task is still in flight), silently no-op so we don't
|
||||
// spawn a second task that would race the first and leave the
|
||||
// worktree in a half-restored state.
|
||||
if self.restoring_tasks.contains_key(&thread_id) {
|
||||
log::debug!("restore already in flight for {thread_id:?}; ignoring duplicate request");
|
||||
return;
|
||||
}
|
||||
let weak_archive_view = match &self.view {
|
||||
SidebarView::Archive(view) => Some(view.downgrade()),
|
||||
_ => None,
|
||||
|
|
@ -2990,10 +3100,29 @@ impl Sidebar {
|
|||
};
|
||||
let path_list = metadata.folder_paths().clone();
|
||||
|
||||
let restore_task = cx.spawn_in(window, async move |this, cx| {
|
||||
// Mark this thread as restoring synchronously, BEFORE spawning, so the
|
||||
// per-window re-entry guard at the top of this function rejects a
|
||||
// double-click before the foreground executor has a chance to run the
|
||||
// spawned body. The map's value type is `Task<()>` for historical
|
||||
// reasons, but it's only ever read for presence — we insert a
|
||||
// `Task::ready(())` placeholder here and detach the real task.
|
||||
//
|
||||
// The reason not to store the real task is that the spawned body's
|
||||
// own error handlers call `finish_restore_ui` -> `restoring_tasks
|
||||
// .remove(...)`. If the map held the real task, that remove would
|
||||
// drop the task we're currently inside — which GPUI tolerates but
|
||||
// is a fragile pattern. Detaching the real task makes the remove a
|
||||
// no-op on a `Task::ready(())`, with no "drop the task we're inside"
|
||||
// hazard.
|
||||
self.restoring_tasks.insert(thread_id, Task::ready(()));
|
||||
|
||||
cx.spawn_in(window, async move |this, cx| {
|
||||
let result: anyhow::Result<()> = async {
|
||||
let archived_worktrees = task.await?;
|
||||
|
||||
// Empty-archive activation has no destructive on-disk
|
||||
// work to serialize against — skip the cross-window claim
|
||||
// entirely and run the simple activation path.
|
||||
if archived_worktrees.is_empty() {
|
||||
this.update_in(cx, |this, window, cx| {
|
||||
this.restoring_tasks.remove(&thread_id);
|
||||
|
|
@ -3035,51 +3164,37 @@ impl Sidebar {
|
|||
return anyhow::Ok(());
|
||||
}
|
||||
|
||||
// Destructive pass: recreate each archived worktree.
|
||||
// We do NOT delete each archived worktree's DB record in
|
||||
// this loop. If we did, a later worktree's failure would
|
||||
// strand the thread: the successful rows would have no
|
||||
// archived metadata left to retry from, leaving on-disk
|
||||
// worktrees orphaned and the thread permanently broken.
|
||||
// Cleanup happens only after the DB-visible state is
|
||||
// committed below.
|
||||
let mut path_replacements: Vec<(PathBuf, PathBuf)> = Vec::new();
|
||||
for row in &archived_worktrees {
|
||||
match thread_worktree_archive::restore_worktree_via_git(
|
||||
row,
|
||||
metadata.remote_connection.as_ref(),
|
||||
&mut *cx,
|
||||
cx,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(restored_path) => {
|
||||
thread_worktree_archive::cleanup_archived_worktree_record(
|
||||
row,
|
||||
metadata.remote_connection.as_ref(),
|
||||
&mut *cx,
|
||||
)
|
||||
.await;
|
||||
path_replacements.push((row.worktree_path.clone(), restored_path));
|
||||
}
|
||||
Err(error) => {
|
||||
log::error!("Failed to restore worktree: {error:#}");
|
||||
this.update_in(cx, |this, _window, cx| {
|
||||
this.restoring_tasks.remove(&thread_id);
|
||||
if let Some(weak_archive_view) = &weak_archive_view {
|
||||
weak_archive_view
|
||||
.update(cx, |view, cx| {
|
||||
view.clear_restoring(&thread_id, cx);
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
|
||||
if let Some(multi_workspace) = this.multi_workspace.upgrade() {
|
||||
let workspace = multi_workspace.read(cx).workspace().clone();
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
struct RestoreWorktreeErrorToast;
|
||||
workspace.show_toast(
|
||||
Toast::new(
|
||||
NotificationId::unique::<RestoreWorktreeErrorToast>(
|
||||
),
|
||||
format!("Failed to restore worktree: {error:#}"),
|
||||
)
|
||||
.autohide(),
|
||||
cx,
|
||||
);
|
||||
});
|
||||
}
|
||||
struct RestoreWorktreeErrorToast;
|
||||
this.fail_restore_with_toast(
|
||||
thread_id,
|
||||
&weak_archive_view,
|
||||
NotificationId::unique::<RestoreWorktreeErrorToast>(),
|
||||
format!("Failed to restore worktree: {error:#}"),
|
||||
cx,
|
||||
);
|
||||
})
|
||||
.ok();
|
||||
return anyhow::Ok(());
|
||||
|
|
@ -3087,29 +3202,22 @@ impl Sidebar {
|
|||
}
|
||||
}
|
||||
|
||||
if !path_replacements.is_empty() {
|
||||
cx.update(|_window, cx| {
|
||||
store.update(cx, |store, cx| {
|
||||
store.update_restored_worktree_paths(thread_id, &path_replacements, cx);
|
||||
});
|
||||
})?;
|
||||
// Commit the DB-visible state through `AsyncApp` so it
|
||||
// survives the user closing the window between the
|
||||
// destructive pass and now (see `commit_db_state_after_
|
||||
// restore`'s docs). The activation `update_in` below is
|
||||
// window-bound and best-effort; correctness lives in the
|
||||
// commit above.
|
||||
let updated_metadata =
|
||||
commit_db_state_after_restore(&store, thread_id, &path_replacements, cx).await;
|
||||
|
||||
let updated_metadata =
|
||||
cx.update(|_window, cx| store.read(cx).entry(thread_id).cloned())?;
|
||||
|
||||
if let Some(updated_metadata) = updated_metadata {
|
||||
match updated_metadata {
|
||||
Some(updated_metadata) => {
|
||||
let new_paths = updated_metadata.folder_paths().clone();
|
||||
let key = ProjectGroupKey::from_worktree_paths(
|
||||
&updated_metadata.worktree_paths,
|
||||
updated_metadata.remote_connection.clone(),
|
||||
);
|
||||
|
||||
cx.update(|_window, cx| {
|
||||
store.update(cx, |store, cx| {
|
||||
store.unarchive(updated_metadata.thread_id, cx);
|
||||
});
|
||||
})?;
|
||||
|
||||
this.update_in(cx, |this, window, cx| {
|
||||
this.restoring_tasks.remove(&thread_id);
|
||||
this.open_workspace_and_activate_thread(
|
||||
|
|
@ -3120,8 +3228,45 @@ impl Sidebar {
|
|||
cx,
|
||||
);
|
||||
this.show_thread_list(window, cx);
|
||||
})?;
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
None if !path_replacements.is_empty() => {
|
||||
// The thread was removed from the store between
|
||||
// the restore loop and this lookup (e.g. another
|
||||
// window deleted it). The on-disk restore has
|
||||
// already succeeded, but there's nothing left
|
||||
// here to activate. Make sure the per-window
|
||||
// "restoring..." state is cleared so the
|
||||
// spinner doesn't get stuck.
|
||||
log::warn!(
|
||||
"Thread {thread_id:?} disappeared from the store mid-restore; \
|
||||
on-disk worktrees were restored but the thread can't be activated."
|
||||
);
|
||||
this.update_in(cx, |this, _window, cx| {
|
||||
this.finish_restore_ui(thread_id, &weak_archive_view, cx);
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
|
||||
// Finally, drop the archive records. This happens last so
|
||||
// a failure or cancellation between the visible state
|
||||
// changes above and this loop only leaves the user with
|
||||
// some stale DB rows (recoverable by a later cleanup
|
||||
// pass), not a thread stuck in an inconsistent
|
||||
// "unarchived but archive rows missing" state.
|
||||
// `cleanup_archived_worktree_record` swallows its own
|
||||
// errors, so a cleanup failure leaks a DB row but doesn't
|
||||
// break the thread.
|
||||
for row in &archived_worktrees {
|
||||
thread_worktree_archive::cleanup_archived_worktree_record(
|
||||
row,
|
||||
metadata.remote_connection.as_ref(),
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
anyhow::Ok(())
|
||||
|
|
@ -3129,9 +3274,20 @@ impl Sidebar {
|
|||
.await;
|
||||
if let Err(error) = result {
|
||||
log::error!("{error:#}");
|
||||
this.update_in(cx, |this, _window, cx| {
|
||||
struct RestoreFailedToast;
|
||||
this.fail_restore_with_toast(
|
||||
thread_id,
|
||||
&weak_archive_view,
|
||||
NotificationId::unique::<RestoreFailedToast>(),
|
||||
format!("Failed to restore thread: {error:#}"),
|
||||
cx,
|
||||
);
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
});
|
||||
self.restoring_tasks.insert(thread_id, restore_task);
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
|
||||
fn expand_selected_entry(
|
||||
|
|
|
|||
|
|
@ -3921,6 +3921,131 @@ async fn init_test_project_with_git(
|
|||
(project, fs)
|
||||
}
|
||||
|
||||
/// Output of [`setup_archived_worktree_fixture`].
|
||||
///
|
||||
/// Holds the FakeFs and entity handles needed by tests that exercise the
|
||||
/// archived-worktree restore flow: a `/project` main repo, a
|
||||
/// `/wt-<branch>` linked worktree pointing back at it, a multi-workspace
|
||||
/// containing both projects, and the staged/unstaged checkpoint hashes
|
||||
/// captured from the worktree.
|
||||
struct ArchivedWorktreeFixture {
|
||||
fs: Arc<FakeFs>,
|
||||
worktree_path: PathBuf,
|
||||
branch_name: String,
|
||||
staged_hash: String,
|
||||
unstaged_hash: String,
|
||||
}
|
||||
|
||||
impl ArchivedWorktreeFixture {
|
||||
/// Builds an `ArchivedGitWorktree` row pointing at this fixture's
|
||||
/// captured checkpoint.
|
||||
fn archived_row(&self) -> agent_ui::thread_metadata_store::ArchivedGitWorktree {
|
||||
agent_ui::thread_metadata_store::ArchivedGitWorktree {
|
||||
id: 1,
|
||||
worktree_path: self.worktree_path.clone(),
|
||||
main_repo_path: PathBuf::from("/project"),
|
||||
branch_name: Some(self.branch_name.clone()),
|
||||
staged_commit_hash: self.staged_hash.clone(),
|
||||
unstaged_commit_hash: self.unstaged_hash.clone(),
|
||||
original_commit_hash: "original-sha".to_string(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Sets up the common FakeFs + multi-workspace + checkpoint state every
|
||||
/// `restore_worktree_via_git` test needs.
|
||||
///
|
||||
/// `extra_worktree_files` is merged into the worktree's `insert_tree`
|
||||
/// payload alongside its `.git` gitfile, so tests can plant fixtures
|
||||
/// without having to repeat the worktree-registration boilerplate.
|
||||
/// Pass `serde_json::json!({})` if nothing extra is needed.
|
||||
async fn setup_archived_worktree_fixture(
|
||||
branch_name: &str,
|
||||
extra_worktree_files: serde_json::Value,
|
||||
cx: &mut TestAppContext,
|
||||
) -> ArchivedWorktreeFixture {
|
||||
init_test(cx);
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let worktree_path = PathBuf::from(format!("/wt-{branch_name}"));
|
||||
|
||||
fs.insert_tree(
|
||||
"/project",
|
||||
serde_json::json!({
|
||||
".git": {
|
||||
"worktrees": {
|
||||
branch_name: {
|
||||
"commondir": "../../",
|
||||
"HEAD": format!("ref: refs/heads/{branch_name}"),
|
||||
},
|
||||
},
|
||||
},
|
||||
"src": {},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
// Build the worktree directory tree: always includes the `.git`
|
||||
// gitfile; merge in caller-provided extras (e.g. a `src/` subtree).
|
||||
let mut worktree_tree = serde_json::json!({
|
||||
".git": format!("gitdir: /project/.git/worktrees/{branch_name}"),
|
||||
});
|
||||
if let (Some(base), Some(extras)) = (
|
||||
worktree_tree.as_object_mut(),
|
||||
extra_worktree_files.as_object(),
|
||||
) {
|
||||
for (key, value) in extras {
|
||||
base.insert(key.clone(), value.clone());
|
||||
}
|
||||
}
|
||||
fs.insert_tree(&worktree_path, worktree_tree).await;
|
||||
|
||||
fs.add_linked_worktree_for_repo(
|
||||
Path::new("/project/.git"),
|
||||
false,
|
||||
git::repository::Worktree {
|
||||
path: worktree_path.clone(),
|
||||
ref_name: Some(format!("refs/heads/{branch_name}").into()),
|
||||
sha: "original-sha".into(),
|
||||
is_main: false,
|
||||
is_bare: false,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
cx.update(|cx| <dyn fs::Fs>::set_global(fs.clone(), cx));
|
||||
|
||||
let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await;
|
||||
let worktree_project = project::Project::test(fs.clone(), [worktree_path.as_path()], cx).await;
|
||||
main_project
|
||||
.update(cx, |p, cx| p.git_scans_complete(cx))
|
||||
.await;
|
||||
worktree_project
|
||||
.update(cx, |p, cx| p.git_scans_complete(cx))
|
||||
.await;
|
||||
|
||||
let (multi_workspace, vcx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx));
|
||||
multi_workspace.update_in(vcx, |mw, window, cx| {
|
||||
mw.test_add_workspace(worktree_project.clone(), window, cx)
|
||||
});
|
||||
|
||||
let wt_repo = worktree_project.read_with(cx, |project, cx| {
|
||||
project.repositories(cx).values().next().unwrap().clone()
|
||||
});
|
||||
let (staged_hash, unstaged_hash) = cx
|
||||
.update(|cx| wt_repo.update(cx, |repo, _| repo.create_archive_checkpoint()))
|
||||
.await
|
||||
.expect("create_archive_checkpoint task should not be canceled")
|
||||
.expect("create_archive_checkpoint should succeed");
|
||||
|
||||
ArchivedWorktreeFixture {
|
||||
fs,
|
||||
worktree_path,
|
||||
branch_name: branch_name.to_string(),
|
||||
staged_hash,
|
||||
unstaged_hash,
|
||||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_search_matches_worktree_name(cx: &mut TestAppContext) {
|
||||
let (project, fs) = init_test_project_with_git("/project", cx).await;
|
||||
|
|
@ -6254,6 +6379,409 @@ async fn test_restore_worktree_when_branch_does_not_exist(cx: &mut TestAppContex
|
|||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_cleans_up_backup_on_success(cx: &mut TestAppContext) {
|
||||
// restore_worktree_via_git should move pre-existing content into a
|
||||
// sibling backup directory before recreating the worktree, then delete
|
||||
// that backup directory once the restore has completed successfully.
|
||||
let fixture =
|
||||
setup_archived_worktree_fixture("feature-success", serde_json::json!({ "src": {} }), cx)
|
||||
.await;
|
||||
let fs = fixture.fs.clone();
|
||||
|
||||
// Drop a sentinel file at the worktree path that is *not* part of the
|
||||
// archive checkpoint, simulating user content that the user agreed to
|
||||
// overwrite when they confirmed the restore prompt.
|
||||
fs.write(
|
||||
Path::new("/wt-feature-success/sentinel.txt"),
|
||||
b"pre-existing user content",
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let row = fixture.archived_row();
|
||||
let result = cx
|
||||
.spawn(|mut cx| async move {
|
||||
agent_ui::thread_worktree_archive::restore_worktree_via_git(&row, None, &mut cx).await
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(result.is_ok(), "restore should succeed: {:?}", result.err());
|
||||
|
||||
// The success-path backup cleanup is scheduled via
|
||||
// `cx.background_spawn(...).detach()`, so we have to drain the
|
||||
// executor before asserting that the backup directory is gone.
|
||||
// Without this, the assertion races the detached cleanup task.
|
||||
cx.run_until_parked();
|
||||
|
||||
// No backup directory should remain in the parent of the worktree path.
|
||||
let leftover_backup = fs.directories(true).into_iter().find(|path| {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.starts_with("zed-restore-backup-"))
|
||||
});
|
||||
assert!(
|
||||
leftover_backup.is_none(),
|
||||
"backup directory should be deleted after a successful restore, found: {leftover_backup:?}"
|
||||
);
|
||||
|
||||
// The restored worktree directory must exist (it was renamed away to a
|
||||
// backup, then recreated by `git worktree add`).
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-success"))
|
||||
.await
|
||||
.unwrap()
|
||||
.is_some(),
|
||||
"worktree path should exist after a successful restore"
|
||||
);
|
||||
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-success/sentinel.txt"))
|
||||
.await
|
||||
.unwrap()
|
||||
.is_none(),
|
||||
"sentinel file from pre-existing content must not survive a successful restore"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_rolls_back_backup_on_failure(cx: &mut TestAppContext) {
|
||||
// When restore_worktree_via_git fails partway through (here, because
|
||||
// the archive checkpoint SHAs are bogus), it must restore the user's
|
||||
// pre-existing content from the backup and not leave a backup
|
||||
// directory lying around.
|
||||
let fixture =
|
||||
setup_archived_worktree_fixture("feature-fail", serde_json::json!({ "src": {} }), cx).await;
|
||||
let fs = fixture.fs.clone();
|
||||
|
||||
// Drop a sentinel file representing pre-existing user content the
|
||||
// user expected to be overwritten by the archived state on success.
|
||||
fs.write(
|
||||
Path::new("/wt-feature-fail/sentinel.txt"),
|
||||
b"important user data",
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Bogus checkpoint SHAs will cause `restore_archive_checkpoint` to
|
||||
// fail, exercising the rollback path.
|
||||
let bogus_sha = "0".repeat(40);
|
||||
let row = agent_ui::thread_metadata_store::ArchivedGitWorktree {
|
||||
staged_commit_hash: bogus_sha.clone(),
|
||||
unstaged_commit_hash: bogus_sha,
|
||||
..fixture.archived_row()
|
||||
};
|
||||
|
||||
let result = cx
|
||||
.spawn(|mut cx| async move {
|
||||
agent_ui::thread_worktree_archive::restore_worktree_via_git(&row, None, &mut cx).await
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"restore should fail when checkpoint SHAs are bogus",
|
||||
);
|
||||
let error_msg = format!("{:#}", result.as_ref().unwrap_err());
|
||||
assert!(
|
||||
error_msg.contains("failed to restore archive checkpoint"),
|
||||
"error should indicate checkpoint failure, got: {error_msg}"
|
||||
);
|
||||
|
||||
// The pre-existing sentinel must be back at the original path.
|
||||
let sentinel_contents = fs
|
||||
.load(Path::new("/wt-feature-fail/sentinel.txt"))
|
||||
.await
|
||||
.expect("sentinel file must be restored from the backup");
|
||||
assert_eq!(
|
||||
sentinel_contents, "important user data",
|
||||
"sentinel content must match what was on disk before the restore",
|
||||
);
|
||||
|
||||
// No backup directory should remain in the parent of the worktree path.
|
||||
let leftover_backup = fs.directories(true).into_iter().find(|path| {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.starts_with("zed-restore-backup-"))
|
||||
});
|
||||
assert!(
|
||||
leftover_backup.is_none(),
|
||||
"backup directory should be cleaned up after a rollback, found: {leftover_backup:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_round_trips_git_admin_state(cx: &mut TestAppContext) {
|
||||
// End-to-end happy-path smoke test for `restore_worktree_via_git`:
|
||||
// plant a checkpoint, tear the worktree directory down to simulate
|
||||
// archival, then call the restore and confirm the captured tree is
|
||||
// reinstated.
|
||||
//
|
||||
// FakeFs limitation: for a linked worktree opened via a `.git` gitfile,
|
||||
// the fake's `create_archive_checkpoint` captures
|
||||
// `repository_dir_path.parent()` (see `crates/fs/src/fake_git_repo.rs`),
|
||||
// which resolves to `<main_repo>/.git/worktrees` — NOT the working
|
||||
// tree at the linked worktree's path. As a result, `restore_archive_
|
||||
// checkpoint` only round-trips the contents of
|
||||
// `<main_repo>/.git/worktrees`, not anything written to the working
|
||||
// directory. We therefore plant our marker inside the captured
|
||||
// location (the linked worktree's `worktrees/<name>` registration
|
||||
// directory) so we have something concrete to assert was actually
|
||||
// moved through the checkpoint pipeline. We still write some user-
|
||||
// facing files to the working tree before checkpointing (and assert
|
||||
// the worktree path itself is recreated) to show that those don't
|
||||
// crash the round-trip even though their contents aren't captured by
|
||||
// the fake.
|
||||
init_test(cx);
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
|
||||
fs.insert_tree(
|
||||
"/project",
|
||||
serde_json::json!({
|
||||
".git": {
|
||||
"worktrees": {
|
||||
"feature-rt": {
|
||||
"commondir": "../../",
|
||||
"HEAD": "ref: refs/heads/feature-rt",
|
||||
},
|
||||
},
|
||||
},
|
||||
"src": {},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
fs.insert_tree(
|
||||
"/wt-feature-rt",
|
||||
serde_json::json!({
|
||||
".git": "gitdir: /project/.git/worktrees/feature-rt",
|
||||
"src": {},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
fs.add_linked_worktree_for_repo(
|
||||
Path::new("/project/.git"),
|
||||
false,
|
||||
git::repository::Worktree {
|
||||
path: PathBuf::from("/wt-feature-rt"),
|
||||
ref_name: Some("refs/heads/feature-rt".into()),
|
||||
sha: "original-sha".into(),
|
||||
is_main: false,
|
||||
is_bare: false,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
cx.update(|cx| <dyn fs::Fs>::set_global(fs.clone(), cx));
|
||||
|
||||
let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await;
|
||||
let worktree_project =
|
||||
project::Project::test(fs.clone(), ["/wt-feature-rt".as_ref()], cx).await;
|
||||
main_project
|
||||
.update(cx, |p, cx| p.git_scans_complete(cx))
|
||||
.await;
|
||||
worktree_project
|
||||
.update(cx, |p, cx| p.git_scans_complete(cx))
|
||||
.await;
|
||||
|
||||
let (multi_workspace, _cx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx));
|
||||
multi_workspace.update_in(_cx, |mw, window, cx| {
|
||||
mw.test_add_workspace(worktree_project.clone(), window, cx)
|
||||
});
|
||||
|
||||
// Working-tree files. The fake doesn't round-trip these, but writing
|
||||
// them and then asserting the restore still completes proves the
|
||||
// function tolerates pre-checkpoint content in the working tree.
|
||||
fs.write(Path::new("/wt-feature-rt/staged.txt"), b"staged contents")
|
||||
.await
|
||||
.expect("writing staged.txt should succeed");
|
||||
fs.write(
|
||||
Path::new("/wt-feature-rt/src/nested.txt"),
|
||||
b"nested contents",
|
||||
)
|
||||
.await
|
||||
.expect("writing src/nested.txt should succeed");
|
||||
|
||||
// Marker inside the captured location. This is the file we'll
|
||||
// actually assert round-trips, since it sits inside what the fake's
|
||||
// `create_archive_checkpoint` snapshots.
|
||||
fs.write(
|
||||
Path::new("/project/.git/worktrees/feature-rt/marker.txt"),
|
||||
b"checkpoint marker",
|
||||
)
|
||||
.await
|
||||
.expect("writing checkpoint marker should succeed");
|
||||
|
||||
let wt_repo = worktree_project.read_with(cx, |project, cx| {
|
||||
project.repositories(cx).values().next().unwrap().clone()
|
||||
});
|
||||
let (staged_hash, unstaged_hash) = cx
|
||||
.update(|cx| wt_repo.update(cx, |repo, _| repo.create_archive_checkpoint()))
|
||||
.await
|
||||
.expect("create_archive_checkpoint task should not be canceled")
|
||||
.expect("create_archive_checkpoint should succeed");
|
||||
|
||||
// Simulate the archive having torn down the worktree directory.
|
||||
fs.remove_dir(
|
||||
Path::new("/wt-feature-rt"),
|
||||
fs::RemoveOptions {
|
||||
recursive: true,
|
||||
ignore_if_not_exists: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("removing the worktree dir should succeed");
|
||||
|
||||
// Also clobber the captured marker so a successful restore is the
|
||||
// only thing that could put it back.
|
||||
fs.remove_file(
|
||||
Path::new("/project/.git/worktrees/feature-rt/marker.txt"),
|
||||
fs::RemoveOptions {
|
||||
recursive: false,
|
||||
ignore_if_not_exists: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("removing checkpoint marker should succeed");
|
||||
|
||||
let result = cx
|
||||
.spawn(|mut cx| async move {
|
||||
agent_ui::thread_worktree_archive::restore_worktree_via_git(
|
||||
&agent_ui::thread_metadata_store::ArchivedGitWorktree {
|
||||
id: 1,
|
||||
worktree_path: PathBuf::from("/wt-feature-rt"),
|
||||
main_repo_path: PathBuf::from("/project"),
|
||||
branch_name: Some("feature-rt".to_string()),
|
||||
staged_commit_hash: staged_hash,
|
||||
unstaged_commit_hash: unstaged_hash,
|
||||
original_commit_hash: "original-sha".to_string(),
|
||||
},
|
||||
None,
|
||||
&mut cx,
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"restore should succeed for a clean round-trip: {:?}",
|
||||
result.err()
|
||||
);
|
||||
|
||||
// The marker we planted in the captured location must come back
|
||||
// with its original contents — this is the actual round-trip
|
||||
// assertion `restore_archive_checkpoint` is exercising in the fake.
|
||||
let marker = fs
|
||||
.load(Path::new("/project/.git/worktrees/feature-rt/marker.txt"))
|
||||
.await
|
||||
.expect("checkpoint marker must be restored");
|
||||
assert_eq!(marker, "checkpoint marker");
|
||||
|
||||
// The worktree directory itself must exist after restore (created
|
||||
// by `create_worktree_detached`), and its `.git` gitfile must be
|
||||
// present so the path is a usable linked worktree again.
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-rt"))
|
||||
.await
|
||||
.expect("metadata for worktree path must succeed")
|
||||
.is_some(),
|
||||
"worktree directory should exist after restore"
|
||||
);
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-rt/.git"))
|
||||
.await
|
||||
.expect("metadata for worktree .git must succeed")
|
||||
.is_some(),
|
||||
"worktree .git gitfile should exist after restore"
|
||||
);
|
||||
|
||||
// Success-path backup cleanup runs in a detached background task; pump
|
||||
// the executor so the assertion below doesn't race it.
|
||||
cx.run_until_parked();
|
||||
|
||||
// No backup directory should remain on success.
|
||||
let leftover_backup = fs.directories(true).into_iter().find(|path| {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.starts_with("zed-restore-backup-"))
|
||||
});
|
||||
assert!(
|
||||
leftover_backup.is_none(),
|
||||
"backup directory should be cleaned up after a successful round-trip, found: {leftover_backup:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_rolls_back_when_create_worktree_detached_fails(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
// Exercises the rollback path that runs when `create_worktree_detached`
|
||||
// itself fails (the early failure branch in `restore_worktree_via_git`,
|
||||
// before any branch / checkpoint operations have run). We force the
|
||||
// failure with `FakeFs::set_create_worktree_error`, which makes the
|
||||
// fake's `create_worktree` bail before producing any side effects.
|
||||
let fixture = setup_archived_worktree_fixture(
|
||||
"feature-create-fail",
|
||||
serde_json::json!({ "src": {} }),
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
let fs = fixture.fs.clone();
|
||||
|
||||
// Sentinel content that the rollback must put back.
|
||||
fs.write(
|
||||
Path::new("/wt-feature-create-fail/sentinel.txt"),
|
||||
b"important user data",
|
||||
)
|
||||
.await
|
||||
.expect("writing sentinel.txt should succeed");
|
||||
|
||||
// Force `create_worktree_detached` to fail when the restore tries it.
|
||||
fs.set_create_worktree_error(
|
||||
Path::new("/project/.git"),
|
||||
Some("simulated create_worktree failure".to_string()),
|
||||
);
|
||||
|
||||
let row = fixture.archived_row();
|
||||
let result = cx
|
||||
.spawn(|mut cx| async move {
|
||||
agent_ui::thread_worktree_archive::restore_worktree_via_git(&row, None, &mut cx).await
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"restore should fail when create_worktree_detached fails",
|
||||
);
|
||||
let error_msg = format!("{:#}", result.as_ref().unwrap_err());
|
||||
assert!(
|
||||
error_msg.contains("failed to create worktree")
|
||||
|| error_msg.contains("simulated create_worktree failure"),
|
||||
"error should indicate worktree creation failure, got: {error_msg}"
|
||||
);
|
||||
|
||||
// The pre-existing sentinel must be back at the original path.
|
||||
let sentinel_contents = fs
|
||||
.load(Path::new("/wt-feature-create-fail/sentinel.txt"))
|
||||
.await
|
||||
.expect("sentinel file must be restored from the backup");
|
||||
assert_eq!(
|
||||
sentinel_contents, "important user data",
|
||||
"sentinel content must match what was on disk before the restore",
|
||||
);
|
||||
|
||||
// No backup directory should remain anywhere on the fake fs.
|
||||
let leftover_backup = fs.directories(true).into_iter().find(|path| {
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.is_some_and(|name| name.starts_with("zed-restore-backup-"))
|
||||
});
|
||||
assert!(
|
||||
leftover_backup.is_none(),
|
||||
"backup directory should be cleaned up after a create_worktree rollback, found: {leftover_backup:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_thread_uses_main_repo_project_group_key(cx: &mut TestAppContext) {
|
||||
// Activating an archived linked worktree thread whose directory has
|
||||
|
|
@ -12030,3 +12558,51 @@ async fn test_cmd_click_project_header_returns_to_last_active_linked_worktree_wo
|
|||
linked-worktree workspace was the last-active one for the group"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_restore_worktree_succeeds_when_path_is_missing(cx: &mut TestAppContext) {
|
||||
// When the worktree path doesn't exist on disk, the destructive
|
||||
// restore should proceed and recreate the worktree (there's nothing
|
||||
// to back up).
|
||||
let fixture = setup_archived_worktree_fixture("feature-empty", serde_json::json!({}), cx).await;
|
||||
let fs = fixture.fs.clone();
|
||||
|
||||
fs.remove_dir(
|
||||
Path::new("/wt-feature-empty"),
|
||||
fs::RemoveOptions {
|
||||
recursive: true,
|
||||
ignore_if_not_exists: false,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-empty"))
|
||||
.await
|
||||
.unwrap()
|
||||
.is_none(),
|
||||
"precondition: worktree directory must not exist"
|
||||
);
|
||||
|
||||
let restore_row = fixture.archived_row();
|
||||
let result = cx
|
||||
.spawn(|mut cx| async move {
|
||||
agent_ui::thread_worktree_archive::restore_worktree_via_git(&restore_row, None, &mut cx)
|
||||
.await
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"restore should succeed when the worktree path does not exist: {:?}",
|
||||
result.err()
|
||||
);
|
||||
|
||||
assert!(
|
||||
fs.metadata(Path::new("/wt-feature-empty"))
|
||||
.await
|
||||
.unwrap()
|
||||
.is_some(),
|
||||
"worktree path should exist after a successful restore"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -661,6 +661,68 @@ pub fn normalize_lexically(path: &Path) -> Result<PathBuf, NormalizeError> {
|
|||
Ok(lexical)
|
||||
}
|
||||
|
||||
/// Returns whether `a` and `b` refer to the same on-disk location.
|
||||
///
|
||||
/// Tries literal equality first. Falls back to canonicalizing the longest
|
||||
/// existing ancestor of each path and joining the unresolved tail, then
|
||||
/// comparing. This handles two cases that a naive `std::fs::canonicalize`
|
||||
/// of each side does not:
|
||||
///
|
||||
/// - **The leaf path doesn't exist.** `canonicalize` requires every
|
||||
/// component to exist; if the worktree directory was deleted but its
|
||||
/// registration is still around, naive canonicalize returns `Err` and
|
||||
/// the caller falls back to literal comparison, which then misses any
|
||||
/// symlink-mediated equivalence.
|
||||
/// - **The path traverses a symlinked ancestor.** Git canonicalizes
|
||||
/// paths when storing them in its registries (notably resolving
|
||||
/// macOS's `/var` -> `/private/var` symlink, or Windows junction
|
||||
/// points), but callers passing paths into Zed APIs typically use the
|
||||
/// un-resolved form. Walking up to the nearest existing ancestor and
|
||||
/// canonicalizing *that* lets the comparison succeed even when the
|
||||
/// leaf is gone.
|
||||
///
|
||||
/// Does sync I/O via `std::fs::canonicalize`; call it from a spawned task
|
||||
/// or background context, not on a foreground async path.
|
||||
pub fn paths_resolve_to_same_location(a: &Path, b: &Path) -> bool {
|
||||
if a == b {
|
||||
return true;
|
||||
}
|
||||
let Some(canon_a) = canonicalize_with_existing_ancestor(a) else {
|
||||
return false;
|
||||
};
|
||||
let Some(canon_b) = canonicalize_with_existing_ancestor(b) else {
|
||||
return false;
|
||||
};
|
||||
canon_a == canon_b
|
||||
}
|
||||
|
||||
/// Canonicalizes the longest existing prefix of `path` and re-appends the
|
||||
/// remaining (unresolved) tail. Returns `None` only if no ancestor of
|
||||
/// `path` can be canonicalized, which on any working filesystem means
|
||||
/// even the root is unreadable.
|
||||
fn canonicalize_with_existing_ancestor(path: &Path) -> Option<PathBuf> {
|
||||
let mut tail = PathBuf::new();
|
||||
let mut current = path.to_path_buf();
|
||||
loop {
|
||||
if let Ok(canon) = std::fs::canonicalize(¤t) {
|
||||
return Some(if tail.as_os_str().is_empty() {
|
||||
canon
|
||||
} else {
|
||||
canon.join(&tail)
|
||||
});
|
||||
}
|
||||
let Some(file_name) = current.file_name().map(|n| n.to_os_string()) else {
|
||||
return None;
|
||||
};
|
||||
let mut new_tail = PathBuf::from(&file_name);
|
||||
new_tail.push(&tail);
|
||||
tail = new_tail;
|
||||
if !current.pop() {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A delimiter to use in `path_query:row_number:column_number` strings parsing.
|
||||
pub const FILE_ROW_COLUMN_DELIMITER: char = ':';
|
||||
|
||||
|
|
@ -3559,4 +3621,64 @@ mod tests {
|
|||
Ok(PathBuf::from("C:\\Users\\file.txt"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_paths_resolve_to_same_location_literal_equality() {
|
||||
let p = Path::new("/some/nonexistent/path/that/will/never/exist/xyz");
|
||||
assert!(paths_resolve_to_same_location(p, p));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_paths_resolve_to_same_location_nonexistent_distinct() {
|
||||
let a = Path::new("/totally/nonexistent/path/a");
|
||||
let b = Path::new("/totally/nonexistent/path/b");
|
||||
assert!(!paths_resolve_to_same_location(a, b));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_paths_resolve_to_same_location_handles_symlinked_ancestor() {
|
||||
// Set up: tmp/real_dir/ exists, tmp/link_dir is a symlink to it.
|
||||
// Compare a leaf under each form. The leaf itself does NOT exist,
|
||||
// so naive `canonicalize` would fail on both and the function
|
||||
// would have to fall back to ancestor-canonicalize. This mirrors
|
||||
// the worktree-removal scenario where git registers the resolved
|
||||
// path and we pass the un-resolved form for a directory that's
|
||||
// already gone.
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let real_dir = tmp.path().join("real_dir");
|
||||
std::fs::create_dir(&real_dir).unwrap();
|
||||
let link_dir = tmp.path().join("link_dir");
|
||||
#[cfg(unix)]
|
||||
std::os::unix::fs::symlink(&real_dir, &link_dir).unwrap();
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// Best-effort; skip the assertion if the test process lacks
|
||||
// SeCreateSymbolicLinkPrivilege.
|
||||
if std::os::windows::fs::symlink_dir(&real_dir, &link_dir).is_err() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let via_real = real_dir.join("missing_leaf");
|
||||
let via_link = link_dir.join("missing_leaf");
|
||||
assert!(
|
||||
paths_resolve_to_same_location(&via_real, &via_link),
|
||||
"paths through a symlinked ancestor must compare equal even when the leaf is missing"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_paths_resolve_to_same_location_distinct_existing_ancestors() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let a_parent = tmp.path().join("a");
|
||||
let b_parent = tmp.path().join("b");
|
||||
std::fs::create_dir(&a_parent).unwrap();
|
||||
std::fs::create_dir(&b_parent).unwrap();
|
||||
let a = a_parent.join("missing");
|
||||
let b = b_parent.join("missing");
|
||||
assert!(
|
||||
!paths_resolve_to_same_location(&a, &b),
|
||||
"distinct parents with the same missing leaf must not compare equal"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue