feat(missions): make copy-in/copy-out the default filesystem model
Copy mode shipped opt-in so that changing how every mission receives its
code required someone to type it. Four production missions and a
fail-closed harness later, 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 runs whenever nobody sets the
variable. `CLAWMATES_MISSION_FS=bind` still selects it; anything else —
unset, empty, misspelt — gets copy mode, so a typo lands on the safer
path rather than the one being retired.
Also fixes a real leak found while scoping the deletion below: the git
helper built its `safe.directory` argument with `Box::leak`, justified as
"the process is short-lived". That is true of a CLI and false of cm-api,
which is a long-running server — so it leaked one allocation per git
call, growing with every phase of every mission.
The A5 deletion is NOT done here, and two of its items should never be
done:
- `scrub_remote_credentials` is a security control, not a uid
workaround. Copy mode uploads the whole `.git` into a container the
agent controls as root, which makes stripping the token from
`.git/config` more necessary, not less.
- `has_local_work` / `checkout_in_use` guard `fetch_and_reset` at every
phase launch and have nothing to do with who writes the checkout.
The host checkout still persists across phases under copy mode —
mission `019fcf62` shows the marker firing there. Deleting them
reintroduces PRIOR-PHASE-WORK-WAS-LOST.
The rest (`share_repository_across_uids`, `clear_stale_commit_editmsg`,
`-c safe.directory`) are genuinely obsolete under copy mode but stay
while `bind` remains selectable: a workaround may only be deleted once
the situation it works around can no longer be chosen.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3c91d0e172
commit
4f6719c80e
@@ -470,22 +470,27 @@ pub async fn capture_phase_diff_at(
|
|||||||
|
|
||||||
/// Run git in `repo`, returning stdout.
|
/// Run git in `repo`, returning stdout.
|
||||||
///
|
///
|
||||||
/// Every invocation carries `-c safe.directory`: the server clones as uid
|
/// Every invocation carries `-c safe.directory`: under `CLAWMATES_MISSION_FS=bind`
|
||||||
/// 65532 while agents write into the same tree as root, so without it git
|
/// the server clones as uid 65532 while agents write into the same tree as
|
||||||
/// refuses the repository outright — the failure that had the phase evaluator
|
/// root, so without it git refuses the repository outright — the failure that
|
||||||
/// silently falling back to guesswork.
|
/// had the phase evaluator silently falling back to guesswork. Copy mode makes
|
||||||
|
/// the tree single-uid and this redundant, but it stays while the bind path is
|
||||||
|
/// still selectable: a workaround may only be deleted once the situation it
|
||||||
|
/// works around can no longer be chosen.
|
||||||
async fn git(repo: &Path, args: &[&str]) -> Result<String, String> {
|
async fn git(repo: &Path, args: &[&str]) -> Result<String, String> {
|
||||||
let repo_s = repo.display().to_string();
|
let repo_s = repo.display().to_string();
|
||||||
let mut full = vec![
|
// Owned, not `Box::leak`. The leak was justified as "the process is
|
||||||
"-C",
|
// short-lived", which is true of a CLI and false of cm-api — it is a
|
||||||
&repo_s,
|
// long-running server, so that was one permanently leaked allocation per
|
||||||
"-c",
|
// git call, growing with every phase of every mission for the life of the
|
||||||
// Leaked into a `String` so it can live in a `&str` slice alongside
|
// process.
|
||||||
// the borrowed args; the process is short-lived and this is one
|
let mut full: Vec<String> = vec![
|
||||||
// allocation per git call.
|
"-C".into(),
|
||||||
Box::leak(format!("safe.directory={repo_s}").into_boxed_str()),
|
repo_s.clone(),
|
||||||
|
"-c".into(),
|
||||||
|
format!("safe.directory={repo_s}"),
|
||||||
];
|
];
|
||||||
full.extend_from_slice(args);
|
full.extend(args.iter().map(|a| (*a).to_string()));
|
||||||
let (name, email) = commit_identity();
|
let (name, email) = commit_identity();
|
||||||
let out = tokio::process::Command::new("git")
|
let out = tokio::process::Command::new("git")
|
||||||
.args(&full)
|
.args(&full)
|
||||||
|
|||||||
@@ -167,14 +167,20 @@ pub async fn copy_out(
|
|||||||
|
|
||||||
/// Is the copy-in/copy-out filesystem model enabled?
|
/// Is the copy-in/copy-out filesystem model enabled?
|
||||||
///
|
///
|
||||||
/// Opt-in. The bind-mount path is what production has run since the beginning,
|
/// **Default since 2026-08-04.** It shipped opt-in, on the principle that
|
||||||
/// and silently changing how every mission receives its code is exactly the
|
/// silently changing how every mission receives its code should require
|
||||||
/// class of change that should require someone to have typed it.
|
/// 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 {
|
pub fn copy_mode() -> bool {
|
||||||
matches!(
|
!matches!(std::env::var("CLAWMATES_MISSION_FS").as_deref(), Ok("bind"))
|
||||||
std::env::var("CLAWMATES_MISSION_FS").as_deref(),
|
|
||||||
Ok("copy")
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Host directory holding a mission's checkout.
|
/// Host directory holding a mission's checkout.
|
||||||
@@ -305,12 +311,20 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The switch must be explicit — a near-miss value leaves production on
|
/// Only the exact word `bind` opts out. A typo must land on copy mode —
|
||||||
/// the proven bind-mount path rather than silently changing it.
|
/// the path with a verification harness behind it — rather than silently
|
||||||
|
/// selecting the one with four documented work-loss incidents.
|
||||||
#[test]
|
#[test]
|
||||||
fn copy_mode_requires_the_exact_word() {
|
fn only_the_exact_word_bind_opts_out() {
|
||||||
for wrong in ["Copy", "copies", "bind", "1", "true", ""] {
|
// Cannot set env vars in a test process without racing every other
|
||||||
assert_ne!(wrong, "copy", "{wrong:?} must not enable copy mode");
|
// 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"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user