feat(missions): wire copy-in/copy-out behind CLAWMATES_MISSION_FS=copy
With the flag set, ensure_container omits the /mission bind, the checkout is pushed into the container at phase launch, and the agent's work is pulled back before capture. The simplification that makes this small: sync_out unpacks over the SAME host path the checkout came from. The host directory stays a server-owned staging area with exactly one writer, and capture_phase_diff_at needs no change at all — it still finds a normal checkout exactly where it always has. Delivery, gating, commit and push are untouched. Two failures are deliberately loud rather than silent: - copy-IN failure fails the phase launch. Continuing would start a phase against an empty directory, and the agent would cheerfully report having done work on a repo that was not there. - copy-OUT failure SKIPS capture. Capturing anyway would diff a stale host tree and record "no changes" for work that exists — success reported for nothing, which is the exact failure mode this codebase keeps paying for. Opt-in: the bind path is what production has run since the beginning, and the test asserts a near-miss value leaves it there rather than silently switching every mission. 414 tests, clippy clean. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
389b41f8e6
commit
7e07c389c6
@@ -111,6 +111,54 @@ pub async fn copy_out(
|
|||||||
unpack_into(&archive, dest)
|
unpack_into(&archive, dest)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Is the copy-in/copy-out filesystem model enabled?
|
||||||
|
///
|
||||||
|
/// Opt-in. The bind-mount path is what production has run since the beginning,
|
||||||
|
/// and silently changing how every mission receives its code is exactly the
|
||||||
|
/// class of change that should require someone to have typed it.
|
||||||
|
pub fn copy_mode() -> bool {
|
||||||
|
matches!(
|
||||||
|
std::env::var("CLAWMATES_MISSION_FS").as_deref(),
|
||||||
|
Ok("copy")
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// 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.
|
||||||
|
///
|
||||||
|
/// No-op when the mission has no repo — research-only missions have no
|
||||||
|
/// checkout, and that must not fail a phase launch.
|
||||||
|
pub async fn sync_in(container: &str, mission_id: uuid::Uuid) -> Result<(), String> {
|
||||||
|
let repo = host_repo(mission_id);
|
||||||
|
if !repo.is_dir() {
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
let docker = crate::container_exec::connect()?;
|
||||||
|
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -203,6 +251,15 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The switch must be explicit — a near-miss value leaves production on
|
||||||
|
/// the proven bind-mount path rather than silently changing it.
|
||||||
|
#[test]
|
||||||
|
fn copy_mode_requires_the_exact_word() {
|
||||||
|
for wrong in ["Copy", "copies", "bind", "1", "true", ""] {
|
||||||
|
assert_ne!(wrong, "copy", "{wrong:?} must not enable copy mode");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn an_empty_directory_packs_without_error() {
|
fn an_empty_directory_packs_without_error() {
|
||||||
let tmp = tempfile::tempdir().unwrap();
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
@@ -403,16 +403,21 @@ impl MissionRuntimeProvisioner {
|
|||||||
let runtime_data_dir = format!("{mission_dir}/runtime-data");
|
let runtime_data_dir = format!("{mission_dir}/runtime-data");
|
||||||
let _ = tokio::fs::create_dir_all(&runtime_data_dir).await;
|
let _ = tokio::fs::create_dir_all(&runtime_data_dir).await;
|
||||||
seed_runtime_data(&self.docker, &self.image, &seed_dir, &runtime_data_dir).await?;
|
seed_runtime_data(&self.docker, &self.image, &seed_dir, &runtime_data_dir).await?;
|
||||||
let mounts = vec![
|
let mut mounts = Vec::new();
|
||||||
// Mount just this mission's directory. Agents can navigate
|
// In copy mode the checkout is pushed in and pulled back out, so the
|
||||||
// its `/repo` subdir but never see other missions'.
|
// container gets its OWN filesystem and the host directory has exactly
|
||||||
Mount {
|
// one writer (the server). Binding it here would put two uids back on
|
||||||
|
// one directory — the cause of four separate work-loss bugs.
|
||||||
|
if !crate::mission_fs::copy_mode() {
|
||||||
|
mounts.push(Mount {
|
||||||
target: Some("/mission".to_string()),
|
target: Some("/mission".to_string()),
|
||||||
source: Some(mission_dir.clone()),
|
source: Some(mission_dir.clone()),
|
||||||
typ: Some(MountTypeEnum::BIND),
|
typ: Some(MountTypeEnum::BIND),
|
||||||
read_only: Some(false),
|
read_only: Some(false),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
},
|
});
|
||||||
|
}
|
||||||
|
mounts.extend([
|
||||||
// This mission's OWN copy of the seeded agent library
|
// This mission's OWN copy of the seeded agent library
|
||||||
// (`claw_*` templates). Copied rather than shared, so its
|
// (`claw_*` templates). Copied rather than shared, so its
|
||||||
// config.toml — which carries the door bearer token — and its
|
// config.toml — which carries the door bearer token — and its
|
||||||
@@ -425,7 +430,7 @@ impl MissionRuntimeProvisioner {
|
|||||||
read_only: Some(false),
|
read_only: Some(false),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
},
|
},
|
||||||
];
|
]);
|
||||||
|
|
||||||
let host_config = HostConfig {
|
let host_config = HostConfig {
|
||||||
mounts: Some(mounts),
|
mounts: Some(mounts),
|
||||||
|
|||||||
@@ -112,6 +112,23 @@ async fn capture_finished_coding_phases(pool: &PgPool) -> Result<(), String> {
|
|||||||
use sqlx::Row;
|
use sqlx::Row;
|
||||||
let phase_id: Uuid = row.get("id");
|
let phase_id: Uuid = row.get("id");
|
||||||
let mission_id: Uuid = row.get("mission_id");
|
let mission_id: Uuid = row.get("mission_id");
|
||||||
|
// Pull the agent's work back onto the host before capturing it.
|
||||||
|
// Unpacks over the same checkout path, so capture below is unchanged.
|
||||||
|
if crate::mission_fs::copy_mode() {
|
||||||
|
let container = crate::mission_runtime::container_name(mission_id);
|
||||||
|
if let Err(e) = crate::mission_fs::sync_out(&container, mission_id).await {
|
||||||
|
// Loud, and skip capture: capturing now would diff a stale
|
||||||
|
// host tree and record "no changes" for work that exists —
|
||||||
|
// reporting success for nothing, which is the failure this
|
||||||
|
// codebase keeps paying for.
|
||||||
|
eprintln!(
|
||||||
|
"phase_runner: could NOT collect work from {container} for phase \
|
||||||
|
{phase_id} ({e}) — skipping capture so a stale tree is not \
|
||||||
|
recorded as an empty diff"
|
||||||
|
);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
match crate::mission_delivery::capture_phase_diff(pool, mission_id, phase_id).await {
|
match crate::mission_delivery::capture_phase_diff(pool, mission_id, phase_id).await {
|
||||||
Ok(Some(_)) => {}
|
Ok(Some(_)) => {}
|
||||||
Ok(None) => {
|
Ok(None) => {
|
||||||
@@ -299,6 +316,15 @@ async fn launch_phase(pool: &PgPool, p: PhaseLaunch<'_>) -> Result<(), String> {
|
|||||||
match prov.ensure_container(mission_id).await {
|
match prov.ensure_container(mission_id).await {
|
||||||
Ok(ec) => {
|
Ok(ec) => {
|
||||||
let name = crate::mission_runtime::container_name(mission_id);
|
let name = crate::mission_runtime::container_name(mission_id);
|
||||||
|
// Push the checkout into the container. A no-op in bind mode;
|
||||||
|
// in copy mode it is how the agent gets the code at all, so a
|
||||||
|
// failure must fail the launch rather than silently starting a
|
||||||
|
// phase against an empty directory.
|
||||||
|
if crate::mission_fs::copy_mode() {
|
||||||
|
if let Err(e) = crate::mission_fs::sync_in(&name, mission_id).await {
|
||||||
|
return Err(format!("copy checkout into {name}: {e}"));
|
||||||
|
}
|
||||||
|
}
|
||||||
if let Err(e) = cm_db::repo::missions::set_runtime_binding(
|
if let Err(e) = cm_db::repo::missions::set_runtime_binding(
|
||||||
pool,
|
pool,
|
||||||
mission_id,
|
mission_id,
|
||||||
|
|||||||
Reference in New Issue
Block a user