fix(missions): let the server and the agent share one git checkout
ci / gates (push) Failing after 5s
ci / rust (push) Skipped
ci / frontend (push) Skipped
ci / e2e (push) Skipped
ci / publish (push) Skipped

Mission 019fc437 lost both phases' work to:

  git add → exit 128: insufficient permission for adding an object
            to repository database .git/objects

cm-api runs as uid 65532; the mission runtime container runs as root;
they share one bind-mounted checkout. Git's .git/objects/xx/ fan-out
directories inherit the ownership of whoever creates them, so an agent
that writes objects first locks the server out of those directories.

The failure is intermittent, which is why the previous run looked clean.
Mission 019fc42b's agents committed their own work, so the blobs already
existed and the server's `git add` never had to write one. Same template,
different agent behaviour, opposite outcome.

`core.sharedRepository` is git's own mechanism for this: objects and refs
are created group- and world-writable, and both parties read the setting
from the shared .git/config. It grants the agent nothing — it is already
root over the whole checkout — and unblocks the server, which was the
party being refused. Applied on clone and on checkout reuse.

Two supporting changes. The artifact now records `commit_error`: this
failure surfaced as `branch: null, push_error: null`, indistinguishable
from a phase that never had work to commit, with the reason only in host
stderr. And the test seeder now calls the production setup function
instead of reimplementing it — building the checkout by hand is what let
a clone-path defect stay invisible to fourteen tests.

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
Omar Sobh
2026-08-02 13:53:06 -07:00
co-authored by Claude Opus 5
parent 8bad869248
commit 5b53705c97
3 changed files with 148 additions and 0 deletions
+8
View File
@@ -236,6 +236,12 @@ pub async fn capture_phase_diff_at(
// Commit only after the patch is safely on disk. If this fails, the work // Commit only after the patch is safely on disk. If this fails, the work
// is still captured and the artifact still lands — the branch is the // is still captured and the artifact still lands — the branch is the
// convenience, the patch is the guarantee. // convenience, the patch is the guarantee.
// Why a phase has no branch belongs in the artifact, not only in the log.
// Mission `019fc437` recorded `branch: null, push_error: null` for both
// phases — indistinguishable from a phase that was never eligible to
// commit. The reason was in stderr on the host, where nothing reading the
// mission would find it.
let mut commit_error: Option<String> = None;
let committed = match commit_phase_work(&repo, mission_id, phase_id, iteration).await { let committed = match commit_phase_work(&repo, mission_id, phase_id, iteration).await {
Ok(c) => c, Ok(c) => c,
Err(e) => { Err(e) => {
@@ -243,6 +249,7 @@ pub async fn capture_phase_diff_at(
"mission_delivery: mission {mission_id} phase {phase_id} captured but not \ "mission_delivery: mission {mission_id} phase {phase_id} captured but not \
committed: {e}" committed: {e}"
); );
commit_error = Some(e.chars().take(500).collect());
None None
} }
}; };
@@ -298,6 +305,7 @@ pub async fn capture_phase_diff_at(
"tests_verified": verified, "tests_verified": verified,
"pushed": published.as_ref().map(|p| p.pushed), "pushed": published.as_ref().map(|p| p.pushed),
"push_error": published.as_ref().and_then(|p| p.error.clone()), "push_error": published.as_ref().and_then(|p| p.error.clone()),
"commit_error": commit_error,
"files_changed": files_changed, "files_changed": files_changed,
"insertions": insertions, "insertions": insertions,
"deletions": deletions, "deletions": deletions,
+59
View File
@@ -67,6 +67,9 @@ pub async fn ensure_checkout(
let auth_url = with_ambient_auth(clone_url); let auth_url = with_ambient_auth(clone_url);
if path.join(".git").exists() { if path.join(".git").exists() {
// Checkouts cloned before this setting existed get it on reuse. It
// governs objects created from now on, which is what delivery needs.
share_repository_across_uids(&path);
fetch_and_reset(&path, default_branch, &auth_url).await?; fetch_and_reset(&path, default_branch, &auth_url).await?;
} else { } else {
clone(&path, &auth_url).await?; clone(&path, &auth_url).await?;
@@ -119,12 +122,68 @@ async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
.collect::<String>() .collect::<String>()
)); ));
} }
share_repository_across_uids(path);
scrub_remote_credentials(path, url); scrub_remote_credentials(path, url);
ignore_agent_scaffolding(path); ignore_agent_scaffolding(path);
record_base_commit(path); record_base_commit(path);
Ok(()) Ok(())
} }
/// Let the server and the agent container both write to this checkout.
///
/// The checkout is one directory bind-mounted into two processes running as
/// different users: cm-api is uid 65532, the mission runtime container is
/// root. Git creates `.git/objects/xx/` fan-out directories on first write and
/// they inherit the writer's ownership, so whichever party commits first locks
/// the other out of that directory:
///
/// ```text
/// git add → exit 128: insufficient permission for adding an object
/// to repository database .git/objects
/// ```
///
/// The failure is intermittent, which is what makes it dangerous. Mission
/// `019fc42b` delivered cleanly because its agents committed their own work,
/// so the blobs already existed and the server's `git add` never had to write
/// one. Mission `019fc437` ran the same template, its agents left the work
/// uncommitted, and delivery lost both phases.
///
/// `core.sharedRepository` is git's own answer to a repository shared between
/// users: it makes git create objects and refs group- and world-writable. Both
/// parties read this config from the shared `.git/config`, so it governs the
/// agent's commits as much as ours.
///
/// This grants the agent no access it lacks. It is already root inside a
/// container with the entire checkout bind-mounted read-write, and could
/// rewrite any of it. The party actually gaining something is the server,
/// which is currently the one being locked out.
pub fn share_repository_across_uids(path: &std::path::Path) {
let out = std::process::Command::new("git")
.args([
"-C",
&path.display().to_string(),
"-c",
&format!("safe.directory={}", path.display()),
"config",
"core.sharedRepository",
"0777",
])
.output();
match out {
Ok(o) if o.status.success() => {}
Ok(o) => eprintln!(
"mission_workspace: could not set core.sharedRepository on {} ({}) — delivery \
may fail to commit if the agent writes git objects first",
path.display(),
String::from_utf8_lossy(&o.stderr).trim()
),
Err(e) => eprintln!(
"mission_workspace: could not set core.sharedRepository on {} ({e})",
path.display()
),
}
}
/// Remember the commit the mission started from. /// Remember the commit the mission started from.
/// ///
/// Delivery needs to answer "what did this mission change", and the obvious /// Delivery needs to answer "what did this mission change", and the obvious
+81
View File
@@ -71,6 +71,9 @@ fn seed_repo(root: &Path, mission: Uuid) -> std::path::PathBuf {
std::fs::write(repo.join("README.md"), "# base\n").unwrap(); std::fs::write(repo.join("README.md"), "# base\n").unwrap();
git(&repo, &["add", "."]); git(&repo, &["add", "."]);
git(&repo, &["commit", "--quiet", "-m", "base"]); git(&repo, &["commit", "--quiet", "-m", "base"]);
// The real clone path applies this; seeding a repo by hand and skipping it
// is what let the uid-split failure reach production untested.
cm_api::mission_workspace::share_repository_across_uids(&repo);
record_base(&repo); record_base(&repo);
repo repo
} }
@@ -655,3 +658,81 @@ async fn a_later_phase_reports_only_its_own_work() {
assert!(listed.contains("ALPHA.md"), "branch is cumulative: {listed}"); assert!(listed.contains("ALPHA.md"), "branch is cumulative: {listed}");
assert!(listed.contains("BETA.md"), "branch is cumulative: {listed}"); assert!(listed.contains("BETA.md"), "branch is cumulative: {listed}");
} }
/// A checkout must stay writable after another user has written to it.
///
/// The production failure (mission `019fc437`) is a uid split: cm-api runs as
/// 65532, the mission runtime container runs as root, and they share one
/// checkout. Git's `.git/objects/xx/` fan-out directories inherit the
/// ownership of whoever creates them, so the agent committing first locked the
/// server out — `git add` returned "insufficient permission for adding an
/// object to repository database".
///
/// A test process cannot become two users, so this asserts the mechanism that
/// makes the two-user case work: the clone sets `core.sharedRepository`, and
/// objects git writes afterwards are group- and world-writable. Without that
/// mode bit the second user is refused regardless of which one arrived first.
#[tokio::test]
async fn a_checkout_is_writable_by_both_uids_that_share_it() {
use std::os::unix::fs::PermissionsExt;
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;
let shared = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["config", "core.sharedRepository"])
.output()
.unwrap();
assert_eq!(
String::from_utf8_lossy(&shared.stdout).trim(),
"0777",
"the checkout must be marked shared, or a second uid cannot write objects"
);
// Only directories created *after* the setting can carry its mode, and
// only those matter: the clone writes its own objects before any config
// exists, but the party that would be blocked by them is the container,
// which runs as root and ignores permission bits. The failing direction is
// the other one — directories the agent creates later, which the server
// must still be able to write into. Snapshot first, then diff.
let objects = repo.join(".git/objects");
let fanout = |dir: &std::path::Path| -> std::collections::HashSet<String> {
std::fs::read_dir(dir)
.map(|rd| {
rd.filter_map(|e| e.ok())
.map(|e| e.file_name().to_string_lossy().into_owned())
.filter(|n| n.len() == 2 && n.chars().all(|c| c.is_ascii_hexdigit()))
.collect()
})
.unwrap_or_default()
};
let before = fanout(&objects);
std::fs::write(repo.join("SHARED.md"), "SHARED\n").unwrap();
capture(&pool, tmp.path(), mission, phase)
.await
.unwrap()
.unwrap();
let mut checked = 0;
for name in fanout(&objects).difference(&before) {
let mode = std::fs::metadata(objects.join(name))
.unwrap()
.permissions()
.mode()
& 0o777;
assert_eq!(
mode & 0o022,
0o022,
"{name} is {mode:o}; the other uid sharing this checkout could not \
write objects into it"
);
checked += 1;
}
assert!(checked > 0, "no object directories were created to check");
}