From 89bf4ae0cf7a622bb880e92d6535744dc4f47626 Mon Sep 17 00:00:00 2001 From: dave Date: Wed, 29 Apr 2026 00:11:52 +0000 Subject: [PATCH] huskies: merge 831 --- server/src/agents/pool/pipeline/merge.rs | 1349 ----------------- .../src/agents/pool/pipeline/merge/control.rs | 69 + server/src/agents/pool/pipeline/merge/mod.rs | 10 + .../src/agents/pool/pipeline/merge/runner.rs | 225 +++ .../src/agents/pool/pipeline/merge/status.rs | 48 + .../src/agents/pool/pipeline/merge/tests.rs | 975 ++++++++++++ server/src/agents/pool/pipeline/merge/time.rs | 37 + 7 files changed, 1364 insertions(+), 1349 deletions(-) delete mode 100644 server/src/agents/pool/pipeline/merge.rs create mode 100644 server/src/agents/pool/pipeline/merge/control.rs create mode 100644 server/src/agents/pool/pipeline/merge/mod.rs create mode 100644 server/src/agents/pool/pipeline/merge/runner.rs create mode 100644 server/src/agents/pool/pipeline/merge/status.rs create mode 100644 server/src/agents/pool/pipeline/merge/tests.rs create mode 100644 server/src/agents/pool/pipeline/merge/time.rs diff --git a/server/src/agents/pool/pipeline/merge.rs b/server/src/agents/pool/pipeline/merge.rs deleted file mode 100644 index 6d9a65fb..00000000 --- a/server/src/agents/pool/pipeline/merge.rs +++ /dev/null @@ -1,1349 +0,0 @@ -//! Pipeline merge step — orchestrates the merge-to-master flow for completed stories. -use crate::slog; -use crate::slog_error; -use crate::slog_warn; -use crate::worktree; -use std::path::Path; -use std::sync::Arc; - -use super::super::super::PipelineStage; -use super::super::super::pipeline_stage; -use super::super::AgentPool; - -/// Returns `true` if the process with the given PID is currently alive. -/// -/// On Unix this sends signal 0 to the PID (no actual signal delivered, but -/// the kernel validates whether the process exists and is reachable). -/// Returns `false` for any error, including ESRCH (no such process). -/// Wall-clock time captured the first time this server process touches the -/// merge subsystem. Used to detect merge_jobs left over from a previous -/// server instance: a re-exec on `rebuild_and_restart` keeps the same PID, -/// so PID alone cannot distinguish "current" vs "previous" server. This -/// timestamp is fresh per-process (the static is reset by execve) and is -/// the source of truth for stale-merge detection. -static SERVER_START_TIME: std::sync::OnceLock = std::sync::OnceLock::new(); - -/// Return this server process's start time (lazily captured on first call). -pub(crate) fn server_start_time() -> f64 { - *SERVER_START_TIME.get_or_init(unix_now) -} - -/// Encode the current server's start-time into the CRDT `error` field for -/// a Running merge job. -fn encode_server_start_time(t: f64) -> String { - format!("{{\"server_start\":{t}}}") -} - -/// Decode the server-start-time from a Running merge job's `error` field. -/// Returns `None` for legacy entries (which encoded `pid` instead) — those -/// are treated as stale by the cleanup pass. -fn decode_server_start_time(error: Option<&str>) -> Option { - error - .and_then(|e| serde_json::from_str::(e).ok()) - .and_then(|v| v["server_start"].as_f64()) -} - -/// Current Unix timestamp in seconds as `f64`. -fn unix_now() -> f64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_secs_f64() -} - -impl AgentPool { - /// Start the merge pipeline as a background task. - /// - /// Returns immediately so the MCP tool call doesn't time out (the full - /// pipeline — squash merge + quality gates — takes well over 60 seconds, - /// exceeding Claude Code's MCP tool-call timeout). - /// - /// The mergemaster agent should poll [`get_merge_status`](Self::get_merge_status) - /// until the job reaches a terminal state. - pub fn start_merge_agent_work( - self: &Arc, - project_root: &Path, - story_id: &str, - ) -> Result<(), String> { - // Sweep stale Running entries left behind by dead processes before - // applying the double-start guard. This handles the case where the - // server crashed mid-merge: the next attempt finds a Running entry - // whose owning process is gone and clears it automatically. - if let Some(jobs) = crate::crdt_state::read_all_merge_jobs() { - let current_boot = server_start_time(); - for job in jobs { - if job.status != "running" { - continue; - } - let stale = match decode_server_start_time(job.error.as_deref()) { - Some(t) => t < current_boot, - None => true, // Legacy (pid-encoded) or malformed: stale - }; - if stale { - slog!( - "[merge] Cleared stale Running merge job for '{}' (server restarted)", - job.story_id - ); - crate::crdt_state::delete_merge_job(&job.story_id); - } - } - } - - // Guard against double-starts; clear any completed/failed entry so the - // caller can retry without needing to call a separate cleanup step. - if let Some(job) = crate::crdt_state::read_merge_job(story_id) { - match job.status.as_str() { - "running" => { - return Err(format!( - "Merge already in progress for '{story_id}'. \ - Use get_merge_status to poll for completion." - )); - } - // Completed or Failed: clear stale entry so we can start fresh. - _ => { - crate::crdt_state::delete_merge_job(story_id); - } - } - } - - // Insert Running job into CRDT. - let started_at = unix_now(); - crate::crdt_state::write_merge_job( - story_id, - "running", - started_at, - None, - Some(&encode_server_start_time(server_start_time())), - ); - - let pool = Arc::clone(self); - let root = project_root.to_path_buf(); - let sid = story_id.to_string(); - - tokio::spawn(async move { - let report = pool.run_merge_pipeline(&root, &sid).await; - let success = matches!(&report, Ok(r) if r.success); - - let finished_at = unix_now(); - - // 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, - }); - } - } - - // Update CRDT with terminal status. - match &report { - Ok(r) => { - let report_json = serde_json::to_string(r).unwrap_or_else(|_| String::new()); - crate::crdt_state::write_merge_job( - &sid, - "completed", - started_at, - Some(finished_at), - Some(&report_json), - ); - } - Err(e) => { - crate::crdt_state::write_merge_job( - &sid, - "failed", - started_at, - Some(finished_at), - Some(e), - ); - } - } - - if !success { - pool.auto_assign_available_work(&root).await; - } - }); - - Ok(()) - } - - /// The actual merge pipeline, run inside a background task. - async fn run_merge_pipeline( - self: &Arc, - project_root: &Path, - story_id: &str, - ) -> Result { - let branch = format!("feature/story-{story_id}"); - let wt_path = worktree::worktree_path(project_root, story_id); - let root = project_root.to_path_buf(); - let sid = story_id.to_string(); - let br = branch.clone(); - - let merge_result = tokio::task::spawn_blocking(move || { - crate::agents::merge::run_squash_merge(&root, &br, &sid) - }) - .await - .map_err(|e| format!("Merge task panicked: {e}"))??; - - if !merge_result.success { - return Ok(crate::agents::merge::MergeReport { - story_id: story_id.to_string(), - success: false, - had_conflicts: merge_result.had_conflicts, - conflicts_resolved: merge_result.conflicts_resolved, - conflict_details: merge_result.conflict_details, - gates_passed: merge_result.gates_passed, - gate_output: merge_result.output, - worktree_cleaned_up: false, - story_archived: false, - }); - } - - let story_archived = crate::agents::lifecycle::move_story_to_done(story_id).is_ok(); - if story_archived { - self.remove_agents_for_story(story_id); - } - - 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() - } else { - false - }; - - self.auto_assign_available_work(project_root).await; - - Ok(crate::agents::merge::MergeReport { - story_id: story_id.to_string(), - success: true, - had_conflicts: merge_result.had_conflicts, - conflicts_resolved: merge_result.conflicts_resolved, - conflict_details: merge_result.conflict_details, - gates_passed: true, - gate_output: merge_result.output, - worktree_cleaned_up, - story_archived, - }) - } - - /// Check the status of a background merge job. - /// - /// Reads from the CRDT `merge_jobs` collection and reconstructs the full - /// [`MergeJob`] struct. The CRDT `error` field encodes the `pid` for - /// Running jobs (as `{"pid":N}`) and the serialised [`MergeReport`] for - /// Completed jobs. - pub fn get_merge_status(&self, story_id: &str) -> Option { - let view = crate::crdt_state::read_merge_job(story_id)?; - let (status, server_start_time) = match view.status.as_str() { - "running" => { - let t = decode_server_start_time(view.error.as_deref()).unwrap_or(0.0); - (crate::agents::merge::MergeJobStatus::Running, t) - } - "completed" => { - let report = view - .error - .as_deref() - .and_then(|e| serde_json::from_str::(e).ok()) - .unwrap_or_else(|| crate::agents::merge::MergeReport { - story_id: story_id.to_string(), - success: false, - had_conflicts: false, - conflicts_resolved: false, - conflict_details: None, - gates_passed: false, - gate_output: String::new(), - worktree_cleaned_up: false, - story_archived: false, - }); - (crate::agents::merge::MergeJobStatus::Completed(report), 0.0) - } - _ => { - let err = view.error.unwrap_or_else(|| "Unknown error".to_string()); - (crate::agents::merge::MergeJobStatus::Failed(err), 0.0) - } - }; - Some(crate::agents::merge::MergeJob { - story_id: story_id.to_string(), - status, - server_start_time, - }) - } - - /// Trigger a deterministic server-side merge for `story_id` without spawning - /// an LLM agent. - /// - /// Constructs an `Arc` 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(), - 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. - /// - /// Sets `merge_failure_reported = true` on the active mergemaster agent so - /// that `run_pipeline_advance` can block advancement to `5_done/` even when - /// the server-owned gate check returns `gates_passed=true` (those gates run - /// in the feature-branch worktree, not on master). - pub fn set_merge_failure_reported(&self, story_id: &str) { - match self.agents.lock() { - Ok(mut lock) => { - let found = lock.iter_mut().find(|(key, agent)| { - let key_story_id = key - .rsplit_once(':') - .map(|(sid, _)| sid) - .unwrap_or(key.as_str()); - key_story_id == story_id - && pipeline_stage(&agent.agent_name) == PipelineStage::Mergemaster - }); - match found { - Some((_, agent)) => { - agent.merge_failure_reported = true; - slog!( - "[pipeline] Merge failure flag set for '{story_id}:{}'", - agent.agent_name - ); - } - None => { - slog_warn!( - "[pipeline] set_merge_failure_reported: no running mergemaster found \ - for story '{story_id}' — flag not set" - ); - } - } - } - Err(e) => { - slog_error!("[pipeline] set_merge_failure_reported: could not lock agents: {e}"); - } - } - } -} - -#[cfg(test)] -mod tests { - use super::super::super::AgentPool; - use super::*; - use crate::agents::merge::{MergeJob, MergeJobStatus}; - use std::process::Command; - - fn init_git_repo(repo: &std::path::Path) { - Command::new("git") - .args(["init"]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["config", "user.email", "test@test.com"]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["config", "user.name", "Test"]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "--allow-empty", "-m", "init"]) - .current_dir(repo) - .output() - .unwrap(); - } - - // ── bug 498: stale Running job blocks retry ─────────────────────────────── - - /// Regression test for bug 498: a Running merge job left behind by a killed - /// mergemaster must not block the next call to start_merge_agent_work. - /// - /// Before the fix: start_merge_agent_work would return "Merge already in - /// progress" when a Running entry existed, even after the mergemaster died. - /// After the fix: the entry is cleared when the mergemaster exits, so a new - /// call succeeds. - #[tokio::test] - async fn stale_running_merge_job_is_cleared_and_retry_succeeds() { - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - let pool = Arc::new(AgentPool::new_test(3001)); - - // Inject a stale Running entry via CRDT, simulating a mergemaster that - // died before the merge pipeline completed. Use the current process PID - // so the stale-lock sweep does NOT auto-remove it — this test verifies - // the double-start guard path. - crate::crdt_state::write_merge_job( - "77_story_stale", - "running", - 1.0, - None, - Some(&encode_server_start_time(server_start_time())), - ); - - // With a stale Running entry, start_merge_agent_work must be blocked. - let blocked = pool.start_merge_agent_work(repo, "77_story_stale"); - assert!( - blocked.is_err(), - "start_merge_agent_work must be blocked while Running job exists" - ); - let err_msg = blocked.unwrap_err(); - assert!( - err_msg.contains("already in progress"), - "unexpected error: {err_msg}" - ); - - // Simulate the mergemaster exit path: clear the stale Running entry. - crate::crdt_state::delete_merge_job("77_story_stale"); - - // After clearing, start_merge_agent_work must succeed (it will fail - // the pipeline because there's no feature branch, but it must not be - // blocked by "Merge already in progress"). - let result = pool.start_merge_agent_work(repo, "77_story_stale"); - assert!( - result.is_ok(), - "start_merge_agent_work must succeed after stale Running job is cleared; got: {result:?}" - ); - } - - // ── story 719: stale-lock recovery on new merge attempts ───────────────── - - /// AC1/AC2/AC3: seeding merge_jobs with an entry whose PID is dead, then - /// triggering a new merge for a *different* story, must automatically remove - /// the stale entry (AC1/AC3) and log at INFO (AC2 — verified structurally - /// because the log path is exercised when the entry is removed). - #[cfg(unix)] - #[tokio::test] - async fn stale_merge_job_with_dead_pid_is_swept_on_new_merge_attempt() { - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - let pool = Arc::new(AgentPool::new_test(3001)); - - // Seed CRDT merge_jobs with a Running entry whose recorded server-start - // time is older than the current server (legacy / previous instance). - crate::crdt_state::write_merge_job( - "719_stale_other", - "running", - 1.0, - None, - Some(&encode_server_start_time(0.0)), // legacy/older boot — should be cleaned up - ); - - // Verify the entry is present before the sweep. - assert!( - crate::crdt_state::read_merge_job("719_stale_other").is_some(), - "stale entry should exist before new merge attempt" - ); - - // Trigger a new merge for a *different* story. The sweep runs at the - // top of start_merge_agent_work and must remove the dead-PID entry. - let _ = pool.start_merge_agent_work(repo, "719_trigger_story"); - - // The stale entry must have been cleared. - assert!( - crate::crdt_state::read_merge_job("719_stale_other").is_none(), - "stale entry with dead pid must be removed when a new merge attempt starts" - ); - } - - // ── merge_agent_work tests ──────────────────────────────────────────────── - - /// Helper: start a merge and poll until terminal state. - async fn run_merge_to_completion( - pool: &Arc, - repo: &std::path::Path, - story_id: &str, - ) -> MergeJob { - pool.start_merge_agent_work(repo, story_id).unwrap(); - loop { - tokio::time::sleep(std::time::Duration::from_millis(50)).await; - if let Some(job) = pool.get_merge_status(story_id) - && !matches!(job.status, MergeJobStatus::Running) - { - return job; - } - } - } - - #[tokio::test] - async fn merge_agent_work_returns_error_when_branch_not_found() { - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - let pool = Arc::new(AgentPool::new_test(3001)); - let job = run_merge_to_completion(&pool, repo, "99_nonexistent").await; - match &job.status { - MergeJobStatus::Completed(report) => { - assert!(!report.success, "should fail when branch missing"); - } - MergeJobStatus::Failed(_) => { - // Also acceptable — the pipeline errored out - } - MergeJobStatus::Running => { - panic!("should not still be running"); - } - } - } - - #[tokio::test] - async fn merge_agent_work_succeeds_on_clean_branch() { - use std::fs; - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - // Create a feature branch with a commit - Command::new("git") - .args(["checkout", "-b", "feature/story-23_test"]) - .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(); - - // Switch back to master (initial branch) - Command::new("git") - .args(["checkout", "master"]) - .current_dir(repo) - .output() - .unwrap(); - - // Create the story file in 4_merge/ so we can test archival - let merge_dir = repo.join(".huskies/work/4_merge"); - fs::create_dir_all(&merge_dir).unwrap(); - let story_file = merge_dir.join("23_test.md"); - fs::write(&story_file, "---\nname: 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)); - let job = run_merge_to_completion(&pool, repo, "23_test").await; - - match &job.status { - MergeJobStatus::Completed(report) => { - assert!(!report.had_conflicts, "should have no conflicts"); - assert!( - report.success - || report.gate_output.contains("Failed to run") - || !report.gates_passed, - "report should be coherent: {report:?}" - ); - if report.story_archived { - let done = repo.join(".huskies/work/5_done/23_test.md"); - assert!(done.exists(), "done file should exist"); - } - } - MergeJobStatus::Failed(e) => { - // Gate failures are acceptable in test env - assert!( - e.contains("Failed") || e.contains("failed"), - "unexpected failure: {e}" - ); - } - MergeJobStatus::Running => panic!("should not still be running"), - } - } - - // ── quality gate ordering test ──────────────────────────────── - - /// Regression test for bug 142: quality gates must run BEFORE the fast-forward - /// to master so that broken code never lands on master. - #[cfg(unix)] - #[test] - fn quality_gates_run_before_fast_forward_to_master() { - 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 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 bash\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 script/test"]) - .current_dir(repo) - .output() - .unwrap(); - - // Create a feature branch with a commit. - Command::new("git") - .args(["checkout", "-b", "feature/story-142_test"]) - .current_dir(repo) - .output() - .unwrap(); - fs::write(repo.join("change.txt"), "feature change").unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "feature work"]) - .current_dir(repo) - .output() - .unwrap(); - - // Switch back to master and record its HEAD. - Command::new("git") - .args(["checkout", "master"]) - .current_dir(repo) - .output() - .unwrap(); - let head_before = String::from_utf8( - Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(repo) - .output() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string(); - - // Run the squash-merge. The failing script/test makes quality gates - // fail → fast-forward must NOT happen. - let result = - crate::agents::merge::run_squash_merge(repo, "feature/story-142_test", "142_test") - .unwrap(); - - let head_after = String::from_utf8( - Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(repo) - .output() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string(); - - // Gates must have failed (script/test exits 1) so master should be untouched. - assert!( - !result.success, - "run_squash_merge must report failure when gates fail" - ); - assert_eq!( - head_before, head_after, - "master HEAD must not advance when quality gates fail (bug 142)" - ); - } - - #[tokio::test] - async fn merge_agent_work_conflict_does_not_break_master() { - use std::fs; - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - // Create a file on master. - fs::write( - repo.join("code.rs"), - "fn main() {\n println!(\"hello\");\n}\n", - ) - .unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "initial code"]) - .current_dir(repo) - .output() - .unwrap(); - - // Feature branch: modify the same line differently. - Command::new("git") - .args(["checkout", "-b", "feature/story-42_story_foo"]) - .current_dir(repo) - .output() - .unwrap(); - fs::write( - repo.join("code.rs"), - "fn main() {\n println!(\"hello\");\n feature_fn();\n}\n", - ) - .unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "feature: add fn call"]) - .current_dir(repo) - .output() - .unwrap(); - - // Master: add different line at same location. - Command::new("git") - .args(["checkout", "master"]) - .current_dir(repo) - .output() - .unwrap(); - fs::write( - repo.join("code.rs"), - "fn main() {\n println!(\"hello\");\n master_fn();\n}\n", - ) - .unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "master: add fn call"]) - .current_dir(repo) - .output() - .unwrap(); - - // Create story file in 4_merge. - let merge_dir = repo.join(".huskies/work/4_merge"); - fs::create_dir_all(&merge_dir).unwrap(); - fs::write(merge_dir.join("42_story_foo.md"), "---\nname: Test\n---\n").unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "add story"]) - .current_dir(repo) - .output() - .unwrap(); - - let pool = Arc::new(AgentPool::new_test(3001)); - let job = run_merge_to_completion(&pool, repo, "42_story_foo").await; - - // Master should NEVER have conflict markers, regardless of merge outcome. - let master_code = fs::read_to_string(repo.join("code.rs")).unwrap(); - assert!( - !master_code.contains("<<<<<<<"), - "master must never contain conflict markers:\n{master_code}" - ); - assert!( - !master_code.contains(">>>>>>>"), - "master must never contain conflict markers:\n{master_code}" - ); - - // The report should accurately reflect what happened. - match &job.status { - MergeJobStatus::Completed(report) => { - assert!(report.had_conflicts, "should report conflicts"); - } - MergeJobStatus::Failed(_) => { - // Acceptable — merge aborted due to conflicts - } - MergeJobStatus::Running => panic!("should not still be running"), - } - } - - // ── bug 675: zero commits ahead must fail with "no commits to merge" ───── - - /// Regression test for bug 675: when the feature branch has zero commits - /// ahead of master the pipeline must fail with a clear "no commits to merge" - /// error and the story must remain in `4_merge` (not advance to `5_done`). - #[tokio::test] - async fn merge_agent_work_zero_commits_ahead_stays_in_merge_stage() { - use std::fs; - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - let tmp = tempdir().unwrap(); - let repo = tmp.path(); - init_git_repo(repo); - - // Feature branch is created at the same commit as master — zero commits ahead. - Command::new("git") - .args(["checkout", "-b", "feature/story-675_zero_commits"]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["checkout", "master"]) - .current_dir(repo) - .output() - .unwrap(); - - // Place the story file in 4_merge so we can verify it stays there. - let merge_dir = repo.join(".huskies/work/4_merge"); - fs::create_dir_all(&merge_dir).unwrap(); - fs::write( - merge_dir.join("675_zero_commits.md"), - "---\nname: Zero commits 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(); - - let pool = Arc::new(AgentPool::new_test(3001)); - let job = run_merge_to_completion(&pool, repo, "675_zero_commits").await; - - // The job must have failed with a "no commits to merge" error. - match &job.status { - MergeJobStatus::Failed(e) => { - assert!( - e.contains("no commits to merge"), - "error must contain 'no commits to merge', got: {e}" - ); - assert!( - e.contains("675_zero_commits"), - "error must name the story_id, got: {e}" - ); - } - MergeJobStatus::Completed(report) => { - panic!( - "expected Failed status, got Completed with success={}: {}", - report.success, report.gate_output - ); - } - MergeJobStatus::Running => panic!("should not still be running"), - } - - // Story file must still be in 4_merge — NOT advanced to 5_done. - assert!( - merge_dir.join("675_zero_commits.md").exists(), - "story file must remain in 4_merge when merge fails" - ); - assert!( - !repo - .join(".huskies/work/5_done/675_zero_commits.md") - .exists(), - "story must NOT advance to 5_done when merge fails with no commits" - ); - } - - // ── 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; - - crate::crdt_state::init_for_test(); - 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; - - crate::crdt_state::init_for_test(); - 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; - - crate::crdt_state::init_for_test(); - 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] - async fn merge_agent_work_one_commit_ahead_merges_successfully() { - use std::fs; - use tempfile::tempdir; - - crate::crdt_state::init_for_test(); - 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-675_one_commit"]) - .current_dir(repo) - .output() - .unwrap(); - fs::write(repo.join("feature_675.txt"), "feature content\n").unwrap(); - Command::new("git") - .args(["add", "."]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["commit", "-m", "add feature file"]) - .current_dir(repo) - .output() - .unwrap(); - Command::new("git") - .args(["checkout", "master"]) - .current_dir(repo) - .output() - .unwrap(); - - // Place the story file in 4_merge. - let merge_dir = repo.join(".huskies/work/4_merge"); - fs::create_dir_all(&merge_dir).unwrap(); - fs::write( - merge_dir.join("675_one_commit.md"), - "---\nname: One commit 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(); - - let pool = Arc::new(AgentPool::new_test(3001)); - let job = run_merge_to_completion(&pool, repo, "675_one_commit").await; - - // The merge must not fail with "no commits to merge". - match &job.status { - MergeJobStatus::Failed(e) => { - assert!( - !e.contains("no commits to merge"), - "one-commit-ahead branch must NOT fail with 'no commits to merge': {e}" - ); - // Gate failures (no script/test) are acceptable in test env. - } - MergeJobStatus::Completed(report) => { - // Success or gate failure — both acceptable; the key invariant is - // that we didn't fail with the zero-commits early-exit. - assert!( - report.success || !report.gates_passed, - "unexpected state: success={} gates_passed={}", - report.success, - report.gates_passed - ); - } - MergeJobStatus::Running => panic!("should not still be running"), - } - } -} diff --git a/server/src/agents/pool/pipeline/merge/control.rs b/server/src/agents/pool/pipeline/merge/control.rs new file mode 100644 index 00000000..b4d22ebb --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/control.rs @@ -0,0 +1,69 @@ +//! Merge control — server-side trigger and failure reporting. +use crate::slog; +use crate::slog_error; +use crate::slog_warn; +use std::sync::Arc; + +use super::super::super::AgentPool; +use crate::agents::{PipelineStage, pipeline_stage}; + +impl AgentPool { + /// Trigger a deterministic server-side merge for `story_id` without spawning + /// an LLM agent. + /// + /// Constructs an `Arc` 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) { + 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(), + 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. + /// + /// Sets `merge_failure_reported = true` on the active mergemaster agent so + /// that `run_pipeline_advance` can block advancement to `5_done/` even when + /// the server-owned gate check returns `gates_passed=true` (those gates run + /// in the feature-branch worktree, not on master). + pub fn set_merge_failure_reported(&self, story_id: &str) { + match self.agents.lock() { + Ok(mut lock) => { + let found = lock.iter_mut().find(|(key, agent)| { + let key_story_id = key + .rsplit_once(':') + .map(|(sid, _)| sid) + .unwrap_or(key.as_str()); + key_story_id == story_id + && pipeline_stage(&agent.agent_name) == PipelineStage::Mergemaster + }); + match found { + Some((_, agent)) => { + agent.merge_failure_reported = true; + slog!( + "[pipeline] Merge failure flag set for '{story_id}:{}'", + agent.agent_name + ); + } + None => { + slog_warn!( + "[pipeline] set_merge_failure_reported: no running mergemaster found \ + for story '{story_id}' — flag not set" + ); + } + } + } + Err(e) => { + slog_error!("[pipeline] set_merge_failure_reported: could not lock agents: {e}"); + } + } + } +} diff --git a/server/src/agents/pool/pipeline/merge/mod.rs b/server/src/agents/pool/pipeline/merge/mod.rs new file mode 100644 index 00000000..69ea7613 --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/mod.rs @@ -0,0 +1,10 @@ +//! Pipeline merge step — orchestrates the merge-to-master flow for completed stories. + +mod control; +mod runner; +mod status; +/// Helpers for encoding and decoding server start-time markers. +pub(crate) mod time; + +#[cfg(test)] +mod tests; diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs new file mode 100644 index 00000000..c0efc4a9 --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -0,0 +1,225 @@ +//! Merge pipeline runner — start_merge_agent_work and run_merge_pipeline. +use crate::slog; +use crate::worktree; +use std::path::Path; +use std::sync::Arc; + +use super::super::super::AgentPool; +use super::time::{ + decode_server_start_time, encode_server_start_time, server_start_time, unix_now, +}; + +impl AgentPool { + /// Start the merge pipeline as a background task. + /// + /// Returns immediately so the MCP tool call doesn't time out (the full + /// pipeline — squash merge + quality gates — takes well over 60 seconds, + /// exceeding Claude Code's MCP tool-call timeout). + /// + /// The mergemaster agent should poll [`get_merge_status`](Self::get_merge_status) + /// until the job reaches a terminal state. + pub fn start_merge_agent_work( + self: &Arc, + project_root: &Path, + story_id: &str, + ) -> Result<(), String> { + // Sweep stale Running entries left behind by dead processes before + // applying the double-start guard. This handles the case where the + // server crashed mid-merge: the next attempt finds a Running entry + // whose owning process is gone and clears it automatically. + if let Some(jobs) = crate::crdt_state::read_all_merge_jobs() { + let current_boot = server_start_time(); + for job in jobs { + if job.status != "running" { + continue; + } + let stale = match decode_server_start_time(job.error.as_deref()) { + Some(t) => t < current_boot, + None => true, // Legacy (pid-encoded) or malformed: stale + }; + if stale { + slog!( + "[merge] Cleared stale Running merge job for '{}' (server restarted)", + job.story_id + ); + crate::crdt_state::delete_merge_job(&job.story_id); + } + } + } + + // Guard against double-starts; clear any completed/failed entry so the + // caller can retry without needing to call a separate cleanup step. + if let Some(job) = crate::crdt_state::read_merge_job(story_id) { + match job.status.as_str() { + "running" => { + return Err(format!( + "Merge already in progress for '{story_id}'. \ + Use get_merge_status to poll for completion." + )); + } + // Completed or Failed: clear stale entry so we can start fresh. + _ => { + crate::crdt_state::delete_merge_job(story_id); + } + } + } + + // Insert Running job into CRDT. + let started_at = unix_now(); + crate::crdt_state::write_merge_job( + story_id, + "running", + started_at, + None, + Some(&encode_server_start_time(server_start_time())), + ); + + let pool = Arc::clone(self); + let root = project_root.to_path_buf(); + let sid = story_id.to_string(); + + tokio::spawn(async move { + let report = pool.run_merge_pipeline(&root, &sid).await; + let success = matches!(&report, Ok(r) if r.success); + + let finished_at = unix_now(); + + // 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, + }); + } + } + + // Update CRDT with terminal status. + match &report { + Ok(r) => { + let report_json = serde_json::to_string(r).unwrap_or_else(|_| String::new()); + crate::crdt_state::write_merge_job( + &sid, + "completed", + started_at, + Some(finished_at), + Some(&report_json), + ); + } + Err(e) => { + crate::crdt_state::write_merge_job( + &sid, + "failed", + started_at, + Some(finished_at), + Some(e), + ); + } + } + + if !success { + pool.auto_assign_available_work(&root).await; + } + }); + + Ok(()) + } + + /// The actual merge pipeline, run inside a background task. + async fn run_merge_pipeline( + self: &Arc, + project_root: &Path, + story_id: &str, + ) -> Result { + let branch = format!("feature/story-{story_id}"); + let wt_path = worktree::worktree_path(project_root, story_id); + let root = project_root.to_path_buf(); + let sid = story_id.to_string(); + let br = branch.clone(); + + let merge_result = tokio::task::spawn_blocking(move || { + crate::agents::merge::run_squash_merge(&root, &br, &sid) + }) + .await + .map_err(|e| format!("Merge task panicked: {e}"))??; + + if !merge_result.success { + return Ok(crate::agents::merge::MergeReport { + story_id: story_id.to_string(), + success: false, + had_conflicts: merge_result.had_conflicts, + conflicts_resolved: merge_result.conflicts_resolved, + conflict_details: merge_result.conflict_details, + gates_passed: merge_result.gates_passed, + gate_output: merge_result.output, + worktree_cleaned_up: false, + story_archived: false, + }); + } + + let story_archived = crate::agents::lifecycle::move_story_to_done(story_id).is_ok(); + if story_archived { + self.remove_agents_for_story(story_id); + } + + 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() + } else { + false + }; + + self.auto_assign_available_work(project_root).await; + + Ok(crate::agents::merge::MergeReport { + story_id: story_id.to_string(), + success: true, + had_conflicts: merge_result.had_conflicts, + conflicts_resolved: merge_result.conflicts_resolved, + conflict_details: merge_result.conflict_details, + gates_passed: true, + gate_output: merge_result.output, + worktree_cleaned_up, + story_archived, + }) + } +} diff --git a/server/src/agents/pool/pipeline/merge/status.rs b/server/src/agents/pool/pipeline/merge/status.rs new file mode 100644 index 00000000..34e5ee74 --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/status.rs @@ -0,0 +1,48 @@ +//! Merge job status queries. +use super::super::super::AgentPool; +use super::time::decode_server_start_time; + +impl AgentPool { + /// Check the status of a background merge job. + /// + /// Reads from the CRDT `merge_jobs` collection and reconstructs the full + /// [`MergeJob`] struct. The CRDT `error` field encodes the `pid` for + /// Running jobs (as `{"pid":N}`) and the serialised [`MergeReport`] for + /// Completed jobs. + pub fn get_merge_status(&self, story_id: &str) -> Option { + let view = crate::crdt_state::read_merge_job(story_id)?; + let (status, server_start_time) = match view.status.as_str() { + "running" => { + let t = decode_server_start_time(view.error.as_deref()).unwrap_or(0.0); + (crate::agents::merge::MergeJobStatus::Running, t) + } + "completed" => { + let report = view + .error + .as_deref() + .and_then(|e| serde_json::from_str::(e).ok()) + .unwrap_or_else(|| crate::agents::merge::MergeReport { + story_id: story_id.to_string(), + success: false, + had_conflicts: false, + conflicts_resolved: false, + conflict_details: None, + gates_passed: false, + gate_output: String::new(), + worktree_cleaned_up: false, + story_archived: false, + }); + (crate::agents::merge::MergeJobStatus::Completed(report), 0.0) + } + _ => { + let err = view.error.unwrap_or_else(|| "Unknown error".to_string()); + (crate::agents::merge::MergeJobStatus::Failed(err), 0.0) + } + }; + Some(crate::agents::merge::MergeJob { + story_id: story_id.to_string(), + status, + server_start_time, + }) + } +} diff --git a/server/src/agents/pool/pipeline/merge/tests.rs b/server/src/agents/pool/pipeline/merge/tests.rs new file mode 100644 index 00000000..44196771 --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/tests.rs @@ -0,0 +1,975 @@ +//! Tests for the merge pipeline module. + +use super::super::super::AgentPool; +use super::time::{encode_server_start_time, server_start_time}; +use crate::agents::merge::{MergeJob, MergeJobStatus}; +use std::process::Command; +use std::sync::Arc; + +fn init_git_repo(repo: &std::path::Path) { + Command::new("git") + .args(["init"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["config", "user.name", "Test"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "--allow-empty", "-m", "init"]) + .current_dir(repo) + .output() + .unwrap(); +} + +// ── bug 498: stale Running job blocks retry ─────────────────────────────── + +/// Regression test for bug 498: a Running merge job left behind by a killed +/// mergemaster must not block the next call to start_merge_agent_work. +/// +/// Before the fix: start_merge_agent_work would return "Merge already in +/// progress" when a Running entry existed, even after the mergemaster died. +/// After the fix: the entry is cleared when the mergemaster exits, so a new +/// call succeeds. +#[tokio::test] +async fn stale_running_merge_job_is_cleared_and_retry_succeeds() { + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + let pool = Arc::new(AgentPool::new_test(3001)); + + // Inject a stale Running entry via CRDT, simulating a mergemaster that + // died before the merge pipeline completed. Use the current process PID + // so the stale-lock sweep does NOT auto-remove it — this test verifies + // the double-start guard path. + crate::crdt_state::write_merge_job( + "77_story_stale", + "running", + 1.0, + None, + Some(&encode_server_start_time(server_start_time())), + ); + + // With a stale Running entry, start_merge_agent_work must be blocked. + let blocked = pool.start_merge_agent_work(repo, "77_story_stale"); + assert!( + blocked.is_err(), + "start_merge_agent_work must be blocked while Running job exists" + ); + let err_msg = blocked.unwrap_err(); + assert!( + err_msg.contains("already in progress"), + "unexpected error: {err_msg}" + ); + + // Simulate the mergemaster exit path: clear the stale Running entry. + crate::crdt_state::delete_merge_job("77_story_stale"); + + // After clearing, start_merge_agent_work must succeed (it will fail + // the pipeline because there's no feature branch, but it must not be + // blocked by "Merge already in progress"). + let result = pool.start_merge_agent_work(repo, "77_story_stale"); + assert!( + result.is_ok(), + "start_merge_agent_work must succeed after stale Running job is cleared; got: {result:?}" + ); +} + +// ── story 719: stale-lock recovery on new merge attempts ───────────────── + +/// AC1/AC2/AC3: seeding merge_jobs with an entry whose PID is dead, then +/// triggering a new merge for a *different* story, must automatically remove +/// the stale entry (AC1/AC3) and log at INFO (AC2 — verified structurally +/// because the log path is exercised when the entry is removed). +#[cfg(unix)] +#[tokio::test] +async fn stale_merge_job_with_dead_pid_is_swept_on_new_merge_attempt() { + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + let pool = Arc::new(AgentPool::new_test(3001)); + + // Seed CRDT merge_jobs with a Running entry whose recorded server-start + // time is older than the current server (legacy / previous instance). + crate::crdt_state::write_merge_job( + "719_stale_other", + "running", + 1.0, + None, + Some(&encode_server_start_time(0.0)), // legacy/older boot — should be cleaned up + ); + + // Verify the entry is present before the sweep. + assert!( + crate::crdt_state::read_merge_job("719_stale_other").is_some(), + "stale entry should exist before new merge attempt" + ); + + // Trigger a new merge for a *different* story. The sweep runs at the + // top of start_merge_agent_work and must remove the dead-PID entry. + let _ = pool.start_merge_agent_work(repo, "719_trigger_story"); + + // The stale entry must have been cleared. + assert!( + crate::crdt_state::read_merge_job("719_stale_other").is_none(), + "stale entry with dead pid must be removed when a new merge attempt starts" + ); +} + +// ── merge_agent_work tests ──────────────────────────────────────────────── + +/// Helper: start a merge and poll until terminal state. +async fn run_merge_to_completion( + pool: &Arc, + repo: &std::path::Path, + story_id: &str, +) -> MergeJob { + pool.start_merge_agent_work(repo, story_id).unwrap(); + loop { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + if let Some(job) = pool.get_merge_status(story_id) + && !matches!(job.status, MergeJobStatus::Running) + { + return job; + } + } +} + +#[tokio::test] +async fn merge_agent_work_returns_error_when_branch_not_found() { + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + let pool = Arc::new(AgentPool::new_test(3001)); + let job = run_merge_to_completion(&pool, repo, "99_nonexistent").await; + match &job.status { + MergeJobStatus::Completed(report) => { + assert!(!report.success, "should fail when branch missing"); + } + MergeJobStatus::Failed(_) => { + // Also acceptable — the pipeline errored out + } + MergeJobStatus::Running => { + panic!("should not still be running"); + } + } +} + +#[tokio::test] +async fn merge_agent_work_succeeds_on_clean_branch() { + use std::fs; + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + // Create a feature branch with a commit + Command::new("git") + .args(["checkout", "-b", "feature/story-23_test"]) + .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(); + + // Switch back to master (initial branch) + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + + // Create the story file in 4_merge/ so we can test archival + let merge_dir = repo.join(".huskies/work/4_merge"); + fs::create_dir_all(&merge_dir).unwrap(); + let story_file = merge_dir.join("23_test.md"); + fs::write(&story_file, "---\nname: 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)); + let job = run_merge_to_completion(&pool, repo, "23_test").await; + + match &job.status { + MergeJobStatus::Completed(report) => { + assert!(!report.had_conflicts, "should have no conflicts"); + assert!( + report.success + || report.gate_output.contains("Failed to run") + || !report.gates_passed, + "report should be coherent: {report:?}" + ); + if report.story_archived { + let done = repo.join(".huskies/work/5_done/23_test.md"); + assert!(done.exists(), "done file should exist"); + } + } + MergeJobStatus::Failed(e) => { + // Gate failures are acceptable in test env + assert!( + e.contains("Failed") || e.contains("failed"), + "unexpected failure: {e}" + ); + } + MergeJobStatus::Running => panic!("should not still be running"), + } +} + +// ── quality gate ordering test ──────────────────────────────── + +/// Regression test for bug 142: quality gates must run BEFORE the fast-forward +/// to master so that broken code never lands on master. +#[cfg(unix)] +#[test] +fn quality_gates_run_before_fast_forward_to_master() { + 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 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 bash\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 script/test"]) + .current_dir(repo) + .output() + .unwrap(); + + // Create a feature branch with a commit. + Command::new("git") + .args(["checkout", "-b", "feature/story-142_test"]) + .current_dir(repo) + .output() + .unwrap(); + fs::write(repo.join("change.txt"), "feature change").unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "feature work"]) + .current_dir(repo) + .output() + .unwrap(); + + // Switch back to master and record its HEAD. + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + let head_before = String::from_utf8( + Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap() + .stdout, + ) + .unwrap() + .trim() + .to_string(); + + // Run the squash-merge. The failing script/test makes quality gates + // fail → fast-forward must NOT happen. + let result = + crate::agents::merge::run_squash_merge(repo, "feature/story-142_test", "142_test").unwrap(); + + let head_after = String::from_utf8( + Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap() + .stdout, + ) + .unwrap() + .trim() + .to_string(); + + // Gates must have failed (script/test exits 1) so master should be untouched. + assert!( + !result.success, + "run_squash_merge must report failure when gates fail" + ); + assert_eq!( + head_before, head_after, + "master HEAD must not advance when quality gates fail (bug 142)" + ); +} + +#[tokio::test] +async fn merge_agent_work_conflict_does_not_break_master() { + use std::fs; + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + // Create a file on master. + fs::write( + repo.join("code.rs"), + "fn main() {\n println!(\"hello\");\n}\n", + ) + .unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "initial code"]) + .current_dir(repo) + .output() + .unwrap(); + + // Feature branch: modify the same line differently. + Command::new("git") + .args(["checkout", "-b", "feature/story-42_story_foo"]) + .current_dir(repo) + .output() + .unwrap(); + fs::write( + repo.join("code.rs"), + "fn main() {\n println!(\"hello\");\n feature_fn();\n}\n", + ) + .unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "feature: add fn call"]) + .current_dir(repo) + .output() + .unwrap(); + + // Master: add different line at same location. + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + fs::write( + repo.join("code.rs"), + "fn main() {\n println!(\"hello\");\n master_fn();\n}\n", + ) + .unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "master: add fn call"]) + .current_dir(repo) + .output() + .unwrap(); + + // Create story file in 4_merge. + let merge_dir = repo.join(".huskies/work/4_merge"); + fs::create_dir_all(&merge_dir).unwrap(); + fs::write(merge_dir.join("42_story_foo.md"), "---\nname: Test\n---\n").unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "add story"]) + .current_dir(repo) + .output() + .unwrap(); + + let pool = Arc::new(AgentPool::new_test(3001)); + let job = run_merge_to_completion(&pool, repo, "42_story_foo").await; + + // Master should NEVER have conflict markers, regardless of merge outcome. + let master_code = fs::read_to_string(repo.join("code.rs")).unwrap(); + assert!( + !master_code.contains("<<<<<<<"), + "master must never contain conflict markers:\n{master_code}" + ); + assert!( + !master_code.contains(">>>>>>>"), + "master must never contain conflict markers:\n{master_code}" + ); + + // The report should accurately reflect what happened. + match &job.status { + MergeJobStatus::Completed(report) => { + assert!(report.had_conflicts, "should report conflicts"); + } + MergeJobStatus::Failed(_) => { + // Acceptable — merge aborted due to conflicts + } + MergeJobStatus::Running => panic!("should not still be running"), + } +} + +// ── bug 675: zero commits ahead must fail with "no commits to merge" ───── + +/// Regression test for bug 675: when the feature branch has zero commits +/// ahead of master the pipeline must fail with a clear "no commits to merge" +/// error and the story must remain in `4_merge` (not advance to `5_done`). +#[tokio::test] +async fn merge_agent_work_zero_commits_ahead_stays_in_merge_stage() { + use std::fs; + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + let tmp = tempdir().unwrap(); + let repo = tmp.path(); + init_git_repo(repo); + + // Feature branch is created at the same commit as master — zero commits ahead. + Command::new("git") + .args(["checkout", "-b", "feature/story-675_zero_commits"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + + // Place the story file in 4_merge so we can verify it stays there. + let merge_dir = repo.join(".huskies/work/4_merge"); + fs::create_dir_all(&merge_dir).unwrap(); + fs::write( + merge_dir.join("675_zero_commits.md"), + "---\nname: Zero commits 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(); + + let pool = Arc::new(AgentPool::new_test(3001)); + let job = run_merge_to_completion(&pool, repo, "675_zero_commits").await; + + // The job must have failed with a "no commits to merge" error. + match &job.status { + MergeJobStatus::Failed(e) => { + assert!( + e.contains("no commits to merge"), + "error must contain 'no commits to merge', got: {e}" + ); + assert!( + e.contains("675_zero_commits"), + "error must name the story_id, got: {e}" + ); + } + MergeJobStatus::Completed(report) => { + panic!( + "expected Failed status, got Completed with success={}: {}", + report.success, report.gate_output + ); + } + MergeJobStatus::Running => panic!("should not still be running"), + } + + // Story file must still be in 4_merge — NOT advanced to 5_done. + assert!( + merge_dir.join("675_zero_commits.md").exists(), + "story file must remain in 4_merge when merge fails" + ); + assert!( + !repo + .join(".huskies/work/5_done/675_zero_commits.md") + .exists(), + "story must NOT advance to 5_done when merge fails with no commits" + ); +} + +// ── 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; + + crate::crdt_state::init_for_test(); + 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; + + crate::crdt_state::init_for_test(); + 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; + + crate::crdt_state::init_for_test(); + 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] +async fn merge_agent_work_one_commit_ahead_merges_successfully() { + use std::fs; + use tempfile::tempdir; + + crate::crdt_state::init_for_test(); + 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-675_one_commit"]) + .current_dir(repo) + .output() + .unwrap(); + fs::write(repo.join("feature_675.txt"), "feature content\n").unwrap(); + Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["commit", "-m", "add feature file"]) + .current_dir(repo) + .output() + .unwrap(); + Command::new("git") + .args(["checkout", "master"]) + .current_dir(repo) + .output() + .unwrap(); + + // Place the story file in 4_merge. + let merge_dir = repo.join(".huskies/work/4_merge"); + fs::create_dir_all(&merge_dir).unwrap(); + fs::write( + merge_dir.join("675_one_commit.md"), + "---\nname: One commit 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(); + + let pool = Arc::new(AgentPool::new_test(3001)); + let job = run_merge_to_completion(&pool, repo, "675_one_commit").await; + + // The merge must not fail with "no commits to merge". + match &job.status { + MergeJobStatus::Failed(e) => { + assert!( + !e.contains("no commits to merge"), + "one-commit-ahead branch must NOT fail with 'no commits to merge': {e}" + ); + // Gate failures (no script/test) are acceptable in test env. + } + MergeJobStatus::Completed(report) => { + // Success or gate failure — both acceptable; the key invariant is + // that we didn't fail with the zero-commits early-exit. + assert!( + report.success || !report.gates_passed, + "unexpected state: success={} gates_passed={}", + report.success, + report.gates_passed + ); + } + MergeJobStatus::Running => panic!("should not still be running"), + } +} diff --git a/server/src/agents/pool/pipeline/merge/time.rs b/server/src/agents/pool/pipeline/merge/time.rs new file mode 100644 index 00000000..1c1453e4 --- /dev/null +++ b/server/src/agents/pool/pipeline/merge/time.rs @@ -0,0 +1,37 @@ +//! Server-start-time utilities for stale-merge detection. + +/// Wall-clock time captured the first time this server process touches the +/// merge subsystem. Used to detect merge_jobs left over from a previous +/// server instance: a re-exec on `rebuild_and_restart` keeps the same PID, +/// so PID alone cannot distinguish "current" vs "previous" server. This +/// timestamp is fresh per-process (the static is reset by execve) and is +/// the source of truth for stale-merge detection. +static SERVER_START_TIME: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Return this server process's start time (lazily captured on first call). +pub(crate) fn server_start_time() -> f64 { + *SERVER_START_TIME.get_or_init(unix_now) +} + +/// Encode the current server's start-time into the CRDT `error` field for +/// a Running merge job. +pub(crate) fn encode_server_start_time(t: f64) -> String { + format!("{{\"server_start\":{t}}}") +} + +/// Decode the server-start-time from a Running merge job's `error` field. +/// Returns `None` for legacy entries (which encoded `pid` instead) — those +/// are treated as stale by the cleanup pass. +pub(crate) fn decode_server_start_time(error: Option<&str>) -> Option { + error + .and_then(|e| serde_json::from_str::(e).ok()) + .and_then(|v| v["server_start"].as_f64()) +} + +/// Current Unix timestamp in seconds as `f64`. +pub(crate) fn unix_now() -> f64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs_f64() +}