From a0aa9442e6bfdc31c1a0f8412b58e39b9a13ab98 Mon Sep 17 00:00:00 2001 From: trac41799 Date: Tue, 29 Sep 2026 21:55:25 +0900 Subject: [PATCH] feat(pipeline): deliver agent work into the project + real build/test gate - wave_executor::merge_completed_agents: commit each completed agent's worktree, merge its branch into the base branch (--no-ff), remove the worktree only after merge; conflicts abort, are recorded, and the run pauses as awaiting_user with deploy skipped. Delivery evidence in report.artifacts[merges]. - PipelineAdapters::deliver enables delivery (production true; hermetic tests keep fake worktrees offline). - install_project_dependencies (npm ci/install, 900s cap, log tail on failure) runs before verification; failure fails the run. - verification::VerifyMode::RequireBuild turns the tooling-dependent checks from Skip into Fail so 'verification passed' cannot mean 'nothing was built'. - fix (found by the new step): exclude .acc/ bookkeeping from the delivery commit - per-agent GUIDELINE.md caused add/add conflicts on every merge after the first. Regression test gives each agent a distinct guideline and expects a clean 2/2 merge. - tests: 192 lib (+4): merge happy/conflict/skip, require-build mode. - docs: ACCEPTANCE-RUN-4.md. Gates: Rust 192 + 42; Web 341; tsc/lint clean. Real 2-agent run: succeeded, both files in the base project, conflicts: []. --- .../handoffs/ACCEPTANCE-RUN-4.md | 61 ++++ src-tauri/src/pipeline.rs | 187 +++++++++- src-tauri/src/pipeline_commands.rs | 2 + src-tauri/src/real_pipeline_test.rs | 46 ++- src-tauri/src/verification.rs | 94 ++++++ src-tauri/src/wave_executor.rs | 319 ++++++++++++++++++ 6 files changed, 694 insertions(+), 15 deletions(-) create mode 100644 docs/2026-09-26-defect-closure/handoffs/ACCEPTANCE-RUN-4.md diff --git a/docs/2026-09-26-defect-closure/handoffs/ACCEPTANCE-RUN-4.md b/docs/2026-09-26-defect-closure/handoffs/ACCEPTANCE-RUN-4.md new file mode 100644 index 0000000..54648aa --- /dev/null +++ b/docs/2026-09-26-defect-closure/handoffs/ACCEPTANCE-RUN-4.md @@ -0,0 +1,61 @@ +# Acceptance Run 4 — agents' work is now actually delivered + +**Date:** 2026-09-29 · branch `hardening/production` + +## The gap this closes + +Before this change the pipeline ran agents in worktrees and then verified the +**base project**, which contained none of their work. The run could report +`succeeded` while every agent's output stayed stranded in a throwaway worktree — +the acceptance test had to assert `agent_wrote_greeting_in_worktree=true` because +`greeting_in_base_project` was false. + +Now `wave_executor::merge_completed_agents` runs between finalize and verify: + +1. commits each completed agent's worktree (excluding `.acc/` bookkeeping), +2. merges its branch into the base branch (`--no-ff`), +3. removes the worktree — only after its work is merged, +4. on conflict: aborts the merge, records it, and the run pauses as + `awaiting_user` with the conflicting agents named (deploy is skipped too). + +A production-mode flag (`PipelineAdapters::deliver`) enables it, alongside +`install_deps`, so hermetic tests stay offline and deterministic. + +## Result (real agents, 2 concurrent) + +``` +status : succeeded + finalize_and_verify done verification passed=true + deploy done deployed via mock → … (+ Dockerfile artifact) +delivery: greeting_in_base_project=true farewell_in_base_project=true +merges : {"conflicts":[],"merged":[{"agent_ref":"1-1.1",…},{"agent_ref":"1-1.2",…}]} +192 lib + 42 integration, 341 web, tsc/lint clean — 69.75s +``` + +Both agents' files are in the base project; every merge names its commit; no +worktrees are left behind. + +## Defect found by this step (and fixed) + +The first delivery attempt failed with +`CONFLICT (add/add): Merge conflict in .acc/GUIDELINE.md` — each agent's worktree +contained its **own** copy of the generated guideline, and every merge after the +first collided on it. Per-agent bookkeeping is not deliverable, so the commit now +excludes `.acc/` via a pathspec (`:(exclude).acc`), which needs no change to the +user's git config. The fail-closed behaviour worked: the run reported +`awaiting_user` and skipped deploy instead of shipping a half-merged project. +Regression test: the merge tests give every agent a distinct +`.acc/GUIDELINE.md` and still expect a clean 2/2 merge. + +## Still not proven + +- **Real build/test gate end-to-end.** `install_deps` + `VerifyMode::RequireBuild` + are implemented and unit-tested (`test_require_build_turns_tooling_skips_into_failures`), + but the acceptance run keeps `install_deps=false` because the synthetic fixture + has no real toolchain (its `build` script references `vite`, which is not + installed). The next milestone is a **from-empty** run: `scaffold_project` + produces a real app → `npm install` → real `build`/`typecheck`/runtime checks + execute and can fail — the point where "verification passed" finally means + "the app was built and tested". +- **GitHub delivery** (private repo, issues, PRs) is still not implemented; it + remains documented in `EXTERNAL-VERIFICATION.md`. diff --git a/src-tauri/src/pipeline.rs b/src-tauri/src/pipeline.rs index 32e4b90..b16fb84 100644 --- a/src-tauri/src/pipeline.rs +++ b/src-tauri/src/pipeline.rs @@ -276,6 +276,13 @@ pub struct PipelineAdapters<'a> { /// None = detect CLIs on this machine. pub cli_status: Option>, pub run_compounder: bool, + /// Install project dependencies before verification and treat an + /// un-runnable build/test as a verification failure. Production `true`; + /// hermetic tests `false` so they stay offline and deterministic. + pub install_deps: bool, + /// Commit each agent's work and merge it into the base branch (delivery). + /// Production `true`; hermetic tests use non-git stand-in worktrees. + pub deliver: bool, } enum StageResult { @@ -296,6 +303,7 @@ struct PipelineState { verification: Option, deploy: Option, compounder_items: usize, + merges: Option, } fn lock<'a>(db: &'a Mutex) -> Result, String> { @@ -439,6 +447,38 @@ pub fn run_pipeline( } } + // Work that could not be merged was not delivered: pause for the operator + // instead of verifying/deploying a project that is missing agent output. + if let Some(merges) = &state.merges { + if !merges.conflicts.is_empty() { + let detail = merges + .conflicts + .iter() + .map(|record| format!("{}: {}", record.agent_ref, record.detail)) + .collect::>() + .join("; "); + let message = format!( + "{} agent branch(es) could not be merged: {detail}", + merges.conflicts.len() + ); + report.status = pipeline_store::STATUS_AWAITING_USER.to_string(); + report.error = Some(message.clone()); + finalize_report(db, opts, &mut report, &state)?; + { + let conn = lock(db)?; + pipeline_store::set_error(&conn, &opts.run_id, &message)?; + pipeline_store::update_status( + &conn, + &opts.run_id, + pipeline_store::STATUS_AWAITING_USER, + Some("report"), + )?; + } + adapters.event_sink.emit("report", "merge_conflict", &message); + return Ok(report); + } + } + // A run whose agents never produced a valid handoff is NOT a success, even // when the project happens to satisfy verification (e.g. it already passed // before the agents ran). Pause instead of overclaiming: the operator can @@ -533,6 +573,11 @@ fn finalize_report( "generate_dockerfile": opts.generate_dockerfile, "agent_timeout_secs": opts.agent_timeout_secs, }); + // Delivery evidence: which agent branches were merged into the base branch. + if let Some(merges) = &state.merges { + report.artifacts["merges"] = + serde_json::to_value(merges).unwrap_or(serde_json::Value::Null); + } let conn = lock(db)?; pipeline_store::set_report(&conn, &opts.run_id, &serde_json::to_value(report).unwrap_or_default()) @@ -552,7 +597,7 @@ fn run_stage( BuildStage::Scaffold => stage_scaffold(opts, state), BuildStage::SeedPlan => stage_seed_plan(db, opts, state), BuildStage::ExecuteWaves => stage_execute_waves(db, opts, adapters, state), - BuildStage::FinalizeAndVerify => stage_finalize_and_verify(db, adapters, state), + BuildStage::FinalizeAndVerify => stage_finalize_and_verify(db, opts, adapters, state), BuildStage::Deploy => stage_deploy(opts, adapters, state), BuildStage::Report => Ok(StageResult::Done("report assembled".to_string())), } @@ -825,6 +870,7 @@ fn stage_execute_waves( fn stage_finalize_and_verify( db: &Mutex, + opts: &PipelineOptions, adapters: &PipelineAdapters, state: &mut PipelineState, ) -> Result { @@ -847,9 +893,47 @@ fn stage_finalize_and_verify( // Feed the compounder: the adapter execution path writes no session events, // so derive them from the handoffs (otherwise compounder_items is always 0). + // Must run before the merge removes the worktrees. record_handoff_events(db, &finalized); - let verification = crate::verification::verify_project(Path::new(&project_path)); + // Deliver: commit each completed agent's work and merge it into the base + // branch. Without this the agents' work stays stranded in worktrees and the + // project being verified/deployed contains none of it. + if adapters.deliver { + let merges = crate::wave_executor::merge_completed_agents( + &finalized, + &project_path, + &opts.base_branch, + ); + adapters.event_sink.emit( + "finalize_and_verify", + "progress", + &format!( + "delivered {} agent branch(es), {} conflict(s)", + merges.merged.len(), + merges.conflicts.len() + ), + ); + state.merges = Some(merges); + } + + // Real build/test gate: install the project's dependencies so the build, + // typecheck and runtime checks actually execute instead of being Skipped. + if adapters.install_deps { + let detail = install_project_dependencies(&project_path)?; + adapters + .event_sink + .emit("finalize_and_verify", "progress", &detail); + } + + let verification = if adapters.install_deps { + crate::verification::verify_project_with( + Path::new(&project_path), + crate::verification::VerifyMode::RequireBuild, + ) + } else { + crate::verification::verify_project(Path::new(&project_path)) + }; let passed = verification.passed; state.verification = Some(verification); @@ -935,6 +1019,85 @@ fn compound_session( Ok(items.len()) } +/// Hard cap on the dependency install so a hung registry cannot wedge a run. +const DEPENDENCY_INSTALL_TIMEOUT_SECS: u64 = 900; + +/// Install the project's dependencies (`npm ci` when a lock file exists). +/// +/// Without this the build/typecheck/runtime checks only had the option to Skip, +/// so "verification passed" could mean "nothing was ever built". +fn install_project_dependencies(project_path: &str) -> Result { + let base = Path::new(project_path); + if !base.join("package.json").exists() { + return Ok("no package.json — no dependencies to install".to_string()); + } + let args: Vec<&str> = if base.join("package-lock.json").exists() { + vec!["ci", "--no-audit", "--no-fund"] + } else { + vec!["install", "--no-audit", "--no-fund"] + }; + + let mut cmd = if cfg!(windows) { + let mut c = std::process::Command::new("cmd"); + c.arg("/C").arg("npm"); + c + } else { + std::process::Command::new("npm") + }; + cmd.args(&args).current_dir(base); + + run_with_timeout(cmd, DEPENDENCY_INSTALL_TIMEOUT_SECS) + .map(|_| format!("dependencies installed (npm {})", args.join(" "))) +} + +/// Run a command with a wall-clock timeout, sending its output to a temp log so +/// a chatty child cannot deadlock on a full pipe. Failures carry the log tail. +fn run_with_timeout(mut cmd: std::process::Command, secs: u64) -> Result<(), String> { + let log_path = + std::env::temp_dir().join(format!("sourceforge-cmd-{}.log", std::process::id())); + let log = std::fs::File::create(&log_path).map_err(|e| e.to_string())?; + let log_err = log.try_clone().map_err(|e| e.to_string())?; + let mut child = cmd + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::from(log)) + .stderr(std::process::Stdio::from(log_err)) + .spawn() + .map_err(|e| format!("cannot start command: {e}"))?; + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(secs); + loop { + match child.try_wait() { + Ok(Some(status)) if status.success() => return Ok(()), + Ok(Some(status)) => { + return Err(format!( + "command failed ({status}); output tail: {}", + tail_of(&log_path) + )); + } + Ok(None) => { + if std::time::Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!( + "timed out after {secs}s; output tail: {}", + tail_of(&log_path) + )); + } + std::thread::sleep(std::time::Duration::from_millis(200)); + } + Err(error) => return Err(error.to_string()), + } + } +} + +fn tail_of(path: &Path) -> String { + let Ok(text) = std::fs::read_to_string(path) else { + return String::new(); + }; + let tail: Vec<&str> = text.lines().rev().take(8).collect(); + tail.into_iter().rev().collect::>().join(" | ") +} + /// Agents that did not finish with a valid handoff, as `(agent_ref, status)`. /// A non-empty result means the promised work was not done. fn incomplete_agents(report: &crate::wave_executor::WaveExecutionReport) -> Vec<(String, String)> { @@ -975,6 +1138,14 @@ fn stage_deploy( // happened to be in the worktree), so gate it like a failed verification // instead of pushing a half-done project to production. if !opts.allow_deploy_on_failed_verification { + if let Some(merges) = &state.merges { + if !merges.conflicts.is_empty() { + return Ok(StageResult::Skipped(format!( + "{} agent branch(es) could not be merged — deploy skipped", + merges.conflicts.len() + ))); + } + } if let Some(wave) = &state.wave_report { let incomplete = incomplete_agents(wave); if !incomplete.is_empty() { @@ -1260,6 +1431,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: false, run_compounder: true, }; @@ -1388,6 +1561,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(status), + install_deps: false, + deliver: false, run_compounder: false, }; @@ -1405,6 +1580,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: false, run_compounder: false, }; @@ -1441,6 +1618,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: false, run_compounder: false, }; @@ -1472,6 +1651,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: false, run_compounder: false, }; @@ -1542,6 +1723,8 @@ mod tests { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: false, run_compounder: false, }; diff --git a/src-tauri/src/pipeline_commands.rs b/src-tauri/src/pipeline_commands.rs index dd7fc52..7dad564 100644 --- a/src-tauri/src/pipeline_commands.rs +++ b/src-tauri/src/pipeline_commands.rs @@ -92,6 +92,8 @@ async fn run_on_blocking_thread( event_sink: &sink, llm: &llm, cli_status: None, + install_deps: true, + deliver: true, run_compounder: true, }; diff --git a/src-tauri/src/real_pipeline_test.rs b/src-tauri/src/real_pipeline_test.rs index 88f469e..66366e9 100644 --- a/src-tauri/src/real_pipeline_test.rs +++ b/src-tauri/src/real_pipeline_test.rs @@ -226,6 +226,8 @@ fn test_real_pipeline_end_to_end() { event_sink: &NoopEventSink, llm: &llm, cli_status: Some(all_clis_installed()), + install_deps: false, + deliver: true, run_compounder: false, }; @@ -310,6 +312,12 @@ fn test_real_pipeline_end_to_end() { project.path().join("Dockerfile").exists(), agent_greeting ); + println!( + "delivery: greeting_in_base_project={} farewell_in_base_project={} merges={}", + project.path().join("src/greeting.ts").exists(), + project.path().join("src/farewell.ts").exists(), + report.artifacts["merges"] + ); println!("========================\n"); // Preserve artifacts for inspection: the worktrees live inside the fixture @@ -331,22 +339,34 @@ fn test_real_pipeline_end_to_end() { ); let wave = report.wave.as_ref().expect("wave report"); assert!(!wave.agents.is_empty(), "no agents were spawned"); - let with_handoff = wave - .agents - .iter() - .filter(|a| { - Path::new(&a.worktree_path) - .join(format!("HANDOFF_{}.md", a.agent_ref)) - .exists() - }) - .count(); - assert_eq!( - with_handoff, - wave.agents.len(), - "agents without a valid handoff: {:?}", + assert!( + wave.agents.iter().all(|agent| agent.status == "done"), + "agents did not complete: {:?}", wave.agents .iter() .map(|a| (a.agent_ref.clone(), a.status.clone())) .collect::>() ); + + // Delivery: every agent branch is merged and the work is in the base project + // (not stranded in a worktree). Before the delivery step this was false. + let merges = &report.artifacts["merges"]; + assert_eq!( + merges["conflicts"].as_array().map(Vec::len), + Some(0), + "no merge conflicts expected: {merges}" + ); + assert_eq!( + merges["merged"].as_array().map(Vec::len), + Some(wave.agents.len()), + "every completed agent must be delivered: {merges}" + ); + assert!( + project.path().join("src/greeting.ts").exists(), + "greeting.ts must be in the base project" + ); + assert!( + project.path().join("src/farewell.ts").exists(), + "farewell.ts must be in the base project" + ); } diff --git a/src-tauri/src/verification.rs b/src-tauri/src/verification.rs index a0196ea..eafc9ec 100644 --- a/src-tauri/src/verification.rs +++ b/src-tauri/src/verification.rs @@ -232,6 +232,49 @@ const REQUIRED_PACKAGE_SCRIPTS: &[&str] = &[ // ── Core Verification ────────────────────────────────────────────────── pub fn verify_project(base: &Path) -> VerificationReport { + verify_project_with(base, VerifyMode::Lenient) +} + +/// How strictly checks that need an installed toolchain are treated. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VerifyMode { + /// Missing tooling → `Skip` (hermetic tests, projects without deps). + Lenient, + /// Missing tooling → `Fail`: "verification passed" must mean the build and + /// runtime checks actually ran. Used by the pipeline when it installed the + /// project's dependencies. + RequireBuild, +} + +/// Checks that must have executed for a `RequireBuild` verification to pass. +const BUILD_GATED_CHECKS: [&str; 3] = [ + "node_modules installed", + "npm run build passes", + "E2E runtime test", +]; + +pub fn verify_project_with(base: &Path, mode: VerifyMode) -> VerificationReport { + let mut report = verify_project_lenient(base); + if mode == VerifyMode::RequireBuild { + let mut downgraded = false; + for check in report.checks.iter_mut() { + if BUILD_GATED_CHECKS.contains(&check.name.as_str()) { + if let CheckStatus::Skip(detail) = &check.status { + let message = format!("required build step did not run: {detail}"); + check.status = CheckStatus::Fail(message.clone()); + check.detail = message; + downgraded = true; + } + } + } + if downgraded { + report.passed = false; + } + } + report +} + +fn verify_project_lenient(base: &Path) -> VerificationReport { let mut report = VerificationReport::new(&base.to_string_lossy()); // ── 1. Project structure ────────────────────────────────────── @@ -1355,4 +1398,55 @@ mod tests { .unwrap(); assert_eq!(n, 1, "review-band README check enqueues a review"); } + + + #[test] + fn test_require_build_turns_tooling_skips_into_failures() { + // A Node project without installed dependencies: lenient verification + // Skips the build/runtime checks (so "passed" could mean "never built"). + let dir = TempDir::new().unwrap(); + std::fs::write( + dir.path().join("package.json"), + r#"{"name":"x","scripts":{"build":"vite build"}}"#, + ) + .unwrap(); + + let gated = [ + "node_modules installed", + "npm run build passes", + "E2E runtime test", + ]; + + let lenient = verify_project(dir.path()); + let skipped = lenient + .checks + .iter() + .filter(|check| { + gated.contains(&check.name.as_str()) + && matches!(check.status, CheckStatus::Skip(_)) + }) + .count(); + assert!(skipped >= 1, "fixture must exercise the skip path"); + + let strict = verify_project_with(dir.path(), VerifyMode::RequireBuild); + assert!(!strict.passed, "un-run build steps must fail the run"); + for name in gated { + let check = strict.checks.iter().find(|c| c.name == name).unwrap(); + assert!( + !matches!(check.status, CheckStatus::Skip(_)), + "{name} must not silently skip in RequireBuild mode: {:?}", + check.status + ); + } + let build = strict + .checks + .iter() + .find(|c| c.name == "npm run build passes") + .unwrap(); + assert!( + format!("{:?}", build.status).contains("required build step did not run"), + "failure should explain why: {:?}", + build.status + ); + } } diff --git a/src-tauri/src/wave_executor.rs b/src-tauri/src/wave_executor.rs index 4fd068e..44033f5 100644 --- a/src-tauri/src/wave_executor.rs +++ b/src-tauri/src/wave_executor.rs @@ -399,6 +399,189 @@ async fn resume_wave_from_state( Ok(report) } +// --------------------------------------------------------------------------- +// Delivery: commit agent work and merge it back (work not merged is not shipped) +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)] +pub struct MergeRecord { + pub agent_ref: String, + pub branch: String, + pub commit: Option, + pub detail: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)] +pub struct MergeReport { + /// Branches committed and merged into the base branch. + pub merged: Vec, + /// Branches that could not be merged (conflict, missing worktree, ...). + pub conflicts: Vec, + /// Agents that produced nothing to deliver (not done / no changes). + pub skipped: Vec, +} + +fn git(args: &[&str]) -> Result<(bool, String), String> { + let output = std::process::Command::new("git") + .args(args) + .output() + .map_err(|e| format!("git {:?} failed to start: {e}", &args[..1.min(args.len())]))?; + let mut text = String::from_utf8_lossy(&output.stdout).to_string(); + text.push_str(&String::from_utf8_lossy(&output.stderr)); + Ok((output.status.success(), text.trim().to_string())) +} + +fn git_or_err(args: &[&str]) -> Result { + let (ok, text) = git(args)?; + if ok { + Ok(text) + } else { + Err(format!("git {} failed: {text}", args.join(" "))) + } +} + +/// Commit each completed agent's worktree and merge its branch into the base +/// branch, then remove the worktree. +/// +/// This is what makes the run a *delivery*: before this, agents wrote only into +/// throwaway worktrees and the base project never received their work. +pub fn merge_completed_agents( + report: &WaveExecutionReport, + base_repo: &str, + base_branch: &str, +) -> MergeReport { + let mut result = MergeReport::default(); + + // Merge into the intended branch regardless of what is currently checked out. + let (ok, head) = git(&["-C", base_repo, "rev-parse", "--abbrev-ref", "HEAD"]).unwrap_or((false, String::new())); + if !ok { + result.conflicts.push(MergeRecord { + agent_ref: "-".to_string(), + branch: base_branch.to_string(), + commit: None, + detail: format!("cannot read the current branch of {base_repo}"), + }); + return result; + } + if head != base_branch { + if let Err(error) = git_or_err(&["-C", base_repo, "checkout", base_branch]) { + result.conflicts.push(MergeRecord { + agent_ref: "-".to_string(), + branch: base_branch.to_string(), + commit: None, + detail: format!("cannot switch to {base_branch}: {error}"), + }); + return result; + } + } + + for agent in &report.agents { + let record = |commit: Option, detail: String| MergeRecord { + agent_ref: agent.agent_ref.clone(), + branch: agent.branch.clone(), + commit, + detail, + }; + + if agent.status != "done" { + result + .skipped + .push(record(None, format!("agent status is {}", agent.status))); + continue; + } + if !Path::new(&agent.worktree_path).exists() { + result.skipped.push(record(None, "worktree is gone".to_string())); + continue; + } + + // 1. Commit whatever the agent left behind (identity supplied inline so + // the user's repo does not need git identity configured). + // + // `.acc/` (GUIDELINE.md, TASK.md) is pipeline bookkeeping, not + // deliverable: every agent's copy differs, so committing it made every + // second agent conflict on add/add. Excluded via pathspec rather than + // touching the user's git config. + const EXCLUDE_BOOKKEEPING: [&str; 2] = [".", ":(exclude).acc"]; + let (_, status) = match git(&[ + "-C", + &agent.worktree_path, + "status", + "--porcelain", + "--", + EXCLUDE_BOOKKEEPING[0], + EXCLUDE_BOOKKEEPING[1], + ]) { + Ok(value) => value, + Err(error) => { + result.conflicts.push(record(None, error)); + continue; + } + }; + let commit = if status.is_empty() { + let (ok, sha) = git(&["-C", &agent.worktree_path, "rev-parse", "HEAD"]).unwrap_or((false, String::new())); + if !ok { + result.skipped.push(record(None, "no changes and no commit".to_string())); + continue; + } + Some(sha) + } else { + let message = format!("agent {}: deliver work from {}", agent.agent_ref, agent.branch); + let staged = git_or_err(&[ + "-C", + &agent.worktree_path, + "add", + "-A", + "--", + EXCLUDE_BOOKKEEPING[0], + EXCLUDE_BOOKKEEPING[1], + ]) + .and_then(|_| { + git_or_err(&[ + "-C", + &agent.worktree_path, + "-c", + "user.name=SourceForge Agent", + "-c", + "user.email=agent@sourceforge.local", + "commit", + "-m", + &message, + ]) + }); + match staged { + Ok(_) => { + let (ok, sha) = git(&["-C", &agent.worktree_path, "rev-parse", "HEAD"]).unwrap_or((false, String::new())); + if !ok { + result.conflicts.push(record(None, "commit created but HEAD unreadable".to_string())); + continue; + } + Some(sha) + } + Err(error) => { + result.conflicts.push(record(None, format!("commit failed: {error}"))); + continue; + } + } + }; + + // 2. Merge the agent branch into the base branch. + let short = commit.as_deref().unwrap_or("work").chars().take(8).collect::(); + let merge_message = format!("merge agent {} ({})", agent.agent_ref, short); + if let Err(error) = git_or_err(&["-C", base_repo, "merge", "--no-ff", "-m", &merge_message, &agent.branch]) { + // Leave the repo clean for the operator to resolve by hand. + let _ = git(&["-C", base_repo, "merge", "--abort"]); + result.conflicts.push(record(commit, format!("merge failed: {error}"))); + continue; + } + + // 3. Only now is the worktree disposable. + let _ = git(&["-C", base_repo, "worktree", "remove", "--force", &agent.worktree_path]); + result.merged.push(record(commit, format!("merged into {base_branch}"))); + } + + result +} + #[cfg(test)] mod tests { use super::*; @@ -477,4 +660,140 @@ mod tests { let path = repo.path().to_str().unwrap(); assert!(std::path::Path::new(path).join(".git").exists()); } + + // ------------------------------------------------------------------ + // Delivery: merging agent work back into the base branch + // ------------------------------------------------------------------ + + fn git(args: &[&str]) { + let out = std::process::Command::new("git").args(args).output().unwrap(); + assert!( + out.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + } + + /// Create a worktree on its own branch, write `file` with `content`, and + /// return the AgentExecution the merge step would receive. + fn agent_worktree( + repo: &TempDir, + agent_ref: &str, + file: &str, + content: &str, + ) -> AgentExecution { + let repo_path = repo.path().to_str().unwrap(); + let worktree = repo.path().join(format!("wt-{agent_ref}")); + let branch = format!("agent/{agent_ref}"); + let wt = worktree.to_str().unwrap(); + git(&["-C", repo_path, "worktree", "add", "-b", &branch, wt, "main"]); + let target = worktree.join(file); + if let Some(parent) = target.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(target, content).unwrap(); + // Per-agent pipeline bookkeeping: must never be delivered (every agent's + // copy differs, which used to make every second merge conflict add/add). + let acc = worktree.join(".acc"); + std::fs::create_dir_all(&acc).unwrap(); + std::fs::write(acc.join("GUIDELINE.md"), format!("# AGENT {agent_ref} GUIDELINE\n")).unwrap(); + AgentExecution { + agent_ref: agent_ref.to_string(), + session_id: format!("session-{agent_ref}"), + worktree_path: wt.to_string(), + branch, + status: "done".to_string(), + guideline_path: String::new(), + cost_usd: 0.0, + retry_count: 0, + } + } + + fn wave_report(agents: Vec) -> WaveExecutionReport { + WaveExecutionReport { + plan_id: "plan-merge".to_string(), + base_repo: String::new(), + agents, + started_at: "now".to_string(), + completed_at: None, + total_cost_usd: 0.0, + } + } + + #[test] + fn test_merge_delivers_agent_work_into_the_base_branch() { + let repo = create_test_repo(); + let repo_path = repo.path().to_str().unwrap(); + let report = wave_report(vec![ + agent_worktree(&repo, "1-1", "src/a.txt", "from a\n"), + agent_worktree(&repo, "1-2", "src/b.txt", "from b\n"), + ]); + + let merges = merge_completed_agents(&report, repo_path, "main"); + + assert_eq!(merges.merged.len(), 2, "both agents delivered: {:?}", merges); + assert!(merges.conflicts.is_empty(), "no conflicts: {:?}", merges.conflicts); + // The work is now in the base project, not just in a worktree. + assert!(repo.path().join("src/a.txt").exists()); + assert!(repo.path().join("src/b.txt").exists()); + // Worktrees are disposable once their work is merged. + for agent in &report.agents { + assert!(!Path::new(&agent.worktree_path).exists(), "worktree removed"); + } + // Every recorded merge names its commit. + assert!(merges.merged.iter().all(|m| m.commit.is_some())); + } + + #[test] + fn test_merge_reports_conflicts_without_leaving_a_dirty_repo() { + let repo = create_test_repo(); + let repo_path = repo.path().to_str().unwrap(); + // Same file, different content → the second merge must conflict. + let report = wave_report(vec![ + agent_worktree(&repo, "1-1", "conflict.txt", "a\n"), + agent_worktree(&repo, "1-2", "conflict.txt", "b\n"), + ]); + + let merges = merge_completed_agents(&report, repo_path, "main"); + + assert_eq!(merges.merged.len(), 1); + assert_eq!(merges.conflicts.len(), 1, "conflicts: {:?}", merges.conflicts); + assert_eq!(merges.conflicts[0].agent_ref, "1-2"); + assert!(merges.conflicts[0].detail.contains("merge failed")); + // The repo is left clean for the operator (merge aborted, not half-done). + // Untracked entries are the still-present worktrees of unmerged agents. + let out = std::process::Command::new("git") + .args(["-C", repo_path, "status", "--porcelain", "--untracked-files=no"]) + .output() + .unwrap(); + assert!( + String::from_utf8_lossy(&out.stdout).trim().is_empty(), + "no half-merged changes: {}", + String::from_utf8_lossy(&out.stdout) + ); + let merge_head = std::process::Command::new("git") + .args(["-C", repo_path, "rev-parse", "-q", "--verify", "MERGE_HEAD"]) + .output() + .unwrap(); + assert!(!merge_head.status.success(), "no merge left in progress"); + } + + #[test] + fn test_merge_skips_agents_that_did_not_complete() { + let repo = create_test_repo(); + let repo_path = repo.path().to_str().unwrap(); + let mut failed = agent_worktree(&repo, "1-1", "x.txt", "x\n"); + failed.status = "failed".to_string(); + let report = wave_report(vec![failed]); + + let merges = merge_completed_agents(&report, repo_path, "main"); + + assert!(merges.merged.is_empty()); + assert!(merges.conflicts.is_empty()); + assert_eq!(merges.skipped.len(), 1); + assert!(merges.skipped[0].detail.contains("failed")); + // Its worktree is left in place for inspection. + assert!(Path::new(&report.agents[0].worktree_path).exists()); + } }