refactor: the missions root has one definition, not five
`mission_workspace::missions_root()` is now the only place that answers "where
does mission state live". It had fragmented into five: this function, private
`env::var("CLAWMATES_MISSIONS_ROOT")` copies in security_scan, benchmark_runner
and mission_outputs, and a hardcoded `MISSIONS_HOST_ROOT` const in
mission_runtime that read no env at all.
They agree on the deployed value, so nothing has broken. The risk is entirely
in what comes next: anything that sweeps or reclaims this tree has to be
sweeping the same tree the writers use, and five definitions cannot promise
that — a reaper written against one would silently leave the others' directories
behind forever, which is how the orphans got there in the first place.
A source-walk test fails any module outside `mission_workspace` that reads the
env var itself.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e5f097c291
commit
91fbd2dc88
@@ -277,9 +277,7 @@ async fn exec_target(
|
|||||||
}
|
}
|
||||||
let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER")
|
let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER")
|
||||||
.unwrap_or_else(|_| "clawmates-runtime".to_string());
|
.unwrap_or_else(|_| "clawmates-runtime".to_string());
|
||||||
let root = std::env::var("CLAWMATES_MISSIONS_ROOT")
|
let workdir = crate::mission_workspace::missions_root()
|
||||||
.unwrap_or_else(|_| "/var/lib/clawmates-missions".to_string());
|
|
||||||
let workdir = std::path::PathBuf::from(root)
|
|
||||||
.join(mission_id.to_string())
|
.join(mission_id.to_string())
|
||||||
.join("repo");
|
.join("repo");
|
||||||
Ok((container, workdir))
|
Ok((container, workdir))
|
||||||
|
|||||||
@@ -305,9 +305,7 @@ pub fn outputs_root_dir() -> PathBuf {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn missions_root() -> PathBuf {
|
fn missions_root() -> PathBuf {
|
||||||
std::env::var("CLAWMATES_MISSIONS_ROOT")
|
crate::mission_workspace::missions_root()
|
||||||
.map(PathBuf::from)
|
|
||||||
.unwrap_or_else(|_| PathBuf::from("/var/lib/clawmates-missions"))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn mime_for(p: &Path) -> &'static str {
|
fn mime_for(p: &Path) -> &'static str {
|
||||||
|
|||||||
@@ -299,7 +299,16 @@ const EDGE_NETWORK: &str = "clawmates_edge";
|
|||||||
/// Host path (as seen by the docker engine, NOT the server container)
|
/// Host path (as seen by the docker engine, NOT the server container)
|
||||||
/// where the mission's checkouts live. Matches the mount source used
|
/// where the mission's checkouts live. Matches the mount source used
|
||||||
/// by `clawmates-runtime.service`.
|
/// by `clawmates-runtime.service`.
|
||||||
const MISSIONS_HOST_ROOT: &str = "/var/lib/clawmates-missions";
|
///
|
||||||
|
/// This was a hardcoded const that read no env at all, while every writer of
|
||||||
|
/// the same tree went through `CLAWMATES_MISSIONS_ROOT`. They agree on this
|
||||||
|
/// deployment; they are not guaranteed to, and a reaper pointed at one of them
|
||||||
|
/// would have silently left the other's directories behind forever.
|
||||||
|
fn missions_host_root() -> String {
|
||||||
|
crate::mission_workspace::missions_root()
|
||||||
|
.to_string_lossy()
|
||||||
|
.into_owned()
|
||||||
|
}
|
||||||
|
|
||||||
/// Host path holding the shared ZeroClaw config + seeded agent library
|
/// Host path holding the shared ZeroClaw config + seeded agent library
|
||||||
/// (built up over time by the shared clawmates-runtime.service). Per-
|
/// (built up over time by the shared clawmates-runtime.service). Per-
|
||||||
@@ -550,7 +559,7 @@ impl MissionRuntimeProvisioner {
|
|||||||
// Create fresh. Ensure the bind source exists first — research-
|
// Create fresh. Ensure the bind source exists first — research-
|
||||||
// only missions (no repo checkout) still need the directory
|
// only missions (no repo checkout) still need the directory
|
||||||
// present or docker start fails with EACCES/ENOENT.
|
// present or docker start fails with EACCES/ENOENT.
|
||||||
let mission_dir = format!("{MISSIONS_HOST_ROOT}/{mission_id}");
|
let mission_dir = format!("{}/{mission_id}", missions_host_root());
|
||||||
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());
|
||||||
@@ -982,7 +991,7 @@ impl MissionRuntimeProvisioner {
|
|||||||
}
|
}
|
||||||
// Remove the per-mission workspace dir (repo checkout + scratch). This
|
// Remove the per-mission workspace dir (repo checkout + scratch). This
|
||||||
// path is bind-mounted into cm-api, so we can reap it directly.
|
// path is bind-mounted into cm-api, so we can reap it directly.
|
||||||
let mission_dir = format!("{MISSIONS_HOST_ROOT}/{mission_id}");
|
let mission_dir = format!("{}/{mission_id}", missions_host_root());
|
||||||
if let Err(e) = tokio::fs::remove_dir_all(&mission_dir).await {
|
if let Err(e) = tokio::fs::remove_dir_all(&mission_dir).await {
|
||||||
if e.kind() != std::io::ErrorKind::NotFound {
|
if e.kind() != std::io::ErrorKind::NotFound {
|
||||||
eprintln!("mission_runtime: rm workspace dir {mission_dir}: {e}");
|
eprintln!("mission_runtime: rm workspace dir {mission_dir}: {e}");
|
||||||
@@ -1480,11 +1489,12 @@ allowed_tools = ["file_read", "file_edit"]
|
|||||||
fn runtime_data_is_scoped_to_one_mission() {
|
fn runtime_data_is_scoped_to_one_mission() {
|
||||||
let a = Uuid::now_v7();
|
let a = Uuid::now_v7();
|
||||||
let b = Uuid::now_v7();
|
let b = Uuid::now_v7();
|
||||||
let path = |id: Uuid| format!("{MISSIONS_HOST_ROOT}/{id}/runtime-data");
|
let root = missions_host_root();
|
||||||
|
let path = |id: Uuid| format!("{root}/{id}/runtime-data");
|
||||||
|
|
||||||
assert_ne!(path(a), path(b), "two missions must not share runtime data");
|
assert_ne!(path(a), path(b), "two missions must not share runtime data");
|
||||||
assert!(
|
assert!(
|
||||||
path(a).starts_with(&format!("{MISSIONS_HOST_ROOT}/{a}")),
|
path(a).starts_with(&format!("{root}/{a}")),
|
||||||
"runtime data must live under the mission dir so teardown removes it"
|
"runtime data must live under the mission dir so teardown removes it"
|
||||||
);
|
);
|
||||||
assert_ne!(
|
assert_ne!(
|
||||||
|
|||||||
@@ -26,7 +26,17 @@ use std::path::PathBuf;
|
|||||||
use tokio::process::Command;
|
use tokio::process::Command;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
pub(crate) fn missions_root() -> PathBuf {
|
/// The ONE definition of where mission state lives on the docker host.
|
||||||
|
///
|
||||||
|
/// There used to be five: this function, three private copies of the same
|
||||||
|
/// `env::var(...).unwrap_or(...)` in `security_scan`, `benchmark_runner` and
|
||||||
|
/// `mission_outputs`, and a hardcoded `MISSIONS_HOST_ROOT` const in
|
||||||
|
/// `mission_runtime` that read no env at all. They agree on today's
|
||||||
|
/// deployment, which is why nothing had broken — but anything that sweeps or
|
||||||
|
/// reclaims this tree has to be sure it is sweeping the same tree the writers
|
||||||
|
/// use, and five definitions cannot promise that. A GC written against one of
|
||||||
|
/// them would silently miss the others.
|
||||||
|
pub 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"))
|
||||||
@@ -1074,4 +1084,43 @@ mod tests {
|
|||||||
mark_phase_started(repo);
|
mark_phase_started(repo);
|
||||||
assert!(checkout_in_use(repo));
|
assert!(checkout_in_use(repo));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Nobody re-derives the missions root.
|
||||||
|
///
|
||||||
|
/// It had fragmented into five definitions — this function, three private
|
||||||
|
/// `env::var("CLAWMATES_MISSIONS_ROOT")` copies, and a hardcoded const
|
||||||
|
/// that read no env at all. They agreed on the deployed value, so nothing
|
||||||
|
/// ever broke; the risk is entirely in what comes next. Anything that
|
||||||
|
/// sweeps, reclaims or reaps this tree has to be sweeping the same tree the
|
||||||
|
/// writers use, and five definitions cannot promise that.
|
||||||
|
#[test]
|
||||||
|
fn the_missions_root_has_exactly_one_definition() {
|
||||||
|
fn walk(dir: &std::path::Path, out: &mut Vec<std::path::PathBuf>) {
|
||||||
|
for entry in std::fs::read_dir(dir).expect("readable source dir") {
|
||||||
|
let path = entry.expect("readable entry").path();
|
||||||
|
if path.is_dir() {
|
||||||
|
walk(&path, out);
|
||||||
|
} else if path.extension().is_some_and(|e| e == "rs") {
|
||||||
|
out.push(path);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
|
||||||
|
let mut files = Vec::new();
|
||||||
|
walk(&root, &mut files);
|
||||||
|
|
||||||
|
for path in files {
|
||||||
|
if path.ends_with("mission_workspace.rs") {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let src = std::fs::read_to_string(&path).expect("readable source");
|
||||||
|
assert!(
|
||||||
|
!src.contains("var(\"CLAWMATES_MISSIONS_ROOT\")"),
|
||||||
|
"{} reads CLAWMATES_MISSIONS_ROOT itself — call \
|
||||||
|
`mission_workspace::missions_root()` so a reaper and a writer \
|
||||||
|
cannot disagree about which tree they are looking at",
|
||||||
|
path.display()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -314,9 +314,7 @@ async fn exec_target(pool: &PgPool, mission_id: Uuid) -> Result<(String, PathBuf
|
|||||||
}
|
}
|
||||||
let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER")
|
let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER")
|
||||||
.unwrap_or_else(|_| "clawmates-runtime".to_string());
|
.unwrap_or_else(|_| "clawmates-runtime".to_string());
|
||||||
let root = std::env::var("CLAWMATES_MISSIONS_ROOT")
|
let workdir = crate::mission_workspace::missions_root()
|
||||||
.unwrap_or_else(|_| "/var/lib/clawmates-missions".to_string());
|
|
||||||
let workdir = PathBuf::from(root)
|
|
||||||
.join(mission_id.to_string())
|
.join(mission_id.to_string())
|
||||||
.join("repo");
|
.join("repo");
|
||||||
Ok((container, workdir))
|
Ok((container, workdir))
|
||||||
|
|||||||
Reference in New Issue
Block a user