From eb8654dba01c4602ff6dd4ae92a723e931ba2e7d Mon Sep 17 00:00:00 2001 From: dave Date: Sat, 4 Apr 2026 15:20:35 +0000 Subject: [PATCH] huskies: merge 475_refactor_deduplicate_lifecycle_rs_move_functions_into_a_shared_parameterised_helper --- server/src/agents/lifecycle.rs | 430 ++++++++++----------------------- 1 file changed, 133 insertions(+), 297 deletions(-) diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index c8e3bb60..f6b4363f 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; use crate::io::story_metadata::{clear_front_matter_field, write_rejection_notes}; @@ -16,53 +16,64 @@ pub(super) fn item_type_from_id(item_id: &str) -> &'static str { } } -/// Return the source directory path for a work item (always work/1_backlog/). -fn item_source_dir(project_root: &Path, _item_id: &str) -> PathBuf { - project_root.join(".huskies").join("work").join("1_backlog") -} +/// Move `{story_id}.md` from the first matching `sources` dir to `target_dir`, clearing +/// `fields_to_clear`. Returns `Ok(Some(src_dir))` on move, `Ok(None)` if idempotent or missing_ok. +fn move_item<'a>( + project_root: &Path, + story_id: &str, + sources: &'a [&'a str], + target_dir: &str, + extra_done_dirs: &[&str], + missing_ok: bool, + fields_to_clear: &[&str], +) -> Result, String> { + let sk = project_root.join(".huskies").join("work"); + let target_dir_path = sk.join(target_dir); + let target_path = target_dir_path.join(format!("{story_id}.md")); -/// Return the done directory path for a work item (always work/5_done/). -fn item_archive_dir(project_root: &Path, _item_id: &str) -> PathBuf { - project_root.join(".huskies").join("work").join("5_done") + if target_path.exists() + || extra_done_dirs + .iter() + .any(|d| sk.join(d).join(format!("{story_id}.md")).exists()) + { + return Ok(None); + } + + let (src_dir, src_path) = match sources.iter().find_map(|&s| { + let p = sk.join(s).join(format!("{story_id}.md")); + p.exists().then_some((s, p)) + }) { + Some(t) => t, + None if missing_ok => { + slog!("[lifecycle] Work item '{story_id}' not found; skipping move to work/{target_dir}/"); + return Ok(None); + } + None => { + let locs = sources.iter().map(|s| format!("work/{s}/")).collect::>().join(" or "); + return Err(format!("Work item '{story_id}' not found in {locs}.")); + } + }; + + std::fs::create_dir_all(&target_dir_path) + .map_err(|e| format!("Failed to create work/{target_dir}/ directory: {e}"))?; + std::fs::rename(&src_path, &target_path) + .map_err(|e| format!("Failed to move '{story_id}' to work/{target_dir}/: {e}"))?; + + for field in fields_to_clear { + if let Err(e) = clear_front_matter_field(&target_path, field) { + slog!("[lifecycle] Warning: could not clear {field} from '{story_id}': {e}"); + } + } + + slog!("[lifecycle] Moved '{story_id}' from work/{src_dir}/ to work/{target_dir}/"); + Ok(Some(src_dir)) } /// Move a work item (story, bug, or spike) from `work/1_backlog/` to `work/2_current/`. /// -/// Idempotent: if the item is already in `2_current/`, returns Ok without committing. -/// If the item is not found in `1_backlog/`, logs a warning and returns Ok. +/// Idempotent: if already in `2_current/`, returns Ok. If not found in `1_backlog/`, logs and returns Ok. pub fn move_story_to_current(project_root: &Path, story_id: &str) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let current_dir = sk.join("2_current"); - let current_path = current_dir.join(format!("{story_id}.md")); - - if current_path.exists() { - // Already in 2_current/ — idempotent, nothing to do. - return Ok(()); - } - - let source_dir = item_source_dir(project_root, story_id); - let source_path = source_dir.join(format!("{story_id}.md")); - - if !source_path.exists() { - slog!( - "[lifecycle] Work item '{story_id}' not found in {}; skipping move to 2_current/", - source_dir.display() - ); - return Ok(()); - } - - std::fs::create_dir_all(¤t_dir) - .map_err(|e| format!("Failed to create work/2_current/ directory: {e}"))?; - - std::fs::rename(&source_path, ¤t_path) - .map_err(|e| format!("Failed to move '{story_id}' to 2_current/: {e}"))?; - - slog!( - "[lifecycle] Moved '{story_id}' from {} to work/2_current/", - source_dir.display() - ); - - Ok(()) + move_item(project_root, story_id, &["1_backlog"], "2_current", &[], true, &[]).map(|_| ()) } /// Check whether a feature branch `feature/story-{story_id}` exists and has @@ -96,190 +107,63 @@ pub fn feature_branch_has_unmerged_changes(project_root: &Path, story_id: &str) } } -/// Move a story from `work/2_current/` to `work/5_done/` and auto-commit. +/// Move a story from `work/2_current/` or `work/4_merge/` to `work/5_done/`. /// -/// * If the story is in `2_current/`, it is moved to `5_done/` and committed. -/// * If the story is in `4_merge/`, it is moved to `5_done/` and committed. -/// * If the story is already in `5_done/` or `6_archived/`, this is a no-op (idempotent). -/// * If the story is not found in `2_current/`, `4_merge/`, `5_done/`, or `6_archived/`, an error is returned. +/// Idempotent if already in `5_done/` or `6_archived/`. Errors if not found in `2_current/` or `4_merge/`. pub fn move_story_to_done(project_root: &Path, story_id: &str) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let current_path = sk.join("2_current").join(format!("{story_id}.md")); - let merge_path = sk.join("4_merge").join(format!("{story_id}.md")); - let done_dir = sk.join("5_done"); - let done_path = done_dir.join(format!("{story_id}.md")); - let archived_path = sk.join("6_archived").join(format!("{story_id}.md")); - - if done_path.exists() || archived_path.exists() { - // Already in done or archived — idempotent, nothing to do. - return Ok(()); - } - - // Check 2_current/ first, then 4_merge/ - let source_path = if current_path.exists() { - current_path.clone() - } else if merge_path.exists() { - merge_path.clone() - } else { - return Err(format!( - "Story '{story_id}' not found in work/2_current/ or work/4_merge/. Cannot accept story." - )); - }; - - std::fs::create_dir_all(&done_dir) - .map_err(|e| format!("Failed to create work/5_done/ directory: {e}"))?; - std::fs::rename(&source_path, &done_path) - .map_err(|e| format!("Failed to move story '{story_id}' to 5_done/: {e}"))?; - - // Strip stale pipeline fields from front matter now that the story is done. - for field in &["merge_failure", "retry_count", "blocked"] { - if let Err(e) = clear_front_matter_field(&done_path, field) { - slog!("[lifecycle] Warning: could not clear {field} from '{story_id}': {e}"); - } - } - - let from_dir = if source_path == current_path { - "work/2_current/" - } else { - "work/4_merge/" - }; - slog!("[lifecycle] Moved story '{story_id}' from {from_dir} to work/5_done/"); - - Ok(()) + move_item( + project_root, + story_id, + &["2_current", "4_merge"], + "5_done", + &["6_archived"], + false, + &["merge_failure", "retry_count", "blocked"], + ) + .map(|_| ()) } /// Move a story/bug from `work/2_current/` or `work/3_qa/` to `work/4_merge/`. /// -/// This stages a work item as ready for the mergemaster to pick up and merge into master. -/// Idempotent: if already in `4_merge/`, returns Ok without committing. +/// Idempotent if already in `4_merge/`. Errors if not found in `2_current/` or `3_qa/`. pub fn move_story_to_merge(project_root: &Path, story_id: &str) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let current_path = sk.join("2_current").join(format!("{story_id}.md")); - let qa_path = sk.join("3_qa").join(format!("{story_id}.md")); - let merge_dir = sk.join("4_merge"); - let merge_path = merge_dir.join(format!("{story_id}.md")); - - if merge_path.exists() { - // Already in 4_merge/ — idempotent, nothing to do. - return Ok(()); - } - - // Accept from 2_current/ (manual trigger) or 3_qa/ (pipeline advancement from QA stage). - let source_path = if current_path.exists() { - current_path.clone() - } else if qa_path.exists() { - qa_path.clone() - } else { - return Err(format!( - "Work item '{story_id}' not found in work/2_current/ or work/3_qa/. Cannot move to 4_merge/." - )); - }; - - std::fs::create_dir_all(&merge_dir) - .map_err(|e| format!("Failed to create work/4_merge/ directory: {e}"))?; - std::fs::rename(&source_path, &merge_path) - .map_err(|e| format!("Failed to move '{story_id}' to 4_merge/: {e}"))?; - - let from_dir = if source_path == current_path { - "work/2_current/" - } else { - "work/3_qa/" - }; - // Reset retry count and blocked for the new stage. - if let Err(e) = clear_front_matter_field(&merge_path, "retry_count") { - slog!("[lifecycle] Warning: could not clear retry_count for '{story_id}': {e}"); - } - if let Err(e) = clear_front_matter_field(&merge_path, "blocked") { - slog!("[lifecycle] Warning: could not clear blocked for '{story_id}': {e}"); - } - - slog!("[lifecycle] Moved '{story_id}' from {from_dir} to work/4_merge/"); - - Ok(()) + move_item( + project_root, + story_id, + &["2_current", "3_qa"], + "4_merge", + &[], + false, + &["retry_count", "blocked"], + ) + .map(|_| ()) } -/// Move a story/bug from `work/2_current/` to `work/3_qa/` and auto-commit. +/// Move a story/bug from `work/2_current/` to `work/3_qa/`. /// -/// This stages a work item for QA review before merging to master. -/// Idempotent: if already in `3_qa/`, returns Ok without committing. +/// Idempotent if already in `3_qa/`. Errors if not found in `2_current/`. pub fn move_story_to_qa(project_root: &Path, story_id: &str) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let current_path = sk.join("2_current").join(format!("{story_id}.md")); - let qa_dir = sk.join("3_qa"); - let qa_path = qa_dir.join(format!("{story_id}.md")); - - if qa_path.exists() { - // Already in 3_qa/ — idempotent, nothing to do. - return Ok(()); - } - - if !current_path.exists() { - return Err(format!( - "Work item '{story_id}' not found in work/2_current/. Cannot move to 3_qa/." - )); - } - - std::fs::create_dir_all(&qa_dir) - .map_err(|e| format!("Failed to create work/3_qa/ directory: {e}"))?; - std::fs::rename(¤t_path, &qa_path) - .map_err(|e| format!("Failed to move '{story_id}' to 3_qa/: {e}"))?; - - // Reset retry count for the new stage. - if let Err(e) = clear_front_matter_field(&qa_path, "retry_count") { - slog!("[lifecycle] Warning: could not clear retry_count for '{story_id}': {e}"); - } - if let Err(e) = clear_front_matter_field(&qa_path, "blocked") { - slog!("[lifecycle] Warning: could not clear blocked for '{story_id}': {e}"); - } - - slog!("[lifecycle] Moved '{story_id}' from work/2_current/ to work/3_qa/"); - - Ok(()) + move_item( + project_root, + story_id, + &["2_current"], + "3_qa", + &[], + false, + &["retry_count", "blocked"], + ) + .map(|_| ()) } -/// Move a story from `work/3_qa/` back to `work/2_current/` and write rejection notes. -/// -/// Used when a human reviewer rejects a story during manual QA. -/// Clears the `review_hold` front matter field and appends rejection notes to the story file. -pub fn reject_story_from_qa( - project_root: &Path, - story_id: &str, - notes: &str, -) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let qa_path = sk.join("3_qa").join(format!("{story_id}.md")); - let current_dir = sk.join("2_current"); - let current_path = current_dir.join(format!("{story_id}.md")); - - if current_path.exists() { - return Ok(()); // Already in 2_current — idempotent. +/// Move a story from `work/3_qa/` back to `work/2_current/`, clearing `review_hold` and writing notes. +pub fn reject_story_from_qa(project_root: &Path, story_id: &str, notes: &str) -> Result<(), String> { + let moved = move_item(project_root, story_id, &["3_qa"], "2_current", &[], false, &["review_hold"])?; + if moved.is_some() && !notes.is_empty() { + let path = project_root.join(".huskies/work/2_current").join(format!("{story_id}.md")); + if let Err(e) = write_rejection_notes(&path, notes) { + slog!("[lifecycle] Warning: could not write rejection notes to '{story_id}': {e}"); + } } - - if !qa_path.exists() { - return Err(format!( - "Work item '{story_id}' not found in work/3_qa/. Cannot reject." - )); - } - - std::fs::create_dir_all(¤t_dir) - .map_err(|e| format!("Failed to create work/2_current/ directory: {e}"))?; - std::fs::rename(&qa_path, ¤t_path) - .map_err(|e| format!("Failed to move '{story_id}' from 3_qa/ to 2_current/: {e}"))?; - - // Clear review_hold since the story is going back for rework. - if let Err(e) = clear_front_matter_field(¤t_path, "review_hold") { - slog!("[lifecycle] Warning: could not clear review_hold from '{story_id}': {e}"); - } - - // Write rejection notes into the story file so the coder can see what needs fixing. - if !notes.is_empty() - && let Err(e) = write_rejection_notes(¤t_path, notes) - { - slog!("[lifecycle] Warning: could not write rejection notes to '{story_id}': {e}"); - } - - slog!("[lifecycle] Rejected '{story_id}' from work/3_qa/ back to work/2_current/"); - Ok(()) } @@ -293,34 +177,7 @@ pub fn move_story_to_stage( story_id: &str, target_stage: &str, ) -> Result<(String, String), String> { - let stage_dirs: &[(&str, &str)] = &[ - ("backlog", "1_backlog"), - ("current", "2_current"), - ("qa", "3_qa"), - ("merge", "4_merge"), - ("done", "5_done"), - ]; - - let target_dir_name = stage_dirs - .iter() - .find(|(name, _)| *name == target_stage) - .map(|(_, dir)| *dir) - .ok_or_else(|| { - format!( - "Invalid target_stage '{target_stage}'. Must be one of: backlog, current, qa, merge, done" - ) - })?; - - let sk = project_root.join(".huskies").join("work"); - let target_dir = sk.join(target_dir_name); - let target_path = target_dir.join(format!("{story_id}.md")); - - if target_path.exists() { - return Ok((target_stage.to_string(), target_stage.to_string())); - } - - // Search all named stages plus the archive stage. - let search_dirs: &[(&str, &str)] = &[ + const STAGES: &[(&str, &str)] = &[ ("backlog", "1_backlog"), ("current", "2_current"), ("qa", "3_qa"), @@ -329,69 +186,48 @@ pub fn move_story_to_stage( ("archived", "6_archived"), ]; - let mut found_path: Option = None; - let mut from_stage = ""; - for (stage_name, dir_name) in search_dirs { - let candidate = sk.join(dir_name).join(format!("{story_id}.md")); - if candidate.exists() { - found_path = Some(candidate); - from_stage = stage_name; - break; + let target_dir = STAGES + .iter() + .filter(|(name, _)| *name != "archived") + .find(|(name, _)| *name == target_stage) + .map(|(_, dir)| *dir) + .ok_or_else(|| { + format!( + "Invalid target_stage '{target_stage}'. Must be one of: backlog, current, qa, merge, done" + ) + })?; + + let all_dirs: Vec<&str> = STAGES.iter().map(|(_, dir)| *dir).collect(); + + match move_item(project_root, story_id, &all_dirs, target_dir, &[], false, &[]) + .map_err(|_| format!("Work item '{story_id}' not found in any pipeline stage."))? + { + Some(src_dir) => { + let from_stage = STAGES + .iter() + .find(|(_, dir)| *dir == src_dir) + .map(|(name, _)| *name) + .unwrap_or(src_dir); + Ok((from_stage.to_string(), target_stage.to_string())) } + None => Ok((target_stage.to_string(), target_stage.to_string())), } - - let source_path = - found_path.ok_or_else(|| format!("Work item '{story_id}' not found in any pipeline stage."))?; - - std::fs::create_dir_all(&target_dir) - .map_err(|e| format!("Failed to create work/{target_dir_name}/ directory: {e}"))?; - std::fs::rename(&source_path, &target_path) - .map_err(|e| format!("Failed to move '{story_id}' to work/{target_dir_name}/: {e}"))?; - - slog!( - "[lifecycle] Moved '{story_id}' from work/{from_stage}/ to work/{target_dir_name}/" - ); - - Ok((from_stage.to_string(), target_stage.to_string())) } -/// Move a bug from `work/2_current/` or `work/1_backlog/` to `work/5_done/` and auto-commit. +/// Move a bug from `work/2_current/` or `work/1_backlog/` to `work/5_done/`. /// -/// * If the bug is in `2_current/`, it is moved to `5_done/` and committed. -/// * If the bug is still in `1_backlog/` (never started), it is moved directly to `5_done/`. -/// * If the bug is already in `5_done/`, this is a no-op (idempotent). -/// * If the bug is not found anywhere, an error is returned. +/// Idempotent if already in `5_done/`. Errors if not found in `2_current/` or `1_backlog/`. pub fn close_bug_to_archive(project_root: &Path, bug_id: &str) -> Result<(), String> { - let sk = project_root.join(".huskies").join("work"); - let current_path = sk.join("2_current").join(format!("{bug_id}.md")); - let backlog_path = sk.join("1_backlog").join(format!("{bug_id}.md")); - let archive_dir = item_archive_dir(project_root, bug_id); - let archive_path = archive_dir.join(format!("{bug_id}.md")); - - if archive_path.exists() { - return Ok(()); - } - - let source_path = if current_path.exists() { - current_path.clone() - } else if backlog_path.exists() { - backlog_path.clone() - } else { - return Err(format!( - "Bug '{bug_id}' not found in work/2_current/ or work/1_backlog/. Cannot close bug." - )); - }; - - std::fs::create_dir_all(&archive_dir) - .map_err(|e| format!("Failed to create work/5_done/ directory: {e}"))?; - std::fs::rename(&source_path, &archive_path) - .map_err(|e| format!("Failed to move bug '{bug_id}' to 5_done/: {e}"))?; - - slog!( - "[lifecycle] Closed bug '{bug_id}' → work/5_done/" - ); - - Ok(()) + move_item( + project_root, + bug_id, + &["2_current", "1_backlog"], + "5_done", + &[], + false, + &[], + ) + .map(|_| ()) } #[cfg(test)]