Files
clawmates/crates/cm-api/src/mission_fs.rs
T
Omar SobhandClaude Opus 5 00160739de
deploy / test (push) Successful in 5m18s
deploy / build (push) Successful in 5m36s
feat(skills): a files arm — progressive disclosure through Read, not a deferred tool
The `index` arm retrieves through `ReadMcpResourceTool`, which is DEFERRED:
absent from the agent's default tool list until `ToolSearch` loads it. Across
three matched production runs (same recipe, same task, same three offered
uris) it retrieved 1 skill in 9 chances:

    01a07812  delegation forced      no instruction    0/3
    01a0842e  no delegation          no instruction    1/3
    01a09877  no delegation          told to load it   0/3

The third run is the decisive one. The preamble said in plain words to run
ToolSearch first; all three prompts carried it; zero ToolSearch calls, and the
three reasoning narratives never mention skills at all. The section was not
declined, it was never engaged with. Instruction is not the lever.

`Read` is a core tool. Never deferred, and every one of those agents used it.
So this arm keeps progressive disclosure exactly as `index` has it — a name, a
`when_to_use`, and a pointer the agent has to follow — and changes only what
the pointer is: a path under /mission/skills instead of an MCP uri. The bodies
are written into the container at launch (every visible skill, one tar upload;
bindings resolve per agent at turn time so a per-mission subset is not knowable
here) and a `Read` of that path is a tapped tool call, so Trigger is exactly as
observable as before.

A third arm and not a replacement, selected per mission like the others, so
the comparison runs against one binary. `resolve` falls back to `inline` when
the files were not written, for the reason `index` does: a pointer to nothing
reads as an agent ignoring its skills.

The writer and reader of a path are one pair of functions
(`skill_file_path` / `skill_from_file_path`), matched by the scorer through
the same seam `parse_uri` uses, and the end-to-end test fails when the matcher
is broken. `Mode::is_retrieval` exists so the next arm cannot silently inherit
`inline`'s "not observable" for what is a miss.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz
2026-09-12 22:33:41 -05:00

599 lines
24 KiB
Rust

//! Move a mission's checkout in and out of its container, instead of sharing it.
//!
//! Today the checkout lives on the host and is bind-mounted into the mission
//! container. That single directory is written by **two users** — cm-api as
//! uid 65532 and the agent as root — and every bug that pattern can produce,
//! it has produced:
//!
//! | Symptom | Fix that was needed |
//! |---|---|
//! | `.git/objects` permission denied | `core.sharedRepository=0777` |
//! | capture base overwritten each phase | advance the base after commit |
//! | `.git/COMMIT_EDITMSG` root-owned | unlink before commit |
//! | `reset --hard` deleting a prior phase | `.git/clawmates-in-use` marker |
//!
//! Four fixes, one cause. `core.sharedRepository` was never a general
//! solution — it covers objects and refs, and every *other* file git touches
//! is a fresh opportunity.
//!
//! Copy-in/copy-out removes the cause: the agent owns its filesystem
//! completely, as root, with no other writer. Nothing on the host is shared,
//! so nothing on the host can collide.
//!
//! # Cost
//!
//! Measured on gw-04 against a real 65 MB checkout of this repository:
//! **0.23s in, 0.18s out**. That was the one open risk in the plan — a
//! monorepo copied per phase — and it is not a risk at this size. Measure
//! again before assuming it holds for a repository an order of magnitude
//! larger.
//!
//! No compression: the payload crosses a local Docker socket, so gzip would
//! spend CPU to save nothing.
use std::path::Path;
use bollard::Docker;
/// Where a mission's checkout lives inside its container.
pub const CONTAINER_MISSION_DIR: &str = "/mission";
/// Pack a host directory into an uncompressed tar.
///
/// `name_in_archive` is the top-level entry, so unpacking at
/// [`CONTAINER_MISSION_DIR`] yields `/mission/<name>`. Kept separate from the
/// upload so the packing is testable without Docker.
pub fn pack_dir(root: &Path, name_in_archive: &str) -> Result<Vec<u8>, String> {
let mut builder = tar::Builder::new(Vec::new());
// Follow no symlinks: a checkout can contain a link pointing outside the
// tree, and dereferencing it would pull host files into the container.
builder.follow_symlinks(false);
append_filtered(&mut builder, root, Path::new(name_in_archive))
.map_err(|e| format!("pack {}: {e}", root.display()))?;
builder
.into_inner()
.map_err(|e| format!("finish archive for {}: {e}", root.display()))
}
/// Directory names never carried across the boundary.
///
/// The same list the delivery diff uses, deliberately: see
/// [`crate::mission_delivery::EXCLUDED_PATHS`]. A build directory is not work —
/// it is regenerable output that dwarfs the source, and shipping it cost a
/// mission its results when `vm_collect` timed out with the agent's finished work
/// still inside the VM.
pub fn transport_excludes() -> &'static [&'static str] {
crate::mission_delivery::EXCLUDED_PATHS
}
/// Should this directory entry be left out of the archive?
///
/// Matched on the entry NAME at any depth, not on a path prefix: a workspace has
/// a `target/` per crate, and excluding only the root one would still ship the
/// rest.
pub fn is_excluded(name: &str) -> bool {
transport_excludes().contains(&name)
}
/// Recursive `append_dir_all` that skips [`transport_excludes`].
///
/// Hand-rolled because `tar::Builder::append_dir_all` takes no filter. Symlinks
/// are added as links rather than followed, matching `follow_symlinks(false)`.
fn append_filtered<W: std::io::Write>(
builder: &mut tar::Builder<W>,
dir: &Path,
prefix: &Path,
) -> std::io::Result<()> {
builder.append_dir(prefix, dir)?;
let mut entries: Vec<_> = std::fs::read_dir(dir)?.collect::<Result<Vec<_>, _>>()?;
// Stable order so an archive of the same tree is byte-identical, which makes
// a size or content difference between two runs mean something.
entries.sort_by_key(|e| e.file_name());
for entry in entries {
let name = entry.file_name();
let name_str = name.to_string_lossy();
let path = entry.path();
let dest = prefix.join(&name);
let meta = std::fs::symlink_metadata(&path)?;
if meta.is_dir() {
if is_excluded(&name_str) {
continue;
}
append_filtered(builder, &path, &dest)?;
} else if meta.is_symlink() {
let mut header = tar::Header::new_gnu();
header.set_metadata(&meta);
header.set_entry_type(tar::EntryType::Symlink);
header.set_size(0);
let target = std::fs::read_link(&path)?;
builder.append_link(&mut header, &dest, &target)?;
} else {
let mut f = std::fs::File::open(&path)?;
builder.append_file(&dest, &mut f)?;
}
}
Ok(())
}
/// Unpack a tar into a host directory.
///
/// `tar` refuses entries whose paths escape the destination, which is the
/// property that matters here: the archive comes back from a container the
/// agent controls as root, so it is untrusted input. A `../../etc` entry must
/// not be able to write outside the collection directory.
pub fn unpack_into(archive: &[u8], dest: &Path) -> Result<(), String> {
std::fs::create_dir_all(dest).map_err(|e| format!("mkdir {}: {e}", dest.display()))?;
let mut ar = tar::Archive::new(archive);
ar.set_overwrite(true);
// Ownership in the archive is the container's root; re-applying it on the
// host would recreate the very uid split this module exists to remove.
ar.set_preserve_permissions(false);
// Filter on the way OUT as well as on the way in.
//
// `pack_dir` (host -> container) skips `transport_excludes`, but `copy_out`
// (container -> host) is the raw Docker archive API, which carries the whole
// tree — `target/` included. The asymmetry was invisible for as long as the
// runtime image had no `cmake`, because nothing could compile and no
// `target/` existed. The moment missions could build, every collection
// failed on a build artifact:
//
// failed to unpack `…/repo/target/debug/build/ahash-…/build_script_build-…`
//
// and `phase_runner` correctly refused to capture a stale tree — so a
// coding phase that HAD done the work delivered nothing, retrying forever.
//
// Entries are skipped by NAME at any depth, the same rule `is_excluded`
// uses, because a workspace has a `target/` per crate.
let mut skipped = 0usize;
for entry in ar
.entries()
.map_err(|e| format!("read archive for {}: {e}", dest.display()))?
{
let mut entry = entry.map_err(|e| format!("read entry for {}: {e}", dest.display()))?;
let path = entry
.path()
.map_err(|e| format!("entry path for {}: {e}", dest.display()))?
.into_owned();
if path
.components()
.any(|c| is_excluded(&c.as_os_str().to_string_lossy()))
{
skipped += 1;
continue;
}
entry
.unpack_in(dest)
.map_err(|e| format!("unpack into {}: {e}", dest.display()))?;
}
if skipped > 0 {
eprintln!(
"mission_fs: unpack into {} skipped {skipped} excluded entr{} (build output)",
dest.display(),
if skipped == 1 { "y" } else { "ies" }
);
}
Ok(())
}
/// Copy a host directory into a running container at [`CONTAINER_MISSION_DIR`].
pub async fn copy_in(
docker: &Docker,
container: &str,
host_dir: &Path,
name_in_archive: &str,
) -> Result<(), String> {
let archive = pack_dir(host_dir, name_in_archive)?;
let opts = bollard::query_parameters::UploadToContainerOptionsBuilder::default()
.path(CONTAINER_MISSION_DIR)
.build();
docker
.upload_to_container(container, Some(opts), bollard::body_full(archive.into()))
.await
.map_err(|e| format!("copy into {container}:{CONTAINER_MISSION_DIR}: {e}"))
}
/// Build a one-entry tar. Split out from [`put_file`] so the size-independence
/// that is the whole point can be tested without Docker.
fn single_file_archive(name: &str, contents: &[u8]) -> Result<Vec<u8>, String> {
let mut header = tar::Header::new_gnu();
header
.set_path(name)
.map_err(|e| format!("tar path {name}: {e}"))?;
header.set_size(contents.len() as u64);
header.set_mode(0o600);
header.set_entry_type(tar::EntryType::Regular);
header.set_cksum();
let mut builder = tar::Builder::new(Vec::new());
builder
.append(&header, contents)
.map_err(|e| format!("tar {name}: {e}"))?;
builder
.into_inner()
.map_err(|e| format!("finish archive for {name}: {e}"))
}
/// Write one file into a container, at any size.
///
/// The obvious way to do this is `sh -c "printf … > file"`, and it works right
/// up until the payload approaches `ARG_MAX`, at which point exec fails with
/// `argument list too long`. That is a size-dependent failure in a code path
/// whose payload grows with use, which makes it a bug that ships green and
/// surfaces in production — as it did, silently unpinning every agent in
/// mission `019fcf62`. Tar has no argv limit.
///
/// The write is not atomic. Callers that need it can upload beside the target
/// and rename; the config writer does not, because the daemon reads its config
/// once at boot and is restarted afterwards.
pub async fn put_file(
docker: &Docker,
container: &str,
path: &str,
contents: &[u8],
) -> Result<(), String> {
let (dir, file) = path
.rsplit_once('/')
.ok_or_else(|| format!("{path} is not an absolute path"))?;
let dir = if dir.is_empty() { "/" } else { dir };
let archive = single_file_archive(file, contents)?;
let opts = bollard::query_parameters::UploadToContainerOptionsBuilder::default()
.path(dir)
.build();
docker
.upload_to_container(container, Some(opts), bollard::body_full(archive.into()))
.await
.map_err(|e| format!("upload {path} to {container}: {e}"))
}
/// Build a flat tar of several files. [`single_file_archive`] for many.
fn files_archive(files: &[(String, Vec<u8>)]) -> Result<Vec<u8>, String> {
let mut builder = tar::Builder::new(Vec::new());
for (name, contents) in files {
let mut header = tar::Header::new_gnu();
header
.set_path(name)
.map_err(|e| format!("tar path {name}: {e}"))?;
header.set_size(contents.len() as u64);
// World-readable, unlike `single_file_archive`'s 0600: that one carries
// a credential, this one carries procedures the agent is meant to read.
header.set_mode(0o644);
header.set_entry_type(tar::EntryType::Regular);
header.set_cksum();
builder
.append(&header, contents.as_slice())
.map_err(|e| format!("tar {name}: {e}"))?;
}
builder
.into_inner()
.map_err(|e| format!("finish archive of {} files: {e}", files.len()))
}
/// Write several files into one directory of a container, in one upload.
///
/// `dir` must already exist — `upload_to_container` will not create it, the
/// same constraint [`sync_in`] works around. Size-independent for the reason
/// [`put_file`] gives; fifty skill bodies would be well past `ARG_MAX` as a
/// printf.
pub async fn put_files(
docker: &Docker,
container: &str,
dir: &str,
files: &[(String, Vec<u8>)],
) -> Result<(), String> {
let archive = files_archive(files)?;
let opts = bollard::query_parameters::UploadToContainerOptionsBuilder::default()
.path(dir)
.build();
docker
.upload_to_container(container, Some(opts), bollard::body_full(archive.into()))
.await
.map_err(|e| format!("upload {} files to {container}:{dir}: {e}", files.len()))
}
/// Copy a directory back out of a container onto the host.
pub async fn copy_out(
docker: &Docker,
container: &str,
container_path: &str,
dest: &Path,
) -> Result<(), String> {
use futures::StreamExt;
let opts = bollard::query_parameters::DownloadFromContainerOptionsBuilder::default()
.path(container_path)
.build();
let mut stream = docker.download_from_container(container, Some(opts));
let mut archive = Vec::new();
while let Some(chunk) = stream.next().await {
let bytes = chunk.map_err(|e| format!("copy out of {container}:{container_path}: {e}"))?;
archive.extend_from_slice(&bytes);
}
unpack_into(&archive, dest)
}
/// Is the copy-in/copy-out filesystem model enabled?
///
/// **Default since 2026-08-04.** It shipped opt-in, on the principle that
/// silently changing how every mission receives its code should require
/// someone to have typed it. Four production missions and a fail-closed
/// harness later (`scripts/verify-mission-delivery.sh`), the opt-in is the
/// riskier setting: the bind path is the one with four documented work-loss
/// incidents, and leaving it as the default means the untested path is what
/// runs when nobody sets the variable.
///
/// `CLAWMATES_MISSION_FS=bind` still selects the old behaviour, so a revert is
/// one line in `.env` rather than a rollback. Anything else — unset, empty,
/// misspelt — gets copy mode, because the failure mode of a typo should be the
/// safer path, not the one being retired.
pub fn copy_mode() -> bool {
!matches!(std::env::var("CLAWMATES_MISSION_FS").as_deref(), Ok("bind"))
}
/// Host directory holding a mission's checkout.
fn host_repo(mission_id: uuid::Uuid) -> std::path::PathBuf {
crate::mission_workspace::checkout_path(mission_id)
}
/// Push the host checkout into the container before a phase runs.
///
/// A repo-less mission has no checkout to push, but it still needs
/// `/mission/repo` to EXIST inside the container: the phase prompt tells the
/// agent that is its working directory, `mission_orchestrator` pins every
/// claw's `workspace.path` to it, and `mission_outputs` copies it back out to
/// register artifacts. This used to return early instead, so none of those three
/// were true — the pin resolved to nothing, ZeroClaw fell back to each agent's
/// own sandbox, and the agents (correctly) reported they had no such directory
/// and refused to work. Creating it empty is what the microVM tier already does,
/// for the same reason: see `microvm_executor::inject` ("the guest needs the
/// workspace to exist before the agent writes into it").
///
/// Creating it host-side rather than `mkdir`-ing in the container keeps the copy
/// cycle symmetric — `sync_out` unpacks over this same path, so work written by
/// one phase survives into the next instead of being wiped by the next
/// `sync_in`.
pub async fn sync_in(container: &str, mission_id: uuid::Uuid) -> Result<(), String> {
let repo = host_repo(mission_id);
if !repo.is_dir() {
tokio::fs::create_dir_all(&repo)
.await
.map_err(|e| format!("create empty workspace {}: {e}", repo.display()))?;
}
let docker = crate::container_exec::connect()?;
// `upload_to_container` requires the DESTINATION to exist: uploading into
// `/mission` when the container has no `/mission` fails with
// "404 Could not find the file /mission in container", which reads like a
// missing source file rather than a missing target directory. Nothing else
// creates it — not the image, not the container spec (in copy mode there is
// no `/mission` bind) — so create it here, immediately before the copy that
// depends on it.
let mkdir = [
"mkdir".to_string(),
"-p".to_string(),
CONTAINER_MISSION_DIR.to_string(),
];
if let Err(e) = crate::container_exec::exec_as_root(
&docker,
container,
None,
&mkdir,
std::time::Duration::from_secs(20),
)
.await
{
return Err(format!("create {CONTAINER_MISSION_DIR} in {container}: {e}"));
}
copy_in(&docker, container, &repo, "repo").await
}
/// Pull the agent's work back onto the host after a phase.
///
/// Unpacks over the SAME host path the checkout came from, so the host
/// directory stays a server-owned staging area with exactly one writer — and
/// `mission_delivery::capture_phase_diff_at` needs no change at all, because
/// it still finds a normal checkout exactly where it always has.
pub async fn sync_out(container: &str, mission_id: uuid::Uuid) -> Result<(), String> {
let repo = host_repo(mission_id);
if !repo.is_dir() {
return Ok(());
}
let parent = repo
.parent()
.ok_or_else(|| format!("{} has no parent", repo.display()))?;
let docker = crate::container_exec::connect()?;
copy_out(&docker, container, "/mission/repo", parent).await
}
#[cfg(test)]
mod tests {
use super::*;
fn seed(root: &Path) {
std::fs::create_dir_all(root.join("src")).unwrap();
std::fs::create_dir_all(root.join(".git")).unwrap();
std::fs::write(root.join("src/lib.rs"), "pub fn x() {}\n").unwrap();
std::fs::write(root.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
}
/// Build output must be dropped on the way BACK, not only on the way out.
///
/// `copy_out` uses the raw Docker archive API, which carries `target/`
/// whatever `pack_dir` did. Unpacking it failed on a build-script binary
/// and took the whole collection down with it, so a coding phase that had
/// really done the work delivered nothing.
#[test]
fn unpacking_drops_build_output_but_keeps_the_source() {
let tmp = tempfile::tempdir().unwrap();
let src = tmp.path().join("repo");
std::fs::create_dir_all(src.join("src")).unwrap();
std::fs::create_dir_all(src.join("target/debug/build")).unwrap();
std::fs::create_dir_all(src.join("crates/inner/target")).unwrap();
std::fs::write(src.join("src/lib.rs"), "pub fn x() {}\n").unwrap();
std::fs::write(src.join("target/debug/build/script"), "ELF").unwrap();
std::fs::write(src.join("crates/inner/target/blob"), "ELF").unwrap();
// Built WITHOUT the filter, the way the Docker API hands it to us.
let mut buf = Vec::new();
{
let mut b = tar::Builder::new(&mut buf);
b.append_dir_all("repo", &src).unwrap();
b.finish().unwrap();
}
let dest = tmp.path().join("out");
unpack_into(&buf, &dest).expect("must not fail on build output");
assert!(dest.join("repo/src/lib.rs").is_file(), "source must survive");
assert!(
!dest.join("repo/target").exists(),
"root target/ must be dropped"
);
assert!(
!dest.join("repo/crates/inner/target").exists(),
"a per-crate target/ must be dropped too — matched by NAME at any depth"
);
}
/// A checkout must survive the round trip intact — including `.git`,
/// without which the whole delivery path (diff, commit, push) is dead.
#[test]
fn a_checkout_round_trips_with_its_git_dir() {
let tmp = tempfile::tempdir().unwrap();
let src = tmp.path().join("repo");
seed(&src);
let archive = pack_dir(&src, "repo").unwrap();
let dest = tmp.path().join("out");
unpack_into(&archive, &dest).unwrap();
assert_eq!(
std::fs::read_to_string(dest.join("repo/src/lib.rs")).unwrap(),
"pub fn x() {}\n"
);
assert!(
dest.join("repo/.git/HEAD").exists(),
"the .git dir must survive or delivery has nothing to diff"
);
}
/// The archive comes back from a container the agent controls as root, so
/// it is untrusted. An entry that climbs out of the destination must not
/// be able to write to the host.
#[test]
fn an_archive_cannot_escape_the_destination() {
let tmp = tempfile::tempdir().unwrap();
let dest = tmp.path().join("dest");
let canary = tmp.path().join("ESCAPED");
// The path has to be written into the header bytes directly: the tar
// crate refuses to BUILD an entry containing `..`, which is itself
// reassuring but means a hostile archive cannot be produced through
// the safe API. A real attacker writes the bytes, so the test does.
let body = b"pwned\n";
let mut header = tar::Header::new_gnu();
header.set_size(body.len() as u64);
header.set_mode(0o644);
header.set_entry_type(tar::EntryType::Regular);
{
let gnu = header.as_gnu_mut().expect("gnu header");
let evil = b"../ESCAPED";
gnu.name[..evil.len()].copy_from_slice(evil);
}
header.set_cksum();
let mut archive = Vec::new();
archive.extend_from_slice(header.as_bytes());
let mut block = [0u8; 512];
block[..body.len()].copy_from_slice(body);
archive.extend_from_slice(&block);
archive.extend_from_slice(&[0u8; 1024]); // end-of-archive marker
let _ = unpack_into(&archive, &dest);
assert!(
!canary.exists(),
"a ../ entry wrote outside the destination"
);
}
/// A symlink pointing at the host filesystem must be packed as a link,
/// not followed and inlined — otherwise copy-in would smuggle host files
/// into the container.
#[test]
fn symlinks_are_not_dereferenced_into_the_archive() {
let tmp = tempfile::tempdir().unwrap();
let src = tmp.path().join("repo");
seed(&src);
let secret = tmp.path().join("host-secret");
std::fs::write(&secret, "TOP SECRET\n").unwrap();
std::os::unix::fs::symlink(&secret, src.join("link")).unwrap();
let archive = pack_dir(&src, "repo").unwrap();
let haystack = String::from_utf8_lossy(&archive);
assert!(
!haystack.contains("TOP SECRET"),
"symlink target contents were inlined into the archive"
);
}
/// Only the exact word `bind` opts out. A typo must land on copy mode —
/// the path with a verification harness behind it — rather than silently
/// selecting the one with four documented work-loss incidents.
#[test]
fn only_the_exact_word_bind_opts_out() {
// Cannot set env vars in a test process without racing every other
// test, so this asserts the predicate the function is built from.
let opts_out = |v: &str| v == "bind";
assert!(opts_out("bind"));
for near_miss in ["Bind", "binds", "bound", "copy", "0", "false", ""] {
assert!(
!opts_out(near_miss),
"{near_miss:?} must NOT select the bind path"
);
}
}
/// The regression this exists for: a config large enough to blow `ARG_MAX`
/// via `sh -c` must round-trip untouched. 2 MB is well past the ~128 KB
/// limit that unpinned every agent in mission `019fcf62`.
#[test]
fn a_file_far_past_arg_max_round_trips() {
let big = "workspace_path = \"/mission/repo\"\n".repeat(64 * 1024);
assert!(big.len() > 2_000_000, "the fixture must exceed ARG_MAX");
let archive = single_file_archive("config.toml", big.as_bytes()).unwrap();
let tmp = tempfile::tempdir().unwrap();
unpack_into(&archive, tmp.path()).unwrap();
assert_eq!(
std::fs::read_to_string(tmp.path().join("config.toml")).unwrap(),
big,
"a large config must survive byte-for-byte"
);
}
/// TOML holding quotes, newlines and backslashes went through a shell
/// before; nothing may depend on quoting now.
#[test]
fn shell_metacharacters_survive_the_archive() {
let nasty = "path = \"/a'b\\\"c\"\n$(rm -rf /) `id` \\\\ \n";
let archive = single_file_archive("config.toml", nasty.as_bytes()).unwrap();
let tmp = tempfile::tempdir().unwrap();
unpack_into(&archive, tmp.path()).unwrap();
assert_eq!(
std::fs::read_to_string(tmp.path().join("config.toml")).unwrap(),
nasty
);
}
#[test]
fn an_empty_directory_packs_without_error() {
let tmp = tempfile::tempdir().unwrap();
let src = tmp.path().join("empty");
std::fs::create_dir_all(&src).unwrap();
let archive = pack_dir(&src, "repo").unwrap();
let dest = tmp.path().join("out");
unpack_into(&archive, &dest).unwrap();
assert!(dest.join("repo").is_dir());
}
}