fix(missions): a root-owned COMMIT_EDITMSG must not block delivery
Mission 019fcd0c produced correct work — a reviewed, tested function plus a REVIEW.md quoting a real cargo test summary — and delivered none of it: git commit → exit 128: could not open '.git/COMMIT_EDITMSG': Permission denied The agent ran `git commit` itself inside the mission container (as root), leaving that file owned by root at 0644. core.sharedRepository covers objects and refs — .git/index lands at 0666, which is why commits work at all — but not COMMIT_EDITMSG, which git writes with the default umask. Unlinking works where overwriting does not: removing a file needs write permission on the DIRECTORY, and .git/ is owned by the server. Silent on failure by design, so the commit reports the real error rather than this speculative cleanup. Third distinct instance of the same uid-split class (objects, then the capture base, now this). The pattern holds: the checkout is one directory written by two users, and each new file git touches is a new opportunity. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
deb60be98d
commit
4ff4e6f7ee
@@ -583,6 +583,7 @@ pub async fn commit_phase_work(
|
|||||||
String::new()
|
String::new()
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
clear_stale_commit_editmsg(repo);
|
||||||
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -878,6 +879,34 @@ impl TestOutcome {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Remove a `COMMIT_EDITMSG` the agent left behind as root.
|
||||||
|
///
|
||||||
|
/// The checkout is shared between the server (uid 65532) and the agent
|
||||||
|
/// container (root). `core.sharedRepository` makes git create *objects and
|
||||||
|
/// refs* group-writable — `.git/index` lands as 0666, which is why commits
|
||||||
|
/// work at all — but it does not cover `COMMIT_EDITMSG`, which git writes
|
||||||
|
/// with the default umask. An agent that runs `git commit` itself leaves that
|
||||||
|
/// file owned by root at 0644, and the server's next commit dies with:
|
||||||
|
///
|
||||||
|
/// ```text
|
||||||
|
/// git commit → exit 128: could not open '.git/COMMIT_EDITMSG': Permission denied
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Observed on mission `019fcd0c`, which produced correct work — a reviewed,
|
||||||
|
/// tested function plus a REVIEW.md quoting a real `cargo test` summary — and
|
||||||
|
/// then delivered none of it.
|
||||||
|
///
|
||||||
|
/// Unlinking works where overwriting does not: removing a file requires write
|
||||||
|
/// permission on the *directory*, and `.git/` is owned by the server. Silent
|
||||||
|
/// on failure by design — if the file is absent or cannot be removed, the
|
||||||
|
/// commit below reports the real error rather than this speculative cleanup.
|
||||||
|
fn clear_stale_commit_editmsg(repo: &Path) {
|
||||||
|
let msg = repo.join(".git/COMMIT_EDITMSG");
|
||||||
|
if msg.exists() {
|
||||||
|
let _ = std::fs::remove_file(&msg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Mark a phase as impossible to capture, so it stops being selected.
|
/// Mark a phase as impossible to capture, so it stops being selected.
|
||||||
///
|
///
|
||||||
/// A phase whose checkout has already been reaped can never be captured. It
|
/// A phase whose checkout has already been reaped can never be captured. It
|
||||||
|
|||||||
@@ -876,3 +876,43 @@ async fn an_unrunnable_suite_is_distinguishable_from_no_suite() {
|
|||||||
"the two must be distinguishable — this is the whole point"
|
"the two must be distinguishable — this is the whole point"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A COMMIT_EDITMSG left by the agent must not block delivery.
|
||||||
|
///
|
||||||
|
/// From mission 019fcd0c: the agent ran `git commit` itself, leaving
|
||||||
|
/// `.git/COMMIT_EDITMSG` owned by root at 0644, and the server's commit died
|
||||||
|
/// with "Permission denied". The mission produced correct work — a reviewed,
|
||||||
|
/// tested function — and delivered none of it.
|
||||||
|
///
|
||||||
|
/// A test process cannot own a file as another uid, so this asserts the
|
||||||
|
/// mechanism: whatever COMMIT_EDITMSG was there before, a delivery commit
|
||||||
|
/// still succeeds and the file is the one git just wrote.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_stale_commit_editmsg_does_not_block_delivery() {
|
||||||
|
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;
|
||||||
|
|
||||||
|
// Stand in for the agent's leftover: content that must not survive.
|
||||||
|
let msg = repo.join(".git/COMMIT_EDITMSG");
|
||||||
|
std::fs::write(&msg, "LEFTOVER FROM THE AGENT\n").unwrap();
|
||||||
|
|
||||||
|
std::fs::write(repo.join("WORK.md"), "work\n").unwrap();
|
||||||
|
let cap = capture(&pool, tmp.path(), mission, phase)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let commit = cap
|
||||||
|
.committed
|
||||||
|
.expect("delivery must commit despite a stale COMMIT_EDITMSG");
|
||||||
|
assert!(!commit.sha.is_empty());
|
||||||
|
|
||||||
|
let body = std::fs::read_to_string(&msg).unwrap_or_default();
|
||||||
|
assert!(
|
||||||
|
!body.contains("LEFTOVER FROM THE AGENT"),
|
||||||
|
"the stale message survived: {body:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user