osobh/clawmates
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(missions): capture a coding phase's diff to durable storage

Browse Source 
First half of mission delivery: the work is captured before anything is
published. A coding mission has until now produced nothing durable — the
checkout is deleted thirty minutes after completion and `register_artifact`
had no callers at all, so the only surviving output was an LLM narrative of
what the agents said they did.

`capture_phase_diff` writes `diff.patch`, `diffstat.txt` and `delivery.json`
under `<missions_root>/_outputs/<mission>/<phase>/` and registers a
`code_diff` artifact. That directory is a *sibling* of the per-mission
directories the sweeper removes, and outside every bind mount handed to a
container — so teardown cannot take the record with it and agents cannot edit
their own evidence.

Three details that decide whether this works at all:

- `git add --intent-to-add` before diffing. Untracked files are invisible to
  `git diff`, and a phase that only *creates* files is the likeliest shape for
  generated code — silently capturing an empty patch would be the worst
  possible failure. The index is reset afterwards so capture leaves the tree
  exactly as the agents left it, which the test asserts.
- Build output is excluded by pathspec (`target`, `node_modules`, `.venv`, …).
  A phase that ran `cargo build` leaves a directory larger than the repo.
- An empty diff is still an artifact, flagged `empty: true`. "This coding
  phase wrote no code" is currently invisible to an operator and is worth
  saying out loud.

`RegisterArtifact` gains `metadata`, which the column has had since 0047 and
nothing ever wrote; the diffstat and base sha go there. No migration needed —
`kind` is unconstrained TEXT and the column already exists.

Tests run against a real `git init` repo rather than a mock: every bug in this
area so far came from git behaving differently than assumed, and a fake git
would have agreed with the assumption. `capture_phase_diff_at` takes explicit
paths so parallel tests cannot race through the process-global
CLAWMATES_MISSIONS_ROOT — the first version of these tests did exactly that
and two of four failed non-deterministically.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
Omar Sobh
2026-08-01 22:46:15 -07:00
co-authored by Claude Opus 5
parent ea3d145aac
commit 716ee9a304
5 changed files with 591 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/cm-api/src/mission_delivery.rs
+356
View File
@@ -0,0 +1,356 @@
//! 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.
const EXCLUDED_PATHS: &[&str] = &[
"target",
"node_modules",
".venv",
"venv",
"dist",
"build",
".next",
"__pycache__",
".pytest_cache",
".mypy_cache",
"vendor",
];
/// 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;
/// 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,
pub truncated: bool,
pub patch_path: PathBuf,
}
/// 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> {
capture_phase_diff_at(
pool,
mission_id,
phase_id,
&mission_workspace::checkout_path(mission_id),
&outputs_root(mission_id),
)
.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,
) -> Result<Option<Capture>, String> {
let repo = repo.to_path_buf();
if !repo.is_dir() {
return Ok(None);
}
let base_sha = git(&repo, &["rev-parse", "HEAD"])
.await
.map(|s| s.trim().to_string())
.unwrap_or_else(|_| "unknown".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;
let mut diff_args = vec!["diff", "HEAD", "--"];
diff_args.extend(excludes.iter().map(String::as_str));
let patch = git(&repo, &diff_args).await.unwrap_or_default();
let mut stat_args = vec!["diff", "HEAD", "--stat", "--"];
stat_args.extend(excludes.iter().map(String::as_str));
let diffstat = git(&repo, &stat_args).await.unwrap_or_default();
// 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);
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()))?;
std::fs::write(dir.join("diffstat.txt"), &diffstat)
.map_err(|e| format!("write diffstat: {e}"))?;
let meta = json!({
"base_sha": base_sha,
"files_changed": files_changed,
"insertions": insertions,
"deletions": deletions,
"empty": empty,
"truncated": 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, matching how
// `pdf_renderer` resolves artifact paths.
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 {
base_sha,
files_changed,
insertions,
deletions,
empty,
truncated,
patch_path,
}))
}
/// Run git in `repo`, returning stdout.
///
/// Every invocation carries `-c safe.directory`: 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.
async fn git(repo: &Path, args: &[&str]) -> Result<String, String> {
let repo_s = repo.display().to_string();
let mut full = vec![
"-C",
&repo_s,
"-c",
// Leaked into a `String` so it can live in a `&str` slice alongside
// the borrowed args; the process is short-lived and this is one
// allocation per git call.
Box::leak(format!("safe.directory={repo_s}").into_boxed_str()),
];
full.extend_from_slice(args);
let out = tokio::process::Command::new("git")
.args(&full)
.output()
.await
.map_err(|e| format!("spawn git: {e}"))?;
if !out.status.success() {
return Err(format!(
"git {} → {}: {}",
args.first().copied().unwrap_or("?"),
out.status,
String::from_utf8_lossy(&out.stderr)
.chars()
.take(300)
.collect::<String>()
));
}
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)
}
#[cfg(test)]
mod tests {
use super::*;
#[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.
#[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 must never reach a patch. A phase that ran `cargo build`
/// leaves a `target/` bigger than the repository.
#[test]
fn build_output_is_excluded() {
for p in ["target", "node_modules", ".venv", "dist", "__pycache__"] {
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"));
}
}