diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 34e8d7e..5e40391 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -33,6 +33,8 @@ export interface PipelineStageItem { error: string | null; merge_failure: string | null; agent: AgentAssignment | null; + review_hold: boolean | null; + manual_qa: boolean | null; } export interface PipelineState { @@ -312,8 +314,42 @@ export const api = { baseUrl, ); }, + /** Approve a story in QA, moving it to merge. */ + approveQa(storyId: string) { + return callMcpTool("approve_qa", { story_id: storyId }); + }, + /** Reject a story in QA, moving it back to current with notes. */ + rejectQa(storyId: string, notes: string) { + return callMcpTool("reject_qa", { story_id: storyId, notes }); + }, + /** Launch the QA app for a story's worktree. */ + launchQaApp(storyId: string) { + return callMcpTool("launch_qa_app", { story_id: storyId }); + }, }; +async function callMcpTool( + toolName: string, + args: Record, +): Promise { + const res = await fetch("/mcp", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: toolName, arguments: args }, + }), + }); + const json = await res.json(); + if (json.error) { + throw new Error(json.error.message); + } + const text = json.result?.content?.[0]?.text ?? ""; + return text; +} + export class ChatWebSocket { private static sharedSocket: WebSocket | null = null; private static refCount = 0; diff --git a/frontend/src/components/WorkItemDetailPanel.tsx b/frontend/src/components/WorkItemDetailPanel.tsx index 10820af..dc2b34c 100644 --- a/frontend/src/components/WorkItemDetailPanel.tsx +++ b/frontend/src/components/WorkItemDetailPanel.tsx @@ -27,6 +27,8 @@ interface WorkItemDetailPanelProps { storyId: string; pipelineVersion: number; onClose: () => void; + /** True when the item is in QA and awaiting human review. */ + reviewHold?: boolean; } function TestCaseRow({ tc }: { tc: TestCaseResult }) { @@ -109,6 +111,7 @@ export function WorkItemDetailPanel({ storyId, pipelineVersion, onClose, + reviewHold: _reviewHold, }: WorkItemDetailPanelProps) { const [content, setContent] = useState(null); const [stage, setStage] = useState(""); diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 71957c7..2907620 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; -use crate::io::story_metadata::clear_front_matter_field; +use crate::io::story_metadata::{clear_front_matter_field, write_rejection_notes}; use crate::slog; pub(super) fn item_type_from_id(item_id: &str) -> &'static str { @@ -219,6 +219,52 @@ pub fn move_story_to_qa(project_root: &Path, story_id: &str) -> Result<(), Strin Ok(()) } +/// Move a story from `work/3_qa/` back to `work/2_current/` and write rejection notes. +/// +/// Used when a human reviewer rejects a story during manual QA. +/// Clears the `review_hold` front matter field and appends rejection notes to the story file. +pub fn reject_story_from_qa( + project_root: &Path, + story_id: &str, + notes: &str, +) -> Result<(), String> { + let sk = project_root.join(".story_kit").join("work"); + let qa_path = sk.join("3_qa").join(format!("{story_id}.md")); + let current_dir = sk.join("2_current"); + let current_path = current_dir.join(format!("{story_id}.md")); + + if current_path.exists() { + return Ok(()); // Already in 2_current — idempotent. + } + + if !qa_path.exists() { + return Err(format!( + "Work item '{story_id}' not found in work/3_qa/. Cannot reject." + )); + } + + std::fs::create_dir_all(¤t_dir) + .map_err(|e| format!("Failed to create work/2_current/ directory: {e}"))?; + std::fs::rename(&qa_path, ¤t_path) + .map_err(|e| format!("Failed to move '{story_id}' from 3_qa/ to 2_current/: {e}"))?; + + // Clear review_hold since the story is going back for rework. + if let Err(e) = clear_front_matter_field(¤t_path, "review_hold") { + slog!("[lifecycle] Warning: could not clear review_hold from '{story_id}': {e}"); + } + + // Write rejection notes into the story file so the coder can see what needs fixing. + if !notes.is_empty() + && let Err(e) = write_rejection_notes(¤t_path, notes) + { + slog!("[lifecycle] Warning: could not write rejection notes to '{story_id}': {e}"); + } + + slog!("[lifecycle] Rejected '{story_id}' from work/3_qa/ back to work/2_current/"); + + Ok(()) +} + /// Move a bug from `work/2_current/` or `work/1_backlog/` to `work/5_done/` and auto-commit. /// /// * If the bug is in `2_current/`, it is moved to `5_done/` and committed. @@ -552,4 +598,51 @@ mod tests { "should return false when no feature branch" ); } + + // ── reject_story_from_qa tests ──────────────────────────────────────────── + + #[test] + fn reject_story_from_qa_moves_to_current() { + use std::fs; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let qa_dir = root.join(".story_kit/work/3_qa"); + let current_dir = root.join(".story_kit/work/2_current"); + fs::create_dir_all(&qa_dir).unwrap(); + fs::create_dir_all(¤t_dir).unwrap(); + fs::write( + qa_dir.join("50_story_test.md"), + "---\nname: Test\nreview_hold: true\n---\n# Story\n", + ) + .unwrap(); + + reject_story_from_qa(root, "50_story_test", "Button color wrong").unwrap(); + + assert!(!qa_dir.join("50_story_test.md").exists()); + assert!(current_dir.join("50_story_test.md").exists()); + let contents = fs::read_to_string(current_dir.join("50_story_test.md")).unwrap(); + assert!(contents.contains("Button color wrong")); + assert!(contents.contains("## QA Rejection Notes")); + assert!(!contents.contains("review_hold")); + } + + #[test] + fn reject_story_from_qa_errors_when_not_in_qa() { + let tmp = tempfile::tempdir().unwrap(); + let result = reject_story_from_qa(tmp.path(), "99_nonexistent", "notes"); + assert!(result.unwrap_err().contains("not found in work/3_qa/")); + } + + #[test] + fn reject_story_from_qa_idempotent_when_in_current() { + use std::fs; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let current_dir = root.join(".story_kit/work/2_current"); + fs::create_dir_all(¤t_dir).unwrap(); + fs::write(current_dir.join("51_story_test.md"), "---\nname: Test\n---\n# Story\n").unwrap(); + + reject_story_from_qa(root, "51_story_test", "notes").unwrap(); + assert!(current_dir.join("51_story_test.md").exists()); + } } diff --git a/server/src/agents/mod.rs b/server/src/agents/mod.rs index fee1216..3c1373c 100644 --- a/server/src/agents/mod.rs +++ b/server/src/agents/mod.rs @@ -9,7 +9,7 @@ use serde::Serialize; pub use lifecycle::{ close_bug_to_archive, feature_branch_has_unmerged_changes, move_story_to_archived, - move_story_to_merge, move_story_to_qa, + move_story_to_merge, move_story_to_qa, reject_story_from_qa, }; pub use pool::AgentPool; diff --git a/server/src/agents/pool.rs b/server/src/agents/pool.rs index 5cc969b..db7ce43 100644 --- a/server/src/agents/pool.rs +++ b/server/src/agents/pool.rs @@ -889,25 +889,37 @@ impl AgentPool { }; if coverage_passed { - // Spikes skip merge — they stay in 3_qa/ for human review. - if super::lifecycle::item_type_from_id(story_id) == "spike" { - // Mark the spike as held for review so auto-assign won't - // restart QA on it. + // Check whether this item needs human review before merging. + let needs_human_review = { + let item_type = super::lifecycle::item_type_from_id(story_id); + if item_type == "spike" { + true // Spikes always need human review. + } else { + // Stories/bugs: check the manual_qa front matter field (defaults to true). + let qa_dir = project_root.join(".story_kit/work/3_qa"); + let story_path = qa_dir.join(format!("{story_id}.md")); + crate::io::story_metadata::requires_manual_qa(&story_path) + } + }; + + if needs_human_review { + // Hold in 3_qa/ for human review. let qa_dir = project_root.join(".story_kit/work/3_qa"); - let spike_path = qa_dir.join(format!("{story_id}.md")); - if let Err(e) = crate::io::story_metadata::write_review_hold(&spike_path) { + let story_path = qa_dir.join(format!("{story_id}.md")); + if let Err(e) = crate::io::story_metadata::write_review_hold(&story_path) { slog_error!("[pipeline] Failed to set review_hold on '{story_id}': {e}"); } slog!( - "[pipeline] QA passed for spike '{story_id}'. \ - Stopping for human review (skipping merge). \ + "[pipeline] QA passed for '{story_id}'. \ + Holding for human review. \ Worktree preserved at: {worktree_path:?}" ); - // Free up the QA slot without advancing the spike. + // Free up the QA slot without advancing. self.auto_assign_available_work(&project_root).await; } else { slog!( - "[pipeline] QA passed gates and coverage for '{story_id}'. Moving to merge." + "[pipeline] QA passed gates and coverage for '{story_id}'. \ + manual_qa: false — moving directly to merge." ); if let Err(e) = super::lifecycle::move_story_to_merge(&project_root, story_id) { slog_error!("[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"); @@ -1746,23 +1758,35 @@ impl AgentPool { }; if coverage_passed { - // Spikes skip the merge stage — stay in 3_qa/ for human review. - if super::lifecycle::item_type_from_id(story_id) == "spike" { - let spike_path = project_root + // Check whether this item needs human review before merging. + let needs_human_review = { + let item_type = super::lifecycle::item_type_from_id(story_id); + if item_type == "spike" { + true + } else { + let story_path = project_root + .join(".story_kit/work/3_qa") + .join(format!("{story_id}.md")); + crate::io::story_metadata::requires_manual_qa(&story_path) + } + }; + + if needs_human_review { + let story_path = project_root .join(".story_kit/work/3_qa") .join(format!("{story_id}.md")); - if let Err(e) = crate::io::story_metadata::write_review_hold(&spike_path) { + if let Err(e) = crate::io::story_metadata::write_review_hold(&story_path) { eprintln!( - "[startup:reconcile] Failed to set review_hold on spike '{story_id}': {e}" + "[startup:reconcile] Failed to set review_hold on '{story_id}': {e}" ); } eprintln!( - "[startup:reconcile] Spike '{story_id}' passed QA — holding for human review." + "[startup:reconcile] '{story_id}' passed QA — holding for human review." ); let _ = progress_tx.send(ReconciliationEvent { story_id: story_id.clone(), status: "review_hold".to_string(), - message: "Spike passed QA — waiting for human review.".to_string(), + message: "Passed QA — waiting for human review.".to_string(), }); } else if let Err(e) = super::lifecycle::move_story_to_merge(project_root, story_id) { eprintln!( @@ -2655,7 +2679,12 @@ mod tests { // Set up story in 3_qa/ let qa_dir = root.join(".story_kit/work/3_qa"); fs::create_dir_all(&qa_dir).unwrap(); - fs::write(qa_dir.join("51_story_test.md"), "test").unwrap(); + // manual_qa: false so the story skips human review and goes straight to merge. + fs::write( + qa_dir.join("51_story_test.md"), + "---\nname: Test\nmanual_qa: false\n---\ntest", + ) + .unwrap(); let pool = AgentPool::new_test(3001); pool.run_pipeline_advance( diff --git a/server/src/http/context.rs b/server/src/http/context.rs index 33f0644..3002ac1 100644 --- a/server/src/http/context.rs +++ b/server/src/http/context.rs @@ -49,6 +49,9 @@ pub struct AppContext { /// Receiver for permission requests. The active WebSocket handler locks /// this and polls for incoming permission forwards. pub perm_rx: Arc>>, + /// Child process of the QA app launched for manual testing. + /// Only one instance runs at a time. + pub qa_app_process: Arc>>, } #[cfg(test)] @@ -69,6 +72,7 @@ impl AppContext { reconciliation_tx, perm_tx, perm_rx: Arc::new(tokio::sync::Mutex::new(perm_rx)), + qa_app_process: Arc::new(std::sync::Mutex::new(None)), } } } diff --git a/server/src/http/mcp.rs b/server/src/http/mcp.rs index 30ebd5a..cfb889e 100644 --- a/server/src/http/mcp.rs +++ b/server/src/http/mcp.rs @@ -1,4 +1,4 @@ -use crate::agents::{close_bug_to_archive, feature_branch_has_unmerged_changes, move_story_to_archived, move_story_to_merge, move_story_to_qa, AgentStatus, PipelineStage}; +use crate::agents::{close_bug_to_archive, feature_branch_has_unmerged_changes, move_story_to_archived, move_story_to_merge, move_story_to_qa, reject_story_from_qa, AgentStatus, PipelineStage}; use crate::config::ProjectConfig; use crate::log_buffer; use crate::slog; @@ -862,6 +862,52 @@ fn handle_tools_list(id: Option) -> JsonRpcResponse { "required": ["story_id"] } }, + { + "name": "approve_qa", + "description": "Approve a story that passed machine QA and is awaiting human review. Moves the story from work/3_qa/ to work/4_merge/ and starts the mergemaster agent.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Story identifier (e.g. '247_story_human_qa_gate')" + } + }, + "required": ["story_id"] + } + }, + { + "name": "reject_qa", + "description": "Reject a story during human QA review. Moves the story from work/3_qa/ back to work/2_current/ with rejection notes so the coder agent can fix the issues.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Story identifier (e.g. '247_story_human_qa_gate')" + }, + "notes": { + "type": "string", + "description": "Explanation of what is broken or needs fixing" + } + }, + "required": ["story_id", "notes"] + } + }, + { + "name": "launch_qa_app", + "description": "Launch the app from a story's worktree for manual QA testing. Automatically assigns a free port, writes it to .story_kit_port, and starts the backend server. Only one QA app instance runs at a time.", + "inputSchema": { + "type": "object", + "properties": { + "story_id": { + "type": "string", + "description": "Story identifier whose worktree app to launch" + } + }, + "required": ["story_id"] + } + }, { "name": "get_pipeline_status", "description": "Return a structured snapshot of the full work item pipeline. Includes all active stages (current, qa, merge, done) with each item's stage, name, and assigned agent. Also includes upcoming backlog items.", @@ -979,6 +1025,9 @@ async fn handle_tools_call( "report_merge_failure" => tool_report_merge_failure(&args, ctx), // QA tools "request_qa" => tool_request_qa(&args, ctx).await, + "approve_qa" => tool_approve_qa(&args, ctx).await, + "reject_qa" => tool_reject_qa(&args, ctx).await, + "launch_qa_app" => tool_launch_qa_app(&args, ctx).await, // Pipeline status "get_pipeline_status" => tool_get_pipeline_status(ctx), // Diagnostics @@ -1947,6 +1996,159 @@ async fn tool_request_qa(args: &Value, ctx: &AppContext) -> Result Result { + 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(".story_kit/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"); + } + + // Move story from work/3_qa/ to work/4_merge/ + 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) + .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}")) +} + +async fn tool_reject_qa(args: &Value, ctx: &AppContext) -> Result { + 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(".story_kit/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)) + .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." + )) +} + +async fn tool_launch_qa_app(args: &Value, ctx: &AppContext) -> Result { + 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 .story_kit_port so the frontend dev server knows where to connect + let port_file = wt_path.join(".story_kit_port"); + std::fs::write(&port_file, port.to_string()) + .map_err(|e| format!("Failed to write .story_kit_port: {e}"))?; + + // Launch the server from the worktree + let child = std::process::Command::new("cargo") + .args(["run"]) + .env("STORYKIT_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`. +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 +} + /// Run `git log ..HEAD --oneline` in the worktree and return the commit /// summaries, or `None` if git is unavailable or there are no new commits. async fn get_worktree_commits(worktree_path: &str, base_branch: &str) -> Option> { @@ -2383,11 +2585,14 @@ mod tests { assert!(names.contains(&"move_story_to_merge")); assert!(names.contains(&"report_merge_failure")); assert!(names.contains(&"request_qa")); + assert!(names.contains(&"approve_qa")); + assert!(names.contains(&"reject_qa")); + assert!(names.contains(&"launch_qa_app")); assert!(names.contains(&"get_server_logs")); assert!(names.contains(&"prompt_permission")); assert!(names.contains(&"get_pipeline_status")); assert!(names.contains(&"rebuild_and_restart")); - assert_eq!(tools.len(), 36); + assert_eq!(tools.len(), 39); } #[test] @@ -3934,6 +4139,80 @@ stage = "coder" assert!(!req_names.contains(&"agent_name")); } + // ── approve_qa in tools list ────────────────────────────────── + + #[test] + fn approve_qa_in_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")); + } + + // ── reject_qa in tools list ────────────────────────────────── + + #[test] + fn reject_qa_in_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")); + } + + // ── launch_qa_app in tools list ────────────────────────────── + + #[test] + fn launch_qa_app_in_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")); + } + + // ── approve_qa missing 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")); + } + + // ── reject_qa missing arguments ────────────────────────────── + + #[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")); + } + // ── tool_validate_stories with file content ─────────────────── #[test] diff --git a/server/src/http/workflow.rs b/server/src/http/workflow.rs index ae1f6f9..45f16d2 100644 --- a/server/src/http/workflow.rs +++ b/server/src/http/workflow.rs @@ -24,6 +24,12 @@ pub struct UpcomingStory { pub merge_failure: Option, /// Active agent working on this item, if any. pub agent: Option, + /// True when the item is held in QA for human review. + #[serde(skip_serializing_if = "Option::is_none")] + pub review_hold: Option, + /// Whether the item requires manual QA (defaults to true when absent). + #[serde(skip_serializing_if = "Option::is_none")] + pub manual_qa: Option, } pub struct StoryValidationResult { @@ -117,12 +123,12 @@ fn load_stage_items( .to_string(); let contents = fs::read_to_string(&path) .map_err(|e| format!("Failed to read story file {}: {e}", path.display()))?; - let (name, error, merge_failure) = match parse_front_matter(&contents) { - Ok(meta) => (meta.name, None, meta.merge_failure), - Err(e) => (None, Some(e.to_string()), None), + let (name, error, merge_failure, review_hold, manual_qa) = match parse_front_matter(&contents) { + Ok(meta) => (meta.name, None, meta.merge_failure, meta.review_hold, meta.manual_qa), + Err(e) => (None, Some(e.to_string()), None, None, None), }; let agent = agent_map.get(&story_id).cloned(); - stories.push(UpcomingStory { story_id, name, error, merge_failure, agent }); + stories.push(UpcomingStory { story_id, name, error, merge_failure, agent, review_hold, manual_qa }); } stories.sort_by(|a, b| a.story_id.cmp(&b.story_id)); diff --git a/server/src/http/ws.rs b/server/src/http/ws.rs index 65ceda4..3028385 100644 --- a/server/src/http/ws.rs +++ b/server/src/http/ws.rs @@ -693,6 +693,8 @@ mod tests { error: None, merge_failure: None, agent: None, + review_hold: None, + manual_qa: None, }; let resp = WsResponse::PipelineState { backlog: vec![story], @@ -830,6 +832,8 @@ mod tests { error: None, merge_failure: None, agent: None, + review_hold: None, + manual_qa: None, }], current: vec![UpcomingStory { story_id: "2_story_b".to_string(), @@ -837,6 +841,8 @@ mod tests { error: None, merge_failure: None, agent: None, + review_hold: None, + manual_qa: None, }], qa: vec![], merge: vec![], @@ -846,6 +852,8 @@ mod tests { error: None, merge_failure: None, agent: None, + review_hold: None, + manual_qa: None, }], }; let resp: WsResponse = state.into(); @@ -1002,6 +1010,8 @@ mod tests { model: Some("claude-3-5-sonnet".to_string()), status: "running".to_string(), }), + review_hold: None, + manual_qa: None, }], qa: vec![], merge: vec![], diff --git a/server/src/io/story_metadata.rs b/server/src/io/story_metadata.rs index af27573..51e7f9c 100644 --- a/server/src/io/story_metadata.rs +++ b/server/src/io/story_metadata.rs @@ -9,6 +9,7 @@ pub struct StoryMetadata { pub merge_failure: Option, pub agent: Option, pub review_hold: Option, + pub manual_qa: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -33,6 +34,7 @@ struct FrontMatter { merge_failure: Option, agent: Option, review_hold: Option, + manual_qa: Option, } pub fn parse_front_matter(contents: &str) -> Result { @@ -67,6 +69,7 @@ fn build_metadata(front: FrontMatter) -> StoryMetadata { merge_failure: front.merge_failure, agent: front.agent, review_hold: front.review_hold, + manual_qa: front.manual_qa, } } @@ -193,6 +196,32 @@ pub fn set_front_matter_field(contents: &str, key: &str, value: &str) -> String result } +/// Append rejection notes to a story file body. +/// +/// Adds a `## QA Rejection Notes` section at the end of the file so the coder +/// agent can see what needs fixing. +pub fn write_rejection_notes(path: &Path, notes: &str) -> Result<(), String> { + let contents = + fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?; + + let section = format!("\n\n## QA Rejection Notes\n\n{notes}\n"); + let updated = format!("{contents}{section}"); + fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?; + Ok(()) +} + +/// Check whether a story requires manual QA (defaults to true). +pub fn requires_manual_qa(path: &Path) -> bool { + let contents = match fs::read_to_string(path) { + Ok(c) => c, + Err(_) => return true, + }; + match parse_front_matter(&contents) { + Ok(meta) => meta.manual_qa.unwrap_or(true), + Err(_) => true, + } +} + pub fn parse_unchecked_todos(contents: &str) -> Vec { contents .lines() @@ -367,4 +396,45 @@ workflow: tdd assert!(contents.contains("review_hold: true")); assert!(contents.contains("name: My Spike")); } + + #[test] + fn parses_manual_qa_from_front_matter() { + let input = "---\nname: Story\nmanual_qa: false\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.manual_qa, Some(false)); + } + + #[test] + fn manual_qa_defaults_to_none() { + let input = "---\nname: Story\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.manual_qa, None); + } + + #[test] + fn requires_manual_qa_defaults_true() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("story.md"); + std::fs::write(&path, "---\nname: Test\n---\n# Story\n").unwrap(); + assert!(requires_manual_qa(&path)); + } + + #[test] + fn requires_manual_qa_false_when_set() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("story.md"); + std::fs::write(&path, "---\nname: Test\nmanual_qa: false\n---\n# Story\n").unwrap(); + assert!(!requires_manual_qa(&path)); + } + + #[test] + fn write_rejection_notes_appends_section() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("story.md"); + std::fs::write(&path, "---\nname: Test\n---\n# Story\n").unwrap(); + write_rejection_notes(&path, "Button color is wrong").unwrap(); + let contents = std::fs::read_to_string(&path).unwrap(); + assert!(contents.contains("## QA Rejection Notes")); + assert!(contents.contains("Button color is wrong")); + } } diff --git a/server/src/main.rs b/server/src/main.rs index 32ea4ad..a33a4e5 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -188,6 +188,7 @@ async fn main() -> Result<(), std::io::Error> { reconciliation_tx, perm_tx, perm_rx, + qa_app_process: Arc::new(std::sync::Mutex::new(None)), }; let app = build_routes(ctx);