3 Commits
Author SHA1 Message Date
Omar SobhandClaude Opus 5 322c1be89c feat(missions): capture runs automatically, and once more before teardown
ci / gates (push) Failing after 5s
ci / rust (push) Skipped
ci / frontend (push) Skipped
ci / e2e (push) Skipped
ci / publish (push) Skipped
Wires diff capture into the two sweeps that matter.

`phase_runner::sweep_once` gains `capture_finished_coding_phases`, guarded by
`NOT EXISTS (code_diff for this phase)`. Deliberately a separate step rather
than a hook on `close_finished_phases` or `evaluate_finished_phases`: a phase
reaches `completed` through one or the other depending on whether it declared
a `done_when`, so hanging capture off either would silently skip half the
missions. The guard also makes it retryable — a capture that errors is simply
re-selected next tick.

`mission_runtime::sweep_once` captures anything still outstanding immediately
before `teardown_container`, which deletes the checkout. This covers what the
phase sweep structurally cannot: a mission that ended `failed` mid-coding
still has real work on disk, and reaping it unexamined destroys the only
evidence of what the agents actually did.

Applies to coding, benchmark and security_scan phases — all three operate on
a repo.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-02 10:03:37 -07:00
Omar SobhandClaude Opus 5 716ee9a304 feat(missions): capture a coding phase's diff to durable storage
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]>
2026-08-01 22:46:15 -07:00
Omar SobhandClaude Opus 5 ea3d145aac fix(missions): stop leaving an access token in every mission checkout
`with_ambient_auth` embeds GITEA_TOKEN in the clone URL, and git persists that
URL verbatim as the `origin` remote. The checkout is bind-mounted into a
container the agents run in as root, so the token sat in a file every mission
agent could read — and it reaches every repository that token reaches, not
just the one being worked on.

The remote is now rewritten to the bare URL immediately after clone. Delivery
does not depend on the stored URL: it will build a fresh authenticated URL at
push time, which also means a rotated token starts working at once rather than
after the next clone. Best-effort and non-fatal — a checkout that keeps its
token still works, and failing a mission over it would trade a real capability
for a situation already logged.

`strip_credentials` only treats an `@` in the *authority* as a separator, so a
path containing `@` (scoped npm-style names) is left alone.

Also, two changes delivery needs:

- `--depth 1` becomes `--filter=blob:none --single-branch`. A shallow clone
  usually cannot push a new branch ("shallow update not allowed"), which is
  exactly what mission delivery must do. A partial clone keeps full history —
  so a base commit stays meaningful and a diff has something to be relative
  to — while fetching blobs on demand.
- `fetch_and_reset` deepens a pre-existing shallow checkout once, up front,
  rather than letting the push fail later with work on the line.

