huskies: merge 529_bug_stale_mergemaster_advance_moves_done_stories_back_to_merge_zombie_merge_loop
This commit is contained in:
@@ -179,7 +179,7 @@ pub fn move_story_to_merge(project_root: &Path, story_id: &str) -> Result<(), St
|
||||
story_id,
|
||||
&["2_current", "3_qa"],
|
||||
"4_merge",
|
||||
&[],
|
||||
&["5_done", "6_archived"],
|
||||
false,
|
||||
&["retry_count", "blocked"],
|
||||
)
|
||||
@@ -195,7 +195,7 @@ pub fn move_story_to_qa(project_root: &Path, story_id: &str) -> Result<(), Strin
|
||||
story_id,
|
||||
&["2_current"],
|
||||
"3_qa",
|
||||
&[],
|
||||
&["5_done", "6_archived"],
|
||||
false,
|
||||
&["retry_count", "blocked"],
|
||||
)
|
||||
|
||||
@@ -231,6 +231,23 @@ impl AgentPool {
|
||||
}
|
||||
}
|
||||
PipelineStage::Mergemaster => {
|
||||
// Bug 529: Guard against stale mergemaster advances. If the story
|
||||
// has already reached done or archived (e.g. a previous mergemaster
|
||||
// succeeded), this advance is a zombie — skip it entirely to avoid
|
||||
// phantom notifications and redundant post-merge test runs.
|
||||
if let Ok(Some(typed_item)) = crate::pipeline_state::read_typed(story_id) {
|
||||
let current_dir = typed_item.stage.dir_name();
|
||||
if current_dir == "5_done" || current_dir == "6_archived" {
|
||||
slog!(
|
||||
"[pipeline] Skipping stale mergemaster advance for '{story_id}': \
|
||||
story is already in work/{current_dir}/"
|
||||
);
|
||||
// Skip pipeline advancement — do not run post-merge tests,
|
||||
// do not emit notifications, do not restart agents.
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Block advancement if the mergemaster explicitly reported a failure.
|
||||
// The server-owned gate check runs in the feature-branch worktree (not
|
||||
// master), so `gates_passed=true` is misleading when no code was merged.
|
||||
@@ -885,4 +902,88 @@ stage = "qa"
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
|
||||
// ── bug 529: stale mergemaster advance for a done story is a no-op ──
|
||||
|
||||
/// Regression test for bug 529: when a stale mergemaster advance fires
|
||||
/// after the story has already reached 5_done, the advance must be a
|
||||
/// no-op — no post-merge tests, no notifications, no agent restarts.
|
||||
#[tokio::test]
|
||||
async fn stale_mergemaster_advance_for_done_story_is_noop() {
|
||||
use std::fs;
|
||||
use std::process::Command;
|
||||
|
||||
// Initialise CRDT so read_typed works.
|
||||
crate::crdt_state::init_for_test();
|
||||
crate::db::ensure_content_store();
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let root = tmp.path();
|
||||
|
||||
// Init a git repo so post-merge tests would pass if they ran.
|
||||
Command::new("git").args(["init"]).current_dir(root).output().unwrap();
|
||||
Command::new("git").args(["config", "user.email", "test@test.com"]).current_dir(root).output().unwrap();
|
||||
Command::new("git").args(["config", "user.name", "Test"]).current_dir(root).output().unwrap();
|
||||
Command::new("git").args(["commit", "--allow-empty", "-m", "init"]).current_dir(root).output().unwrap();
|
||||
|
||||
// Set up pipeline dirs.
|
||||
fs::create_dir_all(root.join(".huskies/work/5_done")).unwrap();
|
||||
|
||||
// Seed the story in 5_done via the DB, which also writes to the CRDT.
|
||||
let story_id = "9929_story_zombie_merge";
|
||||
let content = "---\nname: Zombie Merge Test\n---\n";
|
||||
crate::db::write_content(story_id, content);
|
||||
crate::db::write_item_with_content(story_id, "5_done", content);
|
||||
|
||||
let pool = AgentPool::new_test(3001);
|
||||
let mut rx = pool.watcher_tx.subscribe();
|
||||
|
||||
// Simulate a stale mergemaster advance firing for the already-done story.
|
||||
pool.run_pipeline_advance(
|
||||
story_id,
|
||||
"mergemaster",
|
||||
CompletionReport {
|
||||
summary: "stale advance".to_string(),
|
||||
gates_passed: true,
|
||||
gate_output: String::new(),
|
||||
},
|
||||
Some(root.to_path_buf()),
|
||||
None,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
// No agents should have been started.
|
||||
let agents = pool.agents.lock().unwrap();
|
||||
assert!(
|
||||
agents.is_empty(),
|
||||
"No agents should be started for a stale advance on a done story. \
|
||||
Pool: {:?}",
|
||||
agents.keys().collect::<Vec<_>>()
|
||||
);
|
||||
drop(agents);
|
||||
|
||||
// No StoryBlocked or other events should have been emitted.
|
||||
let mut got_event = false;
|
||||
while let Ok(evt) = rx.try_recv() {
|
||||
// AgentStateChanged from auto_assign is acceptable only if the
|
||||
// advance didn't short-circuit. Since we return early, no events.
|
||||
if matches!(evt, WatcherEvent::StoryBlocked { .. }) {
|
||||
got_event = true;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!got_event,
|
||||
"No StoryBlocked event should be emitted for a stale advance"
|
||||
);
|
||||
|
||||
// The story should still be in 5_done (not moved elsewhere).
|
||||
if let Ok(Some(item)) = crate::pipeline_state::read_typed(story_id) {
|
||||
assert_eq!(
|
||||
item.stage.dir_name(),
|
||||
"5_done",
|
||||
"Story should remain in 5_done after stale mergemaster advance"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user