story-kit: merge 265_story_spikes_skip_merge_and_stop_for_human_review
This commit is contained in:
@@ -4,8 +4,7 @@ use std::process::Command;
|
||||
use crate::io::story_metadata::clear_front_matter_field;
|
||||
use crate::slog;
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn item_type_from_id(item_id: &str) -> &'static str {
|
||||
pub(super) fn item_type_from_id(item_id: &str) -> &'static str {
|
||||
// New format: {digits}_{type}_{slug}
|
||||
let after_num = item_id.trim_start_matches(|c: char| c.is_ascii_digit());
|
||||
if after_num.starts_with("_bug_") {
|
||||
|
||||
@@ -889,6 +889,23 @@ 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.
|
||||
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) {
|
||||
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). \
|
||||
Worktree preserved at: {worktree_path:?}"
|
||||
);
|
||||
// Free up the QA slot without advancing the spike.
|
||||
self.auto_assign_available_work(&project_root).await;
|
||||
} else {
|
||||
slog!(
|
||||
"[pipeline] QA passed gates and coverage for '{story_id}'. Moving to merge."
|
||||
);
|
||||
@@ -904,6 +921,7 @@ impl AgentPool {
|
||||
}
|
||||
// QA slot is now free — pick up any other unassigned work in 3_qa/.
|
||||
self.auto_assign_available_work(&project_root).await;
|
||||
}
|
||||
} else {
|
||||
slog!(
|
||||
"[pipeline] QA coverage gate failed for '{story_id}'. Restarting QA."
|
||||
@@ -1444,6 +1462,12 @@ impl AgentPool {
|
||||
}
|
||||
|
||||
for story_id in &items {
|
||||
// Items marked with review_hold (e.g. spikes after QA passes) stay
|
||||
// in their current stage for human review — don't auto-assign agents.
|
||||
if has_review_hold(project_root, stage_dir, story_id) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Re-acquire the lock on each iteration to see state changes
|
||||
// from previous start_agent calls in the same pass.
|
||||
let preferred_agent =
|
||||
@@ -1707,7 +1731,25 @@ impl AgentPool {
|
||||
};
|
||||
|
||||
if coverage_passed {
|
||||
if let Err(e) = super::lifecycle::move_story_to_merge(project_root, story_id) {
|
||||
// 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
|
||||
.join(".story_kit/work/3_qa")
|
||||
.join(format!("{story_id}.md"));
|
||||
if let Err(e) = crate::io::story_metadata::write_review_hold(&spike_path) {
|
||||
eprintln!(
|
||||
"[startup:reconcile] Failed to set review_hold on spike '{story_id}': {e}"
|
||||
);
|
||||
}
|
||||
eprintln!(
|
||||
"[startup:reconcile] Spike '{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(),
|
||||
});
|
||||
} else 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}"
|
||||
);
|
||||
@@ -1922,6 +1964,24 @@ fn read_story_front_matter_agent(project_root: &Path, stage_dir: &str, story_id:
|
||||
parse_front_matter(&contents).ok()?.agent
|
||||
}
|
||||
|
||||
/// Return `true` if the story file in the given stage has `review_hold: true` in its front matter.
|
||||
fn has_review_hold(project_root: &Path, stage_dir: &str, story_id: &str) -> bool {
|
||||
use crate::io::story_metadata::parse_front_matter;
|
||||
let path = project_root
|
||||
.join(".story_kit")
|
||||
.join("work")
|
||||
.join(stage_dir)
|
||||
.join(format!("{story_id}.md"));
|
||||
let contents = match std::fs::read_to_string(path) {
|
||||
Ok(c) => c,
|
||||
Err(_) => return false,
|
||||
};
|
||||
parse_front_matter(&contents)
|
||||
.ok()
|
||||
.and_then(|m| m.review_hold)
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Return `true` if `agent_name` has no active (pending/running) entry in the pool.
|
||||
fn is_agent_free(agents: &HashMap<String, StoryAgent>, agent_name: &str) -> bool {
|
||||
!agents.values().any(|a| {
|
||||
@@ -4621,4 +4681,76 @@ stage = "coder"
|
||||
"story should be in 2_current/ or 3_qa/ after reconciliation"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn has_review_hold_returns_true_when_set() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let qa_dir = tmp.path().join(".story_kit/work/3_qa");
|
||||
std::fs::create_dir_all(&qa_dir).unwrap();
|
||||
let spike_path = qa_dir.join("10_spike_research.md");
|
||||
std::fs::write(
|
||||
&spike_path,
|
||||
"---\nname: Research spike\nreview_hold: true\n---\n# Spike\n",
|
||||
)
|
||||
.unwrap();
|
||||
assert!(has_review_hold(tmp.path(), "3_qa", "10_spike_research"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn has_review_hold_returns_false_when_not_set() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let qa_dir = tmp.path().join(".story_kit/work/3_qa");
|
||||
std::fs::create_dir_all(&qa_dir).unwrap();
|
||||
let spike_path = qa_dir.join("10_spike_research.md");
|
||||
std::fs::write(
|
||||
&spike_path,
|
||||
"---\nname: Research spike\n---\n# Spike\n",
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!has_review_hold(tmp.path(), "3_qa", "10_spike_research"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn has_review_hold_returns_false_when_file_missing() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
assert!(!has_review_hold(tmp.path(), "3_qa", "99_spike_missing"));
|
||||
}
|
||||
|
||||
/// Story 265: auto_assign_available_work must skip spikes in 3_qa/ that
|
||||
/// have review_hold: true set in their front matter.
|
||||
#[tokio::test]
|
||||
async fn auto_assign_skips_spikes_with_review_hold() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let root = tmp.path();
|
||||
|
||||
// Create project.toml with a QA agent.
|
||||
let sk = root.join(".story_kit");
|
||||
std::fs::create_dir_all(&sk).unwrap();
|
||||
std::fs::write(
|
||||
sk.join("project.toml"),
|
||||
"[[agents]]\nname = \"qa\"\nrole = \"qa\"\nmodel = \"test\"\nprompt = \"test\"\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Put a spike in 3_qa/ with review_hold: true.
|
||||
let qa_dir = root.join(".story_kit/work/3_qa");
|
||||
std::fs::create_dir_all(&qa_dir).unwrap();
|
||||
std::fs::write(
|
||||
qa_dir.join("20_spike_test.md"),
|
||||
"---\nname: Test Spike\nreview_hold: true\n---\n# Spike\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let (watcher_tx, _) = broadcast::channel::<WatcherEvent>(4);
|
||||
let pool = AgentPool::new(3001, watcher_tx);
|
||||
|
||||
pool.auto_assign_available_work(root).await;
|
||||
|
||||
// No agent should have been started for the spike.
|
||||
let agents = pool.agents.lock().unwrap();
|
||||
assert!(
|
||||
agents.is_empty(),
|
||||
"No agents should be assigned to a spike with review_hold"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ pub struct StoryMetadata {
|
||||
pub coverage_baseline: Option<String>,
|
||||
pub merge_failure: Option<String>,
|
||||
pub agent: Option<String>,
|
||||
pub review_hold: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -31,6 +32,7 @@ struct FrontMatter {
|
||||
coverage_baseline: Option<String>,
|
||||
merge_failure: Option<String>,
|
||||
agent: Option<String>,
|
||||
review_hold: Option<bool>,
|
||||
}
|
||||
|
||||
pub fn parse_front_matter(contents: &str) -> Result<StoryMetadata, StoryMetaError> {
|
||||
@@ -64,6 +66,7 @@ fn build_metadata(front: FrontMatter) -> StoryMetadata {
|
||||
coverage_baseline: front.coverage_baseline,
|
||||
merge_failure: front.merge_failure,
|
||||
agent: front.agent,
|
||||
review_hold: front.review_hold,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,6 +101,17 @@ pub fn write_merge_failure(path: &Path, reason: &str) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Write `review_hold: true` to the YAML front matter of a story file.
|
||||
///
|
||||
/// Used to mark spikes that have passed QA and are waiting for human review.
|
||||
pub fn write_review_hold(path: &Path) -> Result<(), String> {
|
||||
let contents =
|
||||
fs::read_to_string(path).map_err(|e| format!("Failed to read story file: {e}"))?;
|
||||
let updated = set_front_matter_field(&contents, "review_hold", "true");
|
||||
fs::write(path, &updated).map_err(|e| format!("Failed to write story file: {e}"))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Remove a key from the YAML front matter of a story file on disk.
|
||||
///
|
||||
/// If front matter is present and contains the key, the line is removed.
|
||||
@@ -328,4 +342,29 @@ workflow: tdd
|
||||
let input = " - [ ] Indented item\n";
|
||||
assert_eq!(parse_unchecked_todos(input), vec!["Indented item"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_review_hold_from_front_matter() {
|
||||
let input = "---\nname: Spike\nreview_hold: true\n---\n# Spike\n";
|
||||
let meta = parse_front_matter(input).expect("front matter");
|
||||
assert_eq!(meta.review_hold, Some(true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_hold_defaults_to_none() {
|
||||
let input = "---\nname: Story\n---\n# Story\n";
|
||||
let meta = parse_front_matter(input).expect("front matter");
|
||||
assert_eq!(meta.review_hold, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_review_hold_sets_field() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let path = tmp.path().join("spike.md");
|
||||
std::fs::write(&path, "---\nname: My Spike\n---\n# Spike\n").unwrap();
|
||||
write_review_hold(&path).unwrap();
|
||||
let contents = std::fs::read_to_string(&path).unwrap();
|
||||
assert!(contents.contains("review_hold: true"));
|
||||
assert!(contents.contains("name: My Spike"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user