huskies: merge 738_refactor_delete_fs_shadow_code_from_lifecycle_rs_and_the_work_directory_watcher
This commit is contained in:
@@ -1,4 +1,7 @@
|
||||
//! Story lifecycle helpers — file creation, archival, and stage transitions for pipeline items.
|
||||
//! Story lifecycle helpers — archival and stage transitions for pipeline items.
|
||||
//!
|
||||
//! All pipeline state lives in the CRDT. These functions never consult the
|
||||
//! filesystem for work-item data — CRDT lookup failures propagate as errors.
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
@@ -25,7 +28,6 @@ pub(crate) fn item_type_from_id(item_id: &str) -> &'static str {
|
||||
/// `sources` stages, then updates the stage. Optionally clears front-matter
|
||||
/// fields from the stored content. Returns the source stage on success.
|
||||
fn move_item<'a>(
|
||||
_project_root: &Path,
|
||||
story_id: &str,
|
||||
sources: &'a [&'a str],
|
||||
target_dir: &str,
|
||||
@@ -81,20 +83,6 @@ fn move_item<'a>(
|
||||
return Ok(Some(src_dir));
|
||||
}
|
||||
|
||||
// Item not found in CRDT — check the content store as a migration
|
||||
// fallback. This handles items that were imported into the DB but
|
||||
// haven't yet been replicated into the CRDT layer. Unlike the old
|
||||
// filesystem fallback (removed — see story 517), this path reads
|
||||
// from the authoritative in-memory DB and cannot cause state drift.
|
||||
if let Some(mut content) = crate::db::read_content(story_id) {
|
||||
for field in fields_to_clear {
|
||||
content = clear_front_matter_field_in_content(&content, field);
|
||||
}
|
||||
crate::db::write_item_with_content(story_id, target_dir, &content);
|
||||
slog!("[lifecycle] Moved '{story_id}' to work/{target_dir}/ (content store fallback)");
|
||||
return Ok(Some(sources[0]));
|
||||
}
|
||||
|
||||
if missing_ok {
|
||||
slog!("[lifecycle] Work item '{story_id}' not found; skipping move to work/{target_dir}/");
|
||||
return Ok(None);
|
||||
@@ -114,17 +102,8 @@ fn move_item<'a>(
|
||||
/// etc.) are left untouched. This prevents coders from accidentally demoting a story
|
||||
/// that has already advanced past the coding stage.
|
||||
/// Idempotent: if already in `2_current/`, returns Ok. If not found, logs and returns Ok.
|
||||
pub fn move_story_to_current(project_root: &Path, story_id: &str) -> Result<(), String> {
|
||||
move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&["1_backlog"],
|
||||
"2_current",
|
||||
&[],
|
||||
true,
|
||||
&[],
|
||||
)
|
||||
.map(|_| ())
|
||||
pub fn move_story_to_current(story_id: &str) -> Result<(), String> {
|
||||
move_item(story_id, &["1_backlog"], "2_current", &[], true, &[]).map(|_| ())
|
||||
}
|
||||
|
||||
/// Check whether a feature branch `feature/story-{story_id}` exists and has
|
||||
@@ -162,9 +141,8 @@ pub fn feature_branch_has_unmerged_changes(project_root: &Path, story_id: &str)
|
||||
///
|
||||
/// Idempotent if already in `5_done/` or `6_archived/`. Errors if not found in any earlier stage.
|
||||
/// Spikes may transition directly from `3_qa/` to `5_done/`, skipping the merge stage.
|
||||
pub fn move_story_to_done(project_root: &Path, story_id: &str) -> Result<(), String> {
|
||||
pub fn move_story_to_done(story_id: &str) -> Result<(), String> {
|
||||
move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&["2_current", "3_qa", "4_merge"],
|
||||
"5_done",
|
||||
@@ -178,9 +156,8 @@ pub fn move_story_to_done(project_root: &Path, story_id: &str) -> Result<(), Str
|
||||
/// Move a story/bug from `work/2_current/` or `work/3_qa/` to `work/4_merge/`.
|
||||
///
|
||||
/// Idempotent if already in `4_merge/`. Errors if not found in `2_current/` or `3_qa/`.
|
||||
pub fn move_story_to_merge(project_root: &Path, story_id: &str) -> Result<(), String> {
|
||||
pub fn move_story_to_merge(story_id: &str) -> Result<(), String> {
|
||||
move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&["2_current", "3_qa"],
|
||||
"4_merge",
|
||||
@@ -194,9 +171,8 @@ pub fn move_story_to_merge(project_root: &Path, story_id: &str) -> Result<(), St
|
||||
/// Move a story/bug from `work/2_current/` to `work/3_qa/`.
|
||||
///
|
||||
/// Idempotent if already in `3_qa/`. Errors if not found in `2_current/`.
|
||||
pub fn move_story_to_qa(project_root: &Path, story_id: &str) -> Result<(), String> {
|
||||
pub fn move_story_to_qa(story_id: &str) -> Result<(), String> {
|
||||
move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&["2_current"],
|
||||
"3_qa",
|
||||
@@ -208,13 +184,8 @@ pub fn move_story_to_qa(project_root: &Path, story_id: &str) -> Result<(), Strin
|
||||
}
|
||||
|
||||
/// Move a story from `work/3_qa/` back to `work/2_current/`, clearing `review_hold` and writing notes.
|
||||
pub fn reject_story_from_qa(
|
||||
project_root: &Path,
|
||||
story_id: &str,
|
||||
notes: &str,
|
||||
) -> Result<(), String> {
|
||||
pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> {
|
||||
let moved = move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&["3_qa"],
|
||||
"2_current",
|
||||
@@ -240,11 +211,7 @@ pub fn reject_story_from_qa(
|
||||
/// Accepts `target_stage` as one of: `backlog`, `current`, `qa`, `merge`, `done`.
|
||||
/// Idempotent: if the item is already in the target stage, returns Ok.
|
||||
/// Returns `(from_stage, to_stage)` on success.
|
||||
pub fn move_story_to_stage(
|
||||
project_root: &Path,
|
||||
story_id: &str,
|
||||
target_stage: &str,
|
||||
) -> Result<(String, String), String> {
|
||||
pub fn move_story_to_stage(story_id: &str, target_stage: &str) -> Result<(String, String), String> {
|
||||
const STAGES: &[(&str, &str)] = &[
|
||||
("backlog", "1_backlog"),
|
||||
("current", "2_current"),
|
||||
@@ -267,16 +234,8 @@ pub fn move_story_to_stage(
|
||||
|
||||
let all_dirs: Vec<&str> = STAGES.iter().map(|(_, dir)| *dir).collect();
|
||||
|
||||
match move_item(
|
||||
project_root,
|
||||
story_id,
|
||||
&all_dirs,
|
||||
target_dir,
|
||||
&[],
|
||||
false,
|
||||
&[],
|
||||
)
|
||||
.map_err(|_| format!("Work item '{story_id}' not found in any pipeline stage."))?
|
||||
match move_item(story_id, &all_dirs, target_dir, &[], false, &[])
|
||||
.map_err(|_| format!("Work item '{story_id}' not found in any pipeline stage."))?
|
||||
{
|
||||
Some(src_dir) => {
|
||||
let from_stage = STAGES
|
||||
@@ -293,9 +252,8 @@ pub fn move_story_to_stage(
|
||||
/// Move a bug from `work/2_current/` or `work/1_backlog/` to `work/5_done/`.
|
||||
///
|
||||
/// Idempotent if already in `5_done/`. Errors if not found in `2_current/` or `1_backlog/`.
|
||||
pub fn close_bug_to_archive(project_root: &Path, bug_id: &str) -> Result<(), String> {
|
||||
pub fn close_bug_to_archive(bug_id: &str) -> Result<(), String> {
|
||||
move_item(
|
||||
project_root,
|
||||
bug_id,
|
||||
&["2_current", "1_backlog"],
|
||||
"5_done",
|
||||
@@ -313,32 +271,57 @@ mod tests {
|
||||
// ── move_story_to_current tests ────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn move_story_to_current_from_content_store() {
|
||||
// Seed via the content store (the DB's in-memory representation).
|
||||
// CRDT is not initialised in unit tests, so move_item uses the
|
||||
// content-store fallback which re-imports to the target stage.
|
||||
fn move_story_to_current_from_crdt() {
|
||||
// Seed via CRDT — the sole source of truth for pipeline state.
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_content(
|
||||
crate::db::write_item_with_content(
|
||||
"99950_story_lifecycle",
|
||||
"1_backlog",
|
||||
"---\nname: Lifecycle Test\n---\n# Story\n",
|
||||
);
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
move_story_to_current(tmp.path(), "99950_story_lifecycle").unwrap();
|
||||
move_story_to_current("99950_story_lifecycle").unwrap();
|
||||
|
||||
// Verify the content store now has the item (imported at target stage).
|
||||
let content = crate::db::read_content("99950_story_lifecycle")
|
||||
.expect("item should be in content store after move");
|
||||
assert!(
|
||||
content.contains("Lifecycle Test"),
|
||||
"content should be preserved after move"
|
||||
// Verify the CRDT now has the item in 2_current.
|
||||
let item = crate::pipeline_state::read_typed("99950_story_lifecycle")
|
||||
.expect("CRDT read should succeed")
|
||||
.expect("item should exist in CRDT after move");
|
||||
assert_eq!(
|
||||
item.stage.dir_name(),
|
||||
"2_current",
|
||||
"item should be in 2_current after move"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_story_to_current_noop_when_not_found() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
assert!(move_story_to_current(tmp.path(), "99_missing").is_ok());
|
||||
assert!(move_story_to_current("99_missing").is_ok());
|
||||
}
|
||||
|
||||
/// Lifecycle operation runs to completion using only CRDT state;
|
||||
/// no `.huskies/work/<stage>/` tree is consulted because no `project_root`
|
||||
/// is passed — the functions operate purely on the CRDT.
|
||||
#[test]
|
||||
fn move_story_uses_only_crdt_no_fs_shadow() {
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_item_with_content(
|
||||
"99951_story_crdt_only",
|
||||
"2_current",
|
||||
"---\nname: CRDT Only Test\n---\n# Story\n",
|
||||
);
|
||||
|
||||
// No filesystem path is involved — lifecycle functions no longer
|
||||
// accept a project_root, proving they never touch the filesystem.
|
||||
move_story_to_done("99951_story_crdt_only").unwrap();
|
||||
|
||||
let item = crate::pipeline_state::read_typed("99951_story_crdt_only")
|
||||
.expect("CRDT read should succeed")
|
||||
.expect("item should exist in CRDT");
|
||||
assert_eq!(
|
||||
item.stage.dir_name(),
|
||||
"5_done",
|
||||
"item should be in 5_done after move"
|
||||
);
|
||||
}
|
||||
|
||||
// ── item_type_from_id tests ────────────────────────────────────────────────
|
||||
|
||||
@@ -64,8 +64,7 @@ impl AgentPool {
|
||||
}
|
||||
// All deps met — promote from backlog to current.
|
||||
slog!("[auto-assign] Story '{story_id}' deps met; promoting from backlog to current.");
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_current(project_root, story_id)
|
||||
{
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_current(story_id) {
|
||||
slog!("[auto-assign] Failed to promote '{story_id}' to current: {e}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -168,7 +168,7 @@ impl AgentPool {
|
||||
|
||||
match qa_mode {
|
||||
crate::io::story_metadata::QaMode::Server => {
|
||||
if let Err(e) = crate::agents::move_story_to_merge(project_root, story_id) {
|
||||
if let Err(e) = crate::agents::move_story_to_merge(story_id) {
|
||||
eprintln!(
|
||||
"[startup:reconcile] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||
);
|
||||
@@ -189,7 +189,7 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
crate::io::story_metadata::QaMode::Agent => {
|
||||
if let Err(e) = crate::agents::move_story_to_qa(project_root, story_id) {
|
||||
if let Err(e) = crate::agents::move_story_to_qa(story_id) {
|
||||
eprintln!(
|
||||
"[startup:reconcile] Failed to move '{story_id}' to 3_qa/: {e}"
|
||||
);
|
||||
@@ -208,7 +208,7 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
crate::io::story_metadata::QaMode::Human => {
|
||||
if let Err(e) = crate::agents::move_story_to_qa(project_root, story_id) {
|
||||
if let Err(e) = crate::agents::move_story_to_qa(story_id) {
|
||||
eprintln!(
|
||||
"[startup:reconcile] Failed to move '{story_id}' to 3_qa/: {e}"
|
||||
);
|
||||
@@ -308,9 +308,7 @@ impl AgentPool {
|
||||
status: "review_hold".to_string(),
|
||||
message: "Passed QA — waiting for human review.".to_string(),
|
||||
});
|
||||
} else if let Err(e) =
|
||||
crate::agents::move_story_to_merge(project_root, story_id)
|
||||
{
|
||||
} else if let Err(e) = crate::agents::move_story_to_merge(story_id) {
|
||||
eprintln!(
|
||||
"[startup:reconcile] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||
);
|
||||
|
||||
@@ -71,10 +71,8 @@ impl AgentPool {
|
||||
"[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \
|
||||
qa: server — moving directly to merge."
|
||||
);
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_merge(
|
||||
&project_root,
|
||||
story_id,
|
||||
) {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_merge(story_id)
|
||||
{
|
||||
slog_error!(
|
||||
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||
);
|
||||
@@ -88,9 +86,7 @@ impl AgentPool {
|
||||
"[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \
|
||||
qa: agent — moving to QA."
|
||||
);
|
||||
if let Err(e) =
|
||||
crate::agents::lifecycle::move_story_to_qa(&project_root, story_id)
|
||||
{
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(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, None)
|
||||
@@ -106,9 +102,7 @@ impl AgentPool {
|
||||
"[pipeline] Coder '{agent_name}' passed gates for '{story_id}'. \
|
||||
qa: human — holding for human review."
|
||||
);
|
||||
if let Err(e) =
|
||||
crate::agents::lifecycle::move_story_to_qa(&project_root, story_id)
|
||||
{
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(story_id) {
|
||||
slog_error!("[pipeline] Failed to move '{story_id}' to 3_qa/: {e}");
|
||||
} else {
|
||||
write_review_hold_to_store(story_id);
|
||||
@@ -150,10 +144,9 @@ impl AgentPool {
|
||||
};
|
||||
match qa_mode {
|
||||
crate::io::story_metadata::QaMode::Server => {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_merge(
|
||||
&project_root,
|
||||
story_id,
|
||||
) {
|
||||
if let Err(e) =
|
||||
crate::agents::lifecycle::move_story_to_merge(story_id)
|
||||
{
|
||||
slog_error!(
|
||||
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||
);
|
||||
@@ -163,10 +156,8 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
crate::io::story_metadata::QaMode::Agent => {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(
|
||||
&project_root,
|
||||
story_id,
|
||||
) {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(story_id)
|
||||
{
|
||||
slog_error!(
|
||||
"[pipeline] Failed to move '{story_id}' to 3_qa/: {e}"
|
||||
);
|
||||
@@ -180,10 +171,8 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
crate::io::story_metadata::QaMode::Human => {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(
|
||||
&project_root,
|
||||
story_id,
|
||||
) {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_qa(story_id)
|
||||
{
|
||||
slog_error!(
|
||||
"[pipeline] Failed to move '{story_id}' to 3_qa/: {e}"
|
||||
);
|
||||
@@ -277,10 +266,8 @@ impl AgentPool {
|
||||
"[pipeline] QA passed gates and coverage for '{story_id}'. \
|
||||
Moving directly to merge."
|
||||
);
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_merge(
|
||||
&project_root,
|
||||
story_id,
|
||||
) {
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_merge(story_id)
|
||||
{
|
||||
slog_error!(
|
||||
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||
);
|
||||
@@ -391,9 +378,7 @@ impl AgentPool {
|
||||
slog!(
|
||||
"[pipeline] Post-merge tests passed for '{story_id}'. Moving to done."
|
||||
);
|
||||
if let Err(e) =
|
||||
crate::agents::lifecycle::move_story_to_done(&project_root, story_id)
|
||||
{
|
||||
if let Err(e) = crate::agents::lifecycle::move_story_to_done(story_id) {
|
||||
slog_error!("[pipeline] Failed to move '{story_id}' to done: {e}");
|
||||
}
|
||||
self.remove_agents_for_story(story_id);
|
||||
|
||||
@@ -57,7 +57,11 @@ async fn mergemaster_blocks_and_sends_story_blocked_when_no_commits_ahead() {
|
||||
)
|
||||
.unwrap();
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_content("9919_story_no_commits", "---\nname: Test\n---\n");
|
||||
crate::db::write_item_with_content(
|
||||
"9919_story_no_commits",
|
||||
"2_current",
|
||||
"---\nname: Test\n---\n",
|
||||
);
|
||||
|
||||
let pool = AgentPool::new_test(3001);
|
||||
let mut rx = pool.watcher_tx.subscribe();
|
||||
|
||||
@@ -155,8 +155,7 @@ impl AgentPool {
|
||||
});
|
||||
}
|
||||
|
||||
let story_archived =
|
||||
crate::agents::lifecycle::move_story_to_done(project_root, story_id).is_ok();
|
||||
let story_archived = crate::agents::lifecycle::move_story_to_done(story_id).is_ok();
|
||||
if story_archived {
|
||||
self.remove_agents_for_story(story_id);
|
||||
}
|
||||
|
||||
@@ -79,7 +79,7 @@ impl AgentPool {
|
||||
.map(|s| s == PipelineStage::Coder)
|
||||
.unwrap_or(true);
|
||||
if starting_a_coder {
|
||||
crate::agents::lifecycle::move_story_to_current(project_root, story_id)?;
|
||||
crate::agents::lifecycle::move_story_to_current(story_id)?;
|
||||
}
|
||||
|
||||
// Validate that the agent's configured stage matches the story's
|
||||
|
||||
@@ -143,7 +143,7 @@ mod tests {
|
||||
let story_content = "test";
|
||||
fs::write(current.join("60_story_cleanup.md"), story_content).unwrap();
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_content("60_story_cleanup", story_content);
|
||||
crate::db::write_item_with_content("60_story_cleanup", "2_current", story_content);
|
||||
|
||||
let pool = AgentPool::new_test(3001);
|
||||
pool.inject_test_agent("60_story_cleanup", "coder-1", AgentStatus::Completed);
|
||||
@@ -152,7 +152,7 @@ mod tests {
|
||||
|
||||
assert_eq!(pool.list_agents().unwrap().len(), 3);
|
||||
|
||||
move_story_to_done(root, "60_story_cleanup").unwrap();
|
||||
move_story_to_done("60_story_cleanup").unwrap();
|
||||
pool.remove_agents_for_story("60_story_cleanup");
|
||||
|
||||
let remaining = pool.list_agents().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user