huskies: merge 1178 bug MergeFailureFinal is a trap state: successful re-merge cannot mark the story done

This commit is contained in:
Huskies Agent
2026-07-16 14:19:55 +00:00
parent 61acf98909
commit e5df6232cf
6 changed files with 226 additions and 34 deletions
@@ -361,15 +361,23 @@ impl AgentPool {
}
let story_archived = crate::agents::lifecycle::move_story_to_done(story_id).is_ok();
if story_archived {
self.remove_agents_for_story(story_id).await;
}
let worktree_cleaned_up = if wt_path.exists() {
let config = crate::config::ProjectConfig::load(project_root).unwrap_or_default();
worktree::remove_worktree_by_story_id(project_root, story_id, &config)
.await
.is_ok()
// Story 1178: only delete the feature branch once the state transition
// to Done is confirmed. Deleting it unconditionally here meant a
// successful squash merge whose CRDT transition failed (e.g. the story
// was in a stage `move_story_to_done` didn't yet handle) would still
// lose its feature branch, making the failure unrecoverable — the
// story couldn't be retried because the branch it needed was gone.
let worktree_cleaned_up = if story_archived {
self.remove_agents_for_story(story_id).await;
if wt_path.exists() {
let config = crate::config::ProjectConfig::load(project_root).unwrap_or_default();
worktree::remove_worktree_by_story_id(project_root, story_id, &config)
.await
.is_ok()
} else {
false
}
} else {
false
};
@@ -409,6 +409,97 @@ async fn merge_agent_work_succeeds_on_clean_branch() {
}
}
/// Regression test (story 1178, AC3): when the squash merge itself succeeds
/// but the CRDT state transition to Done fails (here: no CRDT entry exists
/// for the story, so `move_story_to_done` errors and `story_archived` is
/// false), the feature branch must NOT be deleted — otherwise a retry has
/// nothing to merge from.
#[tokio::test]
async fn merge_success_without_story_archived_keeps_feature_branch() {
let _serial = serial_test_lock();
use std::fs;
use tempfile::tempdir;
crate::crdt_state::init_for_test();
let tmp = tempdir().unwrap();
let repo = tmp.path();
init_git_repo(repo);
let branch = "feature/story-1178_branch_kept";
Command::new("git")
.args(["checkout", "-b", branch])
.current_dir(repo)
.output()
.unwrap();
fs::write(repo.join("feature.txt"), "feature content").unwrap();
Command::new("git")
.args(["add", "."])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["commit", "-m", "add feature"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["checkout", "master"])
.current_dir(repo)
.output()
.unwrap();
let merge_dir = repo.join(".huskies/work/4_merge");
fs::create_dir_all(&merge_dir).unwrap();
fs::write(
merge_dir.join("1178_branch_kept.md"),
"---\nname: Branch Kept Test\n---\n",
)
.unwrap();
Command::new("git")
.args(["add", "."])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["commit", "-m", "add story in merge"])
.current_dir(repo)
.output()
.unwrap();
let pool = Arc::new(AgentPool::new_test(3001));
// Note: no CRDT entry is written for this story, so `move_story_to_done`
// will fail with NotFound — `story_archived` will be false regardless of
// git merge outcome. That is exactly the scenario this test protects.
let job = run_merge_to_completion(&pool, repo, "1178_branch_kept").await;
let MergeJobStatus::Completed(report) = &job.status else {
panic!("expected a completed job, got: {:?}", job.status);
};
if matches!(
report.result,
crate::agents::merge::MergeResult::Success { .. }
) {
assert!(
!report.story_archived,
"story_archived should be false: no CRDT entry exists for this story"
);
assert!(
!report.worktree_cleaned_up,
"worktree/branch must not be cleaned up when story_archived is false"
);
let branch_check = Command::new("git")
.args(["rev-parse", "--verify", branch])
.current_dir(repo)
.output()
.unwrap();
assert!(
branch_check.status.success(),
"feature branch '{branch}' must still exist after a merge whose state \
transition to Done failed, so a retry stays possible"
);
}
}
// ── quality gate ordering test ────────────────────────────────
/// Regression test for bug 142: quality gates must run BEFORE the fast-forward