huskies: merge 519_story_mergemaster_should_detect_no_commits_ahead_of_master_and_fail_loudly_instead_of_exiting_silently
This commit is contained in:
@@ -69,13 +69,8 @@ impl AgentPool {
|
|||||||
slog_error!(
|
slog_error!(
|
||||||
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||||
);
|
);
|
||||||
} else if let Err(e) = self
|
} else {
|
||||||
.start_agent(&project_root, story_id, Some("mergemaster"), None)
|
self.start_mergemaster_or_block(&project_root, story_id).await;
|
||||||
.await
|
|
||||||
{
|
|
||||||
slog_error!(
|
|
||||||
"[pipeline] Failed to start mergemaster for '{story_id}': {e}"
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
crate::io::story_metadata::QaMode::Agent => {
|
crate::io::story_metadata::QaMode::Agent => {
|
||||||
@@ -186,13 +181,8 @@ impl AgentPool {
|
|||||||
slog_error!(
|
slog_error!(
|
||||||
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
"[pipeline] Failed to move '{story_id}' to 4_merge/: {e}"
|
||||||
);
|
);
|
||||||
} else if let Err(e) = self
|
} else {
|
||||||
.start_agent(&project_root, story_id, Some("mergemaster"), None)
|
self.start_mergemaster_or_block(&project_root, story_id).await;
|
||||||
.await
|
|
||||||
{
|
|
||||||
slog_error!(
|
|
||||||
"[pipeline] Failed to start mergemaster for '{story_id}': {e}"
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if let Some(reason) = should_block_story(story_id, config.max_retries, "qa-coverage") {
|
} else if let Some(reason) = should_block_story(story_id, config.max_retries, "qa-coverage") {
|
||||||
@@ -333,6 +323,41 @@ impl AgentPool {
|
|||||||
// become available (bug 295).
|
// become available (bug 295).
|
||||||
self.auto_assign_available_work(&project_root).await;
|
self.auto_assign_available_work(&project_root).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Start the mergemaster agent for `story_id`, but only if the feature
|
||||||
|
/// branch has commits that are not yet on master.
|
||||||
|
///
|
||||||
|
/// If the branch has zero commits ahead of master, this logs an error and
|
||||||
|
/// sends a [`WatcherEvent::StoryBlocked`] instead of spawning a Claude
|
||||||
|
/// session. A no-op merge session was observed spending $0.82 in the
|
||||||
|
/// 2026-04-09 incident (story 519).
|
||||||
|
async fn start_mergemaster_or_block(&self, project_root: &Path, story_id: &str) {
|
||||||
|
let branch = format!("feature/story-{story_id}");
|
||||||
|
if !crate::agents::lifecycle::feature_branch_has_unmerged_changes(project_root, story_id) {
|
||||||
|
slog_error!(
|
||||||
|
"[mergemaster] Branch '{branch}' has no commits ahead of master — \
|
||||||
|
refusing to spawn merge session. \
|
||||||
|
Likely cause: the worktree was reset to master after the feature \
|
||||||
|
branch's commits were created. Investigate the worktree's git state \
|
||||||
|
before retrying. Story '{story_id}' stays in 4_merge/ for human review."
|
||||||
|
);
|
||||||
|
let _ = self.watcher_tx.send(WatcherEvent::StoryBlocked {
|
||||||
|
story_id: story_id.to_string(),
|
||||||
|
reason: format!(
|
||||||
|
"Feature branch '{branch}' has no commits ahead of master — nothing to merge. \
|
||||||
|
The worktree may have been reset to master. \
|
||||||
|
Check the worktree's git state and retry manually."
|
||||||
|
),
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if let Err(e) = self
|
||||||
|
.start_agent(project_root, story_id, Some("mergemaster"), None)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
slog_error!("[pipeline] Failed to start mergemaster for '{story_id}': {e}");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Spawn pipeline advancement as a background task.
|
/// Spawn pipeline advancement as a background task.
|
||||||
@@ -693,6 +718,91 @@ stage = "qa"
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── story 519: mergemaster pre-flight blocks when no commits ahead ──
|
||||||
|
|
||||||
|
/// Regression test for story 519: when the feature branch has zero commits
|
||||||
|
/// ahead of master, mergemaster must not spawn a Claude session. A no-op
|
||||||
|
/// session spent $0.82 in the 2026-04-09 incident because the worktree was
|
||||||
|
/// reset to master before mergemaster ran.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn mergemaster_blocks_and_sends_story_blocked_when_no_commits_ahead() {
|
||||||
|
use std::fs;
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let root = tmp.path();
|
||||||
|
|
||||||
|
// Init a bare git repo on master with one empty commit.
|
||||||
|
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();
|
||||||
|
|
||||||
|
// Create a feature branch that points at master HEAD (zero commits ahead).
|
||||||
|
// This replicates the incident where the worktree was reset to master.
|
||||||
|
Command::new("git")
|
||||||
|
.args(["branch", "feature/story-9919_story_no_commits"])
|
||||||
|
.current_dir(root)
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
// Set up pipeline dirs and story file.
|
||||||
|
let current = root.join(".huskies/work/2_current");
|
||||||
|
fs::create_dir_all(¤t).unwrap();
|
||||||
|
fs::create_dir_all(root.join(".huskies/work/4_merge")).unwrap();
|
||||||
|
fs::write(current.join("9919_story_no_commits.md"), "---\nname: Test\n---\n").unwrap();
|
||||||
|
|
||||||
|
let pool = AgentPool::new_test(3001);
|
||||||
|
let mut rx = pool.watcher_tx.subscribe();
|
||||||
|
|
||||||
|
// Simulate coder completing with gates passed (qa: server → goes to merge).
|
||||||
|
pool.run_pipeline_advance(
|
||||||
|
"9919_story_no_commits",
|
||||||
|
"coder-1",
|
||||||
|
CompletionReport {
|
||||||
|
summary: "done".to_string(),
|
||||||
|
gates_passed: true,
|
||||||
|
gate_output: String::new(),
|
||||||
|
},
|
||||||
|
Some(root.to_path_buf()),
|
||||||
|
None,
|
||||||
|
false,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
// Story should be in 4_merge/ (pipeline moved it there before the block).
|
||||||
|
assert!(
|
||||||
|
root.join(".huskies/work/4_merge/9919_story_no_commits.md").exists(),
|
||||||
|
"story should remain in 4_merge/ — not moved to done"
|
||||||
|
);
|
||||||
|
|
||||||
|
// A StoryBlocked event must have been emitted (triggers chat failure notice,
|
||||||
|
// not the success 🎉 emoji).
|
||||||
|
let mut got_blocked = false;
|
||||||
|
while let Ok(evt) = rx.try_recv() {
|
||||||
|
if let WatcherEvent::StoryBlocked { story_id, .. } = &evt
|
||||||
|
&& story_id == "9919_story_no_commits"
|
||||||
|
{
|
||||||
|
got_blocked = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
got_blocked,
|
||||||
|
"StoryBlocked event must be sent when feature branch has no commits ahead of master"
|
||||||
|
);
|
||||||
|
|
||||||
|
// No mergemaster agent should have been started.
|
||||||
|
let agents = pool.agents.lock().unwrap();
|
||||||
|
let mergemaster_started = agents
|
||||||
|
.values()
|
||||||
|
.any(|a| a.agent_name.contains("mergemaster"));
|
||||||
|
assert!(
|
||||||
|
!mergemaster_started,
|
||||||
|
"mergemaster agent must NOT be started when no commits ahead of master"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// ── bug 295: pipeline advance picks up waiting QA stories ──────────
|
// ── bug 295: pipeline advance picks up waiting QA stories ──────────
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
Reference in New Issue
Block a user