diff --git a/crates/core/src/project.rs b/crates/core/src/project.rs index 731e815d..797ead89 100644 --- a/crates/core/src/project.rs +++ b/crates/core/src/project.rs @@ -140,6 +140,21 @@ pub struct SessionMeta { } impl SessionMeta { + /// Whether `other` works in the worktree this session owns. A fork keeps + /// the source's cwd without the `worktree` ownership marker, so the cwd + /// decides as well as the branch. + pub fn shares_worktree_with(&self, other: &SessionMeta) -> bool { + let Some(worktree) = &self.worktree else { + return false; + }; + other.id != self.id + && (other.cwd.starts_with(&self.cwd) + || other + .worktree + .as_ref() + .is_some_and(|other| other.branch == worktree.branch)) + } + /// The key [`Settings::provider_color`] resolves this thread's color from: /// a user profile id, an ACP agent (`acp:`), or the built-in /// [`provider_key`]. A user profile is its own provider to the user even @@ -540,6 +555,30 @@ mod tests { assert_eq!(acp.provider_color_key(), "acp:gemini"); } + #[test] + fn worktree_is_shared_by_a_fork_in_its_cwd_but_not_by_a_sibling_directory() { + let mut owner = SessionMeta::new(ProviderKind::Codex, PathBuf::from("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/wt/source"), None); + owner.worktree = Some(WorktreeInfo { + root_project_path: PathBuf::from("/repo"), + base: "main".into(), + branch: "tcode/source".into(), + }); + let fork = SessionMeta::new(ProviderKind::Codex, owner.cwd.clone(), None); + let nested = SessionMeta::new(ProviderKind::Codex, owner.cwd.join("crates/core"), None); + let sibling = SessionMeta::new(ProviderKind::Codex, PathBuf::from("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/wt/source-2"), None); + let checkout = SessionMeta::new(ProviderKind::Codex, PathBuf::from("/repo"), None); + + assert!(owner.shares_worktree_with(&fork)); + assert!(owner.shares_worktree_with(&nested)); + assert!(!owner.shares_worktree_with(&sibling)); + assert!(!owner.shares_worktree_with(&checkout)); + assert!(!owner.shares_worktree_with(&owner)); + assert!( + !fork.shares_worktree_with(&owner), + "a fork owns no worktree" + ); + } + fn candidates( sessions: &[SessionMeta], now: u64, diff --git a/crates/runtime/src/app/sessions.rs b/crates/runtime/src/app/sessions.rs index daad0547..92d09119 100644 --- a/crates/runtime/src/app/sessions.rs +++ b/crates/runtime/src/app/sessions.rs @@ -974,10 +974,20 @@ impl AppState { } self.close_orchestrator_children(session_id, cx); let worktree_remove = meta.as_ref().and_then(|meta| { - (remove_worktree && meta.worktree.is_some()).then(|| { - let worktree = meta.worktree.as_ref().unwrap(); - (worktree.root_project_path.clone(), meta.cwd.clone()) - }) + let worktree = meta.worktree.as_ref().filter(|_| remove_worktree)?; + let shared = self + .sessions + .iter() + .chain(self.residents.live.values().map(|session| &session.meta)) + .any(|other| meta.shares_worktree_with(other)); + if shared { + log::info!( + "keeping worktree {} in use by another thread", + meta.cwd.display() + ); + return None; + } + Some((worktree.root_project_path.clone(), meta.cwd.clone())) }); self.settings.last_visited.remove(session_id); self.settings @@ -1110,16 +1120,24 @@ impl AppState { .is_some_and(|&visited| meta.updated_at > visited) } - /// Remove app-owned worktrees whose session is absent from the loaded store. + /// Remove app-owned worktrees that no session in the loaded store owns or + /// works in. pub(crate) fn recover_orphaned_worktrees(&self, cx: &mut HostCx) { let known_ids = self .sessions .iter() .map(|session| session.id.clone()) .collect(); + let cwds: Vec<_> = self + .sessions + .iter() + .map(|session| session.cwd.clone()) + .collect(); let host_cx = cx.clone(); HostCx::spawn_detached(cx, async move { - let summary = host_cx.unblock(move || cleanup_orphans(&known_ids)).await; + let summary = host_cx + .unblock(move || cleanup_orphans(&known_ids, &cwds)) + .await; if !summary.removed.is_empty() || !summary.skipped.is_empty() { log::info!( "worktree orphan recovery removed {}, left {}", diff --git a/crates/services/src/worktree.rs b/crates/services/src/worktree.rs index e7d62e7e..2722c68e 100644 --- a/crates/services/src/worktree.rs +++ b/crates/services/src/worktree.rs @@ -398,15 +398,20 @@ fn worktrees_root() -> PathBuf { }) } -/// Remove old app-owned worktrees whose directory names are not known session ids. +/// Remove old, clean app-owned worktrees that no known session owns or works in. /// /// Tests and isolated processes may set `TCODE_WORKTREES_DIR`; production falls /// back to `~/.tcode/worktrees`. Fresh unknown entries are presumed live and -/// preserved for at least [`ORPHAN_MIN_AGE`]. -pub fn cleanup_orphans(known_session_ids: &HashSet) -> CleanupSummary { +/// preserved for at least [`ORPHAN_MIN_AGE`]. Removal is never forced: a +/// worktree the user kept on delete may hold uncommitted work. +pub fn cleanup_orphans( + known_session_ids: &HashSet, + session_cwds: &[PathBuf], +) -> CleanupSummary { cleanup_orphans_at( &worktrees_root(), known_session_ids, + session_cwds, SystemTime::now(), ORPHAN_MIN_AGE, ) @@ -415,6 +420,7 @@ pub fn cleanup_orphans(known_session_ids: &HashSet) -> CleanupSummary { fn cleanup_orphans_at( worktrees: &Path, known_session_ids: &HashSet, + session_cwds: &[PathBuf], now: SystemTime, minimum_age: Duration, ) -> CleanupSummary { @@ -436,7 +442,10 @@ fn cleanup_orphans_at( let is_directory = entry .file_type() .is_ok_and(|kind| kind.is_dir() && !kind.is_symlink()); - if known_session_ids.contains(&session_id) || !is_directory { + if known_session_ids.contains(&session_id) + || session_cwds.iter().any(|cwd| cwd.starts_with(&path)) + || !is_directory + { continue; } let modified = match entry.metadata().and_then(|metadata| metadata.modified()) { @@ -481,12 +490,7 @@ fn cleanup_orphans_at( }; let output = crate::process::command("git") .current_dir(&removal_root) - .args([ - "worktree", - "remove", - "--force", - ®istered.to_string_lossy(), - ]) + .args(["worktree", "remove", ®istered.to_string_lossy()]) .output(); match output { Ok(output) if output.status.success() => { @@ -994,34 +998,48 @@ mod tests { let orphan = provision_for_test(&root, "orphan", &worktrees).path; let modified = std::fs::metadata(&orphan).unwrap().modified().unwrap(); let kept = provision_for_test(&root, "kept", &worktrees).path; + // A deleted source whose fork still runs in its worktree. + let forked = provision_for_test(&root, "deleted-source", &worktrees).path; + // Deleted with "Keep worktree" while holding uncommitted work. + let dirty = provision_for_test(&root, "kept-on-delete", &worktrees).path; + std::fs::write(dirty.join("notes.txt"), "uncommitted\n").unwrap(); let known = HashSet::from(["kept".to_string()]); + let cwds = [forked.clone()]; - let fresh = cleanup_orphans_at(&worktrees, &known, modified, ORPHAN_MIN_AGE); + let fresh = cleanup_orphans_at(&worktrees, &known, &cwds, modified, ORPHAN_MIN_AGE); assert!(fresh.removed.is_empty()); - assert_eq!(fresh.skipped.as_slice(), std::slice::from_ref(&orphan)); + let mut skipped = fresh.skipped; + skipped.sort(); + assert_eq!(skipped, [dirty.clone(), orphan.clone()]); assert!(orphan.exists()); let old = cleanup_orphans_at( &worktrees, &known, + &cwds, modified + ORPHAN_MIN_AGE + Duration::from_secs(1), ORPHAN_MIN_AGE, ); assert_eq!(old.removed.as_slice(), std::slice::from_ref(&orphan)); - assert!(old.skipped.is_empty()); + assert_eq!(old.skipped.as_slice(), std::slice::from_ref(&dirty)); assert!(!orphan.exists()); assert!(kept.join("tracked.txt").exists()); + assert!(forked.join("tracked.txt").exists()); + assert!(dirty.join("notes.txt").exists()); let unrelated = worktrees.join("unregistered"); std::fs::create_dir(&unrelated).unwrap(); let modified = std::fs::metadata(&unrelated).unwrap().modified().unwrap(); let summary = cleanup_orphans_at( &worktrees, &known, + &cwds, modified + ORPHAN_MIN_AGE + Duration::from_secs(1), ORPHAN_MIN_AGE, ); assert!(summary.removed.is_empty()); - assert_eq!(summary.skipped.as_slice(), std::slice::from_ref(&unrelated)); + let mut skipped = summary.skipped; + skipped.sort(); + assert_eq!(skipped, [dirty, unrelated.clone()]); assert!(unrelated.exists()); let _ = std::fs::remove_dir_all(temp); } diff --git a/crates/ui/src/store/mod.rs b/crates/ui/src/store/mod.rs index 31bb9e63..1833de3a 100644 --- a/crates/ui/src/store/mod.rs +++ b/crates/ui/src/store/mod.rs @@ -2824,16 +2824,15 @@ impl WorkspaceStore { .iter() .find(|meta| meta.id == session_id)?; let worktree = meta.worktree.clone()?; - let shared = self.index_replica.0.iter().any(|meta| { - meta.id != session_id - && meta - .worktree - .as_ref() - .is_some_and(|other| other.branch == worktree.branch) - }) || self - .index_summary - .archived_worktree_branches - .contains(&worktree.branch); + let shared = self + .index_replica + .0 + .iter() + .any(|other| meta.shares_worktree_with(other)) + || self + .index_summary + .archived_worktree_branches + .contains(&worktree.branch); (!shared).then_some(worktree) } }