huskies: merge 988

This commit is contained in:
dave
2026-05-13 17:24:18 +00:00
parent a078d3df7c
commit caed894db9
15 changed files with 169 additions and 81 deletions
+10 -1
View File
@@ -52,7 +52,16 @@ pub(super) async fn scan_and_claim(
} }
// Skip blocked stories (story 945: `Stage::Blocked` is the source of truth). // Skip blocked stories (story 945: `Stage::Blocked` is the source of truth).
if item.stage().is_blocked() { if matches!(
item.stage(),
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
) {
continue; continue;
} }
+13 -1
View File
@@ -679,7 +679,19 @@ mod tests {
"blocked", "blocked",
"should be in blocked after transition_to_blocked" "should be in blocked after transition_to_blocked"
); );
assert!(item.stage.is_blocked(), "is_blocked() should return true"); assert!(
matches!(
item.stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
),
"stage should be a blocked variant"
);
assert!( assert!(
matches!(item.stage, Stage::Blocked { .. }), matches!(item.stage, Stage::Blocked { .. }),
"stage should be Stage::Blocked variant" "stage should be Stage::Blocked variant"
@@ -22,7 +22,7 @@ pub(super) fn read_story_front_matter_agent(story_id: &str) -> Option<String> {
/// `tool_approve_qa`). /// `tool_approve_qa`).
pub(super) fn has_review_hold(story_id: &str) -> bool { pub(super) fn has_review_hold(story_id: &str) -> bool {
crate::crdt_state::read_item(story_id) crate::crdt_state::read_item(story_id)
.map(|w| w.stage().is_review_hold()) .map(|w| matches!(w.stage(), crate::pipeline_state::Stage::ReviewHold { .. }))
.unwrap_or(false) .unwrap_or(false)
} }
@@ -35,7 +35,18 @@ pub(super) fn is_story_blocked(story_id: &str) -> bool {
crate::pipeline_state::read_typed(story_id) crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.flatten() .flatten()
.map(|item| item.stage.is_blocked()) .map(|item| {
matches!(
item.stage,
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
)
})
.unwrap_or(false) .unwrap_or(false)
} }
@@ -70,7 +81,12 @@ pub(super) fn has_content_conflict_failure(story_id: &str) -> bool {
/// the same story after a failed mergemaster session. /// the same story after a failed mergemaster session.
pub(super) fn has_mergemaster_attempted(story_id: &str) -> bool { pub(super) fn has_mergemaster_attempted(story_id: &str) -> bool {
crate::crdt_state::read_item(story_id) crate::crdt_state::read_item(story_id)
.map(|view| view.stage().is_mergemaster_attempted()) .map(|view| {
matches!(
view.stage(),
crate::pipeline_state::Stage::MergeFailureFinal { .. }
)
})
.unwrap_or(false) .unwrap_or(false)
} }
@@ -95,7 +111,7 @@ pub(super) fn check_archived_dependencies(story_id: &str) -> Vec<u32> {
/// `resume_to`. /// `resume_to`.
pub(super) fn is_story_frozen(story_id: &str) -> bool { pub(super) fn is_story_frozen(story_id: &str) -> bool {
crate::crdt_state::read_item(story_id) crate::crdt_state::read_item(story_id)
.map(|view| view.stage().is_frozen()) .map(|view| matches!(view.stage(), crate::pipeline_state::Stage::Frozen { .. }))
.unwrap_or(false) .unwrap_or(false)
} }
+1 -1
View File
@@ -205,7 +205,7 @@ mod tests {
.expect("read_typed should succeed") .expect("read_typed should succeed")
.expect("item should be present"); .expect("item should be present");
assert!( assert!(
item.is_frozen(), matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }),
"stage should be Frozen after freeze: {:?}", "stage should be Frozen after freeze: {:?}",
item.stage item.stage
); );
+11 -2
View File
@@ -218,7 +218,7 @@ fn render_item_line(
Some(item.name.as_str()) Some(item.name.as_str())
}; };
// Use the typed CRDT stage as the sole source of truth (story 945). // Use the typed CRDT stage as the sole source of truth (story 945).
let frozen = item.stage.is_frozen(); let frozen = matches!(item.stage, Stage::Frozen { .. });
let base_label = super::story_short_label(story_id, name_opt); let base_label = super::story_short_label(story_id, name_opt);
let display = if frozen { let display = if frozen {
format!("\u{2744}\u{FE0F} {base_label}") // ❄️ prefix format!("\u{2744}\u{FE0F} {base_label}") // ❄️ prefix
@@ -310,7 +310,16 @@ fn render_item_line(
} }
} }
let blocked = item.stage.is_blocked(); let blocked = matches!(
item.stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
);
// Blocked items with a recovery agent get differentiated indicators. // Blocked items with a recovery agent get differentiated indicators.
if blocked { if blocked {
return match agent.map(|a| &a.status) { return match agent.map(|a| &a.status) {
+30 -3
View File
@@ -362,12 +362,30 @@ fn stage_is_blocked_returns_true_for_archived_blocked() {
reason: "too many retries".to_string(), reason: "too many retries".to_string(),
}, },
}; };
assert!(stage.is_blocked()); assert!(matches!(
stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
));
} }
#[test] #[test]
fn stage_is_blocked_returns_false_for_coding() { fn stage_is_blocked_returns_false_for_coding() {
assert!(!Stage::Coding.is_blocked()); assert!(!matches!(
Stage::Coding,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
));
} }
#[test] #[test]
@@ -376,7 +394,16 @@ fn stage_is_blocked_returns_false_for_archived_completed() {
archived_at: Utc::now(), archived_at: Utc::now(),
reason: ArchiveReason::Completed, reason: ArchiveReason::Completed,
}; };
assert!(!stage.is_blocked()); assert!(!matches!(
stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: ArchiveReason::Blocked { .. },
..
}
));
} }
// -- status output shows idle dot for items with no active agent -------- // -- status output shows idle dot for items with no active agent --------
+10 -1
View File
@@ -86,7 +86,16 @@ fn build_triage_dump(
if let Some(ref w) = crdt_item { if let Some(ref w) = crdt_item {
let mut fields: Vec<String> = Vec::new(); let mut fields: Vec<String> = Vec::new();
// Story 945: `Stage::Blocked` is the source of truth. // Story 945: `Stage::Blocked` is the source of truth.
if w.stage().is_blocked() { if matches!(
w.stage(),
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
) {
fields.push("**blocked:** true".to_string()); fields.push("**blocked:** true".to_string());
} }
if let Some(agent) = w.agent() { if let Some(agent) = w.agent() {
+32 -5
View File
@@ -61,9 +61,18 @@ fn unblock_by_story_id(story_id: &str) -> String {
// Story 945: `Stage::Blocked` / `Stage::MergeFailure` are the single // Story 945: `Stage::Blocked` / `Stage::MergeFailure` are the single
// source of truth — the legacy `blocked` boolean flag is gone. // source of truth — the legacy `blocked` boolean flag is gone.
let typed_item = crate::pipeline_state::read_typed(story_id).ok().flatten(); let typed_item = crate::pipeline_state::read_typed(story_id).ok().flatten();
let typed_blocked = typed_item let typed_blocked = typed_item.as_ref().is_some_and(|item| {
.as_ref() matches!(
.is_some_and(|item| item.stage.is_blocked()); item.stage,
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
)
});
let typed_merge_failure = matches!( let typed_merge_failure = matches!(
typed_item.as_ref().map(|i| &i.stage), typed_item.as_ref().map(|i| &i.stage),
Some(crate::pipeline_state::Stage::MergeFailure { .. }) Some(crate::pipeline_state::Stage::MergeFailure { .. })
@@ -239,7 +248,16 @@ mod tests {
// Story 945: `Stage::Blocked` was the source of truth; after unblock // Story 945: `Stage::Blocked` was the source of truth; after unblock
// the stage must have transitioned out of `Blocked`. // the stage must have transitioned out of `Blocked`.
assert!( assert!(
!item.stage().is_blocked(), !matches!(
item.stage(),
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
),
"stage must no longer be Blocked after unblock" "stage must no longer be Blocked after unblock"
); );
} }
@@ -307,7 +325,16 @@ mod tests {
// Story 945: `Stage::Blocked` was the source of truth; after unblock // Story 945: `Stage::Blocked` was the source of truth; after unblock
// the stage must have transitioned out of `Blocked`. // the stage must have transitioned out of `Blocked`.
assert!( assert!(
!item.stage().is_blocked(), !matches!(
item.stage(),
crate::pipeline_state::Stage::Blocked { .. }
| crate::pipeline_state::Stage::MergeFailure { .. }
| crate::pipeline_state::Stage::MergeFailureFinal { .. }
| crate::pipeline_state::Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
),
"stage must no longer be Blocked after unblock" "stage must no longer be Blocked after unblock"
); );
} }
@@ -75,7 +75,10 @@ mod tests {
let item = crate::pipeline_state::read_typed(story_id) let item = crate::pipeline_state::read_typed(story_id)
.expect("read_typed should succeed") .expect("read_typed should succeed")
.expect("item should be present"); .expect("item should be present");
assert!(item.is_frozen(), "stage should be frozen after MCP freeze"); assert!(
matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }),
"stage should be frozen after MCP freeze"
);
} }
#[test] #[test]
@@ -106,7 +109,7 @@ mod tests {
.expect("read_typed should succeed") .expect("read_typed should succeed")
.expect("item should be present"); .expect("item should be present");
assert!( assert!(
!item.is_frozen(), !matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }),
"stage should not be frozen after MCP unfreeze" "stage should not be frozen after MCP unfreeze"
); );
} }
+23 -5
View File
@@ -115,7 +115,7 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result<PipelineState, String> {
// come from typed CRDT registers. merge_failure detail lives on the // come from typed CRDT registers. merge_failure detail lives on the
// MergeJob CRDT entry (same as status_tools.rs). // MergeJob CRDT entry (same as status_tools.rs).
let view = crate::crdt_state::read_item(sid); let view = crate::crdt_state::read_item(sid);
let review_hold = if item.stage.is_review_hold() { let review_hold = if matches!(item.stage, Stage::ReviewHold { .. }) {
Some(true) Some(true)
} else { } else {
None None
@@ -137,12 +137,21 @@ pub fn load_pipeline_state(ctx: &AppContext) -> Result<PipelineState, String> {
} else { } else {
None None
}, },
blocked: if item.stage.is_blocked() { blocked: if matches!(
item.stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
) {
Some(true) Some(true)
} else { } else {
None None
}, },
frozen: if item.stage.is_frozen() { frozen: if matches!(item.stage, Stage::Frozen { .. }) {
Some(true) Some(true)
} else { } else {
None None
@@ -262,12 +271,21 @@ pub fn load_upcoming_stories(_ctx: &AppContext) -> Result<Vec<UpcomingStory>, St
} else { } else {
None None
}, },
blocked: if item.stage.is_blocked() { blocked: if matches!(
item.stage,
Stage::Blocked { .. }
| Stage::MergeFailure { .. }
| Stage::MergeFailureFinal { .. }
| Stage::Archived {
reason: crate::pipeline_state::ArchiveReason::Blocked { .. },
..
}
) {
Some(true) Some(true)
} else { } else {
None None
}, },
frozen: if item.stage.is_frozen() { frozen: if matches!(item.stage, Stage::Frozen { .. }) {
Some(true) Some(true)
} else { } else {
None None
+1 -1
View File
@@ -36,7 +36,7 @@ pub fn is_story_frozen_in_store(story_id: &str) -> bool {
crate::pipeline_state::read_typed(story_id) crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.flatten() .flatten()
.map(|item| item.is_frozen()) .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }))
.unwrap_or(false) .unwrap_or(false)
} }
+1 -1
View File
@@ -275,7 +275,7 @@ mod tests {
Some("Frozen Story"), Some("Frozen Story"),
); );
let item = PipelineItem::try_from(&view).unwrap(); let item = PipelineItem::try_from(&view).unwrap();
assert!(item.is_frozen()); assert!(matches!(item.stage, Stage::Frozen { .. }));
} }
// ── Event bus tests ───────────────────────────────────────────────── // ── Event bus tests ─────────────────────────────────────────────────
+6 -3
View File
@@ -562,7 +562,7 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() {
let item = read_typed(story_id).unwrap().unwrap(); let item = read_typed(story_id).unwrap().unwrap();
assert!(matches!(item.stage, Stage::Coding)); assert!(matches!(item.stage, Stage::Coding));
assert!(!item.is_frozen()); assert!(!matches!(item.stage, Stage::Frozen { .. }));
super::apply::transition_to_frozen(story_id).expect("freeze should succeed"); super::apply::transition_to_frozen(story_id).expect("freeze should succeed");
@@ -574,7 +574,10 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() {
), ),
other => panic!("stage should be Stage::Frozen after freeze; got {other:?}"), other => panic!("stage should be Stage::Frozen after freeze; got {other:?}"),
} }
assert!(item.is_frozen(), "is_frozen() should be true after freeze"); assert!(
matches!(item.stage, Stage::Frozen { .. }),
"is_frozen() should be true after freeze"
);
super::apply::transition_to_unfrozen(story_id).expect("unfreeze should succeed"); super::apply::transition_to_unfrozen(story_id).expect("unfreeze should succeed");
@@ -585,7 +588,7 @@ fn freeze_transitions_to_frozen_variant_with_resume_to() {
item.stage item.stage
); );
assert!( assert!(
!item.is_frozen(), !matches!(item.stage, Stage::Frozen { .. }),
"is_frozen() should be false after unfreeze" "is_frozen() should be false after unfreeze"
); );
} }
-45
View File
@@ -262,43 +262,6 @@ impl Stage {
stage_dir_name(self) stage_dir_name(self)
} }
/// 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 { .. },
..
}
)
}
/// 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. /// Parse a stage from its filesystem directory name.
/// ///
/// This is the single canonical conversion boundary for turning a loose /// This is the single canonical conversion boundary for turning a loose
@@ -401,14 +364,6 @@ pub struct PipelineItem {
pub retry_count: u32, pub retry_count: u32,
} }
impl PipelineItem {
/// 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.stage.is_frozen()
}
}
// ── Transition errors ─────────────────────────────────────────────────────── // ── Transition errors ───────────────────────────────────────────────────────
/// Error returned when a pipeline event is not valid for the current stage. /// Error returned when a pipeline event is not valid for the current stage.
+6 -6
View File
@@ -31,7 +31,7 @@ pub fn freeze(story_id: &str) -> Result<FreezeStatus, String> {
let already_frozen = crate::pipeline_state::read_typed(story_id) let already_frozen = crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.flatten() .flatten()
.map(|item| item.is_frozen()) .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }))
.unwrap_or(false); .unwrap_or(false);
if already_frozen { if already_frozen {
@@ -51,7 +51,7 @@ pub fn unfreeze(story_id: &str) -> Result<UnfreezeStatus, String> {
let is_frozen = crate::pipeline_state::read_typed(story_id) let is_frozen = crate::pipeline_state::read_typed(story_id)
.ok() .ok()
.flatten() .flatten()
.map(|item| item.is_frozen()) .map(|item| matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }))
.unwrap_or(false); .unwrap_or(false);
if !is_frozen { if !is_frozen {
@@ -92,7 +92,7 @@ mod tests {
.expect("read_typed should succeed") .expect("read_typed should succeed")
.expect("item should be present"); .expect("item should be present");
assert!( assert!(
item.is_frozen(), matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }),
"stage should be Frozen after freeze: {:?}", "stage should be Frozen after freeze: {:?}",
item.stage item.stage
); );
@@ -141,7 +141,7 @@ mod tests {
.expect("read_typed should succeed") .expect("read_typed should succeed")
.expect("item should be present"); .expect("item should be present");
assert!( assert!(
!item.is_frozen(), !matches!(item.stage, crate::pipeline_state::Stage::Frozen { .. }),
"stage should not be Frozen after unfreeze: {:?}", "stage should not be Frozen after unfreeze: {:?}",
item.stage item.stage
); );
@@ -212,12 +212,12 @@ mod tests {
.expect("MCP-path item should be in CRDT"); .expect("MCP-path item should be in CRDT");
assert!( assert!(
state_a.is_frozen(), matches!(state_a.stage, crate::pipeline_state::Stage::Frozen { .. }),
"chat-path CRDT stage must be frozen: {:?}", "chat-path CRDT stage must be frozen: {:?}",
state_a.stage state_a.stage
); );
assert!( assert!(
state_b.is_frozen(), matches!(state_b.stage, crate::pipeline_state::Stage::Frozen { .. }),
"MCP-path CRDT stage must be frozen: {:?}", "MCP-path CRDT stage must be frozen: {:?}",
state_b.stage state_b.stage
); );