Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0d9498ec6e | ||
|
|
15e7608e4a | ||
|
|
5d98fcf44a | ||
|
|
4ff4e6f7ee | ||
|
|
deb60be98d |
@@ -583,6 +583,7 @@ pub async fn commit_phase_work(
|
|||||||
String::new()
|
String::new()
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
clear_stale_commit_editmsg(repo);
|
||||||
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
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.
|
/// 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
|
/// 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
|
/// `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.
|
/// at `running` for ten minutes looked like when this was first switched on.
|
||||||
pub fn forwarded_provider_keys(auth: RuntimeAuth) -> Vec<&'static str> {
|
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 {
|
match auth {
|
||||||
RuntimeAuth::ApiKey => keys.push("ANTHROPIC_API_KEY"),
|
RuntimeAuth::ApiKey => keys.push("ANTHROPIC_API_KEY"),
|
||||||
RuntimeAuth::Subscription => keys.push("CLAUDE_CODE_OAUTH_TOKEN"),
|
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.
|
/// mission so this rarely bites. Long-term: copy-on-write per mission.
|
||||||
const DEFAULT_SEED_DIR: &str = "/root/clawmates-runtime/data";
|
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.
|
/// Deterministic docker container name for a mission's runtime.
|
||||||
/// Uses the full UUID hex — UUIDv7 encodes time in the leading bytes,
|
/// Uses the full UUID hex — UUIDv7 encodes time in the leading bytes,
|
||||||
/// so a short prefix isn't guaranteed unique across missions minted
|
/// 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 _ = tokio::fs::create_dir_all(&mission_dir).await;
|
||||||
let seed_dir = std::env::var("CLAWMATES_RUNTIME_SEED_DIR")
|
let seed_dir = std::env::var("CLAWMATES_RUNTIME_SEED_DIR")
|
||||||
.unwrap_or_else(|_| DEFAULT_SEED_DIR.to_string());
|
.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![
|
let mounts = vec![
|
||||||
// Mount just this mission's directory. Agents can navigate
|
// Mount just this mission's directory. Agents can navigate
|
||||||
// its `/repo` subdir but never see other missions'.
|
// its `/repo` subdir but never see other missions'.
|
||||||
@@ -257,14 +413,14 @@ impl MissionRuntimeProvisioner {
|
|||||||
read_only: Some(false),
|
read_only: Some(false),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
},
|
},
|
||||||
// Share the shared-runtime data dir so this gateway inherits
|
// This mission's OWN copy of the seeded agent library
|
||||||
// the seeded agent library (`claw_*` templates). We then
|
// (`claw_*` templates). Copied rather than shared, so its
|
||||||
// mint a per-mission pairing code via /admin/paircode/new
|
// config.toml — which carries the door bearer token — and its
|
||||||
// below — the mint writes into the shared devices.db but
|
// sqlite files belong to this mission alone and are removed with
|
||||||
// the resulting token is unique to this mission.
|
// it by `teardown_container`.
|
||||||
Mount {
|
Mount {
|
||||||
target: Some("/zeroclaw-data".to_string()),
|
target: Some("/zeroclaw-data".to_string()),
|
||||||
source: Some(seed_dir),
|
source: Some(runtime_data_dir),
|
||||||
typ: Some(MountTypeEnum::BIND),
|
typ: Some(MountTypeEnum::BIND),
|
||||||
read_only: Some(false),
|
read_only: Some(false),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
@@ -810,7 +966,13 @@ mod tests {
|
|||||||
it silently bills the API. Forwarded: {keys:?}"
|
it silently bills the API. Forwarded: {keys:?}"
|
||||||
);
|
);
|
||||||
// Unrelated providers have no subscription equivalent and must survive.
|
// 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");
|
assert!(keys.contains(&k), "{k} should still be forwarded");
|
||||||
}
|
}
|
||||||
// And the subscription credential MUST travel. A mission container
|
// 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");
|
let url = endpoint_url("cm-runtime-mission-abc");
|
||||||
assert_eq!(url, "http://cm-runtime-mission-abc:42617");
|
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");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -876,3 +876,43 @@ async fn an_unrunnable_suite_is_distinguishable_from_no_suite() {
|
|||||||
"the two must be distinguishable — this is the whole point"
|
"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