Three MCP files touched:
- status_tools.rs (story-status JSON dump): every field with a CRDT
equivalent now reads from WorkItem (name, agent, blocked, qa_mode,
retry_count, depends_on, claimed_by, claimed_at) or MergeJob.error
(merge_failure detail). One field — review_hold — has no CRDT register
yet (sub-story 932) and is wrapped in `yaml_residue(parse_front_matter(...))`
so the gap is visible at every code-search.
- qa_tools.rs:
• tool_approve_qa wraps the legacy `clear_front_matter_field("review_hold")`
write in `yaml_residue(...)` pending sub-story 932.
• tool_reject_qa now reads the agent name from the CRDT WorkItem instead
of parsing front matter on disk.
- story_tools/epic.rs: the entire epic feature (item_type, epic link)
has no CRDT analog — sub-story 933. Every parse_front_matter call here
is wrapped in `yaml_residue(...)`.
Also: new identity wrapper `db::yaml_legacy::yaml_residue<T>(v: T) -> T`
that marks a yaml_legacy callsite blocked on a CRDT-register gap. Pure
identity at runtime; the distinctive name makes the residue grep-findable
(`grep -rn yaml_residue`). Sub-stories 932 and 933 enumerate the gaps.
Filed:
- 932: Add CRDT register for review_hold
- 933: Add CRDT registers for the epic mechanism
All 2854 tests pass; fmt + clippy clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
329 lines
12 KiB
Rust
329 lines
12 KiB
Rust
//! MCP QA tools — request, approve, and reject QA reviews for stories.
|
|
//!
|
|
//! This file is a thin adapter: it deserialises MCP payloads, delegates to
|
|
//! `crate::service::qa` for all business logic, and serialises responses.
|
|
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::service::qa::{find_free_port, is_spike, merge_spike_branch_to_master};
|
|
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.services.agents.get_project_root(&ctx.state)?;
|
|
|
|
// Move story from work/2_current/ to work/3_qa/
|
|
move_story_to_qa(story_id)?;
|
|
|
|
// Start the QA agent on the story worktree
|
|
let info = ctx
|
|
.services
|
|
.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.services.agents.get_project_root(&ctx.state)?;
|
|
|
|
// Clear review_hold before moving.
|
|
// review_hold has no CRDT register yet — story 932. Marked with
|
|
// `yaml_residue` so the gap is grep-findable.
|
|
let qa_path = project_root
|
|
.join(".huskies/work/3_qa")
|
|
.join(format!("{story_id}.md"));
|
|
if qa_path.exists() {
|
|
let _ = crate::db::yaml_legacy::yaml_residue(
|
|
crate::db::yaml_legacy::clear_front_matter_field(&qa_path, "review_hold"),
|
|
);
|
|
}
|
|
|
|
if is_spike(story_id) {
|
|
// 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}"))?
|
|
.map_err(|e| e.to_string())?;
|
|
|
|
move_story_to_done(story_id)?;
|
|
|
|
let pool = std::sync::Arc::clone(&ctx.services.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(story_id)?;
|
|
|
|
// Start the mergemaster agent
|
|
let info = ctx
|
|
.services
|
|
.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}"))
|
|
}
|
|
}
|
|
|
|
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.services.agents.get_project_root(&ctx.state)?;
|
|
|
|
// Move story from work/3_qa/ back to work/2_current/ with rejection notes
|
|
reject_story_from_qa(story_id, notes)?;
|
|
|
|
// Restart the coder agent with rejection context.
|
|
// Agent name comes from the CRDT WorkItem register (story 929).
|
|
let agent_name =
|
|
crate::crdt_state::read_item(story_id).and_then(|w| w.agent().map(str::to_string));
|
|
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
|
|
.services
|
|
.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.services.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}"))
|
|
}
|
|
|
|
#[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"));
|
|
}
|
|
}
|