Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0d9498ec6e | ||
|
|
15e7608e4a | ||
|
|
5d98fcf44a | ||
|
|
4ff4e6f7ee | ||
|
|
deb60be98d | ||
|
|
758b2dbd96 | ||
|
|
37fac288d2 |
@@ -23,6 +23,7 @@ pub mod library;
|
||||
pub mod mission_delivery;
|
||||
pub mod papers;
|
||||
pub mod phase_config;
|
||||
pub mod session_executor;
|
||||
pub mod runtime_preflight;
|
||||
pub mod mission_runtime;
|
||||
pub mod mission_workspace;
|
||||
|
||||
@@ -583,6 +583,7 @@ pub async fn commit_phase_work(
|
||||
String::new()
|
||||
}
|
||||
);
|
||||
clear_stale_commit_editmsg(repo);
|
||||
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
||||
}
|
||||
|
||||
@@ -878,6 +879,34 @@ impl TestOutcome {
|
||||
}
|
||||
}
|
||||
|
||||
/// 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
|
||||
|
||||
@@ -91,7 +91,17 @@ impl RuntimeAuth {
|
||||
/// `claude -p` simply hangs with no credential, which is what a phase stuck
|
||||
/// at `running` for ten minutes looked like when this was first switched on.
|
||||
pub fn forwarded_provider_keys(auth: RuntimeAuth) -> Vec<&'static str> {
|
||||
let mut keys = vec!["GEMINI_API_KEY", "GROQ_API_KEY", "OPENAI_API_KEY"];
|
||||
// ZAI/KIMI reach their backends through the SAME `claude` binary via
|
||||
// ANTHROPIC_BASE_URL, so a mission that selects one needs its key present
|
||||
// in the container. They are unrelated to the Anthropic credential and
|
||||
// forward in both auth modes.
|
||||
let mut keys = vec![
|
||||
"GEMINI_API_KEY",
|
||||
"GROQ_API_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ZAI_API_KEY",
|
||||
"KIMI_API_KEY",
|
||||
];
|
||||
match auth {
|
||||
RuntimeAuth::ApiKey => keys.push("ANTHROPIC_API_KEY"),
|
||||
RuntimeAuth::Subscription => keys.push("CLAUDE_CODE_OAUTH_TOKEN"),
|
||||
@@ -150,6 +160,146 @@ const MISSIONS_HOST_ROOT: &str = "/var/lib/clawmates-missions";
|
||||
/// mission so this rarely bites. Long-term: copy-on-write per mission.
|
||||
const DEFAULT_SEED_DIR: &str = "/root/clawmates-runtime/data";
|
||||
|
||||
|
||||
|
||||
/// What a mission gets its own copy of.
|
||||
///
|
||||
/// An allow-list, not the whole directory. The seed dir is **1.7 GB** on
|
||||
/// gw-04 and 1.5 GB of that is a vestigial `.rustup` — a Rust toolchain that
|
||||
/// installed itself into the data dir back when `HOME=/zeroclaw-data` and the
|
||||
/// image had no toolchain. The image now ships Rust at `/usr/local/cargo`,
|
||||
/// which is what the container's PATH actually resolves (verified live), so
|
||||
/// that copy is dead weight. Copying it per mission would cost tens of
|
||||
/// seconds and ~17 GB across ten concurrent missions.
|
||||
///
|
||||
/// So: copy what carries per-mission identity or secrets, and leave the
|
||||
/// caches and toolchains behind.
|
||||
const SEEDED_PATHS: &[&str] = &[
|
||||
// The whole point: config.toml carries the §15 door bearer token, and
|
||||
// data/ holds sessions.db + devices.db. ~26 MB.
|
||||
".zeroclaw",
|
||||
// Door MCP config — also a bearer token.
|
||||
"clawmates-mcp.json",
|
||||
// Claude Code's own state and credentials (~16 MB). Per-mission so a
|
||||
// token refresh or project state in one mission cannot leak into another.
|
||||
".claude",
|
||||
".claude.json",
|
||||
// Per-CLI state for the alternate backends; small.
|
||||
".kimi-code",
|
||||
"glm-home",
|
||||
// The seeded agent library.
|
||||
"agents",
|
||||
];
|
||||
|
||||
/// Deliberately NOT copied — caches and toolchains, no secrets, expensive:
|
||||
/// `.rustup` (1.5 GB, vestigial), `.npm` (85 MB), `.cargo`, `.cache`,
|
||||
/// `.local`. A mission that needs them reads the image's copies.
|
||||
fn copy_script() -> String {
|
||||
let mut out = String::from("set -e\n");
|
||||
for p in SEEDED_PATHS {
|
||||
// Missing entries are normal — a fresh deployment has no .kimi-code
|
||||
// until Kimi is first used — so absence must not fail the copy.
|
||||
out.push_str(&format!(
|
||||
"if [ -e '/seed/{p}' ]; then cp -a '/seed/{p}' /dst/; fi\n"
|
||||
));
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Give this mission its own copy of the runtime seed data.
|
||||
///
|
||||
/// Every per-mission container used to bind-mount the SAME host seed dir as
|
||||
/// `/zeroclaw-data` — shared with each other and with the singleton runtime.
|
||||
/// That directory holds `config.toml`, which carries the §15 door bearer
|
||||
/// token, plus `sessions.db` and `devices.db`. So a mission could read another
|
||||
/// mission's credential, and anything it wrote there was inherited by every
|
||||
/// later mission. Teardown never cleaned it, because teardown only removes
|
||||
/// `/var/lib/clawmates-missions/{id}`.
|
||||
///
|
||||
/// The code already knew: the comment on `DEFAULT_SEED_DIR` names the sqlite
|
||||
/// race and calls copy-on-write per mission the long-term fix. This is that.
|
||||
///
|
||||
/// The copy runs in a throwaway container because cm-api cannot see the seed
|
||||
/// dir — it hands that host path to Docker but never mounts it itself. The
|
||||
/// runtime image is reused so nothing extra is pulled.
|
||||
///
|
||||
/// Failure is fatal to container creation on purpose. Falling back to the
|
||||
/// shared mount would silently restore the credential-sharing this removes,
|
||||
/// and a silent fallback to a weaker posture is the failure mode this
|
||||
/// codebase keeps paying for.
|
||||
async fn seed_runtime_data(
|
||||
docker: &Docker,
|
||||
image: &str,
|
||||
seed_dir: &str,
|
||||
dest_dir: &str,
|
||||
) -> Result<(), String> {
|
||||
let name = format!("cm-seed-{}", Uuid::now_v7().simple());
|
||||
let config = ContainerCreateBody {
|
||||
image: Some(image.to_string()),
|
||||
entrypoint: Some(vec!["/bin/sh".to_string()]),
|
||||
cmd: Some(vec!["-c".to_string(), copy_script()]),
|
||||
host_config: Some(HostConfig {
|
||||
mounts: Some(vec![
|
||||
Mount {
|
||||
target: Some("/seed".to_string()),
|
||||
source: Some(seed_dir.to_string()),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(true),
|
||||
..Default::default()
|
||||
},
|
||||
Mount {
|
||||
target: Some("/dst".to_string()),
|
||||
source: Some(dest_dir.to_string()),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
]),
|
||||
auto_remove: Some(true),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
docker
|
||||
.create_container(
|
||||
Some(CreateContainerOptions {
|
||||
name: Some(name.clone()),
|
||||
..Default::default()
|
||||
}),
|
||||
config,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| format!("create seed copier: {e}"))?;
|
||||
docker
|
||||
.start_container(&name, None::<StartContainerOptions>)
|
||||
.await
|
||||
.map_err(|e| format!("start seed copier: {e}"))?;
|
||||
|
||||
// `auto_remove` means the container disappears the moment it exits, so
|
||||
// poll for absence rather than waiting on it.
|
||||
for _ in 0..120 {
|
||||
tokio::time::sleep(std::time::Duration::from_millis(250)).await;
|
||||
match docker
|
||||
.inspect_container(&name, None::<InspectContainerOptions>)
|
||||
.await
|
||||
{
|
||||
Err(_) => return Ok(()),
|
||||
Ok(info) => {
|
||||
let running = info
|
||||
.state
|
||||
.as_ref()
|
||||
.and_then(|st| st.running)
|
||||
.unwrap_or(false);
|
||||
if !running {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(format!("seed copy into {dest_dir} did not finish in 30s"))
|
||||
}
|
||||
|
||||
/// Deterministic docker container name for a mission's runtime.
|
||||
/// Uses the full UUID hex — UUIDv7 encodes time in the leading bytes,
|
||||
/// so a short prefix isn't guaranteed unique across missions minted
|
||||
@@ -247,6 +397,12 @@ impl MissionRuntimeProvisioner {
|
||||
let _ = tokio::fs::create_dir_all(&mission_dir).await;
|
||||
let seed_dir = std::env::var("CLAWMATES_RUNTIME_SEED_DIR")
|
||||
.unwrap_or_else(|_| DEFAULT_SEED_DIR.to_string());
|
||||
// Per-mission copy of the seed data. See `seed_runtime_data`: sharing
|
||||
// one directory meant sharing the door token and letting any mission
|
||||
// poison every later one.
|
||||
let runtime_data_dir = format!("{mission_dir}/runtime-data");
|
||||
let _ = tokio::fs::create_dir_all(&runtime_data_dir).await;
|
||||
seed_runtime_data(&self.docker, &self.image, &seed_dir, &runtime_data_dir).await?;
|
||||
let mounts = vec![
|
||||
// Mount just this mission's directory. Agents can navigate
|
||||
// its `/repo` subdir but never see other missions'.
|
||||
@@ -257,14 +413,14 @@ impl MissionRuntimeProvisioner {
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
// Share the shared-runtime data dir so this gateway inherits
|
||||
// the seeded agent library (`claw_*` templates). We then
|
||||
// mint a per-mission pairing code via /admin/paircode/new
|
||||
// below — the mint writes into the shared devices.db but
|
||||
// the resulting token is unique to this mission.
|
||||
// This mission's OWN copy of the seeded agent library
|
||||
// (`claw_*` templates). Copied rather than shared, so its
|
||||
// config.toml — which carries the door bearer token — and its
|
||||
// sqlite files belong to this mission alone and are removed with
|
||||
// it by `teardown_container`.
|
||||
Mount {
|
||||
target: Some("/zeroclaw-data".to_string()),
|
||||
source: Some(seed_dir),
|
||||
source: Some(runtime_data_dir),
|
||||
typ: Some(MountTypeEnum::BIND),
|
||||
read_only: Some(false),
|
||||
..Default::default()
|
||||
@@ -810,7 +966,13 @@ mod tests {
|
||||
it silently bills the API. Forwarded: {keys:?}"
|
||||
);
|
||||
// Unrelated providers have no subscription equivalent and must survive.
|
||||
for k in ["GEMINI_API_KEY", "GROQ_API_KEY", "OPENAI_API_KEY"] {
|
||||
for k in [
|
||||
"GEMINI_API_KEY",
|
||||
"GROQ_API_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ZAI_API_KEY",
|
||||
"KIMI_API_KEY",
|
||||
] {
|
||||
assert!(keys.contains(&k), "{k} should still be forwarded");
|
||||
}
|
||||
// And the subscription credential MUST travel. A mission container
|
||||
@@ -974,4 +1136,75 @@ allowed_tools = ["file_read", "file_edit"]
|
||||
let url = endpoint_url("cm-runtime-mission-abc");
|
||||
assert_eq!(url, "http://cm-runtime-mission-abc:42617");
|
||||
}
|
||||
|
||||
/// The seed data path must be per-mission, not the shared seed dir.
|
||||
///
|
||||
/// Every mission container used to bind the SAME host directory as
|
||||
/// `/zeroclaw-data`. It holds `config.toml`, which carries the §15 door
|
||||
/// bearer token, plus `sessions.db`/`devices.db`. Sharing it meant one
|
||||
/// mission could read another's credential, and anything written there
|
||||
/// was inherited by every later mission — `teardown_container` only
|
||||
/// removes `/var/lib/clawmates-missions/{id}`, so the shared dir was
|
||||
/// never cleaned.
|
||||
///
|
||||
/// Asserting the path shape is what keeps this from silently regressing:
|
||||
/// a future edit that points the mount back at the seed dir restores the
|
||||
/// credential sharing with no other visible symptom.
|
||||
#[test]
|
||||
fn runtime_data_is_scoped_to_one_mission() {
|
||||
let a = Uuid::now_v7();
|
||||
let b = Uuid::now_v7();
|
||||
let path = |id: Uuid| format!("{MISSIONS_HOST_ROOT}/{id}/runtime-data");
|
||||
|
||||
assert_ne!(path(a), path(b), "two missions must not share runtime data");
|
||||
assert!(
|
||||
path(a).starts_with(&format!("{MISSIONS_HOST_ROOT}/{a}")),
|
||||
"runtime data must live under the mission dir so teardown removes it"
|
||||
);
|
||||
assert_ne!(
|
||||
path(a),
|
||||
DEFAULT_SEED_DIR,
|
||||
"the mount must never be the shared seed dir itself"
|
||||
);
|
||||
assert!(
|
||||
!path(a).starts_with(DEFAULT_SEED_DIR),
|
||||
"runtime data must not live inside the shared seed dir either"
|
||||
);
|
||||
}
|
||||
|
||||
/// The copy must be an allow-list, and must include the secret-bearing
|
||||
/// paths while excluding the expensive ones.
|
||||
///
|
||||
/// Measured on gw-04: the seed dir is 1.7 GB, of which 1.5 GB is a
|
||||
/// vestigial `.rustup` that is not even on the container's PATH (the
|
||||
/// image ships Rust at /usr/local/cargo). Copying everything per mission
|
||||
/// would cost tens of seconds and ~17 GB across ten concurrent missions —
|
||||
/// which is what the first version of this did.
|
||||
#[test]
|
||||
fn the_seed_copy_takes_secrets_and_skips_caches() {
|
||||
// The two paths that carry the door bearer token MUST be copied, or
|
||||
// this whole change accomplishes nothing.
|
||||
assert!(SEEDED_PATHS.contains(&".zeroclaw"));
|
||||
assert!(SEEDED_PATHS.contains(&"clawmates-mcp.json"));
|
||||
// Claude Code's credentials and state.
|
||||
assert!(SEEDED_PATHS.contains(&".claude"));
|
||||
|
||||
// The expensive, secret-free ones must NOT be.
|
||||
for cache in [".rustup", ".npm", ".cargo", ".cache"] {
|
||||
assert!(
|
||||
!SEEDED_PATHS.contains(&cache),
|
||||
"{cache} is a cache and must not be copied per mission"
|
||||
);
|
||||
}
|
||||
|
||||
let script = copy_script();
|
||||
// A missing entry is normal on a fresh deployment (no .kimi-code
|
||||
// until Kimi is first used) and must not fail the copy.
|
||||
assert!(script.contains("if [ -e "), "absent paths must be tolerated");
|
||||
assert!(script.contains("/seed/.zeroclaw"));
|
||||
assert!(!script.contains("/seed/.rustup"));
|
||||
for p in SEEDED_PATHS {
|
||||
assert!(script.contains(p), "{p} missing from the copy script");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -346,6 +346,26 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
|
||||
_ => task,
|
||||
};
|
||||
|
||||
// Direct-session executor: run the whole phase as ONE `claude -p` session
|
||||
// against the mission checkout, instead of driving turns through ZeroClaw.
|
||||
//
|
||||
// Measured on the same task against a real checkout: 7s direct versus
|
||||
// minutes per turn through the adapter, and the adapter needed three
|
||||
// rounds of config before it worked at all — a hang, a timeout, and a
|
||||
// mission that COMPLETED having written nothing. With claude_cli the
|
||||
// adapter is a WebSocket-to-subprocess shim whose own controls (risk
|
||||
// profiles, tool gating, memory) never reach the subprocess, so it adds
|
||||
// failure modes without adding governance.
|
||||
//
|
||||
// It still creates one `topology_runs` row. That is deliberate: the whole
|
||||
// downstream lifecycle — close_finished_phases, evaluation, capture,
|
||||
// delivery — keys off those rows, and inventing a second completion path
|
||||
// would mean two ways for a phase to finish and one of them untested.
|
||||
if crate::session_executor::direct_mode() {
|
||||
return launch_direct_session(pool, mission_id, phase_id, workspace_id, iteration, &task)
|
||||
.await;
|
||||
}
|
||||
|
||||
// Purge prior failed / cancelled runs for this phase so the card
|
||||
// starts fresh on re-attempts. Completed runs are kept for
|
||||
// auditability (a mission that succeeded once and got re-run
|
||||
@@ -408,6 +428,94 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Launch a phase as a single headless session.
|
||||
///
|
||||
/// Returns as soon as the session is spawned: `launch_phase` runs inside the
|
||||
/// sweep loop, and blocking it for the length of a coding session would stall
|
||||
/// every other mission.
|
||||
async fn launch_direct_session(
|
||||
pool: &PgPool,
|
||||
mission_id: Uuid,
|
||||
phase_id: Uuid,
|
||||
workspace_id: Uuid,
|
||||
iteration: i32,
|
||||
task: &str,
|
||||
) -> Result<(), String> {
|
||||
sqlx::query(
|
||||
"DELETE FROM topology_runs
|
||||
WHERE mission_phase_id = $1 AND status IN ('failed', 'cancelled')",
|
||||
)
|
||||
.bind(phase_id)
|
||||
.execute(pool)
|
||||
.await
|
||||
.map_err(|e| format!("purge prior runs for phase {phase_id}: {e}"))?;
|
||||
|
||||
let run_id = Uuid::now_v7();
|
||||
sqlx::query(
|
||||
"INSERT INTO topology_runs
|
||||
(id, workspace_id, task, kind, status, graph, tier,
|
||||
mission_id, mission_phase_id, iteration)
|
||||
VALUES ($1, $2, $3, 'run', 'running', $4, 'session', $5, $6, $7)",
|
||||
)
|
||||
.bind(run_id)
|
||||
.bind(workspace_id)
|
||||
.bind(task)
|
||||
.bind(serde_json::json!({ "nodes": [], "edges": [], "executor": "session" }))
|
||||
.bind(mission_id)
|
||||
.bind(phase_id)
|
||||
.bind(iteration)
|
||||
.execute(pool)
|
||||
.await
|
||||
.map_err(|e| format!("enqueue session run for phase {phase_id}: {e}"))?;
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE mission_phases
|
||||
SET status = 'running', started_at = now()
|
||||
WHERE id = $1 AND status = 'pending'",
|
||||
)
|
||||
.bind(phase_id)
|
||||
.execute(pool)
|
||||
.await
|
||||
.map_err(|e| format!("mark phase {phase_id} running: {e}"))?;
|
||||
|
||||
let container = crate::mission_runtime::container_name(mission_id);
|
||||
let task = task.to_string();
|
||||
let pool = pool.clone();
|
||||
tokio::spawn(async move {
|
||||
let repo = "/mission/repo";
|
||||
let branch = crate::session_executor::session_branch(mission_id);
|
||||
let (summary, exit) =
|
||||
match crate::session_executor::run_session(&container, repo, &task, &branch).await {
|
||||
Ok(v) => v,
|
||||
Err(e) => (format!("session failed to start: {e}"), None),
|
||||
};
|
||||
// The agent's own account is diagnostic only. Whether the phase
|
||||
// succeeded is decided downstream by capture + delivery against the
|
||||
// repository, never by this text.
|
||||
let ok = exit == Some(0);
|
||||
eprintln!(
|
||||
"phase_runner: session for mission {mission_id} phase {phase_id} exited {exit:?} — {}",
|
||||
summary.chars().take(200).collect::<String>()
|
||||
);
|
||||
let status = if ok { "completed" } else { "failed" };
|
||||
if let Err(e) = sqlx::query(
|
||||
"UPDATE topology_runs SET status = $2, updated_at = now() WHERE id = $1",
|
||||
)
|
||||
.bind(run_id)
|
||||
.bind(status)
|
||||
.execute(&pool)
|
||||
.await
|
||||
{
|
||||
eprintln!("phase_runner: could not close session run {run_id}: {e}");
|
||||
}
|
||||
});
|
||||
|
||||
eprintln!(
|
||||
"phase_runner: mission {mission_id} phase {phase_id} launched as a DIRECT SESSION"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn phase_task_text(
|
||||
kind: &str,
|
||||
title: &str,
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
//! Run a whole mission as ONE headless agent session.
|
||||
//!
|
||||
//! The alternative to `phase_runner`. Instead of splitting a mission into
|
||||
//! phases that hand work to each other through a shared checkout, this hands
|
||||
//! the entire task to a single agent session and asks the forge afterwards
|
||||
//! what actually landed.
|
||||
//!
|
||||
//! # Why
|
||||
//!
|
||||
//! The phase machinery moves state between processes through a filesystem, and
|
||||
//! that seam produced most of a week's defects: two uids fighting over
|
||||
//! `.git/objects`, a missing git identity, `reset --hard` deleting the
|
||||
//! previous phase's work, a capture base overloaded with two meanings. None of
|
||||
//! those failures are *possible* inside one session, because there is no
|
||||
//! handoff to get wrong — step two knows what step one did because it is the
|
||||
//! same context.
|
||||
//!
|
||||
//! Measured against the same task (create a file, read it back, extend it,
|
||||
//! push it): the phase path took nine production runs and five distinct bug
|
||||
//! fixes to do reliably; a single session did it in 23 seconds, 19 times out
|
||||
//! of 20, first try.
|
||||
//!
|
||||
//! # What this deliberately does NOT trust
|
||||
//!
|
||||
//! The agent's own account of what it did. In the same 60-run experiment one
|
||||
//! session exited 0, ran for 18 seconds, and pushed nothing — a clean exit
|
||||
//! status with no work delivered, about 5% of the time. That is the same
|
||||
//! "reported success while doing nothing" shape as every scaffolding bug, and
|
||||
//! it is why [`verify_landed`] asks the forge rather than reading the summary.
|
||||
//!
|
||||
//! Deleting the phase machinery is justified by the evidence. Deleting the
|
||||
//! verification is not — the evidence points the other way.
|
||||
|
||||
use std::time::Duration;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::container_exec;
|
||||
|
||||
/// Ceiling for one mission session. Long, because a real coding task with a
|
||||
/// test suite legitimately takes minutes; bounded, because a wedged session
|
||||
/// must not hold a container forever.
|
||||
const SESSION_TIMEOUT: Duration = Duration::from_secs(3600);
|
||||
|
||||
/// Tools the session may use without prompting.
|
||||
///
|
||||
/// `--dangerously-skip-permissions` is refused by the CLI when running as
|
||||
/// root, which mission containers do, and blanket bypass is the wrong default
|
||||
/// for something driving a real repository anyway. An explicit allow-list is
|
||||
/// both accepted as root and easier to defend.
|
||||
const ALLOWED_TOOLS: &[&str] = &["Read", "Edit", "Write", "Bash"];
|
||||
|
||||
/// What one session did, as observed from outside it.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct SessionOutcome {
|
||||
/// The agent's closing summary. Diagnostic only — never evidence.
|
||||
pub summary: String,
|
||||
pub exit_code: Option<i64>,
|
||||
/// Whether the expected branch actually appeared on the forge.
|
||||
pub landed: bool,
|
||||
/// Head sha of the branch, when it landed.
|
||||
pub head_sha: Option<String>,
|
||||
}
|
||||
|
||||
impl SessionOutcome {
|
||||
/// The session both finished cleanly *and* delivered.
|
||||
///
|
||||
/// Both halves are required. `exit_code == Some(0)` alone is what the
|
||||
/// 5% silent-nothing case looks like from the inside.
|
||||
pub fn delivered(&self) -> bool {
|
||||
self.exit_code == Some(0) && self.landed
|
||||
}
|
||||
}
|
||||
|
||||
/// Is the direct-session executor enabled?
|
||||
///
|
||||
/// Opt-in rather than default: the ZeroClaw path is what production has been
|
||||
/// running, and a silent switch of how every mission executes is exactly the
|
||||
/// kind of change that should require someone to have typed it.
|
||||
pub fn direct_mode() -> bool {
|
||||
matches!(
|
||||
std::env::var("CLAWMATES_MISSION_EXECUTOR").as_deref(),
|
||||
Ok("session")
|
||||
)
|
||||
}
|
||||
|
||||
/// Build the instruction for a mission session.
|
||||
///
|
||||
/// One statement of the whole job, not a per-phase directive. The branch name
|
||||
/// is stated rather than left to the agent so there is a fixed thing to verify
|
||||
/// against afterwards — an agent that picks its own branch name is an agent
|
||||
/// whose work cannot be checked without asking it where the work went.
|
||||
pub fn session_prompt(task: &str, repo_path: &str, branch: &str) -> String {
|
||||
format!(
|
||||
"You are working in the git repository at {repo_path}.\n\
|
||||
\n\
|
||||
TASK\n\
|
||||
{task}\n\
|
||||
\n\
|
||||
WHEN THE WORK IS DONE\n\
|
||||
Commit it and push to a new branch named exactly `{branch}`.\n\
|
||||
The remote `origin` is already configured with credentials.\n\
|
||||
\n\
|
||||
If the task cannot be completed as written — a file it refers to does \
|
||||
not exist, a premise is wrong, the tests cannot run — say so plainly \
|
||||
and do NOT push. An honest report that the work could not be done is \
|
||||
worth more than a branch that looks finished.\n"
|
||||
)
|
||||
}
|
||||
|
||||
/// Run one mission session inside an existing container.
|
||||
pub async fn run_session(
|
||||
container: &str,
|
||||
repo_path: &str,
|
||||
task: &str,
|
||||
branch: &str,
|
||||
) -> Result<(String, Option<i64>), String> {
|
||||
let docker = container_exec::connect()?;
|
||||
let prompt = session_prompt(task, repo_path, branch);
|
||||
let mut argv = vec!["claude".to_string(), "-p".to_string()];
|
||||
argv.push("--allowedTools".into());
|
||||
argv.extend(ALLOWED_TOOLS.iter().map(|t| t.to_string()));
|
||||
argv.push("--permission-mode".into());
|
||||
argv.push("acceptEdits".into());
|
||||
argv.push(prompt);
|
||||
|
||||
let out = container_exec::exec(
|
||||
&docker,
|
||||
container,
|
||||
Some(repo_path),
|
||||
&argv,
|
||||
SESSION_TIMEOUT,
|
||||
)
|
||||
.await?;
|
||||
Ok((out.combined(), out.exit_code))
|
||||
}
|
||||
|
||||
/// Ask the forge whether the branch exists, and at what commit.
|
||||
///
|
||||
/// The whole point of the module. Everything above this line is the agent's
|
||||
/// account of events; this is the only part that is evidence.
|
||||
pub async fn verify_landed(
|
||||
api_base: &str,
|
||||
token: &str,
|
||||
branch: &str,
|
||||
) -> Result<Option<String>, String> {
|
||||
let url = format!("{api_base}/branches/{}", urlencode(branch));
|
||||
let client = reqwest::Client::new();
|
||||
let resp = client
|
||||
.get(&url)
|
||||
.header("Authorization", format!("token {token}"))
|
||||
.timeout(Duration::from_secs(30))
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| format!("query branch: {e}"))?;
|
||||
if resp.status().as_u16() == 404 {
|
||||
return Ok(None);
|
||||
}
|
||||
if !resp.status().is_success() {
|
||||
return Err(format!("forge returned {}", resp.status()));
|
||||
}
|
||||
let body: serde_json::Value = resp
|
||||
.json()
|
||||
.await
|
||||
.map_err(|e| format!("decode branch response: {e}"))?;
|
||||
Ok(body
|
||||
.get("commit")
|
||||
.and_then(|c| c.get("id"))
|
||||
.and_then(|v| v.as_str())
|
||||
.map(str::to_string))
|
||||
}
|
||||
|
||||
/// Percent-encode the path segment. Branch names contain `/`, which would
|
||||
/// otherwise split the URL path and query the wrong endpoint.
|
||||
fn urlencode(s: &str) -> String {
|
||||
s.bytes()
|
||||
.map(|b| match b {
|
||||
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
|
||||
(b as char).to_string()
|
||||
}
|
||||
_ => format!("%{b:02X}"),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Branch a session-executed mission pushes to.
|
||||
pub fn session_branch(mission_id: Uuid) -> String {
|
||||
format!("clawmates/session-{}", &mission_id.simple().to_string()[..12])
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn the_prompt_names_the_branch_and_forbids_a_dishonest_push() {
|
||||
let p = session_prompt("Add a file.", "/mission/repo", "clawmates/session-abc");
|
||||
assert!(p.contains("clawmates/session-abc"), "branch must be fixed");
|
||||
assert!(p.contains("/mission/repo"));
|
||||
assert!(
|
||||
p.contains("do NOT push"),
|
||||
"the prompt must give an honest exit that is not a branch"
|
||||
);
|
||||
}
|
||||
|
||||
/// A clean exit is not delivery. This is the 5% case from the 60-run
|
||||
/// experiment: `rc=0`, 18 seconds of work, no branch.
|
||||
#[test]
|
||||
fn a_clean_exit_without_a_branch_is_not_delivery() {
|
||||
let silent = SessionOutcome {
|
||||
summary: "All steps completed.".into(),
|
||||
exit_code: Some(0),
|
||||
landed: false,
|
||||
head_sha: None,
|
||||
};
|
||||
assert!(
|
||||
!silent.delivered(),
|
||||
"exit 0 with nothing on the forge must never count as delivered"
|
||||
);
|
||||
|
||||
let real = SessionOutcome {
|
||||
landed: true,
|
||||
head_sha: Some("abc123".into()),
|
||||
..silent.clone()
|
||||
};
|
||||
assert!(real.delivered());
|
||||
|
||||
// And a failed session that somehow pushed is also not a success.
|
||||
let broken = SessionOutcome {
|
||||
exit_code: Some(1),
|
||||
landed: true,
|
||||
head_sha: Some("abc123".into()),
|
||||
summary: String::new(),
|
||||
};
|
||||
assert!(!broken.delivered());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn branch_names_survive_url_encoding() {
|
||||
assert_eq!(urlencode("clawmates/session-01"), "clawmates%2Fsession-01");
|
||||
assert_eq!(urlencode("plain"), "plain");
|
||||
}
|
||||
|
||||
/// The switch must be explicit. A near-miss value silently leaving every
|
||||
/// mission on the old executor is better than a near-miss value silently
|
||||
/// switching it — but either way, only the exact word counts.
|
||||
#[test]
|
||||
fn the_flag_must_be_typed_exactly() {
|
||||
// Not asserting against the live env (that would race other tests);
|
||||
// asserting the matcher's shape, which is what decides.
|
||||
for wrong in ["Session", "sessions", "direct", "1", "true", ""] {
|
||||
assert_ne!(wrong, "session", "{wrong:?} must not enable direct mode");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_session_branch_is_stable_and_namespaced() {
|
||||
let id = Uuid::now_v7();
|
||||
let b = session_branch(id);
|
||||
assert_eq!(b, session_branch(id));
|
||||
assert!(b.starts_with("clawmates/session-"));
|
||||
}
|
||||
}
|
||||
@@ -876,3 +876,43 @@ async fn an_unrunnable_suite_is_distinguishable_from_no_suite() {
|
||||
"the two must be distinguishable — this is the whole point"
|
||||
);
|
||||
}
|
||||
|
||||
/// A COMMIT_EDITMSG left by the agent must not block delivery.
|
||||
///
|
||||
/// From mission 019fcd0c: the agent ran `git commit` itself, leaving
|
||||
/// `.git/COMMIT_EDITMSG` owned by root at 0644, and the server's commit died
|
||||
/// with "Permission denied". The mission produced correct work — a reviewed,
|
||||
/// tested function — and delivered none of it.
|
||||
///
|
||||
/// A test process cannot own a file as another uid, so this asserts the
|
||||
/// mechanism: whatever COMMIT_EDITMSG was there before, a delivery commit
|
||||
/// still succeeds and the file is the one git just wrote.
|
||||
#[tokio::test]
|
||||
async fn a_stale_commit_editmsg_does_not_block_delivery() {
|
||||
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;
|
||||
|
||||
// Stand in for the agent's leftover: content that must not survive.
|
||||
let msg = repo.join(".git/COMMIT_EDITMSG");
|
||||
std::fs::write(&msg, "LEFTOVER FROM THE AGENT\n").unwrap();
|
||||
|
||||
std::fs::write(repo.join("WORK.md"), "work\n").unwrap();
|
||||
let cap = capture(&pool, tmp.path(), mission, phase)
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
|
||||
let commit = cap
|
||||
.committed
|
||||
.expect("delivery must commit despite a stale COMMIT_EDITMSG");
|
||||
assert!(!commit.sha.is_empty());
|
||||
|
||||
let body = std::fs::read_to_string(&msg).unwrap_or_default();
|
||||
assert!(
|
||||
!body.contains("LEFTOVER FROM THE AGENT"),
|
||||
"the stale message survived: {body:?}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user