From 40d96008c90823935e5c3b2395c4e73aa87aced4 Mon Sep 17 00:00:00 2001 From: Dave Date: Thu, 26 Feb 2026 14:58:52 +0000 Subject: [PATCH] feat(story-200): auto-prune worktrees when stories are archived - Add `prune_worktree_sync` to worktree.rs: removes a story's worktree if it exists, delegating to `remove_worktree_sync` (best-effort, failures logged internally) - Update `sweep_done_to_archived` to accept `git_root` and call `prune_worktree_sync` after promoting a story from 5_done to 6_archived - Add Part 2 to the sweep: scan 6_archived and prune any stale worktrees for stories already there (catches items archived before this feature) - All worktree removal failures are logged but never block file moves - Add 5 new tests: prune noop, prune real worktree, sweep-on-promote, sweep-stale-archived, sweep-not-blocked-by-removal-failure Co-Authored-By: Claude Sonnet 4.6 --- ...does_not_write_mcp_json_to_project_root.md | 0 ...nal_positional_path_argument_on_startup.md | 0 ...uld_be_selected_by_default_on_first_use.md | 0 ..._claude_code_anthropic_to_anthropic_api.md | 0 ...s_for_archived_stories_in_watcher_sweep.md | 0 ..._interval_configurable_via_project_toml.md | 0 ..._interval_configurable_via_project_toml.md | 0 ...cking_agent_availability_in_start_agent.md | 0 ...ry_rename_storkit_branding_to_story_kit.md | 0 server/src/io/watcher.rs | 258 ++++++++++++++---- server/src/worktree.rs | 42 +++ 11 files changed, 252 insertions(+), 48 deletions(-) rename .story_kit/work/{2_current => 4_merge}/208_bug_project_scaffold_does_not_write_mcp_json_to_project_root.md (100%) rename .story_kit/work/{2_current => 4_merge}/209_story_accept_optional_positional_path_argument_on_startup.md (100%) rename .story_kit/work/{3_qa => 5_done}/206_story_claude_pty_should_be_selected_by_default_on_first_use.md (100%) rename .story_kit/work/{4_merge => 5_done}/207_story_rename_llm_provider_labels_claude_pty_to_claude_code_anthropic_to_anthropic_api.md (100%) rename .story_kit/work/{5_done => 6_archived}/200_story_auto_prune_worktrees_for_archived_stories_in_watcher_sweep.md (100%) rename .story_kit/work/{5_done => 6_archived}/201_story_make_watcher_sweep_interval_configurable_via_project_toml.md (100%) rename .story_kit/work/{5_done => 6_archived}/202_story_make_agent_watchdog_interval_configurable_via_project_toml.md (100%) rename .story_kit/work/{5_done => 6_archived}/203_story_move_story_to_current_before_checking_agent_availability_in_start_agent.md (100%) rename .story_kit/work/{5_done => 6_archived}/204_story_rename_storkit_branding_to_story_kit.md (100%) diff --git a/.story_kit/work/2_current/208_bug_project_scaffold_does_not_write_mcp_json_to_project_root.md b/.story_kit/work/4_merge/208_bug_project_scaffold_does_not_write_mcp_json_to_project_root.md similarity index 100% rename from .story_kit/work/2_current/208_bug_project_scaffold_does_not_write_mcp_json_to_project_root.md rename to .story_kit/work/4_merge/208_bug_project_scaffold_does_not_write_mcp_json_to_project_root.md diff --git a/.story_kit/work/2_current/209_story_accept_optional_positional_path_argument_on_startup.md b/.story_kit/work/4_merge/209_story_accept_optional_positional_path_argument_on_startup.md similarity index 100% rename from .story_kit/work/2_current/209_story_accept_optional_positional_path_argument_on_startup.md rename to .story_kit/work/4_merge/209_story_accept_optional_positional_path_argument_on_startup.md diff --git a/.story_kit/work/3_qa/206_story_claude_pty_should_be_selected_by_default_on_first_use.md b/.story_kit/work/5_done/206_story_claude_pty_should_be_selected_by_default_on_first_use.md similarity index 100% rename from .story_kit/work/3_qa/206_story_claude_pty_should_be_selected_by_default_on_first_use.md rename to .story_kit/work/5_done/206_story_claude_pty_should_be_selected_by_default_on_first_use.md diff --git a/.story_kit/work/4_merge/207_story_rename_llm_provider_labels_claude_pty_to_claude_code_anthropic_to_anthropic_api.md b/.story_kit/work/5_done/207_story_rename_llm_provider_labels_claude_pty_to_claude_code_anthropic_to_anthropic_api.md similarity index 100% rename from .story_kit/work/4_merge/207_story_rename_llm_provider_labels_claude_pty_to_claude_code_anthropic_to_anthropic_api.md rename to .story_kit/work/5_done/207_story_rename_llm_provider_labels_claude_pty_to_claude_code_anthropic_to_anthropic_api.md diff --git a/.story_kit/work/5_done/200_story_auto_prune_worktrees_for_archived_stories_in_watcher_sweep.md b/.story_kit/work/6_archived/200_story_auto_prune_worktrees_for_archived_stories_in_watcher_sweep.md similarity index 100% rename from .story_kit/work/5_done/200_story_auto_prune_worktrees_for_archived_stories_in_watcher_sweep.md rename to .story_kit/work/6_archived/200_story_auto_prune_worktrees_for_archived_stories_in_watcher_sweep.md diff --git a/.story_kit/work/5_done/201_story_make_watcher_sweep_interval_configurable_via_project_toml.md b/.story_kit/work/6_archived/201_story_make_watcher_sweep_interval_configurable_via_project_toml.md similarity index 100% rename from .story_kit/work/5_done/201_story_make_watcher_sweep_interval_configurable_via_project_toml.md rename to .story_kit/work/6_archived/201_story_make_watcher_sweep_interval_configurable_via_project_toml.md diff --git a/.story_kit/work/5_done/202_story_make_agent_watchdog_interval_configurable_via_project_toml.md b/.story_kit/work/6_archived/202_story_make_agent_watchdog_interval_configurable_via_project_toml.md similarity index 100% rename from .story_kit/work/5_done/202_story_make_agent_watchdog_interval_configurable_via_project_toml.md rename to .story_kit/work/6_archived/202_story_make_agent_watchdog_interval_configurable_via_project_toml.md diff --git a/.story_kit/work/5_done/203_story_move_story_to_current_before_checking_agent_availability_in_start_agent.md b/.story_kit/work/6_archived/203_story_move_story_to_current_before_checking_agent_availability_in_start_agent.md similarity index 100% rename from .story_kit/work/5_done/203_story_move_story_to_current_before_checking_agent_availability_in_start_agent.md rename to .story_kit/work/6_archived/203_story_move_story_to_current_before_checking_agent_availability_in_start_agent.md diff --git a/.story_kit/work/5_done/204_story_rename_storkit_branding_to_story_kit.md b/.story_kit/work/6_archived/204_story_rename_storkit_branding_to_story_kit.md similarity index 100% rename from .story_kit/work/5_done/204_story_rename_storkit_branding_to_story_kit.md rename to .story_kit/work/6_archived/204_story_rename_storkit_branding_to_story_kit.md diff --git a/server/src/io/watcher.rs b/server/src/io/watcher.rs index 26f1086..649463e 100644 --- a/server/src/io/watcher.rs +++ b/server/src/io/watcher.rs @@ -206,59 +206,89 @@ fn flush_pending( } /// Scan `work/5_done/` and move any `.md` files whose mtime is older than -/// `done_retention` to `work/6_archived/`. +/// `done_retention` to `work/6_archived/`. After each successful promotion, +/// removes the associated git worktree (if any) via [`crate::worktree::prune_worktree_sync`]. +/// +/// Also scans `work/6_archived/` for stories that still have a live worktree +/// and removes them (catches items that were archived before this sweep was +/// added). +/// +/// Worktree removal failures are logged but never block the file move or other +/// cleanup work. /// /// Called periodically from the watcher thread. File moves will trigger normal /// watcher events, which `flush_pending` will commit and broadcast. -fn sweep_done_to_archived(work_dir: &Path, done_retention: Duration) { +fn sweep_done_to_archived(work_dir: &Path, git_root: &Path, done_retention: Duration) { + // ── Part 1: promote old items from 5_done/ → 6_archived/ ─────────────── let done_dir = work_dir.join("5_done"); - if !done_dir.exists() { - return; + if done_dir.exists() { + let archived_dir = work_dir.join("6_archived"); + + match std::fs::read_dir(&done_dir) { + Err(e) => slog!("[watcher] sweep: failed to read 5_done/: {e}"), + Ok(entries) => { + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().is_none_or(|e| e != "md") { + continue; + } + + let mtime = match entry.metadata().and_then(|m| m.modified()) { + Ok(t) => t, + Err(_) => continue, + }; + + let age = SystemTime::now() + .duration_since(mtime) + .unwrap_or_default(); + + if age >= done_retention { + if let Err(e) = std::fs::create_dir_all(&archived_dir) { + slog!("[watcher] sweep: failed to create 6_archived/: {e}"); + continue; + } + let dest = archived_dir.join(entry.file_name()); + match std::fs::rename(&path, &dest) { + Ok(()) => { + let item_id = path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("unknown"); + slog!("[watcher] sweep: promoted {item_id} → 6_archived/"); + // Prune the worktree for this story (best effort). + if let Err(e) = + crate::worktree::prune_worktree_sync(git_root, item_id) + { + slog!( + "[watcher] sweep: worktree prune failed for {item_id}: {e}" + ); + } + } + Err(e) => { + slog!("[watcher] sweep: failed to move {}: {e}", path.display()); + } + } + } + } + } + } } - let entries = match std::fs::read_dir(&done_dir) { - Ok(e) => e, - Err(e) => { - slog!("[watcher] sweep: failed to read 5_done/: {e}"); - return; - } - }; - + // ── Part 2: prune stale worktrees for items already in 6_archived/ ────── let archived_dir = work_dir.join("6_archived"); - - for entry in entries.flatten() { - let path = entry.path(); - if path.extension().is_none_or(|e| e != "md") { - continue; - } - - let mtime = match entry.metadata().and_then(|m| m.modified()) { - Ok(t) => t, - Err(_) => continue, - }; - - let age = SystemTime::now() - .duration_since(mtime) - .unwrap_or_default(); - - if age >= done_retention { - if let Err(e) = std::fs::create_dir_all(&archived_dir) { - slog!("[watcher] sweep: failed to create 6_archived/: {e}"); + if archived_dir.exists() + && let Ok(entries) = std::fs::read_dir(&archived_dir) + { + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().is_none_or(|e| e != "md") { continue; } - let dest = archived_dir.join(entry.file_name()); - match std::fs::rename(&path, &dest) { - Ok(()) => { - let item_id = path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("unknown"); - slog!("[watcher] sweep: promoted {item_id} → 6_archived/"); - } - Err(e) => { - slog!("[watcher] sweep: failed to move {}: {e}", path.display()); - } + if let Some(item_id) = path.file_stem().and_then(|s| s.to_str()) + && let Err(e) = crate::worktree::prune_worktree_sync(git_root, item_id) + { + slog!("[watcher] sweep: worktree prune failed for {item_id}: {e}"); } } } @@ -410,7 +440,7 @@ pub fn start_watcher( let now = Instant::now(); if now.duration_since(last_sweep) >= sweep_interval { last_sweep = now; - sweep_done_to_archived(&work_dir, done_retention); + sweep_done_to_archived(&work_dir, &git_root, done_retention); } } } @@ -758,7 +788,8 @@ mod tests { .unwrap(); let retention = Duration::from_secs(4 * 60 * 60); - sweep_done_to_archived(&work_dir, retention); + // tmp.path() has no worktrees dir — prune_worktree_sync is a no-op. + sweep_done_to_archived(&work_dir, tmp.path(), retention); assert!(!story_path.exists(), "old item should be moved out of 5_done/"); assert!( @@ -779,7 +810,7 @@ mod tests { fs::write(&story_path, "---\nname: new\n---\n").unwrap(); let retention = Duration::from_secs(4 * 60 * 60); - sweep_done_to_archived(&work_dir, retention); + sweep_done_to_archived(&work_dir, tmp.path(), retention); assert!(story_path.exists(), "recent item should remain in 5_done/"); } @@ -802,7 +833,7 @@ mod tests { .unwrap(); // With a 1-minute retention, the 2-minute-old file should be swept. - sweep_done_to_archived(&work_dir, Duration::from_secs(60)); + sweep_done_to_archived(&work_dir, tmp.path(), Duration::from_secs(60)); assert!( !story_path.exists(), @@ -831,11 +862,142 @@ mod tests { .unwrap(); // With a 1-minute retention, the 30-second-old file should stay. - sweep_done_to_archived(&work_dir, Duration::from_secs(60)); + sweep_done_to_archived(&work_dir, tmp.path(), Duration::from_secs(60)); assert!( story_path.exists(), "item younger than custom retention should remain" ); } + + // ── sweep worktree pruning ───────────────────────────────────────────── + + /// Helper: create a real git worktree at `wt_path` on a new branch. + fn create_git_worktree(git_root: &std::path::Path, wt_path: &std::path::Path, branch: &str) { + use std::process::Command; + // Create the branch first (ignore errors if it already exists). + let _ = Command::new("git") + .args(["branch", branch]) + .current_dir(git_root) + .output(); + Command::new("git") + .args(["worktree", "add", &wt_path.to_string_lossy(), branch]) + .current_dir(git_root) + .output() + .expect("git worktree add"); + } + + #[test] + fn sweep_prunes_worktree_when_story_promoted_to_archived() { + let tmp = TempDir::new().unwrap(); + let git_root = tmp.path().to_path_buf(); + init_git_repo(&git_root); + + let work_dir = git_root.join(".story_kit").join("work"); + let done_dir = work_dir.join("5_done"); + fs::create_dir_all(&done_dir).unwrap(); + + let story_id = "60_story_prune_on_promote"; + let story_path = done_dir.join(format!("{story_id}.md")); + fs::write(&story_path, "---\nname: test\n---\n").unwrap(); + let past = SystemTime::now() + .checked_sub(Duration::from_secs(5 * 60 * 60)) + .unwrap(); + filetime::set_file_mtime(&story_path, filetime::FileTime::from_system_time(past)) + .unwrap(); + + // Create a real git worktree for this story. + let wt_path = crate::worktree::worktree_path(&git_root, story_id); + fs::create_dir_all(wt_path.parent().unwrap()).unwrap(); + create_git_worktree(&git_root, &wt_path, &format!("feature/story-{story_id}")); + assert!(wt_path.exists(), "worktree must exist before sweep"); + + let retention = Duration::from_secs(4 * 60 * 60); + sweep_done_to_archived(&work_dir, &git_root, retention); + + // Story must be archived. + assert!( + !story_path.exists(), + "story should be moved out of 5_done/" + ); + assert!( + work_dir.join("6_archived").join(format!("{story_id}.md")).exists(), + "story should appear in 6_archived/" + ); + // Worktree must be removed. + assert!(!wt_path.exists(), "worktree should be removed after archiving"); + } + + #[test] + fn sweep_prunes_worktrees_for_already_archived_stories() { + let tmp = TempDir::new().unwrap(); + let git_root = tmp.path().to_path_buf(); + init_git_repo(&git_root); + + let work_dir = git_root.join(".story_kit").join("work"); + let archived_dir = work_dir.join("6_archived"); + fs::create_dir_all(&archived_dir).unwrap(); + + // Story is already in 6_archived. + let story_id = "61_story_stale_worktree"; + let story_path = archived_dir.join(format!("{story_id}.md")); + fs::write(&story_path, "---\nname: stale\n---\n").unwrap(); + + // Create a real git worktree that was never cleaned up. + let wt_path = crate::worktree::worktree_path(&git_root, story_id); + fs::create_dir_all(wt_path.parent().unwrap()).unwrap(); + create_git_worktree(&git_root, &wt_path, &format!("feature/story-{story_id}")); + assert!(wt_path.exists(), "stale worktree must exist before sweep"); + + // 5_done/ is empty — only Part 2 runs. + fs::create_dir_all(work_dir.join("5_done")).unwrap(); + let retention = Duration::from_secs(4 * 60 * 60); + sweep_done_to_archived(&work_dir, &git_root, retention); + + // Stale worktree should be pruned. + assert!(!wt_path.exists(), "stale worktree should be pruned by sweep"); + // Story file must remain untouched. + assert!(story_path.exists(), "archived story file must not be removed"); + } + + #[test] + fn sweep_archives_story_even_when_worktree_removal_fails() { + // Use a git repo so prune_worktree_sync can attempt removal, + // but the fake directory is not a registered git worktree so + // `git worktree remove` will fail — the story must still be archived. + let tmp = TempDir::new().unwrap(); + let git_root = tmp.path().to_path_buf(); + init_git_repo(&git_root); + + let work_dir = git_root.join(".story_kit").join("work"); + let done_dir = work_dir.join("5_done"); + fs::create_dir_all(&done_dir).unwrap(); + + let story_id = "62_story_fake_worktree"; + let story_path = done_dir.join(format!("{story_id}.md")); + fs::write(&story_path, "---\nname: test\n---\n").unwrap(); + let past = SystemTime::now() + .checked_sub(Duration::from_secs(5 * 60 * 60)) + .unwrap(); + filetime::set_file_mtime(&story_path, filetime::FileTime::from_system_time(past)) + .unwrap(); + + // Create a plain directory at the expected worktree path — not a real + // git worktree, so `git worktree remove` will fail. + let wt_path = crate::worktree::worktree_path(&git_root, story_id); + fs::create_dir_all(&wt_path).unwrap(); + + let retention = Duration::from_secs(4 * 60 * 60); + sweep_done_to_archived(&work_dir, &git_root, retention); + + // Story must still be archived despite the worktree removal failure. + assert!( + !story_path.exists(), + "story should be archived even when worktree removal fails" + ); + assert!( + work_dir.join("6_archived").join(format!("{story_id}.md")).exists(), + "story should appear in 6_archived/ despite worktree removal failure" + ); + } } diff --git a/server/src/worktree.rs b/server/src/worktree.rs index 6c81882..046ce64 100644 --- a/server/src/worktree.rs +++ b/server/src/worktree.rs @@ -166,6 +166,21 @@ fn configure_sparse_checkout(_wt_path: &Path) -> Result<(), String> { 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 = 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) +} + /// Remove a git worktree and its branch. pub async fn remove_worktree( project_root: &Path, @@ -653,6 +668,33 @@ mod tests { assert!(result.is_ok(), "Expected removal to succeed: {:?}", result.err()); } + // ── prune_worktree_sync ────────────────────────────────────────────────── + + #[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 = 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"); + } + #[tokio::test] async fn create_worktree_succeeds_despite_setup_failure() { let tmp = TempDir::new().unwrap();