Fetch stderr is now redacted too; it can echo the remote URL.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-01 22:03:07 -07:00
7 changed files with 835 additions and 14 deletions
+1
View File
@@ -16,6 +16,7 @@ mod mcp_door;
mod mcp_skills; mod mcp_skills;
pub mod mission_orchestrator; pub mod mission_orchestrator;
pub mod mission_refiner; pub mod mission_refiner;
pub mod mission_delivery;
pub mod mission_runtime; pub mod mission_runtime;
pub mod mission_workspace; pub mod mission_workspace;
pub mod node_rules; pub mod node_rules;
+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"));
}
}
+48
View File
@@ -699,6 +699,14 @@ async fn sweep_once(pool: &sqlx::PgPool, grace: std::time::Duration) -> Result<(
for row in rows { for row in rows {
let id: Uuid = row.get("id"); let id: Uuid = row.get("id");
let workspace_id: Uuid = row.get("workspace_id"); let workspace_id: Uuid = row.get("workspace_id");
// Last chance. `teardown_container` deletes the checkout, so anything
// not captured by now is gone for good. The phase sweep should have
// handled this minutes ago; this covers the cases it cannot — a phase
// that ended `failed` rather than `completed`, or a capture that kept
// erroring until the grace window ran out.
if let Err(e) = capture_outstanding_phases(pool, id).await {
eprintln!("mission_runtime::sweeper: last-chance capture for {id}: {e}");
}
if let Err(e) = prov.teardown_container(id).await { if let Err(e) = prov.teardown_container(id).await {
// A not-found is expected when the container was already // A not-found is expected when the container was already
// reaped by a docker restart or a manual op; log at info // reaped by a docker restart or a manual op; log at info
@@ -716,6 +724,46 @@ async fn sweep_once(pool: &sqlx::PgPool, grace: std::time::Duration) -> Result<(
Ok(()) Ok(())
} }
/// Capture any phase of `mission_id` that has a repo and no `code_diff` yet,
/// regardless of how the phase ended.
///
/// The phase sweep only captures `completed` phases. A mission that failed
/// mid-coding still has real work in its checkout, and deleting it
/// unexamined is how a debugging session loses the only evidence of what the
/// agents actually did.
async fn capture_outstanding_phases(pool: &sqlx::PgPool, mission_id: Uuid) -> Result<(), String> {
use sqlx::Row;
let rows = sqlx::query(
"SELECT mp.id
FROM mission_phases mp
JOIN missions m ON m.id = mp.mission_id
WHERE mp.mission_id = $1
AND m.repo_id IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM mission_artifacts a
WHERE a.mission_id = mp.mission_id
AND a.phase_id = mp.id
AND a.kind = 'code_diff'
)",
)
.bind(mission_id)
.fetch_all(pool)
.await
.map_err(|e| format!("select uncaptured phases: {e}"))?;
for row in rows {
let phase_id: Uuid = row.get("id");
if let Err(e) =
crate::mission_delivery::capture_phase_diff(pool, mission_id, phase_id).await
{
eprintln!(
"mission_runtime::sweeper: capture mission {mission_id} phase {phase_id}: {e}"
);
}
}
Ok(())
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
+140 -12
View File
@@ -23,7 +23,7 @@ use std::path::PathBuf;
use tokio::process::Command; use tokio::process::Command;
use uuid::Uuid; use uuid::Uuid;
fn missions_root() -> PathBuf { pub(crate) fn missions_root() -> PathBuf {
std::env::var("CLAWMATES_MISSIONS_ROOT") std::env::var("CLAWMATES_MISSIONS_ROOT")
.map(PathBuf::from) .map(PathBuf::from)
.unwrap_or_else(|_| PathBuf::from("/var/lib/clawmates-missions")) .unwrap_or_else(|_| PathBuf::from("/var/lib/clawmates-missions"))
@@ -92,8 +92,20 @@ fn with_ambient_auth(url: &str) -> String {
} }
async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> { async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
// `--filter=blob:none` rather than `--depth 1`. A shallow clone cannot
// usually push a new branch back ("shallow update not allowed"), and
// mission delivery needs exactly that. A partial clone keeps full history
// — so the base commit stays meaningful and a diff has something to be
// relative to — while fetching file contents only on demand, which is
// nearly as cheap as a shallow clone for a repo that gets read once.
let out = Command::new("git") let out = Command::new("git")
.args(["clone", "--depth", "1", url, &path.display().to_string()]) .args([
"clone",
"--filter=blob:none",
"--single-branch",
url,
&path.display().to_string(),
])
.output() .output()
.await .await
.map_err(|e| format!("spawn git clone: {e}"))?; .map_err(|e| format!("spawn git clone: {e}"))?;
@@ -107,10 +119,78 @@ async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
.collect::<String>() .collect::<String>()
)); ));
} }
scrub_remote_credentials(path, url);
ignore_agent_scaffolding(path); ignore_agent_scaffolding(path);
Ok(()) Ok(())
} }
/// Take the access token back out of `.git/config`.
///
/// `with_ambient_auth` embeds `GITEA_TOKEN` in the clone URL so the clone can
/// authenticate, and git then persists that URL verbatim as the `origin`
/// remote. The checkout is bind-mounted into a container the agents run in as
/// **root**, so the token sits in a file every mission agent can read, and it
/// reaches every repository that token reaches — not just this one.
///
/// Rewriting the remote to the bare URL costs one command and removes a
/// standing credential from the blast radius of any prompt injection that
/// lands in a mission. Delivery does not depend on the stored URL: it builds a
/// fresh authenticated URL at push time, which also means a rotated token
/// starts working immediately instead of after the next clone.
///
/// Best-effort and non-fatal: a checkout that keeps its token still works, and
/// failing the mission over it would trade a real capability for a marginal
/// improvement in a situation we have already logged.
fn scrub_remote_credentials(path: &std::path::Path, original_url: &str) {
if !original_url.contains('@') && !original_url.contains("oauth2:") {
// Nothing was injected (SSH remote, or no token configured).
return;
}
let bare = strip_credentials(original_url);
let out = std::process::Command::new("git")
.args([
"-C",
&path.display().to_string(),
"remote",
"set-url",
"origin",
&bare,
])
.output();
match out {
Ok(o) if o.status.success() => {}
Ok(o) => eprintln!(
"mission_workspace: could not scrub credentials from {} — the access token \
remains readable in .git/config: {}",
path.display(),
redact_token(&String::from_utf8_lossy(&o.stderr))
.chars()
.take(200)
.collect::<String>()
),
Err(e) => eprintln!(
"mission_workspace: could not scrub credentials from {} ({e}) — the access \
token remains readable in .git/config",
path.display()
),
}
}
/// `https://user:secret@host/path` → `https://host/path`.
fn strip_credentials(url: &str) -> String {
let Some((scheme, rest)) = url.split_once("://") else {
return url.to_string();
};
match rest.split_once('@') {
// Only the *authority* may carry credentials; an `@` later in the path
// is an ordinary character and must not be treated as a separator.
Some((userinfo, host_and_path)) if !userinfo.contains('/') => {
format!("{scheme}://{host_and_path}")
}
_ => url.to_string(),
}
}
/// Files the agent runtime writes into its own workspace, which is pinned to /// Files the agent runtime writes into its own workspace, which is pinned to
/// the repository root (`MissionRuntimeProvisioner::pin_agent_workspaces`). /// the repository root (`MissionRuntimeProvisioner::pin_agent_workspaces`).
/// ///
@@ -183,16 +263,41 @@ fn redact_token(s: &str) -> String {
} }
async fn fetch_and_reset(path: &std::path::Path, branch: &str) -> Result<(), String> { async fn fetch_and_reset(path: &std::path::Path, branch: &str) -> Result<(), String> {
// A checkout cloned before delivery existed is shallow, and a shallow repo
// cannot push a new branch. Deepen it once, here, rather than discovering
// the problem at push time when there is work on the line. `--unshallow`
// errors on a repo that is already complete, so it is only attempted when
// the marker file is present.
if path.join(".git/shallow").exists() {
let deepen = Command::new("git")
.args([
"-C",
&path.display().to_string(),
"fetch",
"--unshallow",
"origin",
])
.output()
.await;
match deepen {
Ok(o) if o.status.success() => {}
Ok(o) => eprintln!(
"mission_workspace: could not deepen shallow checkout at {} — a delivery \
push may be rejected: {}",
path.display(),
redact_token(&String::from_utf8_lossy(&o.stderr))
.chars()
.take(200)
.collect::<String>()
),
Err(e) => eprintln!(
"mission_workspace: could not deepen shallow checkout at {} ({e})",
path.display()
),
}
}
let fetch = Command::new("git") let fetch = Command::new("git")
.args([ .args(["-C", &path.display().to_string(), "fetch", "origin", branch])
"-C",
&path.display().to_string(),
"fetch",
"--depth",
"1",
"origin",
branch,
])
.output() .output()
.await .await
.map_err(|e| format!("spawn git fetch: {e}"))?; .map_err(|e| format!("spawn git fetch: {e}"))?;
@@ -200,7 +305,7 @@ async fn fetch_and_reset(path: &std::path::Path, branch: &str) -> Result<(), Str
return Err(format!( return Err(format!(
"git fetch origin {branch} → exit {}: {}", "git fetch origin {branch} → exit {}: {}",
fetch.status, fetch.status,
String::from_utf8_lossy(&fetch.stderr) redact_token(&String::from_utf8_lossy(&fetch.stderr))
.chars() .chars()
.take(400) .take(400)
.collect::<String>() .collect::<String>()
@@ -255,6 +360,29 @@ mod tests {
assert_eq!(first, second, "re-running must not append a second block"); assert_eq!(first, second, "re-running must not append a second block");
} }
#[test]
fn credentials_are_stripped_from_a_remote_url() {
assert_eq!(
strip_credentials("https://oauth2:[email protected]/o/r.git"),
"https://git.redclaw.dev/o/r.git"
);
// No credentials: unchanged.
assert_eq!(
strip_credentials("https://git.redclaw.dev/o/r.git"),
"https://git.redclaw.dev/o/r.git"
);
// SSH form has no `://` authority to rewrite.
assert_eq!(
strip_credentials("[email protected]:o/r.git"),
"[email protected]:o/r.git"
);
// An `@` inside the path is not a credential separator.
assert_eq!(
strip_credentials("https://host/scope/@org/pkg.git"),
"https://host/scope/@org/pkg.git"
);
}
/// An existing exclude file belongs to the repository; keep it. /// An existing exclude file belongs to the repository; keep it.
#[test] #[test]
fn an_existing_exclude_is_preserved() { fn an_existing_exclude_is_preserved() {
+57
View File
@@ -55,10 +55,67 @@ async fn sweep_once(pool: &PgPool, runtime: &cm_runtime::Runtime) -> Result<(),
// Between "all runs finished" and "phase done" sits the completion // Between "all runs finished" and "phase done" sits the completion
// evaluation, for phases that declare a condition. // evaluation, for phases that declare a condition.
evaluate_finished_phases(pool, runtime).await?; evaluate_finished_phases(pool, runtime).await?;
// Capture before the mission closes and long before the sweeper reaps the
// checkout. Idempotent, so a failure here is retried on the next tick
// rather than losing the phase's work.
capture_finished_coding_phases(pool).await?;
close_finished_missions(pool).await?; close_finished_missions(pool).await?;
Ok(()) Ok(())
} }
/// How many phases to capture per tick. Capture shells out to git against a
/// working tree, so a backlog should be worked through steadily rather than
/// all at once.
const CAPTURE_BATCH: i64 = 5;
/// Write out the diff for coding phases that have finished and not yet been
/// captured.
///
/// Deliberately not hung off `close_finished_phases` or
/// `evaluate_finished_phases`: a phase reaches `completed` through either
/// path depending on whether it declared a `done_when`, and bolting capture
/// onto one of them would silently skip the other. Driving it from the sweep
/// with a `NOT EXISTS` guard covers both and is retryable by construction.
async fn capture_finished_coding_phases(pool: &PgPool) -> Result<(), String> {
let rows = sqlx::query(
"SELECT mp.id, mp.mission_id
FROM mission_phases mp
JOIN missions m ON m.id = mp.mission_id
WHERE mp.status = 'completed'
AND mp.kind IN ('coding', 'benchmark', 'security_scan')
AND m.repo_id IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM mission_artifacts a
WHERE a.mission_id = mp.mission_id
AND a.phase_id = mp.id
AND a.kind = 'code_diff'
)
ORDER BY mp.completed_at NULLS LAST
LIMIT $1",
)
.bind(CAPTURE_BATCH)
.fetch_all(pool)
.await
.map_err(|e| format!("select phases to capture: {e}"))?;
for row in rows {
use sqlx::Row;
let phase_id: Uuid = row.get("id");
let mission_id: Uuid = row.get("mission_id");
if let Err(e) =
crate::mission_delivery::capture_phase_diff(pool, mission_id, phase_id).await
{
// Left uncaptured on purpose: the guard above re-selects it next
// tick. Only a permanently broken checkout keeps failing, and that
// is worth the recurring log line.
eprintln!(
"phase_runner: capturing diff for mission {mission_id} phase {phase_id}: {e}"
);
}
}
Ok(())
}
/// Enqueue topology_runs for every phase whose predecessors are done. /// Enqueue topology_runs for every phase whose predecessors are done.
async fn start_pending_phases(pool: &PgPool) -> Result<(), String> { async fn start_pending_phases(pool: &PgPool) -> Result<(), String> {
// Eligible = pending phase, mission running, all lower-order phases // Eligible = pending phase, mission running, all lower-order phases
+224
View File
@@ -0,0 +1,224 @@
//! Diff capture, against a real git repository.
//!
//! Deliberately not mocked. Every bug this area has produced came from git
//! behaving differently than assumed — a shallow clone refusing a push, an
//! ownership check refusing the repo, untracked files invisible to `git diff`.
//! A fake `git` would agree with whatever the code believed and prove nothing.
use std::path::Path;
use std::process::Command;
use cm_api::mission_delivery;
use uuid::Uuid;
/// Capture against an explicit root, so parallel tests cannot race each other
/// through the process-global `CLAWMATES_MISSIONS_ROOT`.
async fn capture(
pool: &sqlx::PgPool,
root: &Path,
mission: Uuid,
phase: Uuid,
) -> Result<Option<mission_delivery::Capture>, String> {
mission_delivery::capture_phase_diff_at(
pool,
mission,
phase,
&root.join(mission.to_string()).join("repo"),
&root.join("_outputs").join(mission.to_string()),
)
.await
}
fn git(repo: &Path, args: &[&str]) {
let out = Command::new("git")
.arg("-C")
.arg(repo)
.args(args)
.output()
.expect("run git");
assert!(
out.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&out.stderr)
);
}
/// A repo with one commit, at `<root>/<mission>/repo` so `checkout_path`
/// finds it.
fn seed_repo(root: &Path, mission: Uuid) -> std::path::PathBuf {
let repo = root.join(mission.to_string()).join("repo");
std::fs::create_dir_all(&repo).unwrap();
git(&repo, &["init", "--quiet"]);
git(&repo, &["config", "user.email", "[email protected]"]);
git(&repo, &["config", "user.name", "Test"]);
std::fs::write(repo.join("README.md"), "# base\n").unwrap();
git(&repo, &["add", "."]);
git(&repo, &["commit", "--quiet", "-m", "base"]);
repo
}
#[tokio::test]
async fn captures_modified_and_untracked_files() {
let pool = cm_testkit::test_pool().await;
let tmp = tempfile::tempdir().unwrap();
let mission = Uuid::now_v7();
let repo = seed_repo(tmp.path(), mission);
let (ws, phase) = seed_mission_phase(&pool, mission).await;
let _ = ws;
// A modified file and a brand-new one. The new file is the case that
// matters: without `--intent-to-add` it would not appear in `git diff`,
// and a phase that only creates files is the likeliest shape of all.
std::fs::write(repo.join("README.md"), "# base\nchanged\n").unwrap();
std::fs::write(repo.join("new_module.rs"), "fn added() {}\n").unwrap();
let cap = capture(&pool, tmp.path(), mission, phase)
.await
.unwrap()
.expect("mission has a checkout");
assert!(!cap.empty, "the phase changed files");
assert_eq!(cap.files_changed, 2, "one modified, one created");
assert!(cap.insertions >= 2);
let patch = std::fs::read_to_string(&cap.patch_path).unwrap();
assert!(
patch.contains("new_module.rs"),
"untracked file is captured"
);
assert!(patch.contains("fn added()"), "its content is captured");
assert!(patch.contains("changed"), "the modification is captured");
// Capture must leave the working tree exactly as the agents left it — a
// later commit has to see the same thing this diff described.
let status = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["status", "--porcelain"])
.output()
.unwrap();
let status = String::from_utf8_lossy(&status.stdout);
assert!(
status.contains("new_module.rs"),
"the new file is still untracked after capture, not left staged: {status}"
);
let row: (String, serde_json::Value) = sqlx::query_as(
"SELECT kind, metadata FROM mission_artifacts WHERE mission_id = $1 AND phase_id = $2",
)
.bind(mission)
.bind(phase)
.fetch_one(&pool)
.await
.unwrap();
assert_eq!(row.0, "code_diff");
assert_eq!(row.1["files_changed"], 2);
assert_eq!(row.1["empty"], false);
assert!(row.1["base_sha"].as_str().unwrap().len() >= 7);
}
/// "This coding phase wrote no code" is a result, and currently an invisible
/// one. It must still produce an artifact.
#[tokio::test]
async fn an_empty_phase_still_produces_an_artifact() {
let pool = cm_testkit::test_pool().await;
let tmp = tempfile::tempdir().unwrap();
let mission = Uuid::now_v7();
seed_repo(tmp.path(), mission);
let (_, phase) = seed_mission_phase(&pool, mission).await;
let cap = capture(&pool, tmp.path(), mission, phase)
.await
.unwrap()
.unwrap();
assert!(cap.empty);
assert_eq!(cap.files_changed, 0);
let (kind, meta): (String, serde_json::Value) =
sqlx::query_as("SELECT kind, metadata FROM mission_artifacts WHERE mission_id = $1")
.bind(mission)
.fetch_one(&pool)
.await
.unwrap();
assert_eq!(kind, "code_diff");
assert_eq!(meta["empty"], true, "the emptiness is recorded, not hidden");
}
/// Build output must never reach the patch. A phase that ran `cargo build`
/// leaves a `target/` larger than the repository, and committing it would be
/// worse than losing the diff.
#[tokio::test]
async fn build_output_is_not_captured() {
let pool = cm_testkit::test_pool().await;
let tmp = tempfile::tempdir().unwrap();
let mission = Uuid::now_v7();
let repo = seed_repo(tmp.path(), mission);
let (_, phase) = seed_mission_phase(&pool, mission).await;
std::fs::create_dir_all(repo.join("target/debug")).unwrap();
std::fs::write(repo.join("target/debug/huge.bin"), vec![b'x'; 200_000]).unwrap();
std::fs::create_dir_all(repo.join("node_modules/left-pad")).unwrap();
std::fs::write(
repo.join("node_modules/left-pad/index.js"),
"module.exports=0",
)
.unwrap();
std::fs::write(repo.join("real_change.rs"), "fn kept() {}\n").unwrap();
let cap = capture(&pool, tmp.path(), mission, phase)
.await
.unwrap()
.unwrap();
let patch = std::fs::read_to_string(&cap.patch_path).unwrap();
assert!(patch.contains("real_change.rs"), "genuine work is captured");
assert!(!patch.contains("huge.bin"), "target/ is excluded");
assert!(!patch.contains("left-pad"), "node_modules is excluded");
assert_eq!(cap.files_changed, 1, "only the real change counts");
}
/// A research mission has no checkout. That is not an error.
#[tokio::test]
async fn a_mission_without_a_checkout_captures_nothing() {
let pool = cm_testkit::test_pool().await;
let tmp = tempfile::tempdir().unwrap();
let mission = Uuid::now_v7();
let (_, phase) = seed_mission_phase(&pool, mission).await;
assert!(capture(&pool, tmp.path(), mission, phase)
.await
.unwrap()
.is_none());
}
async fn seed_mission_phase(pool: &sqlx::PgPool, mission: Uuid) -> (Uuid, Uuid) {
let ws = Uuid::now_v7();
sqlx::query("INSERT INTO workspaces (id, name, plan) VALUES ($1,'t','team')")
.bind(ws)
.execute(pool)
.await
.unwrap();
sqlx::query(
"INSERT INTO missions (id, workspace_id, title, template_kind, schedule, status, config)
VALUES ($1,$2,'t','research_and_code','{}'::jsonb,'running','{}'::jsonb)",
)
.bind(mission)
.bind(ws)
.execute(pool)
.await
.unwrap();
let phase = Uuid::now_v7();
sqlx::query(
"INSERT INTO mission_phases (id, mission_id, kind, order_idx, status, config)
VALUES ($1,$2,'coding',0,'completed','{}'::jsonb)",
)
.bind(phase)
.bind(mission)
.execute(pool)
.await
.unwrap();
(ws, phase)
}
+9 -2
View File
@@ -556,6 +556,11 @@ pub struct RegisterArtifact<'a> {
pub title: Option<&'a str>, pub title: Option<&'a str>,
pub generated_by_run: Option<Uuid>, pub generated_by_run: Option<Uuid>,
pub render_pdf: bool, pub render_pdf: bool,
/// Free-form facts about the artifact (diffstat, branch, gate verdict).
/// The column has existed since 0047 and was never written — an artifact
/// with no metadata is a path and a kind, which is not enough for a UI to
/// say anything useful about it.
pub metadata: Option<serde_json::Value>,
} }
/// Register an artifact discovered on disk (or produced inline). /// Register an artifact discovered on disk (or produced inline).
@@ -566,14 +571,15 @@ pub async fn register_artifact(pool: &PgPool, a: RegisterArtifact<'_>) -> Result
let row = sqlx::query( let row = sqlx::query(
"INSERT INTO mission_artifacts "INSERT INTO mission_artifacts
(id, mission_id, phase_id, path, kind, mime, title, (id, mission_id, phase_id, path, kind, mime, title,
generated_by_run, render_pdf_status) generated_by_run, render_pdf_status, metadata)
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9) VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,COALESCE($10, '{}'::jsonb))
ON CONFLICT (mission_id, path) DO UPDATE ON CONFLICT (mission_id, path) DO UPDATE
SET kind = EXCLUDED.kind, SET kind = EXCLUDED.kind,
mime = COALESCE(EXCLUDED.mime, mission_artifacts.mime), mime = COALESCE(EXCLUDED.mime, mission_artifacts.mime),
title = COALESCE(EXCLUDED.title, mission_artifacts.title), title = COALESCE(EXCLUDED.title, mission_artifacts.title),
generated_by_run = COALESCE(EXCLUDED.generated_by_run, generated_by_run = COALESCE(EXCLUDED.generated_by_run,
mission_artifacts.generated_by_run), mission_artifacts.generated_by_run),
metadata = COALESCE(EXCLUDED.metadata, mission_artifacts.metadata),
updated_at = now() updated_at = now()
RETURNING id", RETURNING id",
) )
@@ -586,6 +592,7 @@ pub async fn register_artifact(pool: &PgPool, a: RegisterArtifact<'_>) -> Result
.bind(a.title) .bind(a.title)
.bind(a.generated_by_run) .bind(a.generated_by_run)
.bind(render_status) .bind(render_status)
.bind(a.metadata.as_ref())
.fetch_one(pool) .fetch_one(pool)
.await?; .await?;
Ok(row.get("id")) Ok(row.get("id"))