feat(missions): commit captured work to a branch of its own
Second half of delivery, minus the push. After the patch is on disk and the artifact registered, the phase's work is committed onto `clawmates/mission-<mission8>-<phase8>`, with `-i<N>` for re-runs so a second pass cannot collide with the first. Three rules hold throughout: - Never the default branch. The name is derived from the mission and phase, so a mission can only ever add a ref nobody else owns. - Never force. A rejected update gets reported, not overwritten. - The same exclusions as capture. What was too noisy for a patch is too noisy for someone's history — build output, vendored trees, and the workaround files agents write when infrastructure fights them. A test drops a 50 KB binary in `target/` and a `.gitconfig_temp` beside the real change and asserts neither is committed. Ordering is deliberate: commit runs *after* capture, and a commit failure is logged without failing the capture. The patch is the guarantee; the branch is the convenience on top. The branch is created even when there is nothing to stage, because agents often commit their own work — `rust_sdlc` has a committer role — and that commit is unreachable once the checkout is reaped unless a ref points at it. One test changed meaning rather than breaking: it asserted capture left the working tree untouched, which was correct while capture stood alone. Capture now commits, so it asserts the new invariant — work on a namespaced branch, a clean tree, and the created file present in the commit. Push is still deliberately absent. Everything here is local, so a bug costs a retry rather than reaching a remote. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a0e6b16abc
commit
ca1fd46e08
@@ -79,6 +79,15 @@ pub struct Capture {
|
|||||||
pub empty: bool,
|
pub empty: bool,
|
||||||
pub truncated: bool,
|
pub truncated: bool,
|
||||||
pub patch_path: PathBuf,
|
pub patch_path: PathBuf,
|
||||||
|
/// Set once the work has been committed to a mission branch.
|
||||||
|
pub committed: Option<Commit>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A commit made on the mission's own branch.
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct Commit {
|
||||||
|
pub branch: String,
|
||||||
|
pub sha: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Where a mission's durable output lives. Sibling of the swept per-mission
|
/// Where a mission's durable output lives. Sibling of the swept per-mission
|
||||||
@@ -99,12 +108,22 @@ pub async fn capture_phase_diff(
|
|||||||
mission_id: Uuid,
|
mission_id: Uuid,
|
||||||
phase_id: Uuid,
|
phase_id: Uuid,
|
||||||
) -> Result<Option<Capture>, String> {
|
) -> Result<Option<Capture>, String> {
|
||||||
|
// The phase's pass number, so a re-run lands on its own branch instead of
|
||||||
|
// colliding with the previous attempt.
|
||||||
|
let iteration: i32 = sqlx::query_scalar("SELECT iteration FROM mission_phases WHERE id = $1")
|
||||||
|
.bind(phase_id)
|
||||||
|
.fetch_optional(pool)
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.flatten()
|
||||||
|
.unwrap_or(0);
|
||||||
capture_phase_diff_at(
|
capture_phase_diff_at(
|
||||||
pool,
|
pool,
|
||||||
mission_id,
|
mission_id,
|
||||||
phase_id,
|
phase_id,
|
||||||
&mission_workspace::checkout_path(mission_id),
|
&mission_workspace::checkout_path(mission_id),
|
||||||
&outputs_root(mission_id),
|
&outputs_root(mission_id),
|
||||||
|
iteration,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -121,6 +140,7 @@ pub async fn capture_phase_diff_at(
|
|||||||
phase_id: Uuid,
|
phase_id: Uuid,
|
||||||
repo: &Path,
|
repo: &Path,
|
||||||
outputs: &Path,
|
outputs: &Path,
|
||||||
|
iteration: i32,
|
||||||
) -> Result<Option<Capture>, String> {
|
) -> Result<Option<Capture>, String> {
|
||||||
let repo = repo.to_path_buf();
|
let repo = repo.to_path_buf();
|
||||||
if !repo.is_dir() {
|
if !repo.is_dir() {
|
||||||
@@ -198,9 +218,25 @@ pub async fn capture_phase_diff_at(
|
|||||||
std::fs::write(dir.join("diffstat.txt"), &diffstat)
|
std::fs::write(dir.join("diffstat.txt"), &diffstat)
|
||||||
.map_err(|e| format!("write diffstat: {e}"))?;
|
.map_err(|e| format!("write diffstat: {e}"))?;
|
||||||
|
|
||||||
|
// 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
|
||||||
|
// convenience, the patch is the guarantee.
|
||||||
|
let committed = match commit_phase_work(&repo, mission_id, phase_id, iteration).await {
|
||||||
|
Ok(c) => c,
|
||||||
|
Err(e) => {
|
||||||
|
eprintln!(
|
||||||
|
"mission_delivery: mission {mission_id} phase {phase_id} captured but not \
|
||||||
|
committed: {e}"
|
||||||
|
);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
let meta = json!({
|
let meta = json!({
|
||||||
"base_sha": base_sha,
|
"base_sha": base_sha,
|
||||||
"base_recorded": base_is_recorded,
|
"base_recorded": base_is_recorded,
|
||||||
|
"branch": committed.as_ref().map(|c| c.branch.clone()),
|
||||||
|
"head_sha": committed.as_ref().map(|c| c.sha.clone()),
|
||||||
"files_changed": files_changed,
|
"files_changed": files_changed,
|
||||||
"insertions": insertions,
|
"insertions": insertions,
|
||||||
"deletions": deletions,
|
"deletions": deletions,
|
||||||
@@ -246,6 +282,7 @@ pub async fn capture_phase_diff_at(
|
|||||||
);
|
);
|
||||||
|
|
||||||
Ok(Some(Capture {
|
Ok(Some(Capture {
|
||||||
|
committed,
|
||||||
base_sha,
|
base_sha,
|
||||||
files_changed,
|
files_changed,
|
||||||
insertions,
|
insertions,
|
||||||
@@ -324,6 +361,93 @@ pub fn parse_diffstat(stat: &str) -> (usize, usize, usize) {
|
|||||||
(files, ins, del)
|
(files, ins, del)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Commit a phase's work onto a branch of its own.
|
||||||
|
///
|
||||||
|
/// Runs after capture, never before: the patch is already on disk and
|
||||||
|
/// registered, so a commit that goes wrong costs a branch and not the work.
|
||||||
|
///
|
||||||
|
/// Three rules, none of them negotiable:
|
||||||
|
///
|
||||||
|
/// - **Never the default branch.** The branch name is derived from the mission
|
||||||
|
/// and phase, so a mission can only ever add a ref nobody else owns.
|
||||||
|
/// - **Never force.** A rejected update is reported, not overwritten.
|
||||||
|
/// - **Same exclusions as capture.** Whatever was too noisy to put in a patch
|
||||||
|
/// is too noisy to put in someone's history — build output, vendored trees,
|
||||||
|
/// and the workaround files agents write when infrastructure fights them.
|
||||||
|
///
|
||||||
|
/// Returns `Ok(None)` when there is nothing to commit, which is a normal
|
||||||
|
/// outcome and not an error: the phase may have changed nothing, or the agents
|
||||||
|
/// may have committed their own work already.
|
||||||
|
pub async fn commit_phase_work(
|
||||||
|
repo: &Path,
|
||||||
|
mission_id: Uuid,
|
||||||
|
phase_id: Uuid,
|
||||||
|
iteration: i32,
|
||||||
|
) -> Result<Option<Commit>, String> {
|
||||||
|
let branch = branch_name(mission_id, phase_id, iteration);
|
||||||
|
|
||||||
|
// Work already committed by the agents still needs a branch pointing at
|
||||||
|
// it, or it is unreachable once the checkout is reaped. So the branch is
|
||||||
|
// created regardless, and only the staging step is conditional.
|
||||||
|
git(repo, &["checkout", "-B", &branch]).await?;
|
||||||
|
|
||||||
|
let mut add: Vec<&str> = vec!["add", "--", "."];
|
||||||
|
let excludes: Vec<String> = EXCLUDED_PATHS
|
||||||
|
.iter()
|
||||||
|
.map(|p| format!(":(exclude){p}"))
|
||||||
|
.collect();
|
||||||
|
add.extend(excludes.iter().map(String::as_str));
|
||||||
|
git(repo, &add).await?;
|
||||||
|
|
||||||
|
// `--cached` compares the index against HEAD: empty means the agents left
|
||||||
|
// nothing unstaged for us, which is the normal case when they committed
|
||||||
|
// themselves.
|
||||||
|
let staged = git(repo, &["diff", "--cached", "--stat"])
|
||||||
|
.await
|
||||||
|
.unwrap_or_default();
|
||||||
|
if !staged.trim().is_empty() {
|
||||||
|
let message = format!(
|
||||||
|
"clawmates: {} phase work\n\nMission: {mission_id}\nPhase: {phase_id}\n\n Committed by the ClawMates delivery pipeline from the agents' working tree.",
|
||||||
|
if iteration > 0 {
|
||||||
|
format!("pass {}", iteration + 1)
|
||||||
|
} else {
|
||||||
|
"phase".to_string()
|
||||||
|
}
|
||||||
|
);
|
||||||
|
git(repo, &["commit", "--no-verify", "-m", &message]).await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
let sha = git(repo, &["rev-parse", "HEAD"])
|
||||||
|
.await
|
||||||
|
.map(|s| s.trim().to_string())
|
||||||
|
.unwrap_or_default();
|
||||||
|
if sha.is_empty() {
|
||||||
|
return Ok(None);
|
||||||
|
}
|
||||||
|
eprintln!(
|
||||||
|
"mission_delivery: mission {mission_id} phase {phase_id} → branch {branch} at {}",
|
||||||
|
&sha[..sha.len().min(8)]
|
||||||
|
);
|
||||||
|
Ok(Some(Commit { branch, sha }))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The branch a phase's work lands on.
|
||||||
|
///
|
||||||
|
/// Short ids keep it readable; the pair is unique per phase, and the iteration
|
||||||
|
/// suffix keeps a re-run from colliding with the pass before it. Deliberately
|
||||||
|
/// namespaced under `clawmates/` so it is obvious in a branch list who created
|
||||||
|
/// it and safe to delete in bulk.
|
||||||
|
pub fn branch_name(mission_id: Uuid, phase_id: Uuid, iteration: i32) -> String {
|
||||||
|
let m = mission_id.simple().to_string();
|
||||||
|
let p = phase_id.simple().to_string();
|
||||||
|
let base = format!("clawmates/mission-{}-{}", &m[..8], &p[..8]);
|
||||||
|
if iteration > 0 {
|
||||||
|
format!("{base}-i{}", iteration + 1)
|
||||||
|
} else {
|
||||||
|
base
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// 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
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ async fn capture(
|
|||||||
phase,
|
phase,
|
||||||
&root.join(mission.to_string()).join("repo"),
|
&root.join(mission.to_string()).join("repo"),
|
||||||
&root.join("_outputs").join(mission.to_string()),
|
&root.join("_outputs").join(mission.to_string()),
|
||||||
|
0,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -106,18 +107,47 @@ async fn captures_modified_and_untracked_files() {
|
|||||||
assert!(patch.contains("fn added()"), "its content is captured");
|
assert!(patch.contains("fn added()"), "its content is captured");
|
||||||
assert!(patch.contains("changed"), "the modification is captured");
|
assert!(patch.contains("changed"), "the modification is captured");
|
||||||
|
|
||||||
// Capture must leave the working tree exactly as the agents left it — a
|
// Capture is followed by a commit, so the tree the agents left is now on a
|
||||||
// later commit has to see the same thing this diff described.
|
// branch of its own. The patch was written first and is what guarantees
|
||||||
|
// the work survives; the branch is the convenience on top.
|
||||||
|
let branch = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&repo)
|
||||||
|
.args(["rev-parse", "--abbrev-ref", "HEAD"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let branch = String::from_utf8_lossy(&branch.stdout).trim().to_string();
|
||||||
|
assert!(
|
||||||
|
branch.starts_with("clawmates/mission-"),
|
||||||
|
"work lands on a namespaced mission branch, never the default one: {branch}"
|
||||||
|
);
|
||||||
|
|
||||||
let status = Command::new("git")
|
let status = Command::new("git")
|
||||||
.arg("-C")
|
.arg("-C")
|
||||||
.arg(&repo)
|
.arg(&repo)
|
||||||
.args(["status", "--porcelain"])
|
.args(["status", "--porcelain"])
|
||||||
.output()
|
.output()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let status = String::from_utf8_lossy(&status.stdout);
|
|
||||||
assert!(
|
assert!(
|
||||||
status.contains("new_module.rs"),
|
String::from_utf8_lossy(&status.stdout).trim().is_empty(),
|
||||||
"the new file is still untracked after capture, not left staged: {status}"
|
"everything the phase produced is committed, nothing left dangling"
|
||||||
|
);
|
||||||
|
|
||||||
|
let show = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&repo)
|
||||||
|
.args(["show", "--stat", "--oneline", "HEAD"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let show = String::from_utf8_lossy(&show.stdout);
|
||||||
|
assert!(
|
||||||
|
show.contains("new_module.rs"),
|
||||||
|
"the created file is in the commit: {show}"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
cap.committed.is_some(),
|
||||||
|
"the capture records where the work landed"
|
||||||
);
|
);
|
||||||
|
|
||||||
let row: (String, serde_json::Value) = sqlx::query_as(
|
let row: (String, serde_json::Value) = sqlx::query_as(
|
||||||
@@ -312,3 +342,61 @@ async fn committed_and_uncommitted_changes_are_both_captured() {
|
|||||||
assert!(patch.contains("fn wip()"), "uncommitted work");
|
assert!(patch.contains("fn wip()"), "uncommitted work");
|
||||||
assert_eq!(cap.files_changed, 2);
|
assert_eq!(cap.files_changed, 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Build output must stay out of the commit as well as the patch. Putting a
|
||||||
|
/// `target/` directory into someone's history is worse than losing the diff.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn excluded_paths_are_not_committed() {
|
||||||
|
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;
|
||||||
|
|
||||||
|
std::fs::create_dir_all(repo.join("target/debug")).unwrap();
|
||||||
|
std::fs::write(repo.join("target/debug/blob.bin"), vec![b'x'; 50_000]).unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
repo.join(".gitconfig_temp"),
|
||||||
|
"[safe]\n\tdirectory = /mission/repo\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::write(repo.join("real.rs"), "fn kept() {}\n").unwrap();
|
||||||
|
|
||||||
|
capture(&pool, tmp.path(), mission, phase)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let tracked = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&repo)
|
||||||
|
.args(["ls-files"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let tracked = String::from_utf8_lossy(&tracked.stdout);
|
||||||
|
assert!(tracked.contains("real.rs"), "genuine work is committed");
|
||||||
|
assert!(
|
||||||
|
!tracked.contains("blob.bin"),
|
||||||
|
"build output is not committed"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!tracked.contains(".gitconfig_temp"),
|
||||||
|
"an agent's workaround file is not committed into the user's history"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A re-run must not collide with the pass before it.
|
||||||
|
#[test]
|
||||||
|
fn a_rerun_lands_on_its_own_branch() {
|
||||||
|
let m = Uuid::now_v7();
|
||||||
|
let p = Uuid::now_v7();
|
||||||
|
let first = mission_delivery::branch_name(m, p, 0);
|
||||||
|
let second = mission_delivery::branch_name(m, p, 1);
|
||||||
|
assert_ne!(first, second);
|
||||||
|
assert!(first.starts_with("clawmates/mission-"));
|
||||||
|
assert!(
|
||||||
|
second.ends_with("-i2"),
|
||||||
|
"pass 2 is named for the pass, not the index: {second}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user