huskies: merge 1051
This commit is contained in:
@@ -496,6 +496,25 @@ pub fn move_story_to_stage(story_id: &str, target_stage: &str) -> Result<(String
|
||||
Ok((from_name.to_string(), target_stage.to_string()))
|
||||
}
|
||||
|
||||
/// Transition a story from `Stage::MergeFailure` to `Stage::Merge` when the
|
||||
/// deterministic-merge pipeline starts a retry attempt.
|
||||
///
|
||||
/// Preserves `feature_branch` and `commits_ahead` from the preceding
|
||||
/// `MergeFailure` state so the retry continues with the exact same branch.
|
||||
/// If the story is already in `Stage::Merge` or any other stage, this is a
|
||||
/// silent no-op — callers do not need to pre-check the current stage.
|
||||
///
|
||||
/// Called by `start_merge_agent_work` before the squash-merge pipeline starts
|
||||
/// so the UI shows an active merge state rather than a stale failure indicator
|
||||
/// (story 1051, AC 1 and AC 4).
|
||||
pub fn transition_merge_failure_to_retry(story_id: &str) -> Result<(), String> {
|
||||
match apply_transition(story_id, PipelineEvent::MergeRetryStarted, None) {
|
||||
Ok(_) => Ok(()),
|
||||
Err(ApplyError::InvalidTransition(_)) | Err(ApplyError::NotFound(_)) => Ok(()),
|
||||
Err(e) => Err(e.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/`.
|
||||
@@ -1028,6 +1047,113 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── Story 1051: MergeFailure → Merge retry transition ───────────────────
|
||||
|
||||
/// AC1 (story 1051): when the deterministic-merge pipeline starts on a
|
||||
/// MergeFailure story, transition_merge_failure_to_retry moves it to Merge.
|
||||
#[test]
|
||||
fn transition_merge_failure_to_retry_moves_to_merge() {
|
||||
crate::crdt_state::init_for_test();
|
||||
crate::db::ensure_content_store();
|
||||
|
||||
let story_id = "99105_story_merge_retry_1051";
|
||||
crate::db::write_item_with_content(
|
||||
story_id,
|
||||
"merge_failure",
|
||||
"---\nname: Merge Retry Test\n---\n",
|
||||
crate::db::ItemMeta::named("Merge Retry Test"),
|
||||
);
|
||||
|
||||
transition_merge_failure_to_retry(story_id).expect("must succeed for MergeFailure story");
|
||||
|
||||
let item = crate::pipeline_state::read_typed(story_id)
|
||||
.expect("CRDT read must succeed")
|
||||
.expect("item must exist");
|
||||
assert_eq!(
|
||||
item.stage.dir_name(),
|
||||
"merge",
|
||||
"MergeFailure story must be in Merge after retry transition: {:?}",
|
||||
item.stage
|
||||
);
|
||||
assert!(
|
||||
matches!(item.stage, Stage::Merge { .. }),
|
||||
"stage must be Stage::Merge variant: {:?}",
|
||||
item.stage
|
||||
);
|
||||
}
|
||||
|
||||
/// AC1 (story 1051): transition_merge_failure_to_retry is a no-op when the
|
||||
/// story is already in Stage::Merge — callers do not need to pre-check.
|
||||
#[test]
|
||||
fn transition_merge_failure_to_retry_noop_when_already_merge() {
|
||||
crate::crdt_state::init_for_test();
|
||||
crate::db::ensure_content_store();
|
||||
|
||||
let story_id = "99106_story_already_merge_1051";
|
||||
crate::db::write_item_with_content(
|
||||
story_id,
|
||||
"4_merge",
|
||||
"---\nname: Already Merge\n---\n",
|
||||
crate::db::ItemMeta::named("Already Merge"),
|
||||
);
|
||||
|
||||
// Must not error even though story is in Merge, not MergeFailure.
|
||||
assert!(
|
||||
transition_merge_failure_to_retry(story_id).is_ok(),
|
||||
"must be a no-op when story is in Merge"
|
||||
);
|
||||
|
||||
let item = crate::pipeline_state::read_typed(story_id)
|
||||
.expect("CRDT read must succeed")
|
||||
.expect("item must exist");
|
||||
assert_eq!(
|
||||
item.stage.dir_name(),
|
||||
"merge",
|
||||
"story must still be in merge after no-op: {:?}",
|
||||
item.stage
|
||||
);
|
||||
}
|
||||
|
||||
/// AC3 (story 1051): after a MergeRetryStarted transition (MergeFailure →
|
||||
/// Merge), a subsequent MergeFailed event sets a fresh failure — the
|
||||
/// transition's `before` stage is Merge (not MergeFailure), confirming the
|
||||
/// failure flag is not carried over from the previous attempt.
|
||||
#[test]
|
||||
fn merge_retry_followed_by_failure_sets_fresh_merge_failure() {
|
||||
crate::crdt_state::init_for_test();
|
||||
crate::db::ensure_content_store();
|
||||
|
||||
let story_id = "99107_story_fresh_failure_1051";
|
||||
crate::db::write_item_with_content(
|
||||
story_id,
|
||||
"merge_failure",
|
||||
"---\nname: Fresh Failure Test\n---\n",
|
||||
crate::db::ItemMeta::named("Fresh Failure Test"),
|
||||
);
|
||||
|
||||
// Retry start: MergeFailure → Merge.
|
||||
transition_merge_failure_to_retry(story_id).expect("retry start must succeed");
|
||||
|
||||
// Pipeline fails again: Merge → MergeFailure.
|
||||
let fired = transition_to_merge_failure(
|
||||
story_id,
|
||||
MergeFailureKind::GatesFailed("new gate output".to_string()),
|
||||
)
|
||||
.expect("transition_to_merge_failure must succeed after retry");
|
||||
|
||||
// 'before' must be Merge (fresh failure, not a self-loop from MergeFailure).
|
||||
assert!(
|
||||
matches!(fired.before, Stage::Merge { .. }),
|
||||
"before must be Merge, proving this is a fresh failure not a self-loop: {:?}",
|
||||
fired.before
|
||||
);
|
||||
assert!(
|
||||
matches!(fired.after, Stage::MergeFailure { .. }),
|
||||
"after must be MergeFailure: {:?}",
|
||||
fired.after
|
||||
);
|
||||
}
|
||||
|
||||
/// Bug 226: feature_branch_has_unmerged_changes returns false when no
|
||||
/// feature branch exists.
|
||||
#[test]
|
||||
|
||||
@@ -100,6 +100,16 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
|
||||
// Story 1051: if the story is in MergeFailure, transition it to Merge
|
||||
// before starting the pipeline so the UI shows an active retry state
|
||||
// rather than the stale failure indicator (AC 1, AC 4).
|
||||
if let Err(e) = crate::agents::lifecycle::transition_merge_failure_to_retry(story_id) {
|
||||
slog!(
|
||||
"[merge] Could not transition '{story_id}' from MergeFailure to Merge: {e} \
|
||||
(merge will proceed regardless)"
|
||||
);
|
||||
}
|
||||
|
||||
// Insert Running job into CRDT.
|
||||
let started_at = unix_now();
|
||||
crate::crdt_state::write_merge_job(
|
||||
|
||||
@@ -76,6 +76,10 @@ pub enum PipelineEvent {
|
||||
MergeAborted,
|
||||
/// Story 974: user re-opens a Done story for a post-merge hotfix, sending it back to Coding.
|
||||
HotfixRequested,
|
||||
/// Story 1051: deterministic-merge pipeline started a retry on a story in
|
||||
/// `Stage::MergeFailure`; transitions back to `Stage::Merge` so the UI
|
||||
/// shows the active retry rather than the stale failure state.
|
||||
MergeRetryStarted,
|
||||
}
|
||||
|
||||
// ── Per-node execution events ───────────────────────────────────────────────
|
||||
@@ -123,6 +127,7 @@ pub fn event_label(e: &PipelineEvent) -> &'static str {
|
||||
PipelineEvent::ReQueuedForQa => "ReQueuedForQa",
|
||||
PipelineEvent::MergeAborted => "MergeAborted",
|
||||
PipelineEvent::HotfixRequested => "HotfixRequested",
|
||||
PipelineEvent::MergeRetryStarted => "MergeRetryStarted",
|
||||
}
|
||||
}
|
||||
|
||||
@@ -365,6 +370,26 @@ pub fn transition(state: Stage, event: PipelineEvent) -> Result<Stage, Transitio
|
||||
retries: 0,
|
||||
}),
|
||||
|
||||
// ── MergeRetryStarted: MergeFailure → Merge (deterministic retry) ─
|
||||
// When the deterministic-merge pipeline starts a retry on a MergeFailure
|
||||
// story, transition back to Merge so the UI shows the active retry rather
|
||||
// than the stale failure state (story 1051). Preserves feature_branch and
|
||||
// commits_ahead so the retry continues with the same branch.
|
||||
(
|
||||
MergeFailure {
|
||||
feature_branch,
|
||||
commits_ahead,
|
||||
..
|
||||
},
|
||||
MergeRetryStarted,
|
||||
) => Ok(Merge {
|
||||
feature_branch,
|
||||
commits_ahead,
|
||||
claim: None,
|
||||
retries: 0,
|
||||
server_start_time: None,
|
||||
}),
|
||||
|
||||
// ── MergemasterAttempted: MergeFailure → MergeFailureFinal ─────
|
||||
(MergeFailure { kind, .. }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }),
|
||||
(MergeFailureFinal { kind }, MergemasterAttempted) => Ok(MergeFailureFinal { kind }),
|
||||
|
||||
Reference in New Issue
Block a user