Files
huskies/server/src/http/mcp/qa_tools.rs
T
dave 845b85e7a7 fix: add --all to cargo fmt in script/test and autoformat codebase
cargo fmt without --all fails with "Failed to find targets" in
workspace repos. This was blocking every story's gates. Also ran
cargo fmt --all to fix all existing formatting issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-13 14:07:08 +00:00

403 lines
14 KiB
Rust

//! MCP QA tools — request, approve, and reject QA reviews for stories.
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;
use serde_json::{Value, json};
pub(super) async fn tool_request_qa(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let agent_name = args
.get("agent_name")
.and_then(|v| v.as_str())
.unwrap_or("qa");
let project_root = ctx.agents.get_project_root(&ctx.state)?;
// Move story from work/2_current/ to work/3_qa/
move_story_to_qa(&project_root, story_id)?;
// Start the QA agent on the story worktree
let info = ctx
.agents
.start_agent(&project_root, story_id, Some(agent_name), None, None)
.await?;
serde_json::to_string_pretty(&json!({
"story_id": info.story_id,
"agent_name": info.agent_name,
"status": info.status.to_string(),
"worktree_path": info.worktree_path,
"message": format!(
"Story '{story_id}' moved to work/3_qa/ and QA agent '{}' started.",
info.agent_name
),
}))
.map_err(|e| format!("Serialization error: {e}"))
}
pub(super) async fn tool_approve_qa(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let project_root = ctx.agents.get_project_root(&ctx.state)?;
// Clear review_hold before moving
let qa_path = project_root
.join(".huskies/work/3_qa")
.join(format!("{story_id}.md"));
if qa_path.exists() {
let _ = crate::io::story_metadata::clear_front_matter_field(&qa_path, "review_hold");
}
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}"))??;
move_story_to_done(&project_root, story_id)?;
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> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let notes = args
.get("notes")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: notes")?;
let project_root = ctx.agents.get_project_root(&ctx.state)?;
// Move story from work/3_qa/ back to work/2_current/ with rejection notes
reject_story_from_qa(&project_root, story_id, notes)?;
// Restart the coder agent with rejection context
let story_path = project_root
.join(".huskies/work/2_current")
.join(format!("{story_id}.md"));
let agent_name = if story_path.exists() {
let contents = std::fs::read_to_string(&story_path).unwrap_or_default();
crate::io::story_metadata::parse_front_matter(&contents)
.ok()
.and_then(|meta| meta.agent)
} else {
None
};
let agent_name = agent_name.as_deref().unwrap_or("coder-opus");
let context = format!(
"\n\n---\n## QA Rejection\n\
Your previous implementation was rejected during human QA review.\n\
Rejection notes:\n{notes}\n\n\
Please fix the issues described above and try again."
);
if let Err(e) = ctx
.agents
.start_agent(
&project_root,
story_id,
Some(agent_name),
Some(&context),
None,
)
.await
{
slog_warn!("[qa] Failed to restart coder for '{story_id}' after rejection: {e}");
}
Ok(format!(
"Story '{story_id}' rejected and moved back to work/2_current/. Coder agent '{agent_name}' restarted with rejection notes."
))
}
pub(super) async fn tool_launch_qa_app(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let project_root = ctx.agents.get_project_root(&ctx.state)?;
// Find the worktree path for this story
let worktrees = crate::worktree::list_worktrees(&project_root)?;
let wt = worktrees
.iter()
.find(|w| w.story_id == story_id)
.ok_or_else(|| format!("No worktree found for story '{story_id}'"))?;
let wt_path = wt.path.clone();
// Stop any existing QA app instance
{
let mut guard = ctx.qa_app_process.lock().unwrap();
if let Some(mut child) = guard.take() {
let _ = child.kill();
let _ = child.wait();
slog!("[qa-app] Stopped previous QA app instance.");
}
}
// Find a free port starting from 3100
let port = find_free_port(3100);
// Write .huskies_port so the frontend dev server knows where to connect
let port_file = wt_path.join(".huskies_port");
std::fs::write(&port_file, port.to_string())
.map_err(|e| format!("Failed to write .huskies_port: {e}"))?;
// Launch the server from the worktree
let child = std::process::Command::new("cargo")
.args(["run"])
.env("HUSKIES_PORT", port.to_string())
.current_dir(&wt_path)
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.spawn()
.map_err(|e| format!("Failed to launch QA app: {e}"))?;
{
let mut guard = ctx.qa_app_process.lock().unwrap();
*guard = Some(child);
}
serde_json::to_string_pretty(&json!({
"story_id": story_id,
"port": port,
"worktree_path": wt_path.to_string_lossy(),
"message": format!("QA app launched on port {port} from worktree at {}", wt_path.display()),
}))
.map_err(|e| format!("Serialization error: {e}"))
}
/// Find a free TCP port starting from `start`.
pub(super) fn find_free_port(start: u16) -> u16 {
for port in start..start + 100 {
if std::net::TcpListener::bind(("127.0.0.1", port)).is_ok() {
return port;
}
}
start // fallback
}
#[cfg(test)]
mod tests {
use super::*;
use crate::http::test_helpers::test_ctx;
#[test]
fn request_qa_in_tools_list() {
use super::super::handle_tools_list;
let resp = handle_tools_list(Some(json!(1)));
let tools = resp.result.unwrap()["tools"].as_array().unwrap().clone();
let tool = tools.iter().find(|t| t["name"] == "request_qa");
assert!(tool.is_some(), "request_qa missing from tools list");
let t = tool.unwrap();
let required = t["inputSchema"]["required"].as_array().unwrap();
let req_names: Vec<&str> = required.iter().map(|v| v.as_str().unwrap()).collect();
assert!(req_names.contains(&"story_id"));
// agent_name is optional
assert!(!req_names.contains(&"agent_name"));
}
#[test]
fn approve_qa_in_tools_list() {
use super::super::handle_tools_list;
let resp = handle_tools_list(Some(json!(1)));
let tools = resp.result.unwrap()["tools"].as_array().unwrap().clone();
let tool = tools.iter().find(|t| t["name"] == "approve_qa");
assert!(tool.is_some(), "approve_qa missing from tools list");
let t = tool.unwrap();
let required = t["inputSchema"]["required"].as_array().unwrap();
let req_names: Vec<&str> = required.iter().map(|v| v.as_str().unwrap()).collect();
assert!(req_names.contains(&"story_id"));
}
#[test]
fn reject_qa_in_tools_list() {
use super::super::handle_tools_list;
let resp = handle_tools_list(Some(json!(1)));
let tools = resp.result.unwrap()["tools"].as_array().unwrap().clone();
let tool = tools.iter().find(|t| t["name"] == "reject_qa");
assert!(tool.is_some(), "reject_qa missing from tools list");
let t = tool.unwrap();
let required = t["inputSchema"]["required"].as_array().unwrap();
let req_names: Vec<&str> = required.iter().map(|v| v.as_str().unwrap()).collect();
assert!(req_names.contains(&"story_id"));
assert!(req_names.contains(&"notes"));
}
#[test]
fn launch_qa_app_in_tools_list() {
use super::super::handle_tools_list;
let resp = handle_tools_list(Some(json!(1)));
let tools = resp.result.unwrap()["tools"].as_array().unwrap().clone();
let tool = tools.iter().find(|t| t["name"] == "launch_qa_app");
assert!(tool.is_some(), "launch_qa_app missing from tools list");
let t = tool.unwrap();
let required = t["inputSchema"]["required"].as_array().unwrap();
let req_names: Vec<&str> = required.iter().map(|v| v.as_str().unwrap()).collect();
assert!(req_names.contains(&"story_id"));
}
#[tokio::test]
async fn tool_approve_qa_missing_story_id() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let result = tool_approve_qa(&json!({}), &ctx).await;
assert!(result.is_err());
assert!(result.unwrap_err().contains("story_id"));
}
#[tokio::test]
async fn tool_reject_qa_missing_story_id() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let result = tool_reject_qa(&json!({"notes": "broken"}), &ctx).await;
assert!(result.is_err());
assert!(result.unwrap_err().contains("story_id"));
}
#[tokio::test]
async fn tool_reject_qa_missing_notes() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let result = tool_reject_qa(&json!({"story_id": "1_story_test"}), &ctx).await;
assert!(result.is_err());
assert!(result.unwrap_err().contains("notes"));
}
#[tokio::test]
async fn tool_request_qa_missing_story_id() {
let tmp = tempfile::tempdir().unwrap();
let ctx = test_ctx(tmp.path());
let result = tool_request_qa(&json!({}), &ctx).await;
assert!(result.is_err());
assert!(result.unwrap_err().contains("story_id"));
}
}