//! Getting a coding phase's work out of the mission and somewhere durable. //! //! Until now a coding mission produced nothing. Agents cloned a repo, edited //! files, and thirty minutes after the mission finished `teardown_container` //! called `remove_dir_all` on the checkout (`mission_runtime`). Nothing //! pushed, no artifact was registered — `register_artifact` had no callers at //! all — and the only durable output was an LLM-written narrative of what the //! agents *said* they had done. //! //! This module captures the work first and publishes it second, in that //! order and never the reverse. The patch is written to disk before any //! remote is contacted, so a push that fails — a rotated token, a rejected //! ref, an unreachable forge — costs a branch and not the work. //! //! ## Why the server does this and not the agent //! //! An agent that reports "I committed and pushed" is making a claim, and this //! codebase has spent a lot of effort learning not to bank claims (see //! `evaluator`). Server-side git means the exit codes, the shas and the //! diffstat are ours: what lands in `mission_artifacts` is what git actually //! did. It is also the only version that can be gated — a `commit_policy` //! enforced by asking an agent to please check its tests first is a wish. //! //! ## Retention //! //! Output goes to `/_outputs///`, a *sibling* //! of the per-mission directories the sweeper deletes, and outside every bind //! mount handed to a container (`ensure_container` mounts only //! `/`). So agents cannot reach their own delivery record, //! and teardown cannot take it with them. use std::path::{Path, PathBuf}; use serde_json::json; use uuid::Uuid; use crate::mission_workspace; /// Paths never worth capturing: build output and vendored dependencies. A /// coding phase that ran `cargo build` leaves a `target/` directory larger /// than most repositories, and a patch containing it is unreadable as well as /// enormous. /// /// `mission_fs` uses this same list for the TRANSPORT, and that is not a /// convenience — it is the fix for a real failure. The diff excluded `target/` /// while the tar that carried the tree in and out did not, so a phase that ran /// `cargo test` shipped its whole build directory over vsock twice. `vm_collect` /// timed out at 300s on mission 019fd43e with the agent's work finished and /// stranded inside a VM. Two layers, one list. pub(crate) const EXCLUDED_PATHS: &[&str] = &[ "target", "node_modules", ".venv", "venv", "dist", "build", ".next", "__pycache__", ".pytest_cache", ".mypy_cache", "vendor", // Agent workaround debris. `.gitconfig_temp` appeared on mission 019fc3ba // when an agent hit git's ownership check and wrote its own safe.directory // config into the repository root. The cause is fixed (mission containers // now carry GIT_CONFIG_* env), but excluding the artefact keeps a stray // workaround out of a user's repository if an agent invents another one. ".gitconfig_temp", ".gitconfig.tmp", ]; /// Cap on the captured patch. Past this the diff is truncated with a marker /// rather than dropped: a 40 MB patch is a signal in itself (something /// generated or vendored got committed), and the head of it is what an /// operator needs to see to work out what happened. const MAX_PATCH_BYTES: usize = 4 * 1024 * 1024; /// Cap on the recorded path list. A cap that silently truncates is worse than /// no cap, so the metadata carries `files_truncated` beside it — a reader must /// be able to tell "touched 12 files" from "touched at least 500". const MAX_CAPTURED_PATHS: usize = 500; /// Who delivery commits as. /// /// The operator's identity by default, so pushed commits associate with their /// forge account the way their own commits do. Overridable per deployment via /// `CLAWMATES_COMMIT_NAME` / `CLAWMATES_COMMIT_EMAIL` — a shared instance /// wants a bot identity here, not a person's. /// /// What matters for correctness is only that *some* identity is always set: /// the server container has none of its own, so `git commit` fails outright /// without this. The particular value is attribution, not function. /// /// Attribution alone, to be clear — the push credential is `GITEA_TOKEN` and /// is unaffected by any of this. const DEFAULT_COMMIT_NAME: &str = "Omar Sobh"; const DEFAULT_COMMIT_EMAIL: &str = "om.sobh@gmail.com"; pub(crate) fn commit_identity() -> (String, String) { let name = std::env::var("CLAWMATES_COMMIT_NAME") .ok() .filter(|v| !v.trim().is_empty()) .unwrap_or_else(|| DEFAULT_COMMIT_NAME.to_string()); let email = std::env::var("CLAWMATES_COMMIT_EMAIL") .ok() .filter(|v| !v.trim().is_empty()) .unwrap_or_else(|| DEFAULT_COMMIT_EMAIL.to_string()); (name, email) } /// Ceiling on the gate's test run. Long enough for a real suite, short enough /// that a hung test does not hold a phase open indefinitely. const TEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(900); /// What a phase produced. #[derive(Debug, Clone)] pub struct Capture { pub base_sha: String, pub files_changed: usize, pub insertions: usize, pub deletions: usize, /// The phase changed nothing. Still recorded — "this coding phase wrote no /// code" is currently invisible to an operator, and it is worth saying. pub empty: bool, /// Why the diff could not be computed, if it could not be. `empty` is only /// meaningful when this is `None`: otherwise the tree was never read, and /// callers deciding anything on the strength of "no changes" must not. pub diff_error: Option, pub truncated: bool, /// The paths this phase touched, with their `--name-status` letter. The /// diffstat gives counts only; this is what lets anything downstream say /// WHICH files changed. pub files: Vec<(char, String)>, /// The path list hit `MAX_CAPTURED_PATHS`. Recorded so a reader can tell a /// complete list from a clipped one. pub files_truncated: bool, pub patch_path: PathBuf, /// Set once the work has been committed to a mission branch. pub committed: Option, } /// A commit made on the mission's own branch. #[derive(Debug, Clone)] pub struct Commit { pub branch: String, pub sha: String, } /// Where a mission's durable output lives. Sibling of the swept per-mission /// directories, deliberately. pub fn outputs_root(mission_id: Uuid) -> PathBuf { mission_workspace::missions_root() .join("_outputs") .join(mission_id.to_string()) } /// Capture the working-tree diff for one phase and register it as an artifact. /// /// Runs against the host checkout. Returns `Ok(None)` when the mission has no /// repository — a research-only phase has nothing to capture and that is not /// an error. pub async fn capture_phase_diff( pool: &sqlx::PgPool, mission_id: Uuid, phase_id: Uuid, ) -> Result, String> { // The phase's pass number, so a re-run lands on its own branch instead of // colliding with the previous attempt. let iteration: i32 = sqlx::query_scalar("SELECT iteration FROM mission_phases WHERE id = $1") .bind(phase_id) .fetch_optional(pool) .await .ok() .flatten() .unwrap_or(0); // `commit_policy` lives in the phase config, merged there from the // workflow recipe by `phases_for_create`. let policy: Option = sqlx::query_scalar("SELECT config->>'commit_policy' FROM mission_phases WHERE id = $1") .bind(phase_id) .fetch_optional(pool) .await .ok() .flatten(); capture_phase_diff_at( pool, mission_id, phase_id, &mission_workspace::checkout_path(mission_id), &outputs_root(mission_id), iteration, Gate::parse(policy.as_deref()), ) .await } /// [`capture_phase_diff`] with the two paths supplied. /// /// Path resolution reads `CLAWMATES_MISSIONS_ROOT`, which is process-global; /// tests that set it race each other and silently capture the wrong tree. The /// seam keeps the interesting behaviour — what git sees, what lands on disk, /// what is registered — testable in parallel without touching the environment. pub async fn capture_phase_diff_at( pool: &sqlx::PgPool, mission_id: Uuid, phase_id: Uuid, repo: &Path, outputs: &Path, iteration: i32, gate: Gate, ) -> Result, String> { let repo = repo.to_path_buf(); if !repo.is_dir() { return Ok(None); } // Diff from where the mission *started*, not from HEAD. // // `HEAD` is the wrong baseline the moment an agent commits, and committing // is the normal path — `rust_sdlc` has a committer role. A mission that did // its job properly leaves a clean tree, so a HEAD-relative diff reports // nothing changed. That is exactly what happened on mission 019fc372: the // agent committed the file it was asked to create and capture recorded // `empty: true` beside a commit that plainly contained the work. // // Diffing against the recorded clone point covers committed, staged and // unstaged changes in one pass. Falling back to HEAD keeps checkouts made // before the base was recorded working, at the cost of missing committed // work — which is why the fallback says so in the metadata. let recorded_base = mission_workspace::base_commit(&repo); let base_is_recorded = recorded_base.is_some(); let base_sha = match recorded_base { Some(sha) => sha, None => git(&repo, &["rev-parse", "HEAD"]) .await .map(|s| s.trim().to_string()) .unwrap_or_else(|_| "HEAD".to_string()), }; // `--intent-to-add` registers untracked files with the index without // staging their content, which is what makes them appear in `git diff`. // Without it a phase that only *created* files would produce an empty // patch — the most likely shape for generated code, and the worst one to // silently lose. let mut add = vec!["add", "--intent-to-add", "--", "."]; let excludes: Vec = EXCLUDED_PATHS .iter() .map(|p| format!(":(exclude){p}")) .collect(); add.extend(excludes.iter().map(String::as_str)); // A repo with nothing to add is fine; keep going and let the diff be empty. let _ = git(&repo, &add).await; // A failed `git diff` and a phase that changed nothing both yield an empty // string, and `unwrap_or_default` used to erase the difference: a corrupt // index or an unreadable base would land `empty: true, files_changed: 0` — // byte-identical to an honest no-op, and just as quiet. Whatever went // wrong is recorded so the artifact can say which of the two it was. let mut diff_error: Option = None; let mut note_diff_failure = |what: &str, e: String| { eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} could not compute \ {what} against {base_sha}: {e}" ); if diff_error.is_none() { diff_error = Some(format!("{what}: {}", e.chars().take(300).collect::())); } }; let mut diff_args = vec!["diff", base_sha.as_str(), "--"]; diff_args.extend(excludes.iter().map(String::as_str)); let patch = match git(&repo, &diff_args).await { Ok(p) => p, Err(e) => { note_diff_failure("patch", e); String::new() } }; let mut stat_args = vec!["diff", base_sha.as_str(), "--stat", "--"]; stat_args.extend(excludes.iter().map(String::as_str)); let diffstat = match git(&repo, &stat_args).await { Ok(s) => s, Err(e) => { note_diff_failure("diffstat", e); String::new() } }; // The paths themselves, not just the counts. // // The diffstat gives three integers and throws the filenames away, so // nothing downstream could say WHICH files a phase touched — the World // could draw a "coding" station but nothing under it. Same `base_sha` and // the same excludes as the `--stat` call above: if the two disagreed, // `files_changed` and this list would contradict each other and nobody // could tell which one lied. // // Must run BEFORE the reset below — `--intent-to-add` is what makes newly // created files visible to diff at all. let mut name_args = vec!["diff", base_sha.as_str(), "--name-status", "--"]; name_args.extend(excludes.iter().map(String::as_str)); let name_status = match git(&repo, &name_args).await { Ok(s) => s, Err(e) => { note_diff_failure("name-status", e); String::new() } }; // Put the index back. `--intent-to-add` is a mutation of the agent's // workspace, and capture must not change what a later commit would see. let _ = git(&repo, &["reset", "--quiet"]).await; let (files_changed, insertions, deletions) = parse_diffstat(&diffstat); // Shared with auto_merge so the two cannot disagree about what a // `--name-status` line means (renames are three fields; the NEW path is the // one that changed). let all_paths = crate::auto_merge::changed_paths(&name_status); let files_truncated = all_paths.len() > MAX_CAPTURED_PATHS; let files: Vec<(char, String)> = all_paths.into_iter().take(MAX_CAPTURED_PATHS).collect(); let empty = patch.trim().is_empty(); let truncated = patch.len() > MAX_PATCH_BYTES; let stored = if truncated { let head: String = patch.chars().take(MAX_PATCH_BYTES).collect(); format!( "{head}\n\n… patch truncated at {MAX_PATCH_BYTES} bytes \ ({} bytes total) …\n", patch.len() ) } else { patch }; let dir = outputs.join(phase_id.to_string()); std::fs::create_dir_all(&dir).map_err(|e| format!("create {}: {e}", dir.display()))?; let patch_path = dir.join("diff.patch"); std::fs::write(&patch_path, &stored) .map_err(|e| format!("write {}: {e}", patch_path.display()))?; // Raw evidence on disk, independent of the JSONB. When the metadata and // the picture disagree, this is the tiebreaker. let _ = std::fs::write(dir.join("names.txt"), &name_status); std::fs::write(dir.join("diffstat.txt"), &diffstat) .map_err(|e| format!("write diffstat: {e}"))?; // Commit only after the patch is safely on disk. If this fails, the work // is still captured and the artifact still lands — the branch is the // convenience, the patch is the guarantee. // Why a phase has no branch belongs in the artifact, not only in the log. // Mission `019fc437` recorded `branch: null, push_error: null` for both // phases — indistinguishable from a phase that was never eligible to // commit. The reason was in stderr on the host, where nothing reading the // mission would find it. let mut commit_error: Option = None; let committed = match commit_phase_work(&repo, mission_id, phase_id, iteration).await { Ok(c) => c, Err(e) => { eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} captured but not \ committed: {e}" ); commit_error = Some(e.chars().take(500).collect()); None } }; // Hand the next phase a base that excludes this one's work. Done here // rather than inside `commit_phase_work` so the patch on disk is already // written: if the process dies between the two, the worst case is a phase // that re-reports work, not a phase whose work is invisible. if let Some(c) = committed.as_ref() { mission_workspace::advance_base_commit(&repo, &c.sha); } // Gate, then publish. Both are best-effort on top of an artifact that has // already landed: a phase whose tests fail, or whose push is rejected, // still has its patch on disk and its work on a local branch. // // `empty` suppresses publishing, so a diff we could not COMPUTE would // otherwise skip the push and leave `push_error: null` — the phase looking // exactly like one that correctly had nothing to publish. See // [`untrusted_empty_reason`]. let mut outcome: Option = None; let mut published: Option = None; let mut publish_error: Option = untrusted_empty_reason(empty, diff_error.as_deref()); if let Some(c) = committed.as_ref() { if !empty { if gate == Gate::OnGreenTests { let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER") .unwrap_or_else(|_| "clawmates-runtime".to_string()); // Against a COPY, never the checkout. `verify_tests` execs // `cargo test` in a container running as ROOT, which writes // `target/` — in the live tree that leaves root-owned build // output in a checkout owned by uid 65532 and breaks the // single-writer invariant. Measured the first time this gate // ever ran end to end: `uids=0,65532`. // // The gate had been implemented but never exercised (every // harness fixture used `commit_policy: "always"`), which is why // a bug this mechanical survived in it. let gate_root = crate::root_copy::copy_root("_gate", mission_id); crate::root_copy::purge(&container, &gate_root).await; let o = match crate::root_copy::RootCopy::of(&repo, &gate_root) { Ok(copy) => { let r = verify_tests(copy.workdir(), &container).await; crate::root_copy::purge(&container, &gate_root).await; r } // Fail-closed: an unverifiable suite must not license a push. Err(e) => TestOutcome::CouldNotRun(format!( "could not copy the checkout to test it: {e}" )), }; // An infrastructure fault must be loud. The gate degrades // safely either way, but "we could not run the suite" is a // problem with the platform and needs to look like one. if let TestOutcome::CouldNotRun(why) = &o { eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} could NOT \ run the test suite — gating as unverified: {why}" ); } outcome = Some(o); } match push_url_for(pool, mission_id).await { Ok(Some(url)) => { let verified = outcome.as_ref().and_then(TestOutcome::verified); match publish_phase_branch(&repo, &url, &c.branch, gate, verified).await { Ok(p) => published = Some(p), // `publish_phase_branch` only returns Err for a local // git failure; a rejected push is Ok with an error // inside. Both must reach the artifact. Err(e) => { eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} \ could not publish {}: {e}", c.branch ); // Both ends, not the first 500 chars. Git prints its // REASON last — "non-fast-forward", "fetch first", // "protected branch" — so a head-only clamp keeps the // noise and drops the answer. A real push failure was // recorded as two auth lines plus a branch name cut // off mid-word, with the reject reason gone. publish_error = Some(crate::evaluator_tools::clamp_output(&e)); } } } Ok(None) => { // Legitimate: a mission with no repo bound has nowhere to // push. Still recorded, because "not pushed" with no reason // is the ambiguity this whole pass exists to remove. publish_error = Some("mission has no repo bound; work is committed locally only".into()); } Err(e) => { eprintln!( "mission_delivery: mission {mission_id} could not resolve a push \ URL ({e}) — work is committed locally on {} but not published", c.branch ); publish_error = Some(format!("could not resolve push URL: {e}")); } } } } let meta = json!({ "base_sha": base_sha, "base_recorded": base_is_recorded, "branch": published .as_ref() .map(|p| p.branch.clone()) .or_else(|| committed.as_ref().map(|c| c.branch.clone())), "head_sha": committed.as_ref().map(|c| c.sha.clone()), "commit_policy": match gate { Gate::Always => "always", Gate::OnGreenTests => "on_green_tests", Gate::OnReviewerApproval => "on_reviewer_approval", }, // `tests_verified` keeps its original tri-state meaning for existing // readers; `tests_status` is what distinguishes the two ways of being // null — a repo with no suite from a runtime that could not run one. "tests_verified": outcome.as_ref().and_then(TestOutcome::verified), "tests_status": outcome.as_ref().map(TestOutcome::status), "tests_detail": outcome.as_ref().and_then(TestOutcome::detail), "pushed": published.as_ref().map(|p| p.pushed), "push_error": published .as_ref() .and_then(|p| p.error.clone()) .or(publish_error), "commit_error": commit_error, "files_changed": files_changed, "insertions": insertions, "deletions": deletions, "empty": empty, // Non-null means `empty`/`files_changed` describe a failed read, not // an unchanged tree. Readers that treat `empty: true` as "the phase // did nothing" must check this first. "diff_error": diff_error, "truncated": truncated, // WHICH files, not just how many. Same base_sha and the same excludes // as `files_changed`, so the two describe the same diff. "files": files .iter() .map(|(st, path)| serde_json::json!({ "status": st.to_string(), "path": path })) .collect::>(), "files_truncated": files_truncated, "excluded_paths": EXCLUDED_PATHS, }); std::fs::write( dir.join("delivery.json"), serde_json::to_string_pretty(&meta).unwrap_or_default(), ) .map_err(|e| format!("write delivery.json: {e}"))?; // Path is stored relative to the MISSIONS ROOT — the convention every // artifact uses, and what `routes::missions::artifact_content` resolves // against. (The old `pdf_renderer` claimed to match this and did not: it // joined the mission id first, producing a doubled id and ENOENT. It is // gone; this comment named it as the authority, which it never was.) let rel = format!("_outputs/{mission_id}/{phase_id}/diff.patch"); cm_db::repo::missions::register_artifact( pool, cm_db::repo::missions::RegisterArtifact { mission_id, phase_id: Some(phase_id), path: &rel, kind: "code_diff", mime: Some("text/x-patch"), title: Some(if empty { "No code changes" } else { "Code changes" }), generated_by_run: None, render_pdf: false, metadata: Some(meta), }, ) .await .map_err(|e| format!("register code_diff artifact: {e}"))?; eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} → {} \ (+{insertions}/-{deletions} across {files_changed} file(s)){}", if empty { "no changes" } else { "captured" }, if truncated { ", truncated" } else { "" }, ); Ok(Some(Capture { committed, base_sha, files_changed, insertions, deletions, empty, diff_error, truncated, files, files_truncated, patch_path, })) } /// Run git in `repo`, returning stdout. /// /// Every invocation carries `-c safe.directory`: under `CLAWMATES_MISSION_FS=bind` /// the server clones as uid 65532 while agents write into the same tree as /// root, so without it git refuses the repository outright — the failure that /// had the phase evaluator silently falling back to guesswork. Copy mode makes /// the tree single-uid and this redundant, but it stays while the bind path is /// still selectable: a workaround may only be deleted once the situation it /// works around can no longer be chosen. async fn git(repo: &Path, args: &[&str]) -> Result { let repo_s = repo.display().to_string(); // Owned, not `Box::leak`. The leak was justified as "the process is // short-lived", which is true of a CLI and false of cm-api — it is a // long-running server, so that was one permanently leaked allocation per // git call, growing with every phase of every mission for the life of the // process. let mut full: Vec = vec![ "-C".into(), repo_s.clone(), "-c".into(), format!("safe.directory={repo_s}"), ]; full.extend(args.iter().map(|a| (*a).to_string())); let (name, email) = commit_identity(); let mut cmd = tokio::process::Command::new("git"); cmd.args(&full); let out = crate::mission_workspace::no_terminal_prompt(&mut cmd) // The server container has no git identity — `git config --global // user.email` exits 1 — so `git commit` fails with "Author identity // unknown" unless one is supplied. Mission `019fc450` lost its first // phase to exactly that. // // This was the third failure in a row whose trigger was *agent // behaviour rather than our code*: earlier runs committed only because // an agent had happened to run `git config user.email` in the // checkout, leaving a local identity the server then inherited. Config // the agent may or may not have written is not a dependency delivery // can hold, so the identity is supplied here on every call. // // Environment rather than `-c`, because these override config without // needing a leaked string per invocation, and because they name the // committer as the pipeline — which is the truth. The agents' own // commits keep whatever identity they set. .env("GIT_AUTHOR_NAME", &name) .env("GIT_AUTHOR_EMAIL", &email) .env("GIT_COMMITTER_NAME", &name) .env("GIT_COMMITTER_EMAIL", &email) .output() .await .map_err(|e| format!("spawn git: {e}"))?; if !out.status.success() { return Err(format!( "git {} → {}: {}", args.first().copied().unwrap_or("?"), out.status, // Both ends, never a head-only clamp. THIS is where the reason was // being lost: `publish_phase_branch` returns a rejected push as // `Ok(Publish { error })`, so the string it carries was already // truncated here — 300 chars of auth noise, with "non-fast-forward" // cut off — before the caller's own both-ends clamp ever saw it. // Clamping the caller fixed the path that was already fine. crate::mission_workspace::redact_token(&crate::evaluator_tools::clamp_output( &String::from_utf8_lossy(&out.stderr) )) )); } Ok(String::from_utf8_lossy(&out.stdout).into_owned()) } /// Pull `(files, insertions, deletions)` out of `git diff --stat`'s summary. /// /// The last line looks like /// `3 files changed, 12 insertions(+), 4 deletions(-)`, with any of the three /// clauses absent when its count is zero. pub fn parse_diffstat(stat: &str) -> (usize, usize, usize) { let Some(summary) = stat.lines().last() else { return (0, 0, 0); }; let mut files = 0; let mut ins = 0; let mut del = 0; for part in summary.split(',') { let part = part.trim(); let Some((count, rest)) = part.split_once(' ') else { continue; }; let Ok(n) = count.parse::() else { continue; }; if rest.starts_with("file") { files = n; } else if rest.starts_with("insertion") { ins = n; } else if rest.starts_with("deletion") { del = n; } } (files, ins, del) } /// Commit a phase's work onto a branch of its own. /// /// Runs after capture, never before: the patch is already on disk and /// registered, so a commit that goes wrong costs a branch and not the work. /// /// Three rules, none of them negotiable: /// /// - **Never the default branch.** The branch name is derived from the mission /// and phase, so a mission can only ever add a ref nobody else owns. /// - **Never force.** A rejected update is reported, not overwritten. /// - **Same exclusions as capture.** Whatever was too noisy to put in a patch /// is too noisy to put in someone's history — build output, vendored trees, /// and the workaround files agents write when infrastructure fights them. /// /// Returns `Ok(None)` when there is nothing to commit, which is a normal /// outcome and not an error: the phase may have changed nothing, or the agents /// may have committed their own work already. pub async fn commit_phase_work( repo: &Path, mission_id: Uuid, phase_id: Uuid, iteration: i32, ) -> Result, String> { let branch = branch_name(mission_id, phase_id, iteration); // Work already committed by the agents still needs a branch pointing at // it, or it is unreachable once the checkout is reaped. So the branch is // created regardless, and only the staging step is conditional. git(repo, &["checkout", "-B", &branch]).await?; let mut add: Vec<&str> = vec!["add", "--", "."]; let excludes: Vec = EXCLUDED_PATHS .iter() .map(|p| format!(":(exclude){p}")) .collect(); add.extend(excludes.iter().map(String::as_str)); git(repo, &add).await?; // `--cached` compares the index against HEAD: empty means the agents left // nothing unstaged for us, which is the normal case when they committed // themselves. let staged = git(repo, &["diff", "--cached", "--stat"]) .await .unwrap_or_default(); if !staged.trim().is_empty() { let message = format!( // The trailer is the provenance record. It matters more now that // the author line carries a person's name: without it, autonomous // work would be indistinguishable from hand-written commits in // `git log`. Keep it on any change to this message. "clawmates: phase work{}\n\ \n\ Mission: {mission_id}\n\ Phase: {phase_id}\n\ \n\ Committed by the ClawMates delivery pipeline from the agents' \ working tree. Authored by agents, not by the named committer.", if iteration > 0 { format!(" (pass {})", iteration + 1) } else { String::new() } ); clear_stale_commit_editmsg(repo); git(repo, &["commit", "--no-verify", "-m", &message]).await?; } let sha = git(repo, &["rev-parse", "HEAD"]) .await .map(|s| s.trim().to_string()) .unwrap_or_default(); if sha.is_empty() { return Ok(None); } eprintln!( "mission_delivery: mission {mission_id} phase {phase_id} → branch {branch} at {}", &sha[..sha.len().min(8)] ); Ok(Some(Commit { branch, sha })) } /// The branch a phase's work lands on. /// /// Namespaced under `clawmates/` so it is obvious in a branch list who created /// it and safe to delete in bulk. /// /// The two segments are taken from opposite ends of the ids, and that is /// load-bearing. Both are UUIDv7, which leads with a 48-bit timestamp, so ids /// minted in the same millisecond share their leading hex — taking `[..8]` of /// each produced `clawmates/mission-019fc40e-019fc40e` in production, the same /// branch for every phase of the mission, each one silently moving the ref the /// last phase had just set. The mission keeps its time-ordered prefix so /// branches group and sort usefully; the phase contributes its random tail so /// sibling phases cannot collide. pub fn branch_name(mission_id: Uuid, phase_id: Uuid, iteration: i32) -> String { let m = mission_id.simple().to_string(); let p = phase_id.simple().to_string(); let base = format!("clawmates/mission-{}-{}", &m[..8], &p[p.len() - 8..]); if iteration > 0 { format!("{base}-i{}", iteration + 1) } else { base } } /// What a phase's `commit_policy` requires before its branch may be published. /// /// Declared in `templates/workflows/*.toml` and merged into `mission_phases. /// config`. Until now it had no reader at all — three recipes have been /// carrying `commit_policy = "on_green_tests"` that did precisely nothing. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Gate { /// Publish unconditionally. Always, /// Publish to the mission branch only if the project's own tests pass. OnGreenTests, /// Publish to a review branch and wait for a human. OnReviewerApproval, } impl Gate { pub fn parse(policy: Option<&str>) -> Gate { match policy.map(str::trim) { Some("on_green_tests") => Gate::OnGreenTests, Some("on_reviewer_approval") => Gate::OnReviewerApproval, Some("always") | None | Some("") => Gate::Always, Some(other) => { eprintln!( "mission_delivery: unknown commit_policy {other:?} — treating as `always`" ); Gate::Always } } } /// The branch suffix that carries the verdict to a human. /// /// A failed gate never discards work — it changes where the work lands. /// Deleting a red-test branch is how you get back to the old behaviour /// (work destroyed) with extra steps; a `-wip` branch is a thing someone /// can look at, fix, and push properly. pub fn branch_suffix(self, verified: Option) -> &'static str { match (self, verified) { (Gate::Always, _) => "", (Gate::OnGreenTests, Some(true)) => "", // Red, unrunnable, or no test command found — all "not proven". (Gate::OnGreenTests, _) => "-wip", (Gate::OnReviewerApproval, _) => "-review", } } } /// The command that runs a project's own tests, inferred from what is in the /// tree. /// /// Returns `None` when nothing recognisable is present, which /// [`Gate::branch_suffix`] treats as unproven rather than as passing — /// "we could not check" must never read as "it is fine". pub fn discover_test_command(repo: &Path) -> Option> { let has = |f: &str| repo.join(f).exists(); if has("Cargo.toml") { return Some(vec!["cargo".into(), "test".into(), "--quiet".into()]); } if has("package.json") { let pkg = std::fs::read_to_string(repo.join("package.json")).unwrap_or_default(); // Only claim a test command when the project actually declares one; // `npm test` on a package without a test script exits non-zero and // would read as a red suite rather than as "nothing to run". if pkg.contains("\"test\"") { return Some(vec!["npm".into(), "test".into(), "--silent".into()]); } return None; } if has("pyproject.toml") || has("pytest.ini") || repo.join("tests").is_dir() { return Some(vec!["pytest".into(), "-q".into()]); } None } /// The authenticated URL to push this mission's work to. /// /// Built fresh from the repo row and the ambient token rather than read from /// `.git/config`, which no longer carries credentials — the token is scrubbed /// after clone because agents run as root in a container that mounts the /// checkout. Building it here also means a rotated token takes effect /// immediately instead of at the next clone. async fn push_url_for(pool: &sqlx::PgPool, mission_id: Uuid) -> Result, String> { let url: Option = sqlx::query_scalar( "SELECT r.clone_url FROM missions m JOIN repos r ON r.id = m.repo_id WHERE m.id = $1", ) .bind(mission_id) .fetch_optional(pool) .await // `.ok().flatten()` used to collapse a failed query into the same `None` // as a mission with no repo bound, so a database fault was recorded as // "nothing to push to" — the shape that made `commit_error` necessary. .map_err(|e| format!("query push URL: {e}"))? .flatten(); let Some(url) = url else { return Ok(None) }; let auth = mission_workspace::with_ambient_auth(&url); // Fail here, not at the tty. An unauthenticated URL to OUR forge cannot // push, and every second it survives past this point is spent producing a // symptom that looks like something else: git asking for a username, then // `/dev/tty: No such device or address`, then a `push_error` about auth that // sent #55's investigation after credentials which were never the problem. // A third-party host is left alone — ssh keys and .netrc are legitimate. if let Some(why) = &auth.unauthenticated { if auth.is_forge() { return Err(format!( "cannot authenticate the push URL for this mission — {why}. The work \ is committed locally; fix the credential and re-run delivery." )); } eprintln!("mission_delivery: pushing to a non-forge remote unauthenticated — {why}"); } Ok(Some(auth.url)) } /// Did the forge reject this push because our history diverged from the ref? /// /// Git says this several ways depending on version and refspec, and all of them /// mean the same thing here: the branch already exists with commits ours does not /// contain. fn is_non_fast_forward(err: &str) -> bool { let e = err.to_ascii_lowercase(); e.contains("non-fast-forward") || e.contains("fetch first") || e.contains("updates were rejected") || (e.contains("[rejected]") && !e.contains("stale info")) } /// Run the gate, then push the branch if the gate allows it. /// /// Publishing is last on purpose. By the time this runs the patch is on disk, /// the artifact is registered and the work is committed to a local branch — so /// every failure mode here costs a ref that did not reach the forge, and /// nothing that was already captured. /// /// The branch name carries the verdict. A gate that fails redirects to /// `-wip` or `-review` and pushes it anyway: a human can /// inspect, fix and re-push a branch, but cannot recover work that was thrown /// away for failing a test. Deleting a red branch reproduces the old /// behaviour — work destroyed — deliberately rather than by accident. pub async fn publish_phase_branch( repo: &Path, push_url: &str, branch: &str, gate: Gate, verified: Option, ) -> Result { let suffix = gate.branch_suffix(verified); let target = format!("{branch}{suffix}"); if !suffix.is_empty() { // Move the local ref too, so the checkout and the forge agree about // where this work lives. git(repo, &["branch", "-f", &target, "HEAD"]).await?; } // Never force. A rejected update is reported and left alone: the remote // ref belongs to whoever set it, and overwriting it to make delivery look // tidy is how a mission eats someone else's commit. let refspec = format!("HEAD:refs/heads/{target}"); match git(repo, &["push", push_url, &refspec]).await { Ok(_) => { eprintln!("mission_delivery: pushed {target}"); Ok(Publish { branch: target, pushed: true, error: None, }) } // #55: a mission whose checkout was re-cloned — a retry, a container // teardown, disk loss — builds divergent history against its OWN // deterministic branch, and every push it ever attempts is rejected. // Before this, that was terminal: the work stayed on a local branch in a // directory that gets reaped. // // The escape is a NEW ref, not `--force`. Forcing would overwrite // whatever the earlier attempt pushed — which may be the only copy of // that work — to make this attempt look tidy. Suffixing with the commit // sha is deterministic (the same history always lands on the same ref), // self-describing in a branch list, and cannot collide, since divergent // history is by definition a different sha. Err(e) if is_non_fast_forward(&e) => { let sha = git(repo, &["rev-parse", "HEAD"]) .await .map(|s| s.trim().to_string()) .unwrap_or_default(); let Some(short) = sha.get(..8) else { eprintln!("mission_delivery: push of {target} rejected and HEAD unreadable: {e}"); return Ok(Publish { branch: target, pushed: false, error: Some(e), }); }; let alt = format!("{target}-{short}"); eprintln!( "mission_delivery: {target} exists on the forge with history this \ checkout does not contain — pushing to {alt} instead of forcing. \ Original: {e}" ); git(repo, &["branch", "-f", &alt, "HEAD"]).await?; match git(repo, &["push", push_url, &format!("HEAD:refs/heads/{alt}")]).await { Ok(_) => Ok(Publish { branch: alt, pushed: true, // Not an error — the work reached the forge — but the // redirect is a fact the operator needs, or two branches for // one phase look like a bug rather than a rescue. error: None, }), Err(e2) => Ok(Publish { branch: alt, pushed: false, error: Some(format!("{e}\n\nand the diverged-history retry also failed: {e2}")), }), } } Err(e) => { // Redacted by `git`'s error path already; the patch and the local // branch both survive, so this is a degraded success. eprintln!("mission_delivery: push of {target} failed: {e}"); Ok(Publish { branch: target, pushed: false, error: Some(e), }) } } } /// Where a phase's work ended up, and whether the forge has it. #[derive(Debug, Clone)] pub struct Publish { pub branch: String, pub pushed: bool, pub error: Option, } /// Run the project's own tests to decide whether a green-tests gate is met. /// /// `None` means "could not establish", which the gate treats as unproven. That /// is the same fail-closed stance the phase evaluator takes, and for the same /// reason: this codebase has repeatedly found things reporting success while /// doing nothing, and a test suite that never ran must not license a push to a /// mission branch. pub async fn verify_tests(repo: &Path, container: &str) -> TestOutcome { let Some(argv) = discover_test_command(repo) else { return TestOutcome::NoSuite; }; let workdir = repo.display().to_string(); let docker = match crate::container_exec::connect() { Ok(d) => d, Err(e) => return TestOutcome::CouldNotRun(format!("docker unreachable: {e}")), }; match crate::container_exec::exec(&docker, container, Some(&workdir), &argv, TEST_TIMEOUT).await { Ok(out) => { eprintln!( "mission_delivery: {} → exit {:?}", argv.join(" "), out.exit_code ); match out.exit_code { Some(0) => TestOutcome::Passed, // An unreadable status is not a pass, and it is not a red // suite either — the command may never have started. None => TestOutcome::CouldNotRun(format!( "`{}` produced no exit status: {}", argv.join(" "), out.combined().chars().take(300).collect::() )), Some(code) => TestOutcome::Failed(code), } } Err(e) => TestOutcome::CouldNotRun(format!("exec in `{container}` failed: {e}")), } } /// What happened when the gate tried to verify a phase. /// /// This was `Option`, and collapsing four outcomes into `None` is what /// let a missing toolchain hide for days. `clawmates-runtime` shipped without /// `cargo`, so `verify_tests` returned `None` on every mission — identical to /// the reading for "this repository has no test suite", which is what I /// concluded at the time and stated in a summary. The gate behaved correctly /// throughout (unproven is not a pass); it simply could not say *why* it was /// unproven, so nobody could tell a repo without tests from a runtime without /// a test runner. /// /// Only `Passed` clears the gate. The rest differ in what an operator should /// do about them, which is the entire reason they are separate variants. #[derive(Debug, Clone, PartialEq, Eq)] pub enum TestOutcome { /// The suite ran and passed. Passed, /// The suite ran and failed, with its exit code. Failed(i64), /// No test command could be discovered for this repository. NoSuite, /// A suite exists but could not be executed. Always an infrastructure /// fault on our side, never a verdict about the code. CouldNotRun(String), } impl TestOutcome { /// The gate's view: `Some(true)` only when the suite actually passed. /// Preserved so `Gate::branch_suffix` keeps its existing contract. pub fn verified(&self) -> Option { match self { TestOutcome::Passed => Some(true), TestOutcome::Failed(_) => Some(false), TestOutcome::NoSuite | TestOutcome::CouldNotRun(_) => None, } } /// Stable machine-readable label for artifact metadata. pub fn status(&self) -> &'static str { match self { TestOutcome::Passed => "passed", TestOutcome::Failed(_) => "failed", TestOutcome::NoSuite => "no_suite", TestOutcome::CouldNotRun(_) => "could_not_run", } } /// Human-readable detail, when there is any beyond the label. pub fn detail(&self) -> Option { match self { TestOutcome::Passed | TestOutcome::NoSuite => None, TestOutcome::Failed(code) => Some(format!("test command exited {code}")), TestOutcome::CouldNotRun(why) => Some(why.clone()), } } } /// Remove a `COMMIT_EDITMSG` the agent left behind as root. /// /// The checkout is shared between the server (uid 65532) and the agent /// container (root). `core.sharedRepository` makes git create *objects and /// refs* group-writable — `.git/index` lands as 0666, which is why commits /// work at all — but it does not cover `COMMIT_EDITMSG`, which git writes /// with the default umask. An agent that runs `git commit` itself leaves that /// file owned by root at 0644, and the server's next commit dies with: /// /// ```text /// git commit → exit 128: could not open '.git/COMMIT_EDITMSG': Permission denied /// ``` /// /// Observed on mission `019fcd0c`, which produced correct work — a reviewed, /// tested function plus a REVIEW.md quoting a real `cargo test` summary — and /// then delivered none of it. /// /// Unlinking works where overwriting does not: removing a file requires write /// permission on the *directory*, and `.git/` is owned by the server. Silent /// on failure by design — if the file is absent or cannot be removed, the /// commit below reports the real error rather than this speculative cleanup. fn clear_stale_commit_editmsg(repo: &Path) { let msg = repo.join(".git/COMMIT_EDITMSG"); if msg.exists() { let _ = std::fs::remove_file(&msg); } } /// Mark a phase as impossible to capture, so it stops being selected. /// /// A phase whose checkout has already been reaped can never be captured. It /// must still be recorded: the capture batch is bounded, and a row that stays /// eligible forever occupies a slot forever. Enough of them and no live /// mission is ever captured again — head-of-line blocking with a silent /// failure mode, which is how this was found. /// /// The artifact is deliberately honest about *why* it is empty. "No changes" /// and "we lost the checkout before looking" are different facts, and an /// operator reading the mission needs to be able to tell them apart. pub async fn record_uncapturable( pool: &sqlx::PgPool, mission_id: Uuid, phase_id: Uuid, ) -> Result<(), String> { // Write a real file behind the artifact. `_outputs` survives teardown, so // it is still writable even though the checkout is gone — and an artifact // row pointing at a path with nothing behind it turns every reader into a // 404 with no explanation. let dir = outputs_root(mission_id).join(phase_id.to_string()); if std::fs::create_dir_all(&dir).is_ok() { let _ = std::fs::write( dir.join("diff.patch"), "The mission checkout was removed before this phase's changes could be\n captured. Nothing was lost that had already been captured; this phase\n simply finished after its working tree had been reaped.\n", ); } let rel = format!("_outputs/{mission_id}/{phase_id}/diff.patch"); cm_db::repo::missions::register_artifact( pool, cm_db::repo::missions::RegisterArtifact { mission_id, phase_id: Some(phase_id), path: &rel, kind: "code_diff", mime: Some("text/x-patch"), title: Some("Not captured — checkout unavailable"), generated_by_run: None, render_pdf: false, metadata: Some(json!({ "empty": true, "captured": false, "reason": "the mission checkout was removed before the diff could be captured", })), }, ) .await .map(|_| ()) .map_err(|e| format!("register uncapturable marker: {e}")) } /// Why an empty patch must not be believed, if it must not be believed. /// /// An empty patch has two causes that produce identical bytes: the tree really /// did not change, or `git diff` failed and we have no idea what the tree /// looks like. The first is an ordinary outcome; the second is a platform /// fault. Returning `Some` for the second is what stops the fault from being /// filed under the ordinary outcome — the recurring shape where a failure and /// a legitimate negative share one representation. fn untrusted_empty_reason(empty: bool, diff_error: Option<&str>) -> Option { match (empty, diff_error) { (true, Some(why)) => Some(format!( "not published: the diff could not be computed, so an empty patch \ cannot be trusted to mean an unchanged tree ({why})" )), _ => None, } } #[cfg(test)] mod changed_path_capture_tests { /// The path list and `files_changed` must describe the SAME diff. /// /// They come from two separate git invocations — `--stat` and /// `--name-status`. If those are ever given different revisions or /// different exclude pathspecs, the count and the list disagree and there /// is no way to tell which is right: both look like plausible output. #[test] fn both_diff_calls_use_the_same_revision_and_excludes() { let src = include_str!("mission_delivery.rs"); let body = src .split("let mut stat_args") .nth(1) .and_then(|s| s.split("let (files_changed").next()) .expect("the capture block"); assert!( body.contains("let mut name_args = vec![\"diff\", base_sha.as_str(), \"--name-status\", \"--\"]"), "the name-status call must use the same base_sha as --stat" ); assert!( body.contains("name_args.extend(excludes.iter().map(String::as_str))"), "and the same excludes, or files_changed and the path list describe \ different diffs" ); } /// `--name-status` must run before the index is put back, or newly created /// files — which `--intent-to-add` is what makes visible — vanish from the /// list while still being counted by the stat. #[test] fn paths_are_read_before_the_index_reset() { let src = include_str!("mission_delivery.rs"); let name_at = src.find("--name-status").expect("name-status call"); let reset_at = src .find("git(&repo, &[\"reset\", \"--quiet\"])") .expect("index reset"); assert!( name_at < reset_at, "the path list must be captured while --intent-to-add is still in \ effect, or created files are invisible to it" ); } } #[cfg(test)] mod tests { use super::*; /// Git says "your history diverged" several ways, and the one production /// actually produced (`! [rejected] ... (fetch first)`) is not the phrase /// anyone reaches for first. Missing a phrasing means the rescue does not /// fire and the work stays on a local branch in a directory that gets /// reaped — silently, since the push failure is a degraded success. #[test] fn every_way_git_says_diverged_is_recognised() { for e in [ "git push → exit 1: ! [rejected] HEAD -> b (fetch first)\nhint: …", " ! [rejected] HEAD -> b (non-fast-forward)", "hint: Updates were rejected because the remote contains work that you \ do not have locally.", ] { assert!(is_non_fast_forward(e), "not recognised: {e}"); } } /// And it must not fire on failures a new branch cannot fix. Retrying a /// permissions or network error onto a second ref just produces a second /// failure and a confusing branch name. #[test] fn other_push_failures_are_not_mistaken_for_divergence() { for e in [ "fatal: repository 'https://forge/x.git' not found", "remote: error: GH006: Protected branch update failed", "fatal: could not read Username for 'https://forge': terminal prompts disabled", " ! [rejected] (stale info)", ] { assert!(!is_non_fast_forward(e), "wrongly recognised: {e}"); } } // ── The gate ─────────────────────────────────────────────────────── /// Three recipes have declared `commit_policy` since they were written and /// nothing has ever read it. The parse must at least be forgiving about an /// unknown value rather than refusing to deliver. #[test] fn commit_policy_parses_the_declared_values() { assert_eq!(Gate::parse(Some("on_green_tests")), Gate::OnGreenTests); assert_eq!( Gate::parse(Some("on_reviewer_approval")), Gate::OnReviewerApproval ); assert_eq!(Gate::parse(Some("always")), Gate::Always); assert_eq!(Gate::parse(None), Gate::Always); assert_eq!(Gate::parse(Some(" on_green_tests ")), Gate::OnGreenTests); assert_eq!( Gate::parse(Some("nonsense")), Gate::Always, "unknown policy still delivers" ); } /// A failed gate must move the work, never drop it. Deleting a red-test /// branch reproduces the old behaviour — work destroyed — with extra steps. #[test] fn a_failed_gate_redirects_rather_than_discards() { assert_eq!(Gate::OnGreenTests.branch_suffix(Some(true)), ""); assert_eq!(Gate::OnGreenTests.branch_suffix(Some(false)), "-wip"); assert_eq!( Gate::OnReviewerApproval.branch_suffix(Some(true)), "-review" ); assert_eq!( Gate::Always.branch_suffix(Some(false)), "", "always means always" ); } /// "We could not check" must not read as "it passed". An unrunnable or /// undiscoverable test suite lands on `-wip` exactly like a red one. #[test] fn an_unverifiable_suite_is_not_treated_as_green() { assert_eq!(Gate::OnGreenTests.branch_suffix(None), "-wip"); } #[test] fn test_command_is_discovered_from_the_tree() { let dir = tempfile::tempdir().unwrap(); assert_eq!( discover_test_command(dir.path()), None, "nothing recognisable" ); std::fs::write(dir.path().join("Cargo.toml"), "[package]\nname=\"x\"\n").unwrap(); assert_eq!( discover_test_command(dir.path()), Some(vec!["cargo".into(), "test".into(), "--quiet".into()]) ); } /// A `package.json` with no test script must yield None, not `npm test` — /// npm exits non-zero for a missing script, which would look like a red /// suite instead of an absent one. #[test] fn a_package_without_a_test_script_yields_no_command() { let dir = tempfile::tempdir().unwrap(); std::fs::write(dir.path().join("package.json"), r#"{"name":"x"}"#).unwrap(); assert_eq!(discover_test_command(dir.path()), None); std::fs::write( dir.path().join("package.json"), r#"{"name":"x","scripts":{"test":"vitest"}}"#, ) .unwrap(); assert!(discover_test_command(dir.path()).is_some()); } #[test] fn diffstat_summary_is_parsed() { assert_eq!( parse_diffstat(" src/a.rs | 2 +-\n 3 files changed, 12 insertions(+), 4 deletions(-)"), (3, 12, 4) ); // Clauses are omitted when zero. assert_eq!( parse_diffstat(" a.rs | 1 +\n 1 file changed, 1 insertion(+)"), (1, 1, 0) ); assert_eq!( parse_diffstat(" a.rs | 1 -\n 1 file changed, 1 deletion(-)"), (1, 0, 1) ); } /// An empty stat means an empty phase, not a parse failure. This is the /// case that must still produce an artifact. /// The whole point: a tree that genuinely did not change stays silent, and /// a diff that could not be computed does not get to borrow that silence. #[test] fn an_uncomputable_diff_is_not_an_unchanged_tree() { assert_eq!( untrusted_empty_reason(true, None), None, "a genuinely unchanged tree must not report an error" ); let reason = untrusted_empty_reason(true, Some("patch: fatal: bad object")) .expect("an empty patch from a FAILED diff must be reported, not accepted"); assert!( reason.contains("bad object"), "the reason must name what went wrong, got: {reason}" ); assert_eq!( untrusted_empty_reason(false, Some("diffstat: fatal: bad object")), None, "a non-empty patch stands on its own even if the diffstat failed" ); } #[test] fn an_empty_diffstat_is_all_zeroes() { assert_eq!(parse_diffstat(""), (0, 0, 0)); assert_eq!(parse_diffstat("\n"), (0, 0, 0)); } /// Build output and agent workaround debris must never reach a patch. A /// phase that ran `cargo build` leaves a `target/` bigger than the /// repository, and an agent that fought git's ownership check left a /// `.gitconfig_temp` beside the real work. #[test] fn build_output_is_excluded() { for p in [ "target", "node_modules", ".venv", "dist", "__pycache__", ".gitconfig_temp", ] { assert!( EXCLUDED_PATHS.contains(&p), "{p} must be excluded from capture" ); } } /// The outputs directory must be a sibling of the per-mission directories /// the sweeper deletes — not inside one, or teardown takes the record with /// it, and not inside a bind mount, or agents can edit their own evidence. #[test] fn outputs_live_outside_the_swept_mission_directory() { let mission = Uuid::now_v7(); let out = outputs_root(mission); let swept = mission_workspace::checkout_path(mission); assert!( !out.starts_with(swept.parent().unwrap()), "outputs must not sit under the directory teardown removes" ); assert!(out.to_string_lossy().contains("_outputs")); } }