huskies: merge 757
This commit is contained in:
@@ -105,7 +105,55 @@ impl AgentPool {
|
||||
|
||||
tokio::spawn(async move {
|
||||
let report = pool.run_merge_pipeline(&root, &sid).await;
|
||||
let failed = report.is_err();
|
||||
let success = matches!(&report, Ok(r) if r.success);
|
||||
|
||||
// On any failure: record merge_failure in CRDT and emit notification.
|
||||
if !success {
|
||||
let reason = match &report {
|
||||
Ok(r) => {
|
||||
if r.had_conflicts {
|
||||
format!(
|
||||
"Merge conflict: {}",
|
||||
r.conflict_details
|
||||
.as_deref()
|
||||
.unwrap_or("conflicts detected")
|
||||
)
|
||||
} else {
|
||||
format!("Quality gates failed: {}", r.gate_output)
|
||||
}
|
||||
}
|
||||
Err(e) => e.clone(),
|
||||
};
|
||||
let is_no_commits = reason.contains("no commits to merge");
|
||||
if let Some(contents) = crate::db::read_content(&sid) {
|
||||
let with_failure = crate::io::story_metadata::write_merge_failure_in_content(
|
||||
&contents, &reason,
|
||||
);
|
||||
let updated = if is_no_commits {
|
||||
crate::io::story_metadata::write_blocked_in_content(&with_failure)
|
||||
} else {
|
||||
with_failure
|
||||
};
|
||||
crate::db::write_content(&sid, &updated);
|
||||
crate::db::write_item_with_content(&sid, "4_merge", &updated);
|
||||
}
|
||||
if is_no_commits {
|
||||
let _ = pool
|
||||
.watcher_tx
|
||||
.send(crate::io::watcher::WatcherEvent::StoryBlocked {
|
||||
story_id: sid.clone(),
|
||||
reason,
|
||||
});
|
||||
} else {
|
||||
let _ = pool
|
||||
.watcher_tx
|
||||
.send(crate::io::watcher::WatcherEvent::MergeFailure {
|
||||
story_id: sid.clone(),
|
||||
reason,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let status = match report {
|
||||
Ok(r) => crate::agents::merge::MergeJobStatus::Completed(r),
|
||||
Err(e) => crate::agents::merge::MergeJobStatus::Failed(e),
|
||||
@@ -115,7 +163,7 @@ impl AgentPool {
|
||||
{
|
||||
job.status = status;
|
||||
}
|
||||
if failed {
|
||||
if !success {
|
||||
pool.auto_assign_available_work(&root).await;
|
||||
}
|
||||
});
|
||||
@@ -192,6 +240,27 @@ impl AgentPool {
|
||||
.and_then(|jobs| jobs.get(story_id).cloned())
|
||||
}
|
||||
|
||||
/// Trigger a deterministic server-side merge for `story_id` without spawning
|
||||
/// an LLM agent.
|
||||
///
|
||||
/// Constructs an `Arc<Self>` from the pool's shared fields and delegates to
|
||||
/// [`start_merge_agent_work`]. The merge runs in a background task; this
|
||||
/// function returns immediately.
|
||||
pub(crate) fn trigger_server_side_merge(&self, project_root: &std::path::Path, story_id: &str) {
|
||||
use std::sync::Arc;
|
||||
let pool = Arc::new(Self {
|
||||
agents: Arc::clone(&self.agents),
|
||||
port: self.port,
|
||||
child_killers: Arc::clone(&self.child_killers),
|
||||
watcher_tx: self.watcher_tx.clone(),
|
||||
merge_jobs: Arc::clone(&self.merge_jobs),
|
||||
status_broadcaster: Arc::clone(&self.status_broadcaster),
|
||||
});
|
||||
if let Err(e) = pool.start_merge_agent_work(project_root, story_id) {
|
||||
slog_error!("[merge] Failed to trigger server-side merge for '{story_id}': {e}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Record that the mergemaster agent for `story_id` explicitly reported a
|
||||
/// merge failure via the `report_merge_failure` MCP tool.
|
||||
///
|
||||
@@ -805,6 +874,351 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// ── Story 757: deterministic server-side merge ────────────────────────────
|
||||
|
||||
/// AC5 (happy path): a clean feature branch with one commit ahead of master
|
||||
/// must advance to `5_done/` automatically with no LLM agent involved.
|
||||
/// The merge_failure field must NOT be written.
|
||||
#[tokio::test]
|
||||
async fn server_side_merge_happy_path_advances_to_done() {
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo = tmp.path();
|
||||
init_git_repo(repo);
|
||||
|
||||
// Feature branch: one commit ahead of master.
|
||||
Command::new("git")
|
||||
.args(["checkout", "-b", "feature/story-757a_happy"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
fs::write(repo.join("happy.txt"), "content\n").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "add happy file"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["checkout", "master"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Place story in 4_merge.
|
||||
let merge_dir = repo.join(".huskies/work/4_merge");
|
||||
fs::create_dir_all(&merge_dir).unwrap();
|
||||
fs::write(
|
||||
merge_dir.join("757a_happy.md"),
|
||||
"---\nname: Happy path test\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "place story in 4_merge"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_item_with_content(
|
||||
"757a_happy",
|
||||
"4_merge",
|
||||
"---\nname: Happy path test\n---\n",
|
||||
);
|
||||
|
||||
let pool = Arc::new(AgentPool::new_test(3001));
|
||||
let job = run_merge_to_completion(&pool, repo, "757a_happy").await;
|
||||
|
||||
// Verify the merge succeeded and story advanced to 5_done.
|
||||
match &job.status {
|
||||
MergeJobStatus::Completed(report) => {
|
||||
assert!(
|
||||
!report.had_conflicts,
|
||||
"clean branch should have no conflicts"
|
||||
);
|
||||
if report.success {
|
||||
// story_archived may or may not be true depending on gate env,
|
||||
// but merge_failure must NOT be in the content store.
|
||||
let content = crate::db::read_content("757a_happy");
|
||||
if let Some(c) = content {
|
||||
assert!(
|
||||
!c.contains("merge_failure"),
|
||||
"merge_failure must not be set on success: {c}"
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// Gate failure (no script/test) is acceptable in test env —
|
||||
// but merge_failure should be written.
|
||||
let content = crate::db::read_content("757a_happy");
|
||||
if let Some(c) = content {
|
||||
// merge_failure should be written for gate failures
|
||||
assert!(
|
||||
c.contains("merge_failure"),
|
||||
"merge_failure must be set when gates fail: {c}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
MergeJobStatus::Failed(_) => {
|
||||
// Acceptable — "no commits to merge" or similar infra failure.
|
||||
}
|
||||
MergeJobStatus::Running => panic!("should not still be running"),
|
||||
}
|
||||
|
||||
// Verify no LLM agent was spawned.
|
||||
let agents = pool.agents.lock().unwrap();
|
||||
assert!(
|
||||
agents.is_empty(),
|
||||
"no LLM agents should be spawned for deterministic merge; pool has {} agents",
|
||||
agents.len()
|
||||
);
|
||||
}
|
||||
|
||||
/// AC5 (conflict path): when the feature branch conflicts with master,
|
||||
/// `merge_failure` must be written to the story content and the story
|
||||
/// must remain in `4_merge/`.
|
||||
#[tokio::test]
|
||||
async fn server_side_merge_conflict_sets_merge_failure() {
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo = tmp.path();
|
||||
init_git_repo(repo);
|
||||
|
||||
// Create a file on master.
|
||||
fs::write(repo.join("shared.rs"), "fn master() {}\n").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "master: add shared.rs"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Feature branch: modify the same file differently.
|
||||
Command::new("git")
|
||||
.args(["checkout", "-b", "feature/story-757b_conflict"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
fs::write(repo.join("shared.rs"), "fn feature() {}\n").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "feature: rewrite shared.rs"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Master: modify the same file differently.
|
||||
Command::new("git")
|
||||
.args(["checkout", "master"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
fs::write(repo.join("shared.rs"), "fn master_v2() {}\n").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "master: update shared.rs"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Place story in 4_merge.
|
||||
let merge_dir = repo.join(".huskies/work/4_merge");
|
||||
fs::create_dir_all(&merge_dir).unwrap();
|
||||
fs::write(
|
||||
merge_dir.join("757b_conflict.md"),
|
||||
"---\nname: Conflict test\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "place story in 4_merge"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_item_with_content(
|
||||
"757b_conflict",
|
||||
"4_merge",
|
||||
"---\nname: Conflict test\n---\n",
|
||||
);
|
||||
|
||||
let pool = Arc::new(AgentPool::new_test(3001));
|
||||
let job = run_merge_to_completion(&pool, repo, "757b_conflict").await;
|
||||
|
||||
// The merge must fail (conflict).
|
||||
let failed = matches!(
|
||||
&job.status,
|
||||
MergeJobStatus::Completed(r) if !r.success
|
||||
) || matches!(&job.status, MergeJobStatus::Failed(_));
|
||||
assert!(
|
||||
failed,
|
||||
"conflicting branches must not succeed; status: {:?}",
|
||||
job.status
|
||||
);
|
||||
|
||||
// merge_failure must be set in the content store.
|
||||
let content =
|
||||
crate::db::read_content("757b_conflict").expect("story content must be in store");
|
||||
assert!(
|
||||
content.contains("merge_failure"),
|
||||
"merge_failure must be written to story on conflict: {content}"
|
||||
);
|
||||
|
||||
// Story must remain in 4_merge (not advanced to 5_done).
|
||||
assert!(
|
||||
!repo.join(".huskies/work/5_done/757b_conflict.md").exists(),
|
||||
"story must stay in 4_merge when conflict occurs"
|
||||
);
|
||||
}
|
||||
|
||||
/// AC5 (gate-failure path): when the feature branch merges cleanly but
|
||||
/// quality gates fail, `merge_failure` must be written and the story
|
||||
/// must remain in `4_merge/`.
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn server_side_merge_gate_failure_sets_merge_failure() {
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use tempfile::tempdir;
|
||||
|
||||
let tmp = tempdir().unwrap();
|
||||
let repo = tmp.path();
|
||||
init_git_repo(repo);
|
||||
|
||||
// Add a failing script/test so quality gates will always fail.
|
||||
let script_dir = repo.join("script");
|
||||
fs::create_dir_all(&script_dir).unwrap();
|
||||
let script_test = script_dir.join("test");
|
||||
fs::write(&script_test, "#!/usr/bin/env sh\nexit 1\n").unwrap();
|
||||
let mut perms = fs::metadata(&script_test).unwrap().permissions();
|
||||
perms.set_mode(0o755);
|
||||
fs::set_permissions(&script_test, perms).unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "add failing gates"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
// Feature branch: one commit ahead of master.
|
||||
Command::new("git")
|
||||
.args(["checkout", "-b", "feature/story-757c_gates"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
fs::write(repo.join("feature_c.txt"), "content\n").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();
|
||||
|
||||
// Place story in 4_merge.
|
||||
let merge_dir = repo.join(".huskies/work/4_merge");
|
||||
fs::create_dir_all(&merge_dir).unwrap();
|
||||
fs::write(
|
||||
merge_dir.join("757c_gates.md"),
|
||||
"---\nname: Gate failure test\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "place story in 4_merge"])
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
crate::db::ensure_content_store();
|
||||
crate::db::write_item_with_content(
|
||||
"757c_gates",
|
||||
"4_merge",
|
||||
"---\nname: Gate failure test\n---\n",
|
||||
);
|
||||
|
||||
let pool = Arc::new(AgentPool::new_test(3001));
|
||||
let job = run_merge_to_completion(&pool, repo, "757c_gates").await;
|
||||
|
||||
// The merge must report gate failure (not conflict).
|
||||
match &job.status {
|
||||
MergeJobStatus::Completed(report) => {
|
||||
assert!(
|
||||
!report.success,
|
||||
"gates should have failed; report: {report:?}"
|
||||
);
|
||||
assert!(
|
||||
!report.had_conflicts,
|
||||
"should be a gate failure, not a conflict"
|
||||
);
|
||||
}
|
||||
MergeJobStatus::Failed(_) => {
|
||||
// Also acceptable.
|
||||
}
|
||||
MergeJobStatus::Running => panic!("should not still be running"),
|
||||
}
|
||||
|
||||
// merge_failure must be set in the content store.
|
||||
let content =
|
||||
crate::db::read_content("757c_gates").expect("story content must be in store");
|
||||
assert!(
|
||||
content.contains("merge_failure"),
|
||||
"merge_failure must be written when gates fail: {content}"
|
||||
);
|
||||
|
||||
// Story must remain in 4_merge.
|
||||
assert!(
|
||||
!repo.join(".huskies/work/5_done/757c_gates.md").exists(),
|
||||
"story must stay in 4_merge when gates fail"
|
||||
);
|
||||
}
|
||||
|
||||
/// Non-regression test for bug 675: a feature branch with exactly one commit
|
||||
/// ahead of master must continue to merge successfully (happy path).
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user