Omar SobhandClaude Opus 5
53da4d7e6d
`clawmates-runtime` shipped with `gcc` and `make` but no `cmake`, no `g++` and no `python3-dev`. Measured on clawhdf5, three probes: no cmake → "is `cmake` not installed?" exit 101 after 13s no python3-dev → "cannot find -lpython3.11" exit 101 at link with both → cargo test PASSES exit 0 after 69s This is not only the delivery gate. The AGENTS run in this image, so a coding phase was writing Rust it had no way to compile or test — which reframes the last run's 11 agent commits as unverifiable by construction. `images/agent-toolchain/Dockerfile` (the microVM path) has had `cmake build-essential` all along, and its own header warns about precisely this: "if `cargo` is present in one image and absent in another, the same mission passes or fails depending on which backend it landed on, and nothing says why." Both images now install the same set — it was missing `python3-dev` too. `images/runtime-toolchain.Dockerfile` is a thin local overlay so the laptop can run today without recompiling zeroclaw from the fork; it is meant to be deleted once a runtime image built from the corrected deploy/ Dockerfile is published. Also: a build failure is no longer reported as a red suite. Both are cargo exit 101, and `verify_tests` mapped every non-zero to `Failed(code)` — so a missing toolchain was recorded as the USER's tests failing. It now returns `CouldNotRun` with the reason when the output shows a compile or link failure. Deliberately narrow: a failing `assert!` still reads as red, because letting broken code past `on_green_tests` is the expensive direction to be wrong in. Both directions are pinned by tests built from today's two real samples. And the coding phase finally has a loop: `research_and_code.toml` declared `loop = "until_no_more_int_items"`, which `phase_config.rs` lists as DECLARED_BUT_UNREAD. Iteration is driven by `max_iterations` + `done_when`, and with `max_iterations = 1` and no `done_when` the phase ran ONCE and was never judged — reporting `completed` whatever it produced. Now 3 passes against a stated goal, wording per the measured rule (say what the tree must CONTAIN). Co-Authored-By: Claude Opus 5 <[email protected]>
1610 lines
69 KiB
Rust
1610 lines
69 KiB
Rust
//! 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 `<missions_root>/_outputs/<mission>/<phase>/`, a *sibling*
|
|
//! of the per-mission directories the sweeper deletes, and outside every bind
|
|
//! mount handed to a container (`ensure_container` mounts only
|
|
//! `<root>/<mission_id>`). 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 = "[email protected]";
|
|
|
|
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<String>,
|
|
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<Commit>,
|
|
}
|
|
|
|
/// 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<Option<Capture>, 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<String> =
|
|
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<Option<Capture>, 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<String> = 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<String> = 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::<String>()));
|
|
}
|
|
};
|
|
|
|
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<String> = 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<TestOutcome> = None;
|
|
let mut published: Option<Publish> = None;
|
|
let mut publish_error: Option<String> = 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::<Vec<_>>(),
|
|
"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<String, String> {
|
|
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<String> = 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::<usize>() 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<Option<Commit>, 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<String> = EXCLUDED_PATHS
|
|
.iter()
|
|
.map(|p| format!(":(exclude){p}"))
|
|
.collect();
|
|
add.extend(excludes.iter().map(String::as_str));
|
|
// `git add` EXITS 1 whenever the pathspec walked over a gitignored path,
|
|
// even though it staged everything else correctly and even though we
|
|
// excluded that path ourselves. Measured against git 2.x: `-c
|
|
// advice.addIgnoredFile=false`, `--ignore-errors`, `-A` and `:/` all still
|
|
// exit 1, and all still stage the right files. There is no flag that makes
|
|
// this command's exit code mean "nothing was staged".
|
|
//
|
|
// Propagating it with `?` therefore aborted the commit AFTER a successful
|
|
// staging, so the branch was never made, nothing was committed and nothing
|
|
// was pushed — for any repo with a populated `target/`, which is every Rust
|
|
// repo an agent has built in. `capture_phase_diff_at` above already treats
|
|
// the same command as advisory (`let _ = ...`); this is the same command
|
|
// and gets the same policy. The index is the source of truth, and the
|
|
// `diff --cached` immediately below is what actually reads it.
|
|
if let Err(e) = git(repo, &add).await {
|
|
eprintln!(
|
|
"mission_delivery: `git add` reported {e} — continuing, the staged \
|
|
index below is what decides whether there is anything to commit"
|
|
);
|
|
}
|
|
|
|
// `--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<bool>) -> &'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<Vec<String>> {
|
|
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<Option<String>, String> {
|
|
let url: Option<String> = 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
|
|
/// `<branch>-wip` or `<branch>-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<bool>,
|
|
) -> Result<Publish, String> {
|
|
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<String>,
|
|
}
|
|
|
|
/// 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.
|
|
/// Did the toolchain fail to BUILD the project, as opposed to building it and
|
|
/// finding failing tests?
|
|
///
|
|
/// Deliberately narrow. These three phrases are emitted by cargo/rustc only
|
|
/// when compilation or linking did not complete; a failing `assert!` produces
|
|
/// none of them. Anything not matched here stays a red suite, because guessing
|
|
/// "probably an environment problem" over a genuine test failure is the far
|
|
/// more expensive mistake — it would let broken code through the gate.
|
|
fn build_failed(output: &str) -> bool {
|
|
let o = output.to_ascii_lowercase();
|
|
o.contains("error: could not compile")
|
|
|| o.contains("error: linking with")
|
|
|| o.contains("error: failed to run custom build command")
|
|
}
|
|
|
|
/// The first line that explains a build failure, for the artifact.
|
|
fn build_failure_excerpt(output: &str) -> String {
|
|
output
|
|
.lines()
|
|
.find(|l| {
|
|
let l = l.to_ascii_lowercase();
|
|
l.contains("error: could not compile")
|
|
|| l.contains("error: linking with")
|
|
|| l.contains("error: failed to run custom build command")
|
|
|| l.contains("cannot find -l")
|
|
|| l.contains("not installed")
|
|
})
|
|
.unwrap_or("")
|
|
.chars()
|
|
.take(300)
|
|
.collect()
|
|
}
|
|
|
|
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
|
|
);
|
|
let text = out.combined();
|
|
match out.exit_code {
|
|
Some(0) => TestOutcome::Passed,
|
|
// A suite that never COMPILED is not a red suite. Both are
|
|
// non-zero (cargo exits 101 either way), and calling the
|
|
// difference is what stops a missing toolchain being reported
|
|
// as the user's code being broken.
|
|
//
|
|
// Measured twice on clawhdf5 in one sitting: no `cmake` gave
|
|
// "is `cmake` not installed?", and no `python3-dev` gave
|
|
// "cannot find -lpython3.11" — both exit 101, both would have
|
|
// been recorded as `tests_status: "failed"` on a repo whose
|
|
// tests were never run. The branch suffix is `-wip` either way,
|
|
// so nothing ships differently; what changes is that the
|
|
// artifact now says which of the two happened.
|
|
Some(_) if build_failed(&text) => TestOutcome::CouldNotRun(format!(
|
|
"`{}` could not build the project: {}",
|
|
argv.join(" "),
|
|
build_failure_excerpt(&text)
|
|
)),
|
|
// 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(" "),
|
|
text.chars().take(300).collect::<String>()
|
|
)),
|
|
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<bool>`, 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<bool> {
|
|
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<String> {
|
|
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<String> {
|
|
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");
|
|
}
|
|
|
|
/// The regression that destroyed a mission's work.
|
|
///
|
|
/// `git add -- . :(exclude)target` EXITS NON-ZERO when the tree contains a
|
|
/// gitignored `target/`, while correctly staging everything else. The
|
|
/// commit path used to propagate that exit with `?`, so a successful
|
|
/// staging still aborted the commit — no branch, no commit, no push — and
|
|
/// the agents' work was reaped with the container.
|
|
///
|
|
/// This test asserts the git behaviour itself, because the fix is only
|
|
/// correct for as long as the behaviour holds: if a future git makes this
|
|
/// command exit 0, this test fails and tells the next reader the workaround
|
|
/// can go. What must never regress is the second assertion — that the files
|
|
/// ARE staged regardless of the exit code, which is why the index and not
|
|
/// the exit code is the source of truth.
|
|
#[tokio::test]
|
|
async fn git_add_exits_nonzero_over_an_ignored_path_yet_still_stages() {
|
|
let dir = tempfile::tempdir().unwrap();
|
|
let repo = dir.path();
|
|
let run = |args: Vec<&str>| {
|
|
std::process::Command::new("git")
|
|
.args(&args)
|
|
.current_dir(repo)
|
|
.output()
|
|
.expect("git runs")
|
|
};
|
|
run(vec!["init", "-q", "."]);
|
|
run(vec!["config", "user.email", "[email protected]"]);
|
|
run(vec!["config", "user.name", "T"]);
|
|
std::fs::write(repo.join(".gitignore"), "target\n").unwrap();
|
|
run(vec!["add", ".gitignore"]);
|
|
run(vec!["commit", "-qm", "base"]);
|
|
|
|
// The shape every Rust repo an agent has built in ends up with.
|
|
std::fs::create_dir_all(repo.join("target")).unwrap();
|
|
std::fs::write(repo.join("target/build.bin"), "junk").unwrap();
|
|
std::fs::create_dir_all(repo.join("research")).unwrap();
|
|
std::fs::write(repo.join("research/summary.md"), "findings").unwrap();
|
|
|
|
let mut add: Vec<&str> = vec!["add", "--", "."];
|
|
let excludes: Vec<String> = EXCLUDED_PATHS
|
|
.iter()
|
|
.map(|p| format!(":(exclude){p}"))
|
|
.collect();
|
|
add.extend(excludes.iter().map(String::as_str));
|
|
let out = run(add);
|
|
|
|
assert!(
|
|
!out.status.success(),
|
|
"if this now succeeds, git changed and the advisory handling in \
|
|
commit_and_branch can be simplified"
|
|
);
|
|
|
|
let staged = run(vec!["diff", "--cached", "--name-only"]);
|
|
let staged = String::from_utf8_lossy(&staged.stdout);
|
|
assert!(
|
|
staged.contains("research/summary.md"),
|
|
"the work MUST be staged despite the non-zero exit; this is the \
|
|
assertion that stops a phase's output being silently discarded \
|
|
again. staged: {staged:?}"
|
|
);
|
|
assert!(
|
|
!staged.contains("target/"),
|
|
"the exclude pathspec must still keep build output out: {staged:?}"
|
|
);
|
|
}
|
|
|
|
/// A missing toolchain must not be reported as the user's tests failing.
|
|
/// Both are cargo exit 101; only the output distinguishes them, and this
|
|
/// session produced both real samples on clawhdf5.
|
|
#[test]
|
|
fn a_build_failure_is_not_a_red_suite() {
|
|
let no_cmake = "error: failed to run custom build command for `libz-ng-sys v1.1.29`\n\
|
|
is `cmake` not installed?";
|
|
let no_python = "= note: /usr/bin/ld: cannot find -lpython3.11: No such file or directory\n\
|
|
error: could not compile `clawhdf5-py` (lib) due to 1 previous error";
|
|
for sample in [no_cmake, no_python] {
|
|
assert!(build_failed(sample), "must read as a build failure: {sample}");
|
|
assert!(
|
|
!build_failure_excerpt(sample).is_empty(),
|
|
"the artifact needs a reason, not an empty string"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// The half that protects the gate: a genuinely failing test must STAY a
|
|
/// red suite. Mistaking one for an environment problem would let broken
|
|
/// code past `on_green_tests`, which is the expensive direction to be
|
|
/// wrong in.
|
|
#[test]
|
|
fn a_failing_test_is_still_a_red_suite() {
|
|
let red = "running 3 tests\n\
|
|
test math::adds ... FAILED\n\
|
|
failures:\n math::adds\n\
|
|
test result: FAILED. 2 passed; 1 failed; 0 ignored";
|
|
assert!(!build_failed(red), "a failing assertion is not a build failure");
|
|
}
|
|
|
|
#[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"));
|
|
}
|
|
}
|