story-kit: merge 312_bug_auto_assign_assigns_mergemaster_to_coding_stage_stories

server/src/agents/pool.rs

@@ -219,6 +219,33 @@ impl AgentPool {
         // a no-op.
         super::lifecycle::move_story_to_current(project_root, story_id)?;
 
+        // Validate that the agent's configured stage matches the story's
+        // pipeline stage.  This prevents any caller (auto-assign, MCP tool,
+        // pipeline advance, supervisor) from starting a wrong-stage agent on
+        // a story — e.g. mergemaster on a coding-stage story (bug 312).
+        if let Some(name) = agent_name {
+            let agent_stage = config
+                .find_agent(name)
+                .map(agent_config_stage)
+                .unwrap_or_else(|| pipeline_stage(name));
+            if agent_stage != PipelineStage::Other
+                && let Some(story_stage_dir) = find_active_story_stage(project_root, story_id)
+            {
+                let expected_stage = match story_stage_dir {
+                    "2_current" => PipelineStage::Coder,
+                    "3_qa" => PipelineStage::Qa,
+                    "4_merge" => PipelineStage::Mergemaster,
+                    _ => PipelineStage::Other,
+                };
+                if expected_stage != PipelineStage::Other && expected_stage != agent_stage {
+                    return Err(format!(
+                        "Agent '{name}' (stage: {agent_stage:?}) cannot be assigned to \
+                         story '{story_id}' in {story_stage_dir}/ (requires stage: {expected_stage:?})"
+                    ));
+                }
+            }
+        }
+
         // Atomically resolve agent name, check availability, and register as
         // Pending.  When `agent_name` is `None` the first idle coder is
         // selected inside the lock so no TOCTOU race can occur between the
@@ -4205,10 +4232,14 @@ stage = "coder"
         let tmp = tempfile::tempdir().unwrap();
         let root = tmp.path();
 
-        // Minimal project.toml with a "qa" agent.
+        // Minimal project.toml with a coder agent (must match 2_current/ stage).
         let sk_dir = root.join(".story_kit");
         fs::create_dir_all(&sk_dir).unwrap();
-        fs::write(sk_dir.join("project.toml"), "[[agent]]\nname = \"qa\"\n").unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"coder-1\"\nstage = \"coder\"\n",
+        )
+        .unwrap();
 
         // Create the story in upcoming so `move_story_to_current` succeeds,
         // but do NOT init a git repo — `create_worktree` will fail in the spawn.
@@ -4219,7 +4250,7 @@ stage = "coder"
         let pool = AgentPool::new_test(3099);
 
         let result = pool
-            .start_agent(root, "50_story_test", Some("qa"), None)
+            .start_agent(root, "50_story_test", Some("coder-1"), None)
             .await;
 
         // With the non-blocking flow, start_agent returns Ok(Pending) immediately.
@@ -4238,7 +4269,7 @@ stage = "coder"
         // Wait for the background task to reach a terminal state.
         // It must fail (no git repo → create_worktree returns an error).
         let final_info = pool
-            .wait_for_agent("50_story_test", "qa", 5000)
+            .wait_for_agent("50_story_test", "coder-1", 5000)
             .await
             .expect("wait_for_agent should not time out");
         assert_eq!(
@@ -4251,7 +4282,7 @@ stage = "coder"
         let agents = pool.agents.lock().unwrap();
         let failed_entry = agents
             .values()
-            .find(|a| a.agent_name == "qa" && a.status == AgentStatus::Failed);
+            .find(|a| a.agent_name == "coder-1" && a.status == AgentStatus::Failed);
         assert!(
             failed_entry.is_some(),
             "agent pool must retain a Failed entry so the UI can show the error state"
@@ -4261,7 +4292,7 @@ stage = "coder"
         // The AgentEvent::Error must be persisted in the event_log so late
         // subscribers / polling clients can see the failure reason.
         let events = pool
-            .drain_events("50_story_test", "qa")
+            .drain_events("50_story_test", "coder-1")
             .expect("drain_events should succeed");
         let has_error_event = events.iter().any(|e| matches!(e, AgentEvent::Error { .. }));
         assert!(
@@ -4661,6 +4692,198 @@ stage = "coder"
         );
     }
 
+    // ── bug 312: stage-pipeline mismatch guard in start_agent ──────────────
+
+    /// Bug 312: start_agent must reject a mergemaster on a story in 2_current/.
+    #[tokio::test]
+    async fn start_agent_rejects_mergemaster_on_coding_stage_story() {
+        use std::fs;
+
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+
+        let sk_dir = root.join(".story_kit");
+        fs::create_dir_all(sk_dir.join("work/2_current")).unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"coder-1\"\nstage = \"coder\"\n\n\
+             [[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
+        )
+        .unwrap();
+        fs::write(
+            sk_dir.join("work/2_current/310_story_foo.md"),
+            "---\nname: Foo\n---\n",
+        )
+        .unwrap();
+
+        let pool = AgentPool::new_test(3099);
+        let result = pool
+            .start_agent(root, "310_story_foo", Some("mergemaster"), None)
+            .await;
+
+        assert!(
+            result.is_err(),
+            "mergemaster must not be assigned to a story in 2_current/"
+        );
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("stage") && err.contains("2_current"),
+            "error must mention stage mismatch, got: '{err}'"
+        );
+    }
+
+    /// Bug 312: start_agent must reject a coder on a story in 3_qa/.
+    #[tokio::test]
+    async fn start_agent_rejects_coder_on_qa_stage_story() {
+        use std::fs;
+
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+
+        let sk_dir = root.join(".story_kit");
+        fs::create_dir_all(sk_dir.join("work/3_qa")).unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"coder-1\"\nstage = \"coder\"\n\n\
+             [[agent]]\nname = \"qa\"\nstage = \"qa\"\n",
+        )
+        .unwrap();
+        fs::write(
+            sk_dir.join("work/3_qa/42_story_bar.md"),
+            "---\nname: Bar\n---\n",
+        )
+        .unwrap();
+
+        let pool = AgentPool::new_test(3099);
+        let result = pool
+            .start_agent(root, "42_story_bar", Some("coder-1"), None)
+            .await;
+
+        assert!(
+            result.is_err(),
+            "coder must not be assigned to a story in 3_qa/"
+        );
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("stage") && err.contains("3_qa"),
+            "error must mention stage mismatch, got: '{err}'"
+        );
+    }
+
+    /// Bug 312: start_agent must reject a QA agent on a story in 4_merge/.
+    #[tokio::test]
+    async fn start_agent_rejects_qa_on_merge_stage_story() {
+        use std::fs;
+
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+
+        let sk_dir = root.join(".story_kit");
+        fs::create_dir_all(sk_dir.join("work/4_merge")).unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"qa\"\nstage = \"qa\"\n\n\
+             [[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
+        )
+        .unwrap();
+        fs::write(
+            sk_dir.join("work/4_merge/55_story_baz.md"),
+            "---\nname: Baz\n---\n",
+        )
+        .unwrap();
+
+        let pool = AgentPool::new_test(3099);
+        let result = pool
+            .start_agent(root, "55_story_baz", Some("qa"), None)
+            .await;
+
+        assert!(
+            result.is_err(),
+            "qa must not be assigned to a story in 4_merge/"
+        );
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("stage") && err.contains("4_merge"),
+            "error must mention stage mismatch, got: '{err}'"
+        );
+    }
+
+    /// Bug 312: supervisor (stage=other) should be allowed on any pipeline stage.
+    #[tokio::test]
+    async fn start_agent_allows_supervisor_on_any_stage() {
+        use std::fs;
+
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+
+        let sk_dir = root.join(".story_kit");
+        fs::create_dir_all(sk_dir.join("work/2_current")).unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"supervisor\"\nstage = \"other\"\n",
+        )
+        .unwrap();
+        fs::write(
+            sk_dir.join("work/2_current/77_story_sup.md"),
+            "---\nname: Sup\n---\n",
+        )
+        .unwrap();
+
+        let pool = AgentPool::new_test(3099);
+        // start_agent will fail for git/worktree reasons, but NOT for stage
+        // mismatch. We just need to verify it doesn't fail with a stage error.
+        let result = pool
+            .start_agent(root, "77_story_sup", Some("supervisor"), None)
+            .await;
+
+        match result {
+            Ok(_) => {} // Fine — no stage error.
+            Err(e) => {
+                assert!(
+                    !e.contains("stage:") || !e.contains("cannot be assigned"),
+                    "supervisor should not be rejected for stage mismatch, got: '{e}'"
+                );
+            }
+        }
+    }
+
+    /// Bug 312: correct stage agent should still be allowed.
+    #[tokio::test]
+    async fn start_agent_allows_correct_stage_agent() {
+        use std::fs;
+
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+
+        let sk_dir = root.join(".story_kit");
+        fs::create_dir_all(sk_dir.join("work/4_merge")).unwrap();
+        fs::write(
+            sk_dir.join("project.toml"),
+            "[[agent]]\nname = \"mergemaster\"\nstage = \"mergemaster\"\n",
+        )
+        .unwrap();
+        fs::write(
+            sk_dir.join("work/4_merge/88_story_ok.md"),
+            "---\nname: OK\n---\n",
+        )
+        .unwrap();
+
+        let pool = AgentPool::new_test(3099);
+        let result = pool
+            .start_agent(root, "88_story_ok", Some("mergemaster"), None)
+            .await;
+
+        match result {
+            Ok(_) => {} // Fine — correct stage.
+            Err(e) => {
+                assert!(
+                    !e.contains("cannot be assigned"),
+                    "mergemaster on 4_merge/ story should not fail stage check, got: '{e}'"
+                );
+            }
+        }
+    }
+
     // ── merge_agent_work tests ────────────────────────────────────────────────
 
     /// Helper: start a merge and poll until terminal state.