huskies: merge 553_story_accept_spike_state_machine_transition_skips_merge_and_goes_directly_to_done

2026-04-13 12:50:25 +00:00
parent 12497eb4f1
commit 5806156af3
3 changed files with 138 additions and 105 deletions
@@ -7,7 +7,7 @@ use crate::slog;

 type ContentTransform = Option<Box<dyn Fn(&str) -> String>>;

-pub(super) fn item_type_from_id(item_id: &str) -> &'static str {
+pub(crate) fn item_type_from_id(item_id: &str) -> &'static str {
    // New format: {digits}_{type}_{slug}
    let after_num = item_id.trim_start_matches(|c: char| c.is_ascii_digit());
    if after_num.starts_with("_bug_") {
@@ -155,14 +155,15 @@ pub fn feature_branch_has_unmerged_changes(project_root: &Path, story_id: &str)
    }
 }

-/// Move a story from `work/2_current/` or `work/4_merge/` to `work/5_done/`.
+/// Move a story from `work/2_current/`, `work/3_qa/`, or `work/4_merge/` to `work/5_done/`.
 ///
-/// Idempotent if already in `5_done/` or `6_archived/`. Errors if not found in `2_current/` or `4_merge/`.
+/// Idempotent if already in `5_done/` or `6_archived/`. Errors if not found in any earlier stage.
+/// Spikes may transition directly from `3_qa/` to `5_done/`, skipping the merge stage.
 pub fn move_story_to_done(project_root: &Path, story_id: &str) -> Result<(), String> {
    move_item(
        project_root,
        story_id,
-        &["2_current", "4_merge"],
+        &["2_current", "3_qa", "4_merge"],
        "5_done",
        &["6_archived"],
        false,
@@ -1,5 +1,5 @@
 //! MCP QA tools — request, approve, and reject QA reviews for stories.
-use crate::agents::{move_story_to_merge, move_story_to_qa, reject_story_from_qa};
+use crate::agents::{move_story_to_done, move_story_to_merge, move_story_to_qa, reject_story_from_qa};
 use crate::http::context::AppContext;
 use crate::slog;
 use crate::slog_warn;
@@ -55,25 +55,134 @@ pub(super) async fn tool_approve_qa(args: &Value, ctx: &AppContext) -> Result<St
        let _ = crate::io::story_metadata::clear_front_matter_field(&qa_path, "review_hold");
    }

-    // Move story from work/3_qa/ to work/4_merge/
-    move_story_to_merge(&project_root, story_id)?;
+    let item_type = crate::agents::lifecycle::item_type_from_id(story_id);
+    if item_type == "spike" {
+        // Spikes skip the merge stage entirely: merge the feature branch to master
+        // directly (fast-forward or simple merge), then move straight to done.
+        let branch = format!("feature/story-{story_id}");
+        let root = project_root.clone();
+        let br = branch.clone();
+        let sid = story_id.to_string();
+        let merge_ok = tokio::task::spawn_blocking(move || {
+            merge_spike_branch_to_master(&root, &br, &sid)
+        })
+        .await
+        .map_err(|e| format!("Merge task panicked: {e}"))??;

-    // Start the mergemaster agent
-    let info = ctx
-        .agents
-        .start_agent(&project_root, story_id, Some("mergemaster"), None, None)
-        .await?;
+        move_story_to_done(&project_root, story_id)?;

-    serde_json::to_string_pretty(&json!({
-        "story_id": info.story_id,
-        "agent_name": info.agent_name,
-        "status": info.status.to_string(),
-        "message": format!(
-            "Story '{story_id}' approved. Moved to work/4_merge/ and mergemaster agent '{}' started.",
-            info.agent_name
-        ),
-    }))
-    .map_err(|e| format!("Serialization error: {e}"))
+        let pool = std::sync::Arc::clone(&ctx.agents);
+        pool.remove_agents_for_story(story_id);
+
+        let wt_path = crate::worktree::worktree_path(&project_root, story_id);
+        if wt_path.exists() {
+            let config = crate::config::ProjectConfig::load(&project_root).unwrap_or_default();
+            let _ = crate::worktree::remove_worktree_by_story_id(
+                &project_root,
+                story_id,
+                &config,
+            )
+            .await;
+        }
+
+        pool.auto_assign_available_work(&project_root).await;
+
+        serde_json::to_string_pretty(&json!({
+            "story_id": story_id,
+            "message": format!(
+                "Spike '{story_id}' approved. Branch merged to master ({}). Moved directly to work/5_done/.",
+                if merge_ok { "merged" } else { "no changes to merge" }
+            ),
+        }))
+        .map_err(|e| format!("Serialization error: {e}"))
+    } else {
+        // Non-spike items go through the normal merge pipeline.
+        move_story_to_merge(&project_root, story_id)?;
+
+        // Start the mergemaster agent
+        let info = ctx
+            .agents
+            .start_agent(&project_root, story_id, Some("mergemaster"), None, None)
+            .await?;
+
+        serde_json::to_string_pretty(&json!({
+            "story_id": info.story_id,
+            "agent_name": info.agent_name,
+            "status": info.status.to_string(),
+            "message": format!(
+                "Story '{story_id}' approved. Moved to work/4_merge/ and mergemaster agent '{}' started.",
+                info.agent_name
+            ),
+        }))
+        .map_err(|e| format!("Serialization error: {e}"))
+    }
+}
+
+/// Merge a spike's feature branch into master using a fast-forward or simple merge.
+///
+/// Unlike the squash-merge pipeline used for stories, spikes skip quality gates
+/// and preserve their commit history. Returns `true` if a merge was performed,
+/// `false` if the branch had no unmerged commits.
+fn merge_spike_branch_to_master(
+    project_root: &std::path::Path,
+    branch: &str,
+    story_id: &str,
+) -> Result<bool, String> {
+    use std::process::Command;
+
+    // Check the branch exists and has unmerged changes.
+    if !crate::agents::lifecycle::feature_branch_has_unmerged_changes(project_root, story_id) {
+        slog!("[qa] Spike '{story_id}': feature branch has no unmerged changes, skipping merge.");
+        return Ok(false);
+    }
+
+    // Ensure we are on master.
+    let checkout = Command::new("git")
+        .args(["checkout", "master"])
+        .current_dir(project_root)
+        .output()
+        .map_err(|e| format!("git checkout master failed: {e}"))?;
+    if !checkout.status.success() {
+        return Err(format!(
+            "Failed to checkout master: {}",
+            String::from_utf8_lossy(&checkout.stderr)
+        ));
+    }
+
+    // Try fast-forward first, then fall back to a regular merge.
+    let ff = Command::new("git")
+        .args(["merge", "--ff-only", branch])
+        .current_dir(project_root)
+        .output()
+        .map_err(|e| format!("git merge --ff-only failed: {e}"))?;
+
+    if ff.status.success() {
+        slog!("[qa] Spike '{story_id}': fast-forward merged '{branch}' into master.");
+        return Ok(true);
+    }
+
+    // Fast-forward failed (diverged history) — fall back to a regular merge.
+    let merge = Command::new("git")
+        .args([
+            "merge",
+            "--no-ff",
+            branch,
+            "-m",
+            &format!("Merge spike branch '{branch}' into master"),
+        ])
+        .current_dir(project_root)
+        .output()
+        .map_err(|e| format!("git merge failed: {e}"))?;
+
+    if merge.status.success() {
+        slog!("[qa] Spike '{story_id}': merged '{branch}' into master (no-ff).");
+        Ok(true)
+    } else {
+        Err(format!(
+            "Failed to merge spike branch '{branch}' into master: {}",
+            String::from_utf8_lossy(&merge.stderr)
+        ))
+    }
 }

 pub(super) async fn tool_reject_qa(args: &Value, ctx: &AppContext) -> Result<String, String> {