diff --git a/server/src/agents.rs b/server/src/agents.rs index e448d40..c22ab23 100644 --- a/server/src/agents.rs +++ b/server/src/agents.rs @@ -217,6 +217,12 @@ struct StoryAgent { project_root: Option, /// UUID identifying the log file for this session. log_session_id: Option, + /// Set to `true` when the agent calls `report_merge_failure`. + /// Prevents the pipeline from blindly advancing to `5_done/` after a + /// failed merge: the server-owned gate check runs in the feature-branch + /// worktree (which compiles fine) and returns `gates_passed=true` even + /// though the code was never squash-merged onto master. + merge_failure_reported: bool, } /// Build an `AgentInfo` snapshot from a `StoryAgent` map entry. @@ -444,6 +450,7 @@ impl AgentPool { completion: None, project_root: Some(project_root.to_path_buf()), log_session_id: Some(log_session_id.clone()), + merge_failure_reported: false, }, ); } @@ -898,6 +905,7 @@ impl AgentPool { completion: CompletionReport, project_root: Option, worktree_path: Option, + merge_failure_reported: bool, ) { let project_root = match project_root { Some(p) => p, @@ -1023,6 +1031,18 @@ impl AgentPool { } } PipelineStage::Mergemaster => { + // Block advancement if the mergemaster explicitly reported a failure. + // The server-owned gate check runs in the feature-branch worktree (not + // master), so `gates_passed=true` is misleading when no code was merged. + if merge_failure_reported { + slog!( + "[pipeline] Pipeline advancement blocked for '{story_id}': \ + mergemaster explicitly reported a merge failure. \ + Story stays in 4_merge/ for human review." + ); + return; + } + // Run script/test on master (project_root) as the post-merge verification. slog!( "[pipeline] Mergemaster completed for '{story_id}'. Running post-merge tests on master." @@ -1155,7 +1175,7 @@ impl AgentPool { // Extract data for pipeline advance, then remove the entry so // completed agents never appear in list_agents. - let (tx, session_id, project_root_for_advance, wt_path_for_advance) = { + let (tx, session_id, project_root_for_advance, wt_path_for_advance, merge_failure_reported_for_advance) = { let mut agents = self.agents.lock().map_err(|e| e.to_string())?; let agent = agents.get_mut(&key).ok_or_else(|| { format!("Agent '{agent_name}' for story '{story_id}' disappeared during gate check") @@ -1165,8 +1185,9 @@ impl AgentPool { let sid = agent.session_id.clone(); let pr = agent.project_root.clone(); let wt = agent.worktree_info.as_ref().map(|w| w.path.clone()); + let mfr = agent.merge_failure_reported; agents.remove(&key); - (tx, sid, pr, wt) + (tx, sid, pr, wt, mfr) }; // Emit Done so wait_for_agent unblocks. @@ -1197,6 +1218,7 @@ impl AgentPool { report_for_advance, project_root_for_advance, wt_path_for_advance, + merge_failure_reported_for_advance, ) .await; }); @@ -1303,6 +1325,48 @@ impl AgentPool { Some((session_id, project_root)) } + /// Record that the mergemaster agent for `story_id` explicitly reported a + /// merge failure via the `report_merge_failure` MCP tool. + /// + /// Sets `merge_failure_reported = true` on the active mergemaster agent so + /// that `run_pipeline_advance` can block advancement to `5_done/` even when + /// the server-owned gate check returns `gates_passed=true` (those gates run + /// in the feature-branch worktree, not on master). + pub fn set_merge_failure_reported(&self, story_id: &str) { + match self.agents.lock() { + Ok(mut lock) => { + let found = lock.iter_mut().find(|(key, agent)| { + let key_story_id = key + .rsplit_once(':') + .map(|(sid, _)| sid) + .unwrap_or(key.as_str()); + key_story_id == story_id + && pipeline_stage(&agent.agent_name) == PipelineStage::Mergemaster + }); + match found { + Some((_, agent)) => { + agent.merge_failure_reported = true; + slog!( + "[pipeline] Merge failure flag set for '{story_id}:{}'", + agent.agent_name + ); + } + None => { + slog_warn!( + "[pipeline] set_merge_failure_reported: no running mergemaster found \ + for story '{story_id}' — flag not set" + ); + } + } + } + Err(e) => { + slog_error!( + "[pipeline] set_merge_failure_reported: could not lock agents: {e}" + ); + } + } + } + /// Test helper: inject a pre-built agent entry so unit tests can exercise /// wait/subscribe logic without spawning a real process. #[cfg(test)] @@ -1328,6 +1392,7 @@ impl AgentPool { completion: None, project_root: None, log_session_id: None, + merge_failure_reported: false, }, ); tx @@ -1363,6 +1428,7 @@ impl AgentPool { completion: None, project_root: None, log_session_id: None, + merge_failure_reported: false, }, ); tx @@ -1705,6 +1771,7 @@ impl AgentPool { completion: Some(completion), project_root: Some(project_root), log_session_id: None, + merge_failure_reported: false, }, ); tx @@ -1736,6 +1803,7 @@ impl AgentPool { completion: None, project_root: None, log_session_id: None, + merge_failure_reported: false, }, ); tx @@ -2037,7 +2105,7 @@ async fn run_server_owned_completion( // Store completion report, extract data for pipeline advance, then // remove the entry so completed agents never appear in list_agents. - let (tx, project_root_for_advance, wt_path_for_advance) = { + let (tx, project_root_for_advance, wt_path_for_advance, merge_failure_reported_for_advance) = { let mut lock = match agents.lock() { Ok(a) => a, Err(_) => return, @@ -2051,8 +2119,9 @@ async fn run_server_owned_completion( let tx = agent.tx.clone(); let pr = agent.project_root.clone(); let wt = agent.worktree_info.as_ref().map(|w| w.path.clone()); + let mfr = agent.merge_failure_reported; lock.remove(&key); - (tx, pr, wt) + (tx, pr, wt, mfr) }; // Emit Done so wait_for_agent unblocks. @@ -2075,6 +2144,7 @@ async fn run_server_owned_completion( project_root_for_advance, wt_path_for_advance, watcher_tx, + merge_failure_reported_for_advance, ); } @@ -2092,6 +2162,7 @@ fn spawn_pipeline_advance( project_root: Option, worktree_path: Option, watcher_tx: broadcast::Sender, + merge_failure_reported: bool, ) { let sid = story_id.to_string(); let aname = agent_name.to_string(); @@ -2102,8 +2173,15 @@ fn spawn_pipeline_advance( child_killers: Arc::new(Mutex::new(HashMap::new())), watcher_tx, }; - pool.run_pipeline_advance(&sid, &aname, completion, project_root, worktree_path) - .await; + pool.run_pipeline_advance( + &sid, + &aname, + completion, + project_root, + worktree_path, + merge_failure_reported, + ) + .await; }); } @@ -3949,6 +4027,7 @@ mod tests { }, Some(root.to_path_buf()), None, + false, ) .await; @@ -3986,6 +4065,7 @@ mod tests { }, Some(root.to_path_buf()), None, + false, ) .await; @@ -4021,6 +4101,7 @@ mod tests { }, Some(root.to_path_buf()), None, + false, ) .await; @@ -6569,6 +6650,7 @@ stage = "qa" }, Some(root.to_path_buf()), None, + false, ) .await; diff --git a/server/src/http/mcp.rs b/server/src/http/mcp.rs index 0a9fe24..eac30f7 100644 --- a/server/src/http/mcp.rs +++ b/server/src/http/mcp.rs @@ -899,7 +899,7 @@ async fn handle_tools_call( // Mergemaster tools "merge_agent_work" => tool_merge_agent_work(&args, ctx).await, "move_story_to_merge" => tool_move_story_to_merge(&args, ctx).await, - "report_merge_failure" => tool_report_merge_failure(&args), + "report_merge_failure" => tool_report_merge_failure(&args, ctx), // QA tools "request_qa" => tool_request_qa(&args, ctx).await, // Diagnostics @@ -1645,7 +1645,7 @@ async fn tool_move_story_to_merge(args: &Value, ctx: &AppContext) -> Result Result { +fn tool_report_merge_failure(args: &Value, ctx: &AppContext) -> Result { let story_id = args .get("story_id") .and_then(|v| v.as_str()) @@ -1656,6 +1656,7 @@ fn tool_report_merge_failure(args: &Value) -> Result { .ok_or("Missing required argument: reason")?; slog!("[mergemaster] Merge failure reported for '{story_id}': {reason}"); + ctx.agents.set_merge_failure_reported(story_id); Ok(format!( "Merge failure for '{story_id}' recorded. Story remains in work/4_merge/. Reason: {reason}" @@ -2638,24 +2639,33 @@ mod tests { #[test] fn tool_report_merge_failure_missing_story_id() { - let result = tool_report_merge_failure(&json!({"reason": "conflicts"})); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_report_merge_failure(&json!({"reason": "conflicts"}), &ctx); assert!(result.is_err()); assert!(result.unwrap_err().contains("story_id")); } #[test] fn tool_report_merge_failure_missing_reason() { - let result = tool_report_merge_failure(&json!({"story_id": "42_story_foo"})); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_report_merge_failure(&json!({"story_id": "42_story_foo"}), &ctx); assert!(result.is_err()); assert!(result.unwrap_err().contains("reason")); } #[test] fn tool_report_merge_failure_returns_confirmation() { - let result = tool_report_merge_failure(&json!({ - "story_id": "42_story_foo", - "reason": "Unresolvable merge conflicts in src/main.rs" - })); + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let result = tool_report_merge_failure( + &json!({ + "story_id": "42_story_foo", + "reason": "Unresolvable merge conflicts in src/main.rs" + }), + &ctx, + ); assert!(result.is_ok()); let msg = result.unwrap(); assert!(msg.contains("42_story_foo"));