story-kit: merge 306_story_replace_manual_qa_boolean_with_configurable_qa_mode_field
This commit is contained in:
Dave
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user