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

huskies: merge 945

Browse Source
This commit is contained in:
dave
2026-05-13 06:05:01 +00:00
parent 3a8894ea8f
commit 9ce5a8df0c
53 changed files with 497 additions and 654 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/pipeline_state/apply.rs
+7 -21
View File
@@ -100,31 +100,17 @@ pub fn apply_transition_str(
/// Freeze a story.
///
/// Story 934, stage 4: `frozen` is now a CRDT flag orthogonal to [`Stage`],
/// so the story stays at its current stage and only the boolean register
/// changes. Returns `Err(NotFound)` if no item exists for `story_id`.
/// Story 945: `Stage::Frozen { resume_to }` is the single source of truth;
/// the previous `frozen: bool` flag has been removed. Transitions any
/// non-terminal stage to `Stage::Frozen { resume_to: <previous stage> }`.
pub fn transition_to_frozen(story_id: &str) -> Result<(), ApplyError> {
if read_typed(story_id)?.is_none() {
return Err(ApplyError::NotFound(story_id.to_string()));
}
crate::crdt_state::set_frozen(story_id, true);
crate::slog!("[pipeline/transition] #{}: Freeze (flag set)", story_id);
Ok(())
apply_transition(story_id, PipelineEvent::Freeze, None).map(|_| ())
}
/// Unfreeze a story.
///
/// Story 934, stage 4: paired with [`transition_to_frozen`]; clears the
/// CRDT `frozen` flag without touching the stage register. Returns
/// `Err(NotFound)` if no item exists for `story_id`.
/// Story 945: returns the story to the `resume_to` stage stored on
/// `Stage::Frozen`.
pub fn transition_to_unfrozen(story_id: &str) -> Result<(), ApplyError> {
if read_typed(story_id)?.is_none() {
return Err(ApplyError::NotFound(story_id.to_string()));
}
crate::crdt_state::set_frozen(story_id, false);
crate::slog!(
"[pipeline/transition] #{}: Unfreeze (flag cleared)",
story_id
);
Ok(())
apply_transition(story_id, PipelineEvent::Unfreeze, None).map(|_| ())
}
server/src/pipeline_state/projection.rs
+19 -109
View File
@@ -10,7 +10,7 @@ use std::fmt;
use crate::crdt_state::PipelineItemView;
use super::{ArchiveReason, PipelineItem, Stage, StoryId, stage_dir_name};
use super::{PipelineItem, StoryId};
/// Errors from projecting loose CRDT data into typed enums.
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -60,30 +60,10 @@ impl TryFrom<&PipelineItemView> for PipelineItem {
stage: view.stage().clone(),
depends_on,
retry_count,
frozen: view.frozen(),
})
}
}
// ── Reverse projection: PipelineItem → stage dir string ─────────────────────
impl PipelineItem {
/// Convert back to the loose fields that the CRDT write path expects.
/// Returns `(stage_dir, blocked)`.
pub fn to_crdt_fields(&self) -> (&'static str, bool) {
let dir = stage_dir_name(&self.stage);
let blocked = matches!(
self.stage,
Stage::Blocked { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
);
(dir, blocked)
}
}
// ── Bridge to existing CRDT reads ───────────────────────────────────────────
/// Read all pipeline items from the CRDT and project them into typed enums.
@@ -120,7 +100,7 @@ pub fn read_typed(story_id: &str) -> Result<Option<PipelineItem>, ProjectionErro
#[cfg(test)]
mod tests {
use super::*;
use crate::pipeline_state::{BranchName, GitSha};
use crate::pipeline_state::{ArchiveReason, BranchName, GitSha, Stage};
use chrono::Utc;
use std::num::NonZeroU32;
@@ -130,9 +110,6 @@ mod tests {
fn fb(name: &str) -> BranchName {
BranchName(name.to_string())
}
fn sha(s: &str) -> GitSha {
GitSha(s.to_string())
}
fn make_view(story_id: &str, stage: Stage, name: Option<&str>) -> PipelineItemView {
PipelineItemView::for_test(
@@ -148,10 +125,6 @@ mod tests {
None,
None,
None,
None,
None,
None,
None,
)
}
@@ -170,7 +143,6 @@ mod tests {
Some("Test Story".to_string()),
None,
None,
None,
Some(vec![10, 20]),
None,
None,
@@ -178,9 +150,6 @@ mod tests {
None,
None,
None,
None,
None,
None,
);
let item = PipelineItem::try_from(&view).unwrap();
assert_eq!(item.story_id, StoryId("42_story_test".to_string()));
@@ -205,10 +174,6 @@ mod tests {
None,
None,
None,
None,
None,
None,
None,
);
let item = PipelineItem::try_from(&view).unwrap();
assert!(matches!(item.stage, Stage::Coding));
@@ -263,10 +228,6 @@ mod tests {
Some("Test".to_string()),
None,
None,
Some(true),
None,
None,
None,
None,
None,
None,
@@ -296,10 +257,6 @@ mod tests {
Some("Test".to_string()),
None,
None,
Some(false),
None,
None,
None,
None,
None,
None,
@@ -318,71 +275,17 @@ mod tests {
));
}
// ── Reverse projection tests ────────────────────────────────────────
#[test]
fn reverse_projection_stage_dirs() {
let cases: Vec<(Stage, &str, bool)> = vec![
(Stage::Upcoming, "upcoming", false),
(Stage::Backlog, "backlog", false),
(Stage::Coding, "coding", false),
(
Stage::Blocked {
reason: "stuck".into(),
},
"blocked",
true,
),
(Stage::Qa, "qa", false),
(
Stage::Merge {
feature_branch: fb("f"),
commits_ahead: nz(1),
},
"merge",
false,
),
(
Stage::Done {
merged_at: Utc::now(),
merge_commit: sha("abc"),
},
"done",
false,
),
(
Stage::Archived {
archived_at: Utc::now(),
reason: ArchiveReason::Completed,
},
"archived",
false,
),
(
Stage::Archived {
archived_at: Utc::now(),
reason: ArchiveReason::Blocked {
reason: "stuck".into(),
},
},
"archived",
true,
),
];
for (stage, expected_dir, expected_blocked) in cases {
let item = PipelineItem {
story_id: StoryId("test".into()),
name: "test".into(),
stage,
depends_on: vec![],
retry_count: 0,
frozen: false,
};
let (dir, blocked) = item.to_crdt_fields();
assert_eq!(dir, expected_dir);
assert_eq!(blocked, expected_blocked);
}
fn project_frozen_item() {
let view = make_view(
"42_story_test",
Stage::Frozen {
resume_to: Box::new(Stage::Coding),
},
Some("Frozen Story"),
);
let item = PipelineItem::try_from(&view).unwrap();
assert!(item.is_frozen());
}
// ── Event bus tests ─────────────────────────────────────────────────
@@ -395,4 +298,11 @@ mod tests {
let err = ProjectionError::MissingField("story_id");
assert_eq!(err.to_string(), "missing required field: story_id");
}
// Compile-time check that GitSha is reachable from the test imports
// (mirrors the previous reverse_projection test that used it).
#[test]
fn git_sha_constructs() {
let _ = GitSha("abc".to_string());
}
}
server/src/pipeline_state/tests.rs
+24 -16
View File
@@ -300,6 +300,9 @@ fn supersede_from_any_active_or_done() {
#[test]
fn review_hold_from_active_stages() {
// Story 945: `ReviewHold` transitions to `Stage::ReviewHold { resume_to }`
// with the resume_to set to the originating stage, replacing the legacy
// boolean flag.
for s in [Stage::Backlog, Stage::Coding, Stage::Qa] {
let result = transition(
s.clone(),
@@ -307,13 +310,14 @@ fn review_hold_from_active_stages() {
reason: "review".into(),
},
);
assert!(matches!(
result,
Ok(Stage::Archived {
reason: ArchiveReason::ReviewHeld { .. },
..
})
));
let resumed = match result {
Ok(Stage::ReviewHold { resume_to, .. }) => *resume_to,
other => panic!("ReviewHold should produce Stage::ReviewHold; got {other:?}"),
};
assert_eq!(
resumed, s,
"resume_to should preserve the originating stage"
);
}
}
@@ -578,7 +582,9 @@ fn cannot_reject_from_archived() {
/// to "restore". Tests the freeze/unfreeze API on the apply layer, since
/// freeze/unfreeze are no longer pure stage transitions.
#[test]
fn freeze_sets_flag_without_changing_stage() {
fn freeze_transitions_to_frozen_variant_with_resume_to() {
// Story 945: freeze/unfreeze move the typed stage to `Stage::Frozen
// { resume_to }` and back, replacing the orthogonal boolean flag.
crate::crdt_state::init_for_test();
crate::db::ensure_content_store();
@@ -597,24 +603,26 @@ fn freeze_sets_flag_without_changing_stage() {
super::apply::transition_to_frozen(story_id).expect("freeze should succeed");
let item = read_typed(story_id).unwrap().unwrap();
assert!(
matches!(item.stage, Stage::Coding),
"stage register stays at Coding after freeze: {:?}",
item.stage
);
assert!(item.is_frozen(), "frozen flag should be set after freeze");
match &item.stage {
Stage::Frozen { resume_to } => assert!(
matches!(**resume_to, Stage::Coding),
"resume_to should preserve the previous stage; got {resume_to:?}"
),
other => panic!("stage should be Stage::Frozen after freeze; got {other:?}"),
}
assert!(item.is_frozen(), "is_frozen() should be true after freeze");
super::apply::transition_to_unfrozen(story_id).expect("unfreeze should succeed");
let item = read_typed(story_id).unwrap().unwrap();
assert!(
matches!(item.stage, Stage::Coding),
"stage register still at Coding after unfreeze: {:?}",
"stage should return to Coding after unfreeze: {:?}",
item.stage
);
assert!(
!item.is_frozen(),
"frozen flag should be cleared after unfreeze"
"is_frozen() should be false after unfreeze"
);
}
server/src/pipeline_state/transition.rs
+53 -7
View File
@@ -59,6 +59,15 @@ pub enum PipelineEvent {
Close,
/// Manual demotion back to backlog from an active stage.
Demote,
/// Story 945: freeze a story at its current stage.
Freeze,
/// Story 945: unfreeze a frozen story, returning to `resume_to`.
Unfreeze,
/// Story 945: clear a `ReviewHold`, returning to `resume_to`.
ReviewHoldCleared,
/// Story 945: mergemaster has been auto-spawned and gave up; transitions
/// `Stage::MergeFailure` → `Stage::MergeFailureFinal`.
MergemasterAttempted,
}
// ── Per-node execution events ───────────────────────────────────────────────
@@ -98,6 +107,10 @@ pub fn event_label(e: &PipelineEvent) -> &'static str {
PipelineEvent::Triage => "Triage",
PipelineEvent::Close => "Close",
PipelineEvent::Demote => "Demote",
PipelineEvent::Freeze => "Freeze",
PipelineEvent::Unfreeze => "Unfreeze",
PipelineEvent::ReviewHoldCleared => "ReviewHoldCleared",
PipelineEvent::MergemasterAttempted => "MergemasterAttempted",
}
}
@@ -172,13 +185,16 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
| (Qa, Block { reason })
| (Merge { .. }, Block { reason }) => Ok(Blocked { reason }),
(Backlog, ReviewHold { reason })
| (Coding, ReviewHold { reason })
| (Qa, ReviewHold { reason })
| (Merge { .. }, ReviewHold { reason }) => Ok(Archived {
archived_at: now,
reason: ArchiveReason::ReviewHeld { reason },
}),
// Story 945: ReviewHold no longer auto-archives. It transitions the
// story to `Stage::ReviewHold { resume_to, reason }`, preserving the
// current stage as the resume target so a reviewer can clear the
// hold and continue.
(s @ (Backlog | Coding | Qa | Merge { .. }), PipelineEvent::ReviewHold { reason }) => {
Ok(Stage::ReviewHold {
resume_to: Box::new(s),
reason,
})
}
// ── MergeFailed: Merge → MergeFailure (recoverable intermediate) ──
(Merge { .. }, MergeFailed { reason }) => Ok(MergeFailure { reason }),
@@ -239,6 +255,36 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
merge_commit: GitSha("closed".to_string()),
}),
// ── Freeze: any non-terminal stage → Frozen { resume_to } ──────
(
s @ (Upcoming
| Backlog
| Coding
| Qa
| Merge { .. }
| Blocked { .. }
| MergeFailure { .. }
| MergeFailureFinal { .. }
| Stage::ReviewHold { .. }),
Freeze,
) => Ok(Frozen {
resume_to: Box::new(s),
}),
// ── Unfreeze: Frozen → resume_to ───────────────────────────────
(Frozen { resume_to }, Unfreeze) => Ok(*resume_to),
// ── ReviewHoldCleared: ReviewHold → resume_to ──────────────────
(Stage::ReviewHold { resume_to, .. }, ReviewHoldCleared) => Ok(*resume_to),
// ── MergemasterAttempted: MergeFailure → MergeFailureFinal ─────
(MergeFailure { reason }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }),
(MergeFailureFinal { reason }, MergemasterAttempted) => Ok(MergeFailureFinal { reason }),
// ── Unblock: from Frozen/ReviewHold → resume_to ────────────────
(Frozen { resume_to }, Unblock) => Ok(*resume_to),
(Stage::ReviewHold { resume_to, .. }, Unblock) => Ok(*resume_to),
// ── Unblock: Blocked → Coding ─────────────────────────────────
(Blocked { .. }, Unblock) => Ok(Coding),
server/src/pipeline_state/types.rs
+67 -9
View File
@@ -111,6 +111,29 @@ pub enum Stage {
/// this is a recoverable intermediate state — `Unblock` returns to `Coding`
/// (immediate agent retry) and `Demote` returns to `Backlog` (manual park).
MergeFailure { reason: String },
/// Merge pipeline failed AND mergemaster has already been auto-spawned to
/// recover; the agent gave up. The story stays here awaiting human
/// intervention — the auto-assigner will NOT spawn mergemaster again.
/// Replaces the legacy `mergemaster_attempted: true` boolean flag.
MergeFailureFinal { reason: String },
/// Story is frozen — kept at this stage as a snapshot of its previous
/// stage. Replaces the legacy `frozen: true` boolean flag: there is no
/// "frozen AND coding" combinatorial state; a story is either Frozen
/// (with the stage it would resume to) or not. The auto-assigner skips
/// frozen stories; `Unfreeze` transitions back to `resume_to`.
Frozen { resume_to: Box<Stage> },
/// Story is held for human review at a pipeline-stage boundary (e.g.
/// spikes after QA passes, human-QA items after gates pass). Replaces
/// the legacy `review_hold: true` boolean flag: the held story has a
/// definite resume target stored on the variant. The auto-assigner
/// skips review-held stories; clearing the hold returns to `resume_to`.
ReviewHold {
resume_to: Box<Stage>,
reason: String,
},
}
/// Why a story was archived. Subsumes the old `blocked`, `merge_failure`,
@@ -151,13 +174,15 @@ impl Stage {
stage_dir_name(self)
}
/// Returns true if this is the `Blocked` or `MergeFailure` variant (or the
/// legacy `Archived(Blocked)` for backward-compatible reads).
/// Returns true if this is the `Blocked`, `MergeFailure`, or
/// `MergeFailureFinal` variant (or the legacy `Archived(Blocked)` for
/// backward-compatible reads).
pub fn is_blocked(&self) -> bool {
matches!(
self,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
@@ -165,6 +190,27 @@ impl Stage {
)
}
/// Returns true if this is the `Frozen` variant. Story 945: replaces
/// the legacy `frozen: true` boolean flag. The auto-assigner skips
/// frozen stories.
pub fn is_frozen(&self) -> bool {
matches!(self, Stage::Frozen { .. })
}
/// Returns true if this is the `ReviewHold` variant. Story 945:
/// replaces the legacy `review_hold: true` boolean flag. The
/// auto-assigner skips review-held stories.
pub fn is_review_hold(&self) -> bool {
matches!(self, Stage::ReviewHold { .. })
}
/// Returns true if mergemaster has already been auto-spawned for this
/// story (`MergeFailureFinal`). Story 945: replaces the legacy
/// `mergemaster_attempted: true` boolean flag.
pub fn is_mergemaster_attempted(&self) -> bool {
matches!(self, Stage::MergeFailureFinal { .. })
}
/// Parse a stage from its filesystem directory name.
///
/// This is the single canonical conversion boundary for turning a loose
@@ -189,6 +235,16 @@ impl Stage {
"merge_failure" => Some(Stage::MergeFailure {
reason: String::new(),
}),
"merge_failure_final" => Some(Stage::MergeFailureFinal {
reason: String::new(),
}),
"frozen" => Some(Stage::Frozen {
resume_to: Box::new(Stage::Coding),
}),
"review_hold" => Some(Stage::ReviewHold {
resume_to: Box::new(Stage::Coding),
reason: String::new(),
}),
"done" => Some(Stage::Done {
merged_at: DateTime::<Utc>::UNIX_EPOCH,
merge_commit: GitSha(String::new()),
@@ -242,17 +298,13 @@ pub struct PipelineItem {
pub stage: Stage,
pub depends_on: Vec<StoryId>,
pub retry_count: u32,
/// Whether the item is frozen — orthogonal to [`Self::stage`].
/// Frozen items remain at their current stage but are skipped by the
/// auto-assigner until explicitly unfrozen (story 934, stage 4).
pub frozen: bool,
}
impl PipelineItem {
/// Whether the item is frozen. Frozen items stay at their current
/// [`Stage`] but are skipped by the auto-assigner until unfrozen.
/// Whether the item is frozen — story 945: `Stage::Frozen` is now the
/// single source of truth, replacing the legacy boolean flag.
pub fn is_frozen(&self) -> bool {
self.frozen
self.stage.is_frozen()
}
}
@@ -287,8 +339,11 @@ pub fn stage_label(s: &Stage) -> &'static str {
Stage::Qa => "Qa",
Stage::Merge { .. } => "Merge",
Stage::MergeFailure { .. } => "MergeFailure",
Stage::MergeFailureFinal { .. } => "MergeFailureFinal",
Stage::Done { .. } => "Done",
Stage::Blocked { .. } => "Blocked",
Stage::Frozen { .. } => "Frozen",
Stage::ReviewHold { .. } => "ReviewHold",
Stage::Archived { .. } => "Archived",
}
}
@@ -307,6 +362,9 @@ pub fn stage_dir_name(s: &Stage) -> &'static str {
Stage::Qa => "qa",
Stage::Merge { .. } => "merge",
Stage::MergeFailure { .. } => "merge_failure",
Stage::MergeFailureFinal { .. } => "merge_failure_final",
Stage::Frozen { .. } => "frozen",
Stage::ReviewHold { .. } => "review_hold",
Stage::Done { .. } => "done",
Stage::Archived { .. } => "archived",
}