dave/storkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

story-kit: merge 247_story_human_qa_gate_with_rejection_flow

Browse Source
This commit is contained in:
Dave
2026-03-18 15:45:45 +00:00
parent 1faacd7812
commit 9352443555
11 changed files with 557 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

95
server/src/agents/lifecycle.rs
View File

@@ -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(&current_dir)
.map_err(|e| format!("Failed to create work/2_current/ directory: {e}"))?;
std::fs::rename(&qa_path, &current_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(&current_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(&current_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(&current_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(&current_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());
}
}

2
server/src/agents/mod.rs
View File

@@ -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;

65
server/src/agents/pool.rs
View File

@@ -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(