diff --git a/server/src/agent_mode/mod.rs b/server/src/agent_mode/mod.rs index f53e01f8..e0e005da 100644 --- a/server/src/agent_mode/mod.rs +++ b/server/src/agent_mode/mod.rs @@ -99,10 +99,11 @@ pub async fn run( && let Some(root) = crdt_prune_root.as_ref().cloned() { let story_id = evt.story_id.clone(); - tokio::task::spawn_blocking(move || { - if let Err(e) = crate::worktree::prune_worktree_sync(&root, &story_id) { - slog!("[agent-mode] worktree prune failed for {story_id}: {e}"); - } + tokio::spawn(async move { + let config = ProjectConfig::load(&root).unwrap_or_default(); + crate::worktree::remove_worktree_by_story_id(&root, &story_id, &config) + .await + .ok(); }); } let (action, commit_msg) = diff --git a/server/src/chat/transport/matrix/rmtree.rs b/server/src/chat/transport/matrix/rmtree.rs index d5143eba..5f190d3d 100644 --- a/server/src/chat/transport/matrix/rmtree.rs +++ b/server/src/chat/transport/matrix/rmtree.rs @@ -103,7 +103,10 @@ pub async fn handle_rmtree( } // Remove the worktree. - if let Err(e) = crate::worktree::prune_worktree_sync(project_root, &story_id) { + let config = crate::config::ProjectConfig::load(project_root).unwrap_or_default(); + if let Err(e) = + crate::worktree::remove_worktree_by_story_id(project_root, &story_id, &config).await + { return format!("Failed to remove worktree for story {story_number}: {e}"); } diff --git a/server/src/startup/tick_loop.rs b/server/src/startup/tick_loop.rs index 2f7e9f72..34099065 100644 --- a/server/src/startup/tick_loop.rs +++ b/server/src/startup/tick_loop.rs @@ -34,10 +34,12 @@ pub(crate) fn spawn_event_bridges( && let Some(root) = crdt_prune_root.as_ref().cloned() { let story_id = evt.story_id.clone(); - tokio::task::spawn_blocking(move || { - if let Err(e) = crate::worktree::prune_worktree_sync(&root, &story_id) { - crate::slog!("[crdt] worktree prune failed for {story_id}: {e}"); - } + tokio::spawn(async move { + let config = + crate::config::ProjectConfig::load(&root).unwrap_or_default(); + crate::worktree::remove_worktree_by_story_id(&root, &story_id, &config) + .await + .ok(); }); } let (action, commit_msg) = diff --git a/server/src/worktree/git.rs b/server/src/worktree/git.rs index 3d794353..3844645d 100644 --- a/server/src/worktree/git.rs +++ b/server/src/worktree/git.rs @@ -124,21 +124,6 @@ pub(crate) fn remove_worktree_sync( Ok(()) } -/// Remove the git worktree for a story if it exists, deriving the path and -/// branch name deterministically from `project_root` and `story_id`. -/// -/// Returns `Ok(())` if the worktree was removed or did not exist. -/// Removal is best-effort: `remove_worktree_sync` logs failures internally -/// but always returns `Ok`. -pub fn prune_worktree_sync(project_root: &Path, story_id: &str) -> Result<(), String> { - let wt_path = super::worktree_path(project_root, story_id); - if !wt_path.exists() { - return Ok(()); - } - let branch = branch_name(story_id); - remove_worktree_sync(project_root, &wt_path, &branch) -} - /// Migrate filesystem artifacts for story IDs that were rewritten from slug form /// (`664_story_my_feature`) to numeric-only form (`664`). /// @@ -393,42 +378,4 @@ mod tests { remove_worktree_sync(&project_root, &wt_path, "feature/test-rm").unwrap(); assert!(!wt_path.exists()); } - - #[test] - fn prune_worktree_sync_noop_when_no_worktree_dir() { - let tmp = TempDir::new().unwrap(); - // No worktree directory exists — must return Ok without touching git. - let result = prune_worktree_sync(tmp.path(), "42_story_nonexistent"); - assert!( - result.is_ok(), - "Expected Ok when worktree dir absent: {:?}", - result.err() - ); - } - - #[test] - fn prune_worktree_sync_removes_real_worktree() { - let tmp = TempDir::new().unwrap(); - let project_root = tmp.path().join("my-project"); - fs::create_dir_all(&project_root).unwrap(); - init_git_repo(&project_root); - - let story_id = "55_story_prune_test"; - let wt_path = super::super::worktree_path(&project_root, story_id); - create_worktree_sync( - &project_root, - &wt_path, - &format!("feature/story-{story_id}"), - ) - .unwrap(); - assert!(wt_path.exists(), "worktree dir should exist before prune"); - - let result = prune_worktree_sync(&project_root, story_id); - assert!( - result.is_ok(), - "prune_worktree_sync must return Ok: {:?}", - result.err() - ); - assert!(!wt_path.exists(), "worktree dir should be gone after prune"); - } } diff --git a/server/src/worktree/mod.rs b/server/src/worktree/mod.rs index 8805203b..9648a5d8 100644 --- a/server/src/worktree/mod.rs +++ b/server/src/worktree/mod.rs @@ -9,7 +9,7 @@ mod sweep; pub use cleanup::{format_report, run_cleanup}; pub use create::create_worktree; -pub use git::{migrate_slug_paths, prune_worktree_sync}; +pub use git::migrate_slug_paths; pub use remove::remove_worktree_by_story_id; pub use sweep::sweep_orphaned_worktrees; diff --git a/server/src/worktree/remove.rs b/server/src/worktree/remove.rs index f6cd65bc..e84fa8a2 100644 --- a/server/src/worktree/remove.rs +++ b/server/src/worktree/remove.rs @@ -133,6 +133,55 @@ mod tests { ); } + #[tokio::test] + async fn remove_worktree_by_story_id_cleans_git_metadata_and_branch() { + let tmp = TempDir::new().unwrap(); + let project_root = tmp.path().join("my-project"); + fs::create_dir_all(&project_root).unwrap(); + init_git_repo(&project_root); + + let story_id = "89_regression_remove"; + let expected_branch = format!("feature/story-{story_id}"); + + super::super::create::create_worktree(&project_root, story_id, &empty_config(), 3001) + .await + .unwrap(); + + let wt_path = super::super::worktree_path(&project_root, story_id); + assert!(wt_path.exists(), "worktree must exist before removal"); + + let result = remove_worktree_by_story_id(&project_root, story_id, &empty_config()).await; + assert!( + result.is_ok(), + "Expected removal to succeed: {:?}", + result.err() + ); + + // Regression: `git worktree list` must not show the removed worktree. + let wt_list_output = std::process::Command::new("git") + .args(["worktree", "list", "--porcelain"]) + .current_dir(&project_root) + .output() + .expect("git worktree list"); + let wt_list = String::from_utf8_lossy(&wt_list_output.stdout); + assert!( + !wt_list.contains(&*wt_path.to_string_lossy()), + "git worktree list should not contain the removed worktree path, but got:\n{wt_list}" + ); + + // Regression: feature branch must be deleted after worktree removal. + let branch_output = std::process::Command::new("git") + .args(["branch", "--list", &expected_branch]) + .current_dir(&project_root) + .output() + .expect("git branch --list"); + let branch_list = String::from_utf8_lossy(&branch_output.stdout); + assert!( + branch_list.trim().is_empty(), + "Feature branch '{expected_branch}' should be deleted, but found: {branch_list}" + ); + } + #[tokio::test] async fn remove_worktree_async_removes_directory() { let tmp = TempDir::new().unwrap();