diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 46b2fff3..f0501568 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -2,14 +2,21 @@ //! //! All pipeline state lives in the CRDT. These functions never consult the //! filesystem for work-item data — CRDT lookup failures propagate as errors. +//! +//! Every lifecycle function routes through the typed state machine +//! ([`crate::pipeline_state::apply_transition`]) so that illegal transitions +//! are rejected and every stage change emits a [`TransitionFired`] event. +use std::num::NonZeroU32; use std::path::Path; use std::process::Command; use crate::io::story_metadata::clear_front_matter_field_in_content; +use crate::pipeline_state::{ + ApplyError, ArchiveReason, BranchName, GitSha, PipelineEvent, Stage, apply_transition, + stage_label, +}; use crate::slog; -type ContentTransform = Option String>>; - /// Determine the item type ("story", "bug", "spike", or "refactor") from the item ID. /// /// For slug-format IDs (e.g. `"4_bug_login_crash"`), the type is embedded in the ID. @@ -40,78 +47,21 @@ pub(crate) fn item_type_from_id(item_id: &str) -> &'static str { "story" } -/// Move a work item to a new pipeline stage via the database. -/// -/// Looks up the item in the CRDT to verify it exists in one of the expected -/// `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>( - story_id: &str, - sources: &'a [&'a str], - target_dir: &str, - extra_done_dirs: &[&str], - missing_ok: bool, - fields_to_clear: &[&str], -) -> Result, String> { - // Check if the item is already in the target stage or a done stage. - // Use the typed projection for compile-safe stage comparison. - if let Ok(Some(typed_item)) = crate::pipeline_state::read_typed(story_id) { - let current_dir = typed_item.stage.dir_name(); - if current_dir == target_dir || extra_done_dirs.contains(¤t_dir) { - return Ok(None); // Idempotent: already there. +type ContentTransform = Box String>; + +/// Build a content-transform closure that clears the given front-matter fields. +fn fields_to_clear_transform(fields: &[&str]) -> Option { + if fields.is_empty() { + return None; + } + let fields: Vec = fields.iter().map(|s| s.to_string()).collect(); + Some(Box::new(move |content: &str| { + let mut result = content.to_string(); + for field in &fields { + result = clear_front_matter_field_in_content(&result, field); } - - // Verify it's in one of the expected source stages. - let src_dir = sources.iter().find(|&&s| current_dir == s).copied(); - let src_dir = match src_dir { - Some(s) => s, - None if missing_ok => { - // Item is in CRDT but not in an expected source stage — do not - // overwrite its current stage. This prevents promote_ready_backlog_stories - // from demoting a story that has already advanced to merge/done (bug 524). - return Ok(None); - } - None => { - let locs = sources - .iter() - .map(|s| format!("work/{s}/")) - .collect::>() - .join(" or "); - return Err(format!("Work item '{story_id}' not found in {locs}.")); - } - }; - - // Optionally clear front-matter fields from the stored content. - let transform: ContentTransform = if fields_to_clear.is_empty() { - None - } else { - let fields: Vec = fields_to_clear.iter().map(|s| s.to_string()).collect(); - Some(Box::new(move |content: &str| { - let mut result = content.to_string(); - for field in &fields { - result = clear_front_matter_field_in_content(&result, field); - } - result - })) - }; - - crate::db::move_item_stage(story_id, target_dir, transform.as_ref().map(|f| f.as_ref())); - - slog!("[lifecycle] Moved '{story_id}' from work/{src_dir}/ to work/{target_dir}/"); - return Ok(Some(src_dir)); - } - - if missing_ok { - slog!("[lifecycle] Work item '{story_id}' not found; skipping move to work/{target_dir}/"); - return Ok(None); - } - - let locs = sources - .iter() - .map(|s| format!("work/{s}/")) - .collect::>() - .join(" or "); - Err(format!("Work item '{story_id}' not found in {locs}.")) + result + })) } /// Move a work item (story, bug, or spike) from `1_backlog` to `work/2_current/`. @@ -121,7 +71,18 @@ fn move_item<'a>( /// 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(story_id: &str) -> Result<(), String> { - move_item(story_id, &["1_backlog"], "2_current", &[], true, &[]).map(|_| ()) + match apply_transition(story_id, PipelineEvent::DepsMet, None) { + Ok(_) => Ok(()), + Err(ApplyError::NotFound(_)) => { + slog!("[lifecycle] Work item '{story_id}' not found; skipping move to work/2_current/"); + Ok(()) + } + Err(ApplyError::InvalidTransition(_)) => { + // Already promoted or in a later stage — idempotent no-op. + Ok(()) + } + Err(ApplyError::Projection(_)) => Ok(()), + } } /// Check whether a feature branch `feature/story-{story_id}` exists and has @@ -160,68 +121,165 @@ 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(story_id: &str) -> Result<(), String> { - move_item( - story_id, - &["2_current", "3_qa", "4_merge"], - "5_done", - &["6_archived"], - false, - &["merge_failure", "blocked"], - ) - .map(|_| ()) + let item = read_typed_or_err(story_id)?; + let dir = item.stage.dir_name(); + + // Idempotent: already at or past done. + if dir >= "5_done" { + return Ok(()); + } + + let event = match &item.stage { + Stage::Merge { .. } => PipelineEvent::MergeSucceeded { + merge_commit: GitSha("accepted".to_string()), + }, + Stage::Coding | Stage::Qa | Stage::Backlog => PipelineEvent::Close, + _ => { + return Err(format!( + "Work item '{story_id}' is in {} — cannot move to done.", + stage_label(&item.stage) + )); + } + }; + + let transform = fields_to_clear_transform(&["merge_failure", "blocked"]); + apply_transition(story_id, event, transform.as_ref().map(|f| f.as_ref())) + .map(|_| ()) + .map_err(|e| e.to_string()) } /// 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(story_id: &str) -> Result<(), String> { - move_item( - story_id, - &["2_current", "3_qa"], - "4_merge", - &["5_done", "6_archived"], - false, - &["blocked"], - ) - .map(|_| ()) + let item = read_typed_or_err(story_id)?; + let dir = item.stage.dir_name(); + + // Idempotent: already at or past merge. + if dir >= "4_merge" { + return Ok(()); + } + + let branch = BranchName(format!("feature/story-{story_id}")); + let commits = NonZeroU32::new(1).expect("1 is non-zero"); + + let event = match &item.stage { + Stage::Coding => PipelineEvent::QaSkipped { + feature_branch: branch, + commits_ahead: commits, + }, + Stage::Qa => PipelineEvent::GatesPassed { + feature_branch: branch, + commits_ahead: commits, + }, + _ => { + return Err(format!( + "Work item '{story_id}' not found in work/2_current/ or work/3_qa/." + )); + } + }; + + let transform = fields_to_clear_transform(&["blocked"]); + apply_transition(story_id, event, transform.as_ref().map(|f| f.as_ref())) + .map(|_| ()) + .map_err(|e| e.to_string()) } /// 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(story_id: &str) -> Result<(), String> { - move_item( + let item = read_typed_or_err(story_id)?; + let dir = item.stage.dir_name(); + + // Idempotent: already at or past qa. + if dir >= "3_qa" { + return Ok(()); + } + + let transform = fields_to_clear_transform(&["blocked"]); + apply_transition( story_id, - &["2_current"], - "3_qa", - &["5_done", "6_archived"], - false, - &["blocked"], + PipelineEvent::GatesStarted, + transform.as_ref().map(|f| f.as_ref()), ) .map(|_| ()) + .map_err(|e| e.to_string()) } /// Move a story from `work/3_qa/` back to `work/2_current/`, clearing `review_hold` and writing notes. pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> { - let moved = move_item( - story_id, - &["3_qa"], - "2_current", - &[], - false, - &["review_hold"], - )?; - if moved.is_some() && !notes.is_empty() { - // Append rejection notes to the stored content. - if let Some(content) = crate::db::read_content(story_id) { - let updated = - crate::io::story_metadata::write_rejection_notes_to_content(&content, notes); - crate::db::write_content(story_id, &updated); - // Re-sync to DB. - crate::db::write_item_with_content(story_id, "2_current", &updated); + let notes_owned = notes.to_string(); + let transform: Box String> = Box::new(move |content: &str| { + let mut result = clear_front_matter_field_in_content(content, "review_hold"); + if !notes_owned.is_empty() { + result = + crate::io::story_metadata::write_rejection_notes_to_content(&result, ¬es_owned); } + result + }); + + apply_transition( + story_id, + PipelineEvent::GatesFailed { + reason: notes.to_string(), + }, + Some(&*transform), + ) + .map(|_| ()) + .map_err(|e| e.to_string()) +} + +/// Map a (current stage, target stage name) pair to the appropriate PipelineEvent. +fn map_stage_move_to_event( + from: &Stage, + target: &str, + story_id: &str, +) -> Result { + let branch = || BranchName(format!("feature/story-{story_id}")); + let nz1 = || NonZeroU32::new(1).expect("1 is non-zero"); + + match (from, target) { + (Stage::Upcoming, "backlog") => Ok(PipelineEvent::Triage), + (Stage::Backlog, "current") => Ok(PipelineEvent::DepsMet), + (Stage::Coding, "qa") => Ok(PipelineEvent::GatesStarted), + (Stage::Coding, "merge") => Ok(PipelineEvent::QaSkipped { + feature_branch: branch(), + commits_ahead: nz1(), + }), + (Stage::Qa, "merge") => Ok(PipelineEvent::GatesPassed { + feature_branch: branch(), + commits_ahead: nz1(), + }), + (Stage::Coding, "backlog") | (Stage::Qa, "backlog") | (Stage::Merge { .. }, "backlog") => { + Ok(PipelineEvent::Demote) + } + (Stage::Qa, "current") => Ok(PipelineEvent::GatesFailed { + reason: "manual move".to_string(), + }), + (Stage::Merge { .. }, "done") => Ok(PipelineEvent::MergeSucceeded { + merge_commit: GitSha("manual".to_string()), + }), + (Stage::Coding | Stage::Qa | Stage::Backlog, "done") => Ok(PipelineEvent::Close), + ( + Stage::Archived { + reason: ArchiveReason::Blocked { .. }, + .. + }, + "backlog", + ) + | ( + Stage::Archived { + reason: ArchiveReason::MergeFailed { .. }, + .. + }, + "backlog", + ) => Ok(PipelineEvent::Unblock), + _ => Err(format!( + "Invalid target_stage '{target}'. Cannot transition from {} to {target}.", + stage_label(from), + )), } - Ok(()) } /// Move any work item to an arbitrary pipeline stage by searching all stages. @@ -230,56 +288,68 @@ pub fn reject_story_from_qa(story_id: &str, notes: &str) -> Result<(), String> { /// 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> { - const STAGES: &[(&str, &str)] = &[ - ("backlog", "1_backlog"), - ("current", "2_current"), - ("qa", "3_qa"), - ("merge", "4_merge"), - ("done", "5_done"), - ("archived", "6_archived"), - ]; - - let target_dir = STAGES - .iter() - .filter(|(name, _)| *name != "archived") - .find(|(name, _)| *name == target_stage) - .map(|(_, dir)| *dir) - .ok_or_else(|| { - format!( + // Validate target. + let target_dir = match target_stage { + "backlog" => "1_backlog", + "current" => "2_current", + "qa" => "3_qa", + "merge" => "4_merge", + "done" => "5_done", + _ => { + return Err(format!( "Invalid target_stage '{target_stage}'. Must be one of: backlog, current, qa, merge, done" - ) - })?; - - let all_dirs: Vec<&str> = STAGES.iter().map(|(_, dir)| *dir).collect(); - - 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 - .iter() - .find(|(_, dir)| *dir == src_dir) - .map(|(name, _)| *name) - .unwrap_or(src_dir); - Ok((from_stage.to_string(), target_stage.to_string())) + )); } - None => Ok((target_stage.to_string(), target_stage.to_string())), + }; + + let item = read_typed_or_err(story_id)?; + let from_name = stage_to_name(&item.stage); + + // Idempotent: already in the target stage. + if item.stage.dir_name() == target_dir { + return Ok((target_stage.to_string(), target_stage.to_string())); } + + let event = map_stage_move_to_event(&item.stage, target_stage, story_id)?; + + apply_transition(story_id, event, None).map_err(|e| e.to_string())?; + + Ok((from_name.to_string(), target_stage.to_string())) } /// 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(bug_id: &str) -> Result<(), String> { - move_item( - bug_id, - &["2_current", "1_backlog"], - "5_done", - &[], - false, - &[], - ) - .map(|_| ()) + let item = read_typed_or_err(bug_id)?; + + if item.stage.dir_name() >= "5_done" { + return Ok(()); + } + + apply_transition(bug_id, PipelineEvent::Close, None) + .map(|_| ()) + .map_err(|e| e.to_string()) +} + +/// Read a typed pipeline item or return a user-facing error. +fn read_typed_or_err(story_id: &str) -> Result { + crate::pipeline_state::read_typed(story_id) + .map_err(|e| format!("Work item '{story_id}': {e}"))? + .ok_or_else(|| format!("Work item '{story_id}' not found in any pipeline stage.")) +} + +/// Map a Stage variant to the short name used by `move_story_to_stage` return values. +fn stage_to_name(s: &Stage) -> &'static str { + match s { + Stage::Upcoming => "upcoming", + Stage::Backlog => "backlog", + Stage::Coding => "current", + Stage::Qa => "qa", + Stage::Merge { .. } => "merge", + Stage::Done { .. } => "done", + Stage::Archived { .. } => "archived", + } } #[cfg(test)] diff --git a/server/src/chat/transport/matrix/delete.rs b/server/src/chat/transport/matrix/delete.rs index 4e2e2b38..a3322178 100644 --- a/server/src/chat/transport/matrix/delete.rs +++ b/server/src/chat/transport/matrix/delete.rs @@ -122,6 +122,7 @@ pub async fn handle_delete( fn stage_display_name(stage: &str) -> &str { use crate::pipeline_state::Stage; match Stage::from_dir(stage) { + Some(Stage::Upcoming) => "upcoming", Some(Stage::Backlog) => "backlog", Some(Stage::Coding) => "in-progress", Some(Stage::Qa) => "QA", diff --git a/server/src/http/workflow/pipeline.rs b/server/src/http/workflow/pipeline.rs index 44f1b22d..cd9d807e 100644 --- a/server/src/http/workflow/pipeline.rs +++ b/server/src/http/workflow/pipeline.rs @@ -138,6 +138,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result { }, }; match &item.stage { + Stage::Upcoming => state.backlog.push(story), // upcoming shown with backlog Stage::Backlog => state.backlog.push(story), Stage::Coding => state.current.push(story), Stage::Qa => state.qa.push(story), diff --git a/server/src/io/watcher/mod.rs b/server/src/io/watcher/mod.rs index d364d1b3..8e4df853 100644 --- a/server/src/io/watcher/mod.rs +++ b/server/src/io/watcher/mod.rs @@ -49,6 +49,7 @@ pub fn is_config_file(path: &Path, git_root: &Path) -> bool { pub fn stage_metadata(stage: &str, item_id: &str) -> Option<(&'static str, String)> { use crate::pipeline_state::Stage; let (action, msg) = match Stage::from_dir(stage)? { + Stage::Upcoming => ("create", format!("huskies: triage {item_id}")), Stage::Backlog => ("create", format!("huskies: create {item_id}")), Stage::Coding => ("start", format!("huskies: start {item_id}")), Stage::Qa => ("qa", format!("huskies: queue {item_id} for QA")), diff --git a/server/src/pipeline_state/apply.rs b/server/src/pipeline_state/apply.rs new file mode 100644 index 00000000..49b0558b --- /dev/null +++ b/server/src/pipeline_state/apply.rs @@ -0,0 +1,99 @@ +//! Single entry point for pipeline state transitions. +//! +//! [`apply_transition`] is the **only** function that should mutate a story's +//! pipeline stage. It reads the current typed stage from the CRDT, validates +//! the transition via the pure [`super::transition()`] function, writes the new +//! stage back to the CRDT, and returns a [`TransitionFired`] event for +//! downstream subscribers. + +use super::{ + PipelineEvent, StoryId, TransitionFired, event_label, read_typed, stage_label, transition, +}; +use chrono::Utc; + +/// Error type for [`apply_transition`]. +#[derive(Debug)] +pub enum ApplyError { + /// The story was not found in the CRDT. + NotFound(String), + /// The CRDT projection failed. + Projection(super::ProjectionError), + /// The transition was invalid for the current stage. + InvalidTransition(super::TransitionError), +} + +impl std::fmt::Display for ApplyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotFound(id) => write!(f, "story '{id}' not found in CRDT"), + Self::Projection(e) => write!(f, "projection error: {e}"), + Self::InvalidTransition(e) => write!(f, "{e}"), + } + } +} + +impl std::error::Error for ApplyError {} + +impl From for ApplyError { + fn from(e: super::ProjectionError) -> Self { + Self::Projection(e) + } +} + +impl From for ApplyError { + fn from(e: super::TransitionError) -> Self { + Self::InvalidTransition(e) + } +} + +/// Apply a pipeline event to a story, validating the transition and writing +/// the new stage to the CRDT. +/// +/// This is the single canonical entry point for all pipeline stage mutations. +/// Returns a [`TransitionFired`] describing what happened, suitable for +/// broadcasting to event-bus subscribers and CRDT log consumers. +/// +/// An optional `content_transform` allows callers to modify the stored content +/// (e.g. clearing front-matter fields) atomically with the stage change. +pub fn apply_transition( + story_id: &str, + event: PipelineEvent, + content_transform: Option<&dyn Fn(&str) -> String>, +) -> Result { + let item = read_typed(story_id)?.ok_or_else(|| ApplyError::NotFound(story_id.to_string()))?; + + let before = item.stage.clone(); + let after = transition(before.clone(), event.clone())?; + let new_dir = after.dir_name(); + + // Write the new stage to the CRDT (with optional content transform). + crate::db::move_item_stage(story_id, new_dir, content_transform); + + let fired = TransitionFired { + story_id: StoryId(story_id.to_string()), + before, + after, + event, + at: Utc::now(), + }; + + crate::slog!( + "[pipeline/transition] #{}: {} + {} → {}", + story_id, + stage_label(&fired.before), + event_label(&fired.event), + stage_label(&fired.after), + ); + + Ok(fired) +} + +/// Convenience: apply a transition, returning an `Err(String)` on failure +/// (matches the existing lifecycle function signatures). +pub fn apply_transition_str( + story_id: &str, + event: PipelineEvent, + content_transform: Option<&dyn Fn(&str) -> String>, +) -> Result { + apply_transition(story_id, event, content_transform).map_err(|e| e.to_string()) +} diff --git a/server/src/pipeline_state/mod.rs b/server/src/pipeline_state/mod.rs index f56e6c7a..b1abfa1e 100644 --- a/server/src/pipeline_state/mod.rs +++ b/server/src/pipeline_state/mod.rs @@ -23,6 +23,7 @@ // the dead_code lint is suppressed for the module. #![allow(dead_code)] +mod apply; mod events; mod projection; mod subscribers; @@ -52,6 +53,9 @@ pub use events::{EventBus, TransitionFired, TransitionSubscriber}; pub use projection::{ProjectionError, project_stage}; pub use projection::{read_all_typed, read_typed}; +#[allow(unused_imports)] +pub use apply::{ApplyError, apply_transition, apply_transition_str}; + #[allow(unused_imports)] pub use subscribers::{ AutoAssignSubscriber, FileRendererSubscriber, MatrixBotSubscriber, PipelineItemsSubscriber, diff --git a/server/src/pipeline_state/projection.rs b/server/src/pipeline_state/projection.rs index 33a810cc..2e1e2b1e 100644 --- a/server/src/pipeline_state/projection.rs +++ b/server/src/pipeline_state/projection.rs @@ -70,6 +70,7 @@ impl TryFrom<&PipelineItemView> for PipelineItem { /// loose CRDT data becomes typed. pub fn project_stage(view: &PipelineItemView) -> Result { match view.stage.as_str() { + "0_upcoming" => Ok(Stage::Upcoming), "1_backlog" => Ok(Stage::Backlog), "2_current" => Ok(Stage::Coding), "3_qa" => Ok(Stage::Qa), @@ -192,6 +193,26 @@ mod tests { StoryId(s.to_string()) } + #[test] + fn project_upcoming_item() { + let view = PipelineItemView { + story_id: "42_story_test".to_string(), + stage: "0_upcoming".to_string(), + name: Some("Test Story".to_string()), + agent: None, + retry_count: None, + blocked: None, + depends_on: None, + claimed_by: None, + claimed_at: None, + merged_at: None, + qa_mode: None, + mergemaster_attempted: None, + }; + let item = PipelineItem::try_from(&view).unwrap(); + assert!(matches!(item.stage, Stage::Upcoming)); + } + #[test] fn project_backlog_item() { let view = PipelineItemView { @@ -345,6 +366,7 @@ mod tests { #[test] fn reverse_projection_stage_dirs() { let cases: Vec<(Stage, &str, bool)> = vec![ + (Stage::Upcoming, "0_upcoming", false), (Stage::Backlog, "1_backlog", false), (Stage::Coding, "2_current", false), (Stage::Qa, "3_qa", false), diff --git a/server/src/pipeline_state/tests.rs b/server/src/pipeline_state/tests.rs index aa2456f5..358c6861 100644 --- a/server/src/pipeline_state/tests.rs +++ b/server/src/pipeline_state/tests.rs @@ -406,4 +406,136 @@ fn transition_error_display() { assert_eq!(err.to_string(), "invalid transition: Backlog + Accepted"); } +// ── Upcoming / Triage ────────────────────────────────────────────── + +#[test] +fn triage_upcoming_to_backlog() { + let s = Stage::Upcoming; + let s = transition(s, PipelineEvent::Triage).unwrap(); + assert!(matches!(s, Stage::Backlog)); +} + +#[test] +fn cannot_triage_from_backlog() { + let result = transition(Stage::Backlog, PipelineEvent::Triage); + assert!(matches!( + result, + Err(TransitionError::InvalidTransition { .. }) + )); +} + +#[test] +fn abandon_from_upcoming() { + let result = transition(Stage::Upcoming, PipelineEvent::Abandon).unwrap(); + assert!(matches!( + result, + Stage::Archived { + reason: ArchiveReason::Abandoned, + .. + } + )); +} + +#[test] +fn supersede_from_upcoming() { + let result = transition( + Stage::Upcoming, + PipelineEvent::Supersede { + by: sid("999_story_new"), + }, + ) + .unwrap(); + assert!(matches!( + result, + Stage::Archived { + reason: ArchiveReason::Superseded { .. }, + .. + } + )); +} + +#[test] +fn cannot_deps_met_from_upcoming() { + let result = transition(Stage::Upcoming, PipelineEvent::DepsMet); + assert!(matches!( + result, + Err(TransitionError::InvalidTransition { .. }) + )); +} + +// ── Reject ───────────────────────────────────────────────────────── + +#[test] +fn reject_from_active_stages() { + for s in [Stage::Backlog, Stage::Coding, Stage::Qa] { + let result = transition( + s.clone(), + PipelineEvent::Reject { + reason: "not needed".into(), + }, + ); + assert!(matches!( + result, + Ok(Stage::Archived { + reason: ArchiveReason::Rejected { .. }, + .. + }) + )); + } + + let m = Stage::Merge { + feature_branch: fb("f"), + commits_ahead: nz(1), + }; + let result = transition( + m, + PipelineEvent::Reject { + reason: "not needed".into(), + }, + ); + assert!(matches!( + result, + Ok(Stage::Archived { + reason: ArchiveReason::Rejected { .. }, + .. + }) + )); +} + +#[test] +fn cannot_reject_from_done() { + let s = Stage::Done { + merged_at: chrono::Utc::now(), + merge_commit: sha("abc"), + }; + let result = transition( + s, + PipelineEvent::Reject { + reason: "too late".into(), + }, + ); + assert!(matches!( + result, + Err(TransitionError::InvalidTransition { .. }) + )); +} + +#[test] +fn cannot_reject_from_archived() { + let s = Stage::Archived { + archived_at: chrono::Utc::now(), + reason: ArchiveReason::Completed, + }; + let result = transition( + s, + PipelineEvent::Reject { + reason: "already done".into(), + }, + ); + assert!(matches!( + result, + Err(TransitionError::InvalidTransition { .. }) + )); +} + // ── ProjectionError Display ───────────────────────────────────────── diff --git a/server/src/pipeline_state/transition.rs b/server/src/pipeline_state/transition.rs index e86492d6..ceb244da 100644 --- a/server/src/pipeline_state/transition.rs +++ b/server/src/pipeline_state/transition.rs @@ -47,6 +47,15 @@ pub enum PipelineEvent { Supersede { by: StoryId }, /// Story put on review hold. ReviewHold { reason: String }, + /// Story rejected by QA or reviewer. + Reject { reason: String }, + /// Story triaged from upcoming to backlog. + Triage, + /// Direct completion — item closed without going through the full pipeline + /// (e.g. spike auto-merge, bug closure, manual acceptance). + Close, + /// Manual demotion back to backlog from an active stage. + Demote, } // ── Per-node execution events ─────────────────────────────────────────────── @@ -81,6 +90,10 @@ pub fn event_label(e: &PipelineEvent) -> &'static str { PipelineEvent::Abandon => "Abandon", PipelineEvent::Supersede { .. } => "Supersede", PipelineEvent::ReviewHold { .. } => "ReviewHold", + PipelineEvent::Reject { .. } => "Reject", + PipelineEvent::Triage => "Triage", + PipelineEvent::Close => "Close", + PipelineEvent::Demote => "Demote", } } @@ -103,6 +116,9 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Ok(Backlog), + // ── Forward path ──────────────────────────────────────────────── (Backlog, DepsMet) => Ok(Coding), (Coding, GatesStarted) => Ok(Qa), @@ -161,7 +177,8 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result Result Result Ok(Archived { + archived_at: now, + reason: ArchiveReason::Rejected { reason }, + }), + + // ── Demote: send an active item back to backlog ──────────────── + (Coding, Demote) | (Qa, Demote) | (Merge { .. }, Demote) => Ok(Backlog), + + // ── Close: direct completion from any active stage ───────────── + (Backlog, Close) | (Coding, Close) | (Qa, Close) | (Merge { .. }, Close) => Ok(Done { + merged_at: now, + merge_commit: GitSha("closed".to_string()), + }), + + // ── Unblock: from Archived(Blocked) or Archived(MergeFailed) → Backlog ( Archived { reason: ArchiveReason::Blocked { .. }, .. }, Unblock, + ) + | ( + Archived { + reason: ArchiveReason::MergeFailed { .. }, + .. + }, + Unblock, ) => Ok(Backlog), // ── Everything else is invalid ────────────────────────────────── diff --git a/server/src/pipeline_state/types.rs b/server/src/pipeline_state/types.rs index 59e4f937..36e92278 100644 --- a/server/src/pipeline_state/types.rs +++ b/server/src/pipeline_state/types.rs @@ -48,8 +48,30 @@ impl fmt::Display for AgentName { /// - `agent` — local execution state, not pipeline state /// - `retry_count` — also local /// - `blocked` — folded into `Archived { reason: Blocked { .. } }` +/// +/// ## Canonical state machine (story 857) +/// +/// The following named lifecycle states map to `Stage` variants: +/// +/// | Lifecycle state | Stage variant | +/// |-----------------|-----------------------------------| +/// | upcoming | `Upcoming` | +/// | backlog | `Backlog` | +/// | current | `Coding` | +/// | qa_pending | `Qa` | +/// | merge_pending | `Merge { .. }` | +/// | done | `Done { .. }` | +/// | blocked | `Archived { Blocked { .. } }` | +/// | merge_failure | `Archived { MergeFailed { .. } }` | +/// | archived | `Archived { Completed }` | +/// | superseded | `Archived { Superseded { .. } }` | +/// | rejected | `Archived { Rejected { .. } }` | +/// | abandoned | `Archived { Abandoned }` | #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum Stage { + /// Story has been created but not yet triaged into the backlog. + Upcoming, + /// Story exists, waiting for dependencies or auto-assign promotion. Backlog, @@ -96,6 +118,8 @@ pub enum ArchiveReason { MergeFailed { reason: String }, /// Held in review at human request. ReviewHeld { reason: String }, + /// Story rejected by QA or reviewer with an explanation. + Rejected { reason: String }, } // ── Stage convenience methods ────────────────────────────────────────────── @@ -106,6 +130,11 @@ impl Stage { matches!(self, Stage::Coding | Stage::Qa | Stage::Merge { .. }) } + /// Returns true if this is the Upcoming variant. + pub fn is_upcoming(&self) -> bool { + matches!(self, Stage::Upcoming) + } + /// Returns the filesystem directory name for this stage. pub fn dir_name(&self) -> &'static str { stage_dir_name(self) @@ -132,6 +161,7 @@ impl Stage { /// accessing the rich metadata fields. pub fn from_dir(s: &str) -> Option { match s { + "0_upcoming" => Some(Stage::Upcoming), "1_backlog" => Some(Stage::Backlog), "2_current" => Some(Stage::Coding), "3_qa" => Some(Stage::Qa), @@ -219,6 +249,7 @@ impl std::error::Error for TransitionError {} /// Human-readable label for a `Stage` variant. pub fn stage_label(s: &Stage) -> &'static str { match s { + Stage::Upcoming => "Upcoming", Stage::Backlog => "Backlog", Stage::Coding => "Coding", Stage::Qa => "Qa", @@ -231,6 +262,7 @@ pub fn stage_label(s: &Stage) -> &'static str { /// Map a Stage to the filesystem directory name used by the work pipeline. pub fn stage_dir_name(s: &Stage) -> &'static str { match s { + Stage::Upcoming => "0_upcoming", Stage::Backlog => "1_backlog", Stage::Coding => "2_current", Stage::Qa => "3_qa", diff --git a/server/src/service/agents/mod.rs b/server/src/service/agents/mod.rs index f4fda78b..61939521 100644 --- a/server/src/service/agents/mod.rs +++ b/server/src/service/agents/mod.rs @@ -202,6 +202,7 @@ pub fn get_work_item_content( let stage = item .as_ref() .map(|i| match &i.stage { + crate::pipeline_state::Stage::Upcoming => "upcoming", crate::pipeline_state::Stage::Backlog => "backlog", crate::pipeline_state::Stage::Coding => "current", crate::pipeline_state::Stage::Qa => "qa", diff --git a/server/src/service/notifications/format.rs b/server/src/service/notifications/format.rs index d8cb3182..8beeb7fa 100644 --- a/server/src/service/notifications/format.rs +++ b/server/src/service/notifications/format.rs @@ -10,6 +10,7 @@ use crate::service::common::item_id::extract_item_number; pub fn stage_display_name(stage: &str) -> &'static str { use crate::pipeline_state::Stage; match Stage::from_dir(stage) { + Some(Stage::Upcoming) => "Upcoming", Some(Stage::Backlog) => "Backlog", Some(Stage::Coding) => "Current", Some(Stage::Qa) => "QA",