crashlabs/huskies
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 27 Wiki Activity

feat(934): typed Stage enum replaces directory-string state model

Browse Source 
The state machine's `Stage` enum becomes the source of truth for pipeline
state. Six stages of work land together:

  1. Clean wire vocabulary (`coding`, `merge`, `merge_failure`, ...) replaces
     legacy directory-style strings (`2_current`, `4_merge`, ...) on the wire.
     `Stage::from_dir` accepted both during deployment; new writes always
     emit the clean form via `stage_dir_name`. Lexicographic `dir >= "5_done"`
     checks in lifecycle.rs become typed `matches!` checks since the new
     vocabulary doesn't sort in pipeline order.
  2. `crdt_state::write_item` takes typed `&Stage`, serialising via
     `stage_dir_name` at the CRDT boundary. `#[cfg(test)] write_item_str`
     parses legacy strings for test fixtures.
  3. `WorkItem::stage()` returns typed `crdt_state::Stage`; `stage_str()`
     is gone from the public API. Projection dispatches on the typed enum.
  4. `frozen` becomes an orthogonal CRDT register. `Stage::Frozen` and
     `PipelineEvent::Freeze`/`Unfreeze` are removed; `transition_to_frozen`/
     `unfrozen` set the flag directly without touching the stage register.
  5. Watcher sweep and `tool_update_story`'s `blocked` setter route through
     `apply_transition` so the typed transition table validates every
     stage change. `update_story` gains a `frozen` field for symmetry.
  6. One-shot startup migration rewrites pre-934 directory-style stage
     registers (and sets `frozen=true` on items previously at `7_frozen`).
     `Stage::from_dir` drops legacy aliases. The db boundary keeps a small
     normaliser so callers with legacy strings (MCP, tests) still work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Timmy
2026-05-12 22:31:59 +01:00
parent 93443e2ff1
commit d78dd9e8f9
55 changed files with 783 additions and 584 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/agents/lifecycle.rs
+37 -29
View File
@@ -103,10 +103,9 @@ pub fn feature_branch_has_unmerged_changes(project_root: &Path, story_id: &str)
/// Spikes may transition directly from `3_qa/` to `5_done/`, skipping the merge stage.
pub fn move_story_to_done(story_id: &str) -> Result<(), String> {
let item = read_typed_or_err(story_id)?;
let dir = item.stage.dir_name();
// Idempotent: already at or past done.
if dir >= "5_done" {
if matches!(item.stage, Stage::Done { .. } | Stage::Archived { .. }) {
return Ok(());
}
@@ -134,10 +133,15 @@ pub fn move_story_to_done(story_id: &str) -> Result<(), String> {
/// Idempotent if already in `4_merge/`. Errors if not found in `2_current/` or `3_qa/`.
pub fn move_story_to_merge(story_id: &str) -> Result<(), String> {
let item = read_typed_or_err(story_id)?;
let dir = item.stage.dir_name();
// Idempotent: already at or past merge.
if dir >= "4_merge" {
if matches!(
item.stage,
Stage::Merge { .. }
| Stage::MergeFailure { .. }
| Stage::Done { .. }
| Stage::Archived { .. }
) {
return Ok(());
}
@@ -170,10 +174,16 @@ pub fn move_story_to_merge(story_id: &str) -> Result<(), String> {
/// Idempotent if already in `3_qa/`. Errors if not found in `2_current/`.
pub fn move_story_to_qa(story_id: &str) -> Result<(), String> {
let item = read_typed_or_err(story_id)?;
let dir = item.stage.dir_name();
// Idempotent: already at or past qa.
if dir >= "3_qa" {
if matches!(
item.stage,
Stage::Qa
| Stage::Merge { .. }
| Stage::MergeFailure { .. }
| Stage::Done { .. }
| Stage::Archived { .. }
) {
return Ok(());
}
@@ -349,16 +359,19 @@ fn map_stage_move_to_event(
/// Move any work item to an arbitrary pipeline stage by searching all stages.
///
/// Accepts `target_stage` as one of: `backlog`, `current`, `qa`, `merge`, `done`.
/// (`current` is the user-facing alias for the `coding` stage.)
/// 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(story_id: &str, target_stage: &str) -> Result<(String, String), String> {
// Validate target.
let target_dir = match target_stage {
"backlog" => "1_backlog",
"current" => "2_current",
"qa" => "3_qa",
"merge" => "4_merge",
"done" => "5_done",
// Validate target. We accept the user-facing aliases (which include
// "current" as the historical alias for "coding") and normalise to the
// canonical clean wire form for the idempotency check.
let target_wire = match target_stage {
"backlog" => "backlog",
"current" => "coding",
"qa" => "qa",
"merge" => "merge",
"done" => "done",
_ => {
return Err(format!(
"Invalid target_stage '{target_stage}'. Must be one of: backlog, current, qa, merge, done"
@@ -370,7 +383,7 @@ pub fn move_story_to_stage(story_id: &str, target_stage: &str) -> Result<(String
let from_name = stage_to_name(&item.stage);
// Idempotent: already in the target stage.
if item.stage.dir_name() == target_dir {
if item.stage.dir_name() == target_wire {
return Ok((target_stage.to_string(), target_stage.to_string()));
}
@@ -387,7 +400,7 @@ pub fn move_story_to_stage(story_id: &str, target_stage: &str) -> Result<(String
pub fn close_bug_to_archive(bug_id: &str) -> Result<(), String> {
let item = read_typed_or_err(bug_id)?;
if item.stage.dir_name() >= "5_done" {
if matches!(item.stage, Stage::Done { .. } | Stage::Archived { .. }) {
return Ok(());
}
@@ -415,7 +428,6 @@ fn stage_to_name(s: &Stage) -> &'static str {
Stage::MergeFailure { .. } => "merge_failure",
Stage::Done { .. } => "done",
Stage::Archived { .. } => "archived",
Stage::Frozen { .. } => "frozen",
}
}
@@ -444,8 +456,8 @@ mod tests {
.expect("item should exist in CRDT after move");
assert_eq!(
item.stage.dir_name(),
"2_current",
"item should be in 2_current after move"
"coding",
"item should be in coding after move"
);
}
@@ -476,8 +488,8 @@ mod tests {
.expect("item should exist in CRDT");
assert_eq!(
item.stage.dir_name(),
"5_done",
"item should be in 5_done after move"
"done",
"item should be in done after move"
);
}
@@ -540,11 +552,7 @@ mod tests {
let item = crate::pipeline_state::read_typed("99866_story_block_test")
.expect("read should succeed")
.expect("item should exist");
assert_eq!(
item.stage.dir_name(),
"2_current",
"should start in 2_current"
);
assert_eq!(item.stage.dir_name(), "coding", "should start in coding");
// Block via the state machine.
transition_to_blocked("99866_story_block_test", "retry limit exceeded")
@@ -556,8 +564,8 @@ mod tests {
.expect("item should exist after block");
assert_eq!(
item.stage.dir_name(),
"2_blocked",
"should be in 2_blocked after transition_to_blocked"
"blocked",
"should be in blocked after transition_to_blocked"
);
assert!(item.stage.is_blocked(), "is_blocked() should return true");
assert!(
@@ -575,8 +583,8 @@ mod tests {
.expect("item should exist after unblock");
assert_eq!(
item.stage.dir_name(),
"2_current",
"should return to 2_current after unblock"
"coding",
"should return to coding after unblock"
);
assert!(
matches!(item.stage, Stage::Coding),
server/src/agents/pool/auto_assign/scan.rs
+21 -1
View File
@@ -22,9 +22,29 @@ pub(super) fn scan_stage_items(_project_root: &Path, stage_dir: &str) -> Vec<Str
use std::collections::BTreeSet;
let mut items = BTreeSet::new();
// Accept legacy directory-style strings (`"2_current"`, `"4_merge"`,
// etc.) at the boundary; `Stage::from_dir` itself is strict post-934
// stage 6, so we normalise here.
let normalised = match stage_dir {
"0_upcoming" => "upcoming",
"1_backlog" => "backlog",
"2_current" => "coding",
"2_blocked" => "blocked",
"3_qa" => "qa",
"4_merge" => "merge",
"4_merge_failure" => "merge_failure",
"5_done" => "done",
"6_archived" => "archived",
other => other,
};
let Some(want) = crate::pipeline_state::Stage::from_dir(normalised) else {
return Vec::new();
};
let want = want.dir_name();
// CRDT is the only source of truth — no filesystem fallback.
for item in crate::pipeline_state::read_all_typed() {
if item.stage.dir_name() == stage_dir {
if item.stage.dir_name() == want {
items.insert(item.story_id.0.clone());
}
}
server/src/agents/pool/auto_assign/story_checks.rs
+15 -16
View File
@@ -116,14 +116,13 @@ pub(super) fn check_archived_dependencies(
crate::crdt_state::check_archived_deps_crdt(story_id)
}
/// Return `true` if the story is in the `Frozen` pipeline stage.
/// Return `true` if the story's `frozen` CRDT flag is set (story 934, stage 4).
///
/// Checks the typed CRDT stage via `read_typed`.
/// `frozen` is orthogonal to [`Stage`]: a frozen story keeps its current stage
/// register but is skipped by the auto-assigner.
pub(super) fn is_story_frozen(_project_root: &Path, _stage_dir: &str, story_id: &str) -> bool {
crate::pipeline_state::read_typed(story_id)
.ok()
.flatten()
.map(|item| item.stage.is_frozen())
crate::crdt_state::read_item(story_id)
.map(|view| view.frozen())
.unwrap_or(false)
}
@@ -140,7 +139,7 @@ mod tests {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"890_spike_held",
"3_qa",
Some("Held Spike"),
@@ -161,7 +160,7 @@ mod tests {
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"890_spike_active_qa",
"3_qa",
Some("Active QA Spike"),
@@ -253,7 +252,7 @@ mod tests {
fn has_unmet_dependencies_returns_true_when_dep_not_done() {
crate::crdt_state::init_for_test();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"10_story_blocked",
"2_current",
Some("Blocked"),
@@ -276,7 +275,7 @@ mod tests {
fn has_unmet_dependencies_returns_false_when_dep_done() {
crate::crdt_state::init_for_test();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"999_story_dep",
"5_done",
Some("Dep"),
@@ -288,7 +287,7 @@ mod tests {
None,
None,
);
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"10_story_ok",
"2_current",
Some("Ok"),
@@ -311,7 +310,7 @@ mod tests {
fn has_unmet_dependencies_returns_false_when_no_deps() {
crate::crdt_state::init_for_test();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"5_story_free",
"2_current",
Some("Free"),
@@ -337,7 +336,7 @@ mod tests {
fn check_archived_dependencies_returns_archived_ids() {
crate::crdt_state::init_for_test();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"500_spike_crdt",
"6_archived",
Some("CRDT Spike"),
@@ -349,7 +348,7 @@ mod tests {
None,
None,
);
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"503_story_dependent",
"1_backlog",
Some("Dependent"),
@@ -371,7 +370,7 @@ mod tests {
fn check_archived_dependencies_empty_when_dep_in_done() {
crate::crdt_state::init_for_test();
let tmp = tempfile::tempdir().unwrap();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"490_story_done",
"5_done",
Some("Done"),
@@ -383,7 +382,7 @@ mod tests {
None,
None,
);
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"503_story_waiting",
"1_backlog",
Some("Waiting"),
server/src/agents/pool/auto_assign/watchdog/tests/limits_tests.rs
+15 -15
View File
@@ -243,7 +243,7 @@ max_turns = 10
let story_id = "42_story_runaway";
let initial = "---\nname: Runaway Story\n---\n# Runaway Story\n";
crate::db::write_content(story_id, initial);
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
story_id,
"2_current",
Some("Runaway Story"),
@@ -274,10 +274,10 @@ max_turns = 10
let item = crate::crdt_state::read_item(story_id)
.expect("story must be in CRDT after watchdog termination");
assert_eq!(
item.stage_str(),
"2_blocked",
item.stage().as_dir(),
"blocked",
"story stage must be 2_blocked after limit termination with max_retries=1 — got: {}",
item.stage_str()
item.stage().as_dir()
);
// Sanity: the agent itself is also Failed with the right reason.
@@ -371,7 +371,7 @@ max_turns = 10
let story_id = "story_e_per_session";
crate::db::write_content(story_id, "---\nname: Per-Session Test\n---\n");
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
story_id,
"2_current",
Some("Per-Session Test"),
@@ -416,8 +416,8 @@ max_turns = 10
let item = crate::crdt_state::read_item(story_id)
.expect("story must be in CRDT after per-session overrun");
assert_eq!(
item.stage_str(),
"2_blocked",
item.stage().as_dir(),
"blocked",
"story stage must be 2_blocked after per-session overrun with max_retries=1"
);
}
@@ -451,7 +451,7 @@ max_turns = 10
let initial = "---\nname: Retry Test\n---\n";
crate::crdt_state::init_for_test();
crate::db::write_content(story_id, initial);
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
story_id,
"2_current",
Some("Retry Test"),
@@ -478,8 +478,8 @@ max_turns = 10
"after session 1, retry_count should be 1 in CRDT"
);
assert_ne!(
item.stage_str(),
"2_blocked",
item.stage().as_dir(),
"blocked",
"story should NOT be blocked after session 1"
);
}
@@ -498,8 +498,8 @@ max_turns = 10
"after session 2, retry_count should be 2 in CRDT"
);
assert_ne!(
item.stage_str(),
"2_blocked",
item.stage().as_dir(),
"blocked",
"story should NOT be blocked after session 2"
);
}
@@ -513,10 +513,10 @@ max_turns = 10
let item = crate::crdt_state::read_item(story_id).expect("story must be in CRDT");
assert_eq!(
item.stage_str(),
"2_blocked",
item.stage().as_dir(),
"blocked",
"story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got: {}",
item.stage_str()
item.stage().as_dir()
);
// retry_count resets to 0 on stage transition (Bug 780) — the fact
// that the story reached 2_blocked proves the retry limit was hit.
server/src/agents/pool/pipeline/advance/tests_regression.rs
+3 -3
View File
@@ -298,12 +298,12 @@ async fn stale_mergemaster_advance_for_done_story_is_noop() {
"No StoryBlocked event should be emitted for a stale advance"
);
// The story should still be in 5_done (not moved elsewhere).
// The story should still be in done (not moved elsewhere).
if let Ok(Some(item)) = crate::pipeline_state::read_typed(story_id) {
assert_eq!(
item.stage.dir_name(),
"5_done",
"Story should remain in 5_done after stale mergemaster advance"
"done",
"Story should remain in done after stale mergemaster advance"
);
}
}
server/src/agents/pool/start/tests_concurrency.rs
+6 -6
View File
@@ -443,11 +443,11 @@ async fn start_agent_rejects_mergemaster_on_coding_stage_story() {
assert!(
result.is_err(),
"mergemaster must not be assigned to a story in 2_current/"
"mergemaster must not be assigned to a story in coding stage"
);
let err = result.unwrap_err();
assert!(
err.contains("stage") && err.contains("2_current"),
err.contains("stage") && err.contains("coding"),
"error must mention stage mismatch, got: '{err}'"
);
}
@@ -482,11 +482,11 @@ async fn start_agent_rejects_coder_on_qa_stage_story() {
assert!(
result.is_err(),
"coder must not be assigned to a story in 3_qa/"
"coder must not be assigned to a story in qa stage"
);
let err = result.unwrap_err();
assert!(
err.contains("stage") && err.contains("3_qa"),
err.contains("stage") && err.contains("qa"),
"error must mention stage mismatch, got: '{err}'"
);
}
@@ -521,11 +521,11 @@ async fn start_agent_rejects_qa_on_merge_stage_story() {
assert!(
result.is_err(),
"qa must not be assigned to a story in 4_merge/"
"qa must not be assigned to a story in merge stage"
);
let err = result.unwrap_err();
assert!(
err.contains("stage") && err.contains("4_merge"),
err.contains("stage") && err.contains("merge"),
"error must mention stage mismatch, got: '{err}'"
);
}
server/src/agents/pool/start/tests_selection.rs
+1 -1
View File
@@ -283,7 +283,7 @@ stage = "coder"
crate::db::write_content("368_story_test", story_content);
// Story 929: agent pin comes from the CRDT register, not YAML. Seed it.
crate::crdt_state::init_for_test();
crate::crdt_state::write_item(
crate::crdt_state::write_item_str(
"368_story_test",
"2_current",
Some("Test Story"),