From 2067abb2e5005a3f86605e7e0d117bc6133ed3aa Mon Sep 17 00:00:00 2001 From: Dave Date: Thu, 19 Mar 2026 11:56:39 +0000 Subject: [PATCH] story-kit: merge 306_story_replace_manual_qa_boolean_with_configurable_qa_mode_field --- .story_kit/project.toml | 4 + frontend/src/api/client.ts | 2 +- .../src/components/LozengeFlyContext.test.tsx | 56 ++-- frontend/src/components/StagePanel.test.tsx | 32 +-- server/src/agents/pool.rs | 249 +++++++++++++++--- server/src/config.rs | 21 ++ server/src/http/mcp.rs | 2 +- server/src/http/workflow.rs | 14 +- server/src/http/ws.rs | 10 +- server/src/io/fs.rs | 6 +- server/src/io/story_metadata.rs | 135 ++++++++-- server/src/worktree.rs | 12 + 12 files changed, 418 insertions(+), 125 deletions(-) diff --git a/.story_kit/project.toml b/.story_kit/project.toml index 3a152c6..5d51e69 100644 --- a/.story_kit/project.toml +++ b/.story_kit/project.toml @@ -1,3 +1,7 @@ +# Project-wide default QA mode: "server", "agent", or "human". +# Per-story `qa` front matter overrides this setting. +default_qa = "server" + [[component]] name = "frontend" path = "frontend" diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index e2ed9e1..41ba633 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -34,7 +34,7 @@ export interface PipelineStageItem { merge_failure: string | null; agent: AgentAssignment | null; review_hold: boolean | null; - manual_qa: boolean | null; + qa: string | null; } export interface PipelineState { diff --git a/frontend/src/components/LozengeFlyContext.test.tsx b/frontend/src/components/LozengeFlyContext.test.tsx index 57f12c6..b6961a7 100644 --- a/frontend/src/components/LozengeFlyContext.test.tsx +++ b/frontend/src/components/LozengeFlyContext.test.tsx @@ -60,7 +60,7 @@ describe("AgentLozenge fixed intrinsic width", () => { merge_failure: null, agent: { agent_name: "coder-1", model: "sonnet", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ]; const pipeline = makePipeline({ current: items }); @@ -114,7 +114,7 @@ describe("LozengeFlyProvider fly-in visibility", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -156,7 +156,7 @@ describe("LozengeFlyProvider fly-in visibility", () => { status: "running", }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -220,7 +220,7 @@ describe("LozengeFlyProvider fly-in clone", () => { merge_failure: null, agent: { agent_name: "coder-1", model: "sonnet", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -263,7 +263,7 @@ describe("LozengeFlyProvider fly-in clone", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -312,7 +312,7 @@ describe("LozengeFlyProvider fly-in clone", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -383,7 +383,7 @@ describe("LozengeFlyProvider fly-out", () => { merge_failure: null, agent: { agent_name: "coder-1", model: "haiku", status: "completed" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -410,7 +410,7 @@ describe("LozengeFlyProvider fly-out", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -444,7 +444,7 @@ describe("AgentLozenge idle vs active appearance", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ]; const { container } = render( @@ -470,7 +470,7 @@ describe("AgentLozenge idle vs active appearance", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "pending" }, review_hold: null, - manual_qa: null, + qa: null, }, ]; const { container } = render( @@ -496,7 +496,7 @@ describe("AgentLozenge idle vs active appearance", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ]; const { container } = render( @@ -549,7 +549,7 @@ describe("hiddenRosterAgents: assigned agents are absent from roster", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -572,7 +572,7 @@ describe("hiddenRosterAgents: assigned agents are absent from roster", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -596,7 +596,7 @@ describe("hiddenRosterAgents: assigned agents are absent from roster", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -658,7 +658,7 @@ describe("hiddenRosterAgents: fly-out keeps agent hidden until clone lands", () merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "completed" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -671,7 +671,7 @@ describe("hiddenRosterAgents: fly-out keeps agent hidden until clone lands", () merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -715,7 +715,7 @@ describe("hiddenRosterAgents: fly-out keeps agent hidden until clone lands", () merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "completed" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -728,7 +728,7 @@ describe("hiddenRosterAgents: fly-out keeps agent hidden until clone lands", () merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -803,7 +803,7 @@ describe("LozengeFlyProvider agent swap (name change)", () => { merge_failure: null, agent: { agent_name: "coder-1", model: "sonnet", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -816,7 +816,7 @@ describe("LozengeFlyProvider agent swap (name change)", () => { merge_failure: null, agent: { agent_name: "coder-2", model: "haiku", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -902,7 +902,7 @@ describe("LozengeFlyProvider fly-out without roster element", () => { status: "completed", }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -915,7 +915,7 @@ describe("LozengeFlyProvider fly-out without roster element", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -988,7 +988,7 @@ describe("FlyingLozengeClone initial non-flying render", () => { merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -1065,7 +1065,7 @@ describe("Bug 137: no animation actions lost during rapid pipeline updates", () merge_failure: null, agent: { agent_name: "coder-1", model: "sonnet", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -1078,7 +1078,7 @@ describe("Bug 137: no animation actions lost during rapid pipeline updates", () merge_failure: null, agent: { agent_name: "coder-2", model: "haiku", status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -1146,7 +1146,7 @@ describe("Bug 137: no animation actions lost during rapid pipeline updates", () merge_failure: null, agent: { agent_name: "coder-1", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -1159,7 +1159,7 @@ describe("Bug 137: no animation actions lost during rapid pipeline updates", () merge_failure: null, agent: { agent_name: "coder-2", model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); @@ -1246,7 +1246,7 @@ describe("Bug 137: animations remain functional through sustained agent activity merge_failure: null, agent: { agent_name: agentName, model: null, status: "running" }, review_hold: null, - manual_qa: null, + qa: null, }, ], }); diff --git a/frontend/src/components/StagePanel.test.tsx b/frontend/src/components/StagePanel.test.tsx index dcbedb3..36e4f91 100644 --- a/frontend/src/components/StagePanel.test.tsx +++ b/frontend/src/components/StagePanel.test.tsx @@ -18,7 +18,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -40,7 +40,7 @@ describe("StagePanel", () => { status: "running", }, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -61,7 +61,7 @@ describe("StagePanel", () => { status: "running", }, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -81,7 +81,7 @@ describe("StagePanel", () => { status: "pending", }, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -97,7 +97,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -113,7 +113,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -129,7 +129,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -147,7 +147,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -165,7 +165,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -183,7 +183,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -201,7 +201,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -222,7 +222,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -240,7 +240,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -258,7 +258,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -276,7 +276,7 @@ describe("StagePanel", () => { merge_failure: "Squash merge failed: conflicts in Cargo.lock", agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); @@ -297,7 +297,7 @@ describe("StagePanel", () => { merge_failure: null, agent: null, review_hold: null, - manual_qa: null, + qa: null, }, ]; render(); diff --git a/server/src/agents/pool.rs b/server/src/agents/pool.rs index 059db77..541abfc 100644 --- a/server/src/agents/pool.rs +++ b/server/src/agents/pool.rs @@ -854,16 +854,75 @@ impl AgentPool { } PipelineStage::Coder => { if completion.gates_passed { - slog!( - "[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. Moving to QA." - ); - if let Err(e) = super::lifecycle::move_story_to_qa(&project_root, story_id) { - slog_error!("[pipeline] Failed to move '{story_id}' to 3_qa/: {e}"); - } else if let Err(e) = self - .start_agent(&project_root, story_id, Some("qa"), None) - .await - { - slog_error!("[pipeline] Failed to start qa agent for '{story_id}': {e}"); + // Determine effective QA mode for this story. + let qa_mode = { + let item_type = super::lifecycle::item_type_from_id(story_id); + if item_type == "spike" { + crate::io::story_metadata::QaMode::Human + } else { + let default_qa = config.default_qa_mode(); + // Story is in 2_current/ when a coder completes. + let story_path = project_root + .join(".story_kit/work/2_current") + .join(format!("{story_id}.md")); + crate::io::story_metadata::resolve_qa_mode(&story_path, default_qa) + } + }; + + match qa_mode { + crate::io::story_metadata::QaMode::Server => { + slog!( + "[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \ + qa: server — 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}" + ); + } else if let Err(e) = self + .start_agent(&project_root, story_id, Some("mergemaster"), None) + .await + { + slog_error!( + "[pipeline] Failed to start mergemaster for '{story_id}': {e}" + ); + } + } + crate::io::story_metadata::QaMode::Agent => { + slog!( + "[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \ + qa: agent — moving to QA." + ); + if let Err(e) = super::lifecycle::move_story_to_qa(&project_root, story_id) { + slog_error!("[pipeline] Failed to move '{story_id}' to 3_qa/: {e}"); + } else if let Err(e) = self + .start_agent(&project_root, story_id, Some("qa"), None) + .await + { + slog_error!("[pipeline] Failed to start qa agent for '{story_id}': {e}"); + } + } + crate::io::story_metadata::QaMode::Human => { + slog!( + "[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \ + qa: human — holding for human review." + ); + if let Err(e) = super::lifecycle::move_story_to_qa(&project_root, story_id) { + slog_error!("[pipeline] Failed to move '{story_id}' to 3_qa/: {e}"); + } else { + let qa_dir = project_root.join(".story_kit/work/3_qa"); + 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}" + ); + } + } + } } } else { slog!( @@ -911,10 +970,13 @@ impl AgentPool { if item_type == "spike" { true // Spikes always need human review. } else { - // Stories/bugs: check the manual_qa front matter field (defaults to false). 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) + let default_qa = config.default_qa_mode(); + matches!( + crate::io::story_metadata::resolve_qa_mode(&story_path, default_qa), + crate::io::story_metadata::QaMode::Human + ) } }; @@ -937,7 +999,7 @@ impl AgentPool { } else { slog!( "[pipeline] QA passed gates and coverage for '{story_id}'. \ - manual_qa: false — moving directly to merge." + Moving directly to merge." ); if let Err(e) = super::lifecycle::move_story_to_merge(&project_root, story_id) @@ -1730,21 +1792,82 @@ impl AgentPool { eprintln!("[startup:reconcile] Gates passed for '{story_id}' (stage: {stage_dir}/)."); if stage_dir == "2_current" { - // Coder stage → advance to QA. - if let Err(e) = super::lifecycle::move_story_to_qa(project_root, story_id) { - eprintln!("[startup:reconcile] Failed to move '{story_id}' to 3_qa/: {e}"); - let _ = progress_tx.send(ReconciliationEvent { - story_id: story_id.clone(), - status: "failed".to_string(), - message: format!("Failed to advance to QA: {e}"), - }); - } else { - eprintln!("[startup:reconcile] Moved '{story_id}' → 3_qa/."); - let _ = progress_tx.send(ReconciliationEvent { - story_id: story_id.clone(), - status: "advanced".to_string(), - message: "Gates passed — moved to QA.".to_string(), - }); + // Coder stage — determine qa mode to decide next step. + let qa_mode = { + let item_type = super::lifecycle::item_type_from_id(story_id); + if item_type == "spike" { + crate::io::story_metadata::QaMode::Human + } else { + let default_qa = crate::config::ProjectConfig::load(project_root) + .unwrap_or_default() + .default_qa_mode(); + let story_path = project_root + .join(".story_kit/work/2_current") + .join(format!("{story_id}.md")); + crate::io::story_metadata::resolve_qa_mode(&story_path, default_qa) + } + }; + + match qa_mode { + crate::io::story_metadata::QaMode::Server => { + if let Err(e) = super::lifecycle::move_story_to_merge(project_root, story_id) { + eprintln!("[startup:reconcile] Failed to move '{story_id}' to 4_merge/: {e}"); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "failed".to_string(), + message: format!("Failed to advance to merge: {e}"), + }); + } else { + eprintln!("[startup:reconcile] Moved '{story_id}' → 4_merge/ (qa: server)."); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "advanced".to_string(), + message: "Gates passed — moved to merge (qa: server).".to_string(), + }); + } + } + crate::io::story_metadata::QaMode::Agent => { + if let Err(e) = super::lifecycle::move_story_to_qa(project_root, story_id) { + eprintln!("[startup:reconcile] Failed to move '{story_id}' to 3_qa/: {e}"); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "failed".to_string(), + message: format!("Failed to advance to QA: {e}"), + }); + } else { + eprintln!("[startup:reconcile] Moved '{story_id}' → 3_qa/."); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "advanced".to_string(), + message: "Gates passed — moved to QA.".to_string(), + }); + } + } + crate::io::story_metadata::QaMode::Human => { + if let Err(e) = super::lifecycle::move_story_to_qa(project_root, story_id) { + eprintln!("[startup:reconcile] Failed to move '{story_id}' to 3_qa/: {e}"); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "failed".to_string(), + message: format!("Failed to advance to QA: {e}"), + }); + } else { + 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(&story_path) { + eprintln!( + "[startup:reconcile] Failed to set review_hold on '{story_id}': {e}" + ); + } + eprintln!("[startup:reconcile] Moved '{story_id}' → 3_qa/ (qa: human — holding for review)."); + let _ = progress_tx.send(ReconciliationEvent { + story_id: story_id.clone(), + status: "review_hold".to_string(), + message: "Gates passed — holding for human review.".to_string(), + }); + } + } } } else if stage_dir == "3_qa" { // QA stage → run coverage gate before advancing to merge. @@ -1788,7 +1911,13 @@ impl AgentPool { 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) + let default_qa = crate::config::ProjectConfig::load(project_root) + .unwrap_or_default() + .default_qa_mode(); + matches!( + crate::io::story_metadata::resolve_qa_mode(&story_path, default_qa), + crate::io::story_metadata::QaMode::Human + ) } }; @@ -2681,18 +2810,17 @@ mod tests { // ── pipeline advance tests ──────────────────────────────────────────────── #[tokio::test] - async fn pipeline_advance_coder_gates_pass_moves_story_to_qa() { + async fn pipeline_advance_coder_gates_pass_server_qa_moves_to_merge() { use std::fs; let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - // Set up story in 2_current/ + // Set up story in 2_current/ (no qa frontmatter → uses project default "server") let current = root.join(".story_kit/work/2_current"); fs::create_dir_all(¤t).unwrap(); fs::write(current.join("50_story_test.md"), "test").unwrap(); let pool = AgentPool::new_test(3001); - // Call pipeline advance directly with completion data. pool.run_pipeline_advance( "50_story_test", "coder-1", @@ -2707,8 +2835,49 @@ mod tests { ) .await; - // Story should have moved to 3_qa/ (start_agent for qa will fail in tests but - // the file move happens before that). + // With default qa: server, story skips QA and goes straight to 4_merge/ + assert!( + root.join(".story_kit/work/4_merge/50_story_test.md") + .exists(), + "story should be in 4_merge/" + ); + assert!( + !current.join("50_story_test.md").exists(), + "story should not still be in 2_current/" + ); + } + + #[tokio::test] + async fn pipeline_advance_coder_gates_pass_agent_qa_moves_to_qa() { + use std::fs; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + + // Set up story in 2_current/ with qa: agent frontmatter + let current = root.join(".story_kit/work/2_current"); + fs::create_dir_all(¤t).unwrap(); + fs::write( + current.join("50_story_test.md"), + "---\nname: Test\nqa: agent\n---\ntest", + ) + .unwrap(); + + let pool = AgentPool::new_test(3001); + pool.run_pipeline_advance( + "50_story_test", + "coder-1", + CompletionReport { + summary: "done".to_string(), + gates_passed: true, + gate_output: String::new(), + }, + Some(root.to_path_buf()), + None, + false, + ) + .await; + + // With qa: agent, story should move to 3_qa/ assert!( root.join(".story_kit/work/3_qa/50_story_test.md").exists(), "story should be in 3_qa/" @@ -2728,10 +2897,10 @@ 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(); - // manual_qa: false so the story skips human review and goes straight to merge. + // qa: server 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", + "---\nname: Test\nqa: server\n---\ntest", ) .unwrap(); @@ -2815,6 +2984,8 @@ mod tests { fs::write( root.join(".story_kit/project.toml"), r#" +default_qa = "agent" + [[agent]] name = "coder-1" role = "Coder" @@ -4984,12 +5155,12 @@ stage = "qa" // simulating the "stuck" state from bug 295. fs::write( qa_dir.join("292_story_first.md"), - "---\nname: First\nmanual_qa: true\n---\n", + "---\nname: First\nqa: human\n---\n", ) .unwrap(); fs::write( qa_dir.join("293_story_second.md"), - "---\nname: Second\nmanual_qa: true\n---\n", + "---\nname: Second\nqa: human\n---\n", ) .unwrap(); @@ -5015,7 +5186,7 @@ stage = "qa" // Pipeline advance for QA with gates_passed=true will: // 1. Run coverage gate (will "pass" trivially in test — no script/test_coverage) - // 2. Set review_hold on 292 (manual_qa: true) + // 2. Set review_hold on 292 (qa: human) // 3. Call auto_assign_available_work (the fix from bug 295) // 4. auto_assign should find 293 in 3_qa/ with no agent and start qa on it pool.run_pipeline_advance( diff --git a/server/src/config.rs b/server/src/config.rs index bf6fb18..b25ed75 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -11,6 +11,10 @@ pub struct ProjectConfig { pub agent: Vec, #[serde(default)] pub watcher: WatcherConfig, + /// Project-wide default QA mode: "server", "agent", or "human". + /// Per-story `qa` front matter overrides this. Default: "server". + #[serde(default = "default_qa")] + pub default_qa: String, } /// Configuration for the filesystem watcher's sweep behaviour. @@ -46,6 +50,10 @@ fn default_done_retention_secs() -> u64 { 4 * 60 * 60 // 4 hours } +fn default_qa() -> String { + "server".to_string() +} + #[derive(Debug, Clone, Deserialize)] #[allow(dead_code)] pub struct ComponentConfig { @@ -124,6 +132,8 @@ struct LegacyProjectConfig { agent: Option, #[serde(default)] watcher: WatcherConfig, + #[serde(default = "default_qa")] + default_qa: String, } impl Default for ProjectConfig { @@ -145,6 +155,7 @@ impl Default for ProjectConfig { inactivity_timeout_secs: default_inactivity_timeout_secs(), }], watcher: WatcherConfig::default(), + default_qa: default_qa(), } } } @@ -186,6 +197,7 @@ impl ProjectConfig { component: legacy.component, agent: vec![agent], watcher: legacy.watcher, + default_qa: legacy.default_qa, }; validate_agents(&config.agent)?; return Ok(config); @@ -206,6 +218,7 @@ impl ProjectConfig { component: legacy.component, agent: vec![agent], watcher: legacy.watcher, + default_qa: legacy.default_qa, }; validate_agents(&config.agent)?; Ok(config) @@ -214,12 +227,20 @@ impl ProjectConfig { component: legacy.component, agent: Vec::new(), watcher: legacy.watcher, + default_qa: legacy.default_qa, }) } } } } + /// Return the project-wide default QA mode parsed from `default_qa`. + /// Falls back to `Server` if the value is unrecognised. + pub fn default_qa_mode(&self) -> crate::io::story_metadata::QaMode { + crate::io::story_metadata::QaMode::from_str(&self.default_qa) + .unwrap_or(crate::io::story_metadata::QaMode::Server) + } + /// Look up an agent config by name. pub fn find_agent(&self, name: &str) -> Option<&AgentConfig> { self.agent.iter().find(|a| a.name == name) diff --git a/server/src/http/mcp.rs b/server/src/http/mcp.rs index 85e28d6..8808a6f 100644 --- a/server/src/http/mcp.rs +++ b/server/src/http/mcp.rs @@ -639,7 +639,7 @@ fn handle_tools_list(id: Option) -> JsonRpcResponse { }, { "name": "update_story", - "description": "Update an existing story file. Can replace the '## User Story' and/or '## Description' section content, and/or set YAML front matter fields (e.g. agent, manual_qa). Auto-commits via the filesystem watcher.", + "description": "Update an existing story file. Can replace the '## User Story' and/or '## Description' section content, and/or set YAML front matter fields (e.g. agent, qa). Auto-commits via the filesystem watcher.", "inputSchema": { "type": "object", "properties": { diff --git a/server/src/http/workflow.rs b/server/src/http/workflow.rs index 45f16d2..67a03e8 100644 --- a/server/src/http/workflow.rs +++ b/server/src/http/workflow.rs @@ -27,9 +27,9 @@ pub struct UpcomingStory { /// 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). + /// QA mode for this item: "human", "server", or "agent". #[serde(skip_serializing_if = "Option::is_none")] - pub manual_qa: Option, + pub qa: Option, } pub struct StoryValidationResult { @@ -123,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, review_hold, manual_qa) = match parse_front_matter(&contents) { - Ok(meta) => (meta.name, None, meta.merge_failure, meta.review_hold, meta.manual_qa), + let (name, error, merge_failure, review_hold, qa) = match parse_front_matter(&contents) { + Ok(meta) => (meta.name, None, meta.merge_failure, meta.review_hold, meta.qa.map(|m| m.as_str().to_string())), 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, review_hold, manual_qa }); + stories.push(UpcomingStory { story_id, name, error, merge_failure, agent, review_hold, qa }); } stories.sort_by(|a, b| a.story_id.cmp(&b.story_id)); @@ -1691,12 +1691,12 @@ mod tests { fs::write(&filepath, "---\nname: T\n---\n\n## User Story\n\nSome story\n").unwrap(); let mut fields = HashMap::new(); - fields.insert("manual_qa".to_string(), "true".to_string()); + fields.insert("qa".to_string(), "human".to_string()); fields.insert("priority".to_string(), "high".to_string()); update_story_in_file(tmp.path(), "25_test", None, None, Some(&fields)).unwrap(); let result = fs::read_to_string(&filepath).unwrap(); - assert!(result.contains("manual_qa: \"true\""), "manual_qa field should be set"); + assert!(result.contains("qa: \"human\""), "qa field should be set"); assert!(result.contains("priority: \"high\""), "priority field should be set"); assert!(result.contains("name: T"), "name field preserved"); } diff --git a/server/src/http/ws.rs b/server/src/http/ws.rs index c144b08..df1a3c4 100644 --- a/server/src/http/ws.rs +++ b/server/src/http/ws.rs @@ -738,7 +738,7 @@ mod tests { merge_failure: None, agent: None, review_hold: None, - manual_qa: None, + qa: None, }; let resp = WsResponse::PipelineState { backlog: vec![story], @@ -877,7 +877,7 @@ mod tests { merge_failure: None, agent: None, review_hold: None, - manual_qa: None, + qa: None, }], current: vec![UpcomingStory { story_id: "2_story_b".to_string(), @@ -886,7 +886,7 @@ mod tests { merge_failure: None, agent: None, review_hold: None, - manual_qa: None, + qa: None, }], qa: vec![], merge: vec![], @@ -897,7 +897,7 @@ mod tests { merge_failure: None, agent: None, review_hold: None, - manual_qa: None, + qa: None, }], }; let resp: WsResponse = state.into(); @@ -1055,7 +1055,7 @@ mod tests { status: "running".to_string(), }), review_hold: None, - manual_qa: None, + qa: None, }], qa: vec![], merge: vec![], diff --git a/server/src/io/fs.rs b/server/src/io/fs.rs index 49ee0eb..da571d8 100644 --- a/server/src/io/fs.rs +++ b/server/src/io/fs.rs @@ -99,7 +99,11 @@ const STORY_KIT_CLAUDE_SETTINGS: &str = r#"{ } "#; -const DEFAULT_PROJECT_AGENTS_TOML: &str = r#"[[agent]] +const DEFAULT_PROJECT_AGENTS_TOML: &str = r#"# Project-wide default QA mode: "server", "agent", or "human". +# Per-story `qa` front matter overrides this setting. +default_qa = "server" + +[[agent]] name = "coder-1" stage = "coder" role = "Full-stack engineer. Implements features across all components." diff --git a/server/src/io/story_metadata.rs b/server/src/io/story_metadata.rs index 23129ae..074aa35 100644 --- a/server/src/io/story_metadata.rs +++ b/server/src/io/story_metadata.rs @@ -2,6 +2,45 @@ use serde::Deserialize; use std::fs; use std::path::Path; +/// QA mode for a story: determines how the pipeline handles post-coder review. +/// +/// - `Server` — skip the QA agent; rely on server gate checks (clippy + tests). +/// If gates pass, advance straight to merge. +/// - `Agent` — spin up a QA agent (Claude session) to review code and run gates. +/// - `Human` — hold in QA for human approval after server gates pass. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum QaMode { + Server, + Agent, + Human, +} + +impl QaMode { + /// Parse a string into a `QaMode`. Returns `None` for unrecognised values. + pub fn from_str(s: &str) -> Option { + match s.trim().to_lowercase().as_str() { + "server" => Some(Self::Server), + "agent" => Some(Self::Agent), + "human" => Some(Self::Human), + _ => None, + } + } + + pub fn as_str(&self) -> &'static str { + match self { + Self::Server => "server", + Self::Agent => "agent", + Self::Human => "human", + } + } +} + +impl std::fmt::Display for QaMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct StoryMetadata { pub name: Option, @@ -9,7 +48,7 @@ pub struct StoryMetadata { pub merge_failure: Option, pub agent: Option, pub review_hold: Option, - pub manual_qa: Option, + pub qa: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -34,6 +73,9 @@ struct FrontMatter { merge_failure: Option, agent: Option, review_hold: Option, + /// New configurable QA mode field: "human", "server", or "agent". + qa: Option, + /// Legacy boolean field — mapped to `qa: human` (true) or ignored (false/absent). manual_qa: Option, } @@ -63,13 +105,20 @@ pub fn parse_front_matter(contents: &str) -> Result StoryMetadata { + // Resolve qa mode: prefer the new `qa` field, fall back to legacy `manual_qa`. + let qa = if let Some(ref qa_str) = front.qa { + QaMode::from_str(qa_str) + } else { + front.manual_qa.and_then(|v| if v { Some(QaMode::Human) } else { None }) + }; + StoryMetadata { name: front.name, coverage_baseline: front.coverage_baseline, merge_failure: front.merge_failure, agent: front.agent, review_hold: front.review_hold, - manual_qa: front.manual_qa, + qa, } } @@ -210,15 +259,19 @@ pub fn write_rejection_notes(path: &Path, notes: &str) -> Result<(), String> { Ok(()) } -/// Check whether a story requires manual QA (defaults to false). -pub fn requires_manual_qa(path: &Path) -> bool { +/// Resolve the effective QA mode for a story file. +/// +/// Reads the `qa` front matter field. If absent, falls back to `default`. +/// Spikes are **not** handled here — the caller is responsible for overriding +/// to `Human` for spikes. +pub fn resolve_qa_mode(path: &Path, default: QaMode) -> QaMode { let contents = match fs::read_to_string(path) { Ok(c) => c, - Err(_) => return false, + Err(_) => return default, }; match parse_front_matter(&contents) { - Ok(meta) => meta.manual_qa.unwrap_or(false), - Err(_) => false, + Ok(meta) => meta.qa.unwrap_or(default), + Err(_) => default, } } @@ -398,41 +451,69 @@ workflow: tdd } #[test] - fn parses_manual_qa_from_front_matter() { - let input = "---\nname: Story\nmanual_qa: false\n---\n# Story\n"; + fn parses_qa_mode_from_front_matter() { + let input = "---\nname: Story\nqa: server\n---\n# Story\n"; let meta = parse_front_matter(input).expect("front matter"); - assert_eq!(meta.manual_qa, Some(false)); + assert_eq!(meta.qa, Some(QaMode::Server)); + + let input = "---\nname: Story\nqa: agent\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.qa, Some(QaMode::Agent)); + + let input = "---\nname: Story\nqa: human\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.qa, Some(QaMode::Human)); } #[test] - fn manual_qa_defaults_to_none() { + fn qa_mode_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); + assert_eq!(meta.qa, None); } #[test] - fn requires_manual_qa_defaults_false() { + fn legacy_manual_qa_true_maps_to_human() { + let input = "---\nname: Story\nmanual_qa: true\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.qa, Some(QaMode::Human)); + } + + #[test] + fn legacy_manual_qa_false_maps_to_none() { + let input = "---\nname: Story\nmanual_qa: false\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.qa, None); + } + + #[test] + fn qa_field_takes_precedence_over_manual_qa() { + let input = "---\nname: Story\nqa: server\nmanual_qa: true\n---\n# Story\n"; + let meta = parse_front_matter(input).expect("front matter"); + assert_eq!(meta.qa, Some(QaMode::Server)); + } + + #[test] + fn resolve_qa_mode_uses_file_value() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("story.md"); + std::fs::write(&path, "---\nname: Test\nqa: human\n---\n# Story\n").unwrap(); + assert_eq!(resolve_qa_mode(&path, QaMode::Server), QaMode::Human); + } + + #[test] + fn resolve_qa_mode_falls_back_to_default() { 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)); + assert_eq!(resolve_qa_mode(&path, QaMode::Server), QaMode::Server); + assert_eq!(resolve_qa_mode(&path, QaMode::Agent), QaMode::Agent); } #[test] - fn requires_manual_qa_true_when_set() { - let tmp = tempfile::tempdir().unwrap(); - let path = tmp.path().join("story.md"); - std::fs::write(&path, "---\nname: Test\nmanual_qa: true\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)); + fn resolve_qa_mode_missing_file_uses_default() { + let path = std::path::Path::new("/nonexistent/story.md"); + assert_eq!(resolve_qa_mode(path, QaMode::Server), QaMode::Server); } #[test] diff --git a/server/src/worktree.rs b/server/src/worktree.rs index 046ce64..6d9ed30 100644 --- a/server/src/worktree.rs +++ b/server/src/worktree.rs @@ -507,6 +507,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Should complete without panic run_setup_commands(tmp.path(), &config).await; @@ -524,6 +525,7 @@ mod tests { }], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Should complete without panic run_setup_commands(tmp.path(), &config).await; @@ -541,6 +543,7 @@ mod tests { }], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Setup command failures are non-fatal — should not panic or propagate run_setup_commands(tmp.path(), &config).await; @@ -558,6 +561,7 @@ mod tests { }], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Teardown failures are best-effort — should not propagate assert!(run_teardown_commands(tmp.path(), &config).await.is_ok()); @@ -574,6 +578,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; let info = create_worktree(&project_root, "42_fresh_test", &config, 3001) .await @@ -597,6 +602,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // First creation let _info1 = create_worktree(&project_root, "43_reuse_test", &config, 3001) @@ -636,6 +642,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; let result = remove_worktree_by_story_id(tmp.path(), "99_nonexistent", &config).await; @@ -658,6 +665,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; create_worktree(&project_root, "88_remove_by_id", &config, 3001) .await @@ -711,6 +719,7 @@ mod tests { }], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Even though setup commands fail, create_worktree must succeed // so the agent can start and fix the problem itself. @@ -736,6 +745,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // First creation — no setup commands, should succeed create_worktree(&project_root, "173_reuse_fail", &empty_config, 3001) @@ -751,6 +761,7 @@ mod tests { }], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; // Second call — worktree exists, setup commands fail, must still succeed let result = @@ -773,6 +784,7 @@ mod tests { component: vec![], agent: vec![], watcher: WatcherConfig::default(), + default_qa: "server".to_string(), }; let info = create_worktree(&project_root, "77_remove_async", &config, 3001) .await