feat(missions): publish the mission branch, gated by commit_policy
Completes delivery. A phase's work is now captured, committed, gated and pushed — in that order, so every failure costs strictly less than the one before it. Publishing is last for a reason. By the time it runs the patch is on disk, the artifact is registered and the work is on a local branch, so a rejected ref, a rotated token or an unreachable forge costs a push and nothing else. A test pushes at a path that does not exist and asserts the commit is still there afterwards. The gate decides the branch name, never whether the work survives: - green, or policy `always` → `clawmates/mission-<m8>-<p8>` - red / unrunnable / no suite → `…-wip` - `on_reviewer_approval` → `…-review` Both land on the forge. A human can inspect, fix and re-push a branch; nobody can recover work discarded for failing a test. Deleting a red branch reproduces the old behaviour on purpose rather than by accident. `verify_tests` runs the project's own suite through the runtime container and returns `Option<bool>` — `None` for "could not establish", which the gate treats as unproven. An unreadable exit status is not a pass. That is the same fail-closed stance as the phase evaluator, and it is here because this tranche has now found four separate things reporting success while doing nothing. Never force-push. A rejected update is reported and left alone: the remote ref belongs to whoever set it, and overwriting it to make delivery look tidy is how a mission eats someone else's commit. The push URL is built fresh from the repo row and the ambient token, not read from `.git/config` — which no longer carries credentials, since agents run as root in a container that mounts the checkout. Tests push to a real `git init --bare` remote and assert the ref and its content actually arrived. A mock would have accepted anything. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3ea288dbb5
commit
e2871c4361
@@ -67,6 +67,10 @@ const EXCLUDED_PATHS: &[&str] = &[
|
|||||||
/// operator needs to see to work out what happened.
|
/// operator needs to see to work out what happened.
|
||||||
const MAX_PATCH_BYTES: usize = 4 * 1024 * 1024;
|
const MAX_PATCH_BYTES: usize = 4 * 1024 * 1024;
|
||||||
|
|
||||||
|
/// Ceiling on the gate's test run. Long enough for a real suite, short enough
|
||||||
|
/// that a hung test does not hold a phase open indefinitely.
|
||||||
|
const TEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(900);
|
||||||
|
|
||||||
/// What a phase produced.
|
/// What a phase produced.
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct Capture {
|
pub struct Capture {
|
||||||
@@ -117,6 +121,15 @@ pub async fn capture_phase_diff(
|
|||||||
.ok()
|
.ok()
|
||||||
.flatten()
|
.flatten()
|
||||||
.unwrap_or(0);
|
.unwrap_or(0);
|
||||||
|
// `commit_policy` lives in the phase config, merged there from the
|
||||||
|
// workflow recipe by `phases_for_create`.
|
||||||
|
let policy: Option<String> =
|
||||||
|
sqlx::query_scalar("SELECT config->>'commit_policy' FROM mission_phases WHERE id = $1")
|
||||||
|
.bind(phase_id)
|
||||||
|
.fetch_optional(pool)
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.flatten();
|
||||||
capture_phase_diff_at(
|
capture_phase_diff_at(
|
||||||
pool,
|
pool,
|
||||||
mission_id,
|
mission_id,
|
||||||
@@ -124,6 +137,7 @@ pub async fn capture_phase_diff(
|
|||||||
&mission_workspace::checkout_path(mission_id),
|
&mission_workspace::checkout_path(mission_id),
|
||||||
&outputs_root(mission_id),
|
&outputs_root(mission_id),
|
||||||
iteration,
|
iteration,
|
||||||
|
Gate::parse(policy.as_deref()),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -141,6 +155,7 @@ pub async fn capture_phase_diff_at(
|
|||||||
repo: &Path,
|
repo: &Path,
|
||||||
outputs: &Path,
|
outputs: &Path,
|
||||||
iteration: i32,
|
iteration: i32,
|
||||||
|
gate: Gate,
|
||||||
) -> 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() {
|
||||||
@@ -232,11 +247,49 @@ pub async fn capture_phase_diff_at(
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Gate, then publish. Both are best-effort on top of an artifact that has
|
||||||
|
// already landed: a phase whose tests fail, or whose push is rejected,
|
||||||
|
// still has its patch on disk and its work on a local branch.
|
||||||
|
let mut verified: Option<bool> = None;
|
||||||
|
let mut published: Option<Publish> = None;
|
||||||
|
if let Some(c) = committed.as_ref() {
|
||||||
|
if !empty {
|
||||||
|
if gate == Gate::OnGreenTests {
|
||||||
|
let container = std::env::var("CLAWMATES_RUNTIME_CONTAINER")
|
||||||
|
.unwrap_or_else(|_| "clawmates-runtime".to_string());
|
||||||
|
verified = verify_tests(&repo, &container).await;
|
||||||
|
}
|
||||||
|
match push_url_for(pool, mission_id).await {
|
||||||
|
Some(url) => {
|
||||||
|
published = publish_phase_branch(&repo, &url, &c.branch, gate, verified)
|
||||||
|
.await
|
||||||
|
.ok();
|
||||||
|
}
|
||||||
|
None => eprintln!(
|
||||||
|
"mission_delivery: mission {mission_id} has no push URL — work is \
|
||||||
|
committed locally on {} but not published",
|
||||||
|
c.branch
|
||||||
|
),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
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()),
|
"branch": published
|
||||||
|
.as_ref()
|
||||||
|
.map(|p| p.branch.clone())
|
||||||
|
.or_else(|| committed.as_ref().map(|c| c.branch.clone())),
|
||||||
"head_sha": committed.as_ref().map(|c| c.sha.clone()),
|
"head_sha": committed.as_ref().map(|c| c.sha.clone()),
|
||||||
|
"commit_policy": match gate {
|
||||||
|
Gate::Always => "always",
|
||||||
|
Gate::OnGreenTests => "on_green_tests",
|
||||||
|
Gate::OnReviewerApproval => "on_reviewer_approval",
|
||||||
|
},
|
||||||
|
"tests_verified": verified,
|
||||||
|
"pushed": published.as_ref().map(|p| p.pushed),
|
||||||
|
"push_error": published.as_ref().and_then(|p| p.error.clone()),
|
||||||
"files_changed": files_changed,
|
"files_changed": files_changed,
|
||||||
"insertions": insertions,
|
"insertions": insertions,
|
||||||
"deletions": deletions,
|
"deletions": deletions,
|
||||||
@@ -529,6 +582,109 @@ pub fn discover_test_command(repo: &Path) -> Option<Vec<String>> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The authenticated URL to push this mission's work to.
|
||||||
|
///
|
||||||
|
/// Built fresh from the repo row and the ambient token rather than read from
|
||||||
|
/// `.git/config`, which no longer carries credentials — the token is scrubbed
|
||||||
|
/// after clone because agents run as root in a container that mounts the
|
||||||
|
/// checkout. Building it here also means a rotated token takes effect
|
||||||
|
/// immediately instead of at the next clone.
|
||||||
|
async fn push_url_for(pool: &sqlx::PgPool, mission_id: Uuid) -> Option<String> {
|
||||||
|
let url: Option<String> = sqlx::query_scalar(
|
||||||
|
"SELECT r.clone_url FROM missions m JOIN repos r ON r.id = m.repo_id WHERE m.id = $1",
|
||||||
|
)
|
||||||
|
.bind(mission_id)
|
||||||
|
.fetch_optional(pool)
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.flatten();
|
||||||
|
url.map(|u| mission_workspace::with_ambient_auth(&u))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Run the gate, then push the branch if the gate allows it.
|
||||||
|
///
|
||||||
|
/// Publishing is last on purpose. By the time this runs the patch is on disk,
|
||||||
|
/// the artifact is registered and the work is committed to a local branch — so
|
||||||
|
/// every failure mode here costs a ref that did not reach the forge, and
|
||||||
|
/// nothing that was already captured.
|
||||||
|
///
|
||||||
|
/// The branch name carries the verdict. A gate that fails redirects to
|
||||||
|
/// `<branch>-wip` or `<branch>-review` and pushes it anyway: a human can
|
||||||
|
/// inspect, fix and re-push a branch, but cannot recover work that was thrown
|
||||||
|
/// away for failing a test. Deleting a red branch reproduces the old
|
||||||
|
/// behaviour — work destroyed — deliberately rather than by accident.
|
||||||
|
pub async fn publish_phase_branch(
|
||||||
|
repo: &Path,
|
||||||
|
push_url: &str,
|
||||||
|
branch: &str,
|
||||||
|
gate: Gate,
|
||||||
|
verified: Option<bool>,
|
||||||
|
) -> Result<Publish, String> {
|
||||||
|
let suffix = gate.branch_suffix(verified);
|
||||||
|
let target = format!("{branch}{suffix}");
|
||||||
|
if !suffix.is_empty() {
|
||||||
|
// Move the local ref too, so the checkout and the forge agree about
|
||||||
|
// where this work lives.
|
||||||
|
git(repo, &["branch", "-f", &target, "HEAD"]).await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Never force. A rejected update is reported and left alone: the remote
|
||||||
|
// ref belongs to whoever set it, and overwriting it to make delivery look
|
||||||
|
// tidy is how a mission eats someone else's commit.
|
||||||
|
let refspec = format!("HEAD:refs/heads/{target}");
|
||||||
|
match git(repo, &["push", push_url, &refspec]).await {
|
||||||
|
Ok(_) => {
|
||||||
|
eprintln!("mission_delivery: pushed {target}");
|
||||||
|
Ok(Publish {
|
||||||
|
branch: target,
|
||||||
|
pushed: true,
|
||||||
|
error: None,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
Err(e) => {
|
||||||
|
// Redacted by `git`'s error path already; the patch and the local
|
||||||
|
// branch both survive, so this is a degraded success.
|
||||||
|
eprintln!("mission_delivery: push of {target} failed: {e}");
|
||||||
|
Ok(Publish {
|
||||||
|
branch: target,
|
||||||
|
pushed: false,
|
||||||
|
error: Some(e),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Where a phase's work ended up, and whether the forge has it.
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct Publish {
|
||||||
|
pub branch: String,
|
||||||
|
pub pushed: bool,
|
||||||
|
pub error: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Run the project's own tests to decide whether a green-tests gate is met.
|
||||||
|
///
|
||||||
|
/// `None` means "could not establish", which the gate treats as unproven. That
|
||||||
|
/// is the same fail-closed stance the phase evaluator takes, and for the same
|
||||||
|
/// reason: this codebase has repeatedly found things reporting success while
|
||||||
|
/// doing nothing, and a test suite that never ran must not license a push to a
|
||||||
|
/// mission branch.
|
||||||
|
pub async fn verify_tests(repo: &Path, container: &str) -> Option<bool> {
|
||||||
|
let argv = discover_test_command(repo)?;
|
||||||
|
let workdir = repo.display().to_string();
|
||||||
|
let docker = crate::container_exec::connect().ok()?;
|
||||||
|
let out = crate::container_exec::exec(&docker, container, Some(&workdir), &argv, TEST_TIMEOUT)
|
||||||
|
.await
|
||||||
|
.ok()?;
|
||||||
|
eprintln!(
|
||||||
|
"mission_delivery: {} → exit {:?}",
|
||||||
|
argv.join(" "),
|
||||||
|
out.exit_code
|
||||||
|
);
|
||||||
|
// An unreadable status is not a pass.
|
||||||
|
Some(out.success())
|
||||||
|
}
|
||||||
|
|
||||||
/// 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
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ pub async fn ensure_checkout(
|
|||||||
/// environment, rewrite it to include the token as basic-auth. Returns
|
/// environment, rewrite it to include the token as basic-auth. Returns
|
||||||
/// the URL unchanged otherwise. The token is never logged (we only
|
/// the URL unchanged otherwise. The token is never logged (we only
|
||||||
/// pass the rewritten URL into `git clone` via argv).
|
/// pass the rewritten URL into `git clone` via argv).
|
||||||
fn with_ambient_auth(url: &str) -> String {
|
pub(crate) fn with_ambient_auth(url: &str) -> String {
|
||||||
let Ok(token) = std::env::var("GITEA_TOKEN") else {
|
let Ok(token) = std::env::var("GITEA_TOKEN") else {
|
||||||
return url.to_string();
|
return url.to_string();
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ async fn capture(
|
|||||||
&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,
|
0,
|
||||||
|
mission_delivery::Gate::Always,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -426,3 +427,161 @@ fn a_rerun_lands_on_its_own_branch() {
|
|||||||
"pass 2 is named for the pass, not the index: {second}"
|
"pass 2 is named for the pass, not the index: {second}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Push against a real bare repository.
|
||||||
|
///
|
||||||
|
/// A mock remote would accept whatever we sent and prove nothing; the failures
|
||||||
|
/// worth catching here — a rejected ref, a branch that never arrives, work
|
||||||
|
/// pushed to the wrong name — are all things only a real git remote reports.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_gated_push_reaches_the_remote() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let remote = tmp.path().join("remote.git");
|
||||||
|
std::fs::create_dir_all(&remote).unwrap();
|
||||||
|
Command::new("git")
|
||||||
|
.args(["init", "--bare", "--quiet"])
|
||||||
|
.arg(&remote)
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let mission = Uuid::now_v7();
|
||||||
|
let repo = seed_repo(tmp.path(), mission);
|
||||||
|
std::fs::write(repo.join("work.rs"), "fn shipped() {}\n").unwrap();
|
||||||
|
git(&repo, &["add", "."]);
|
||||||
|
git(&repo, &["commit", "--quiet", "-m", "work"]);
|
||||||
|
git(
|
||||||
|
&repo,
|
||||||
|
&["checkout", "-B", "clawmates/mission-test-aaaaaaaa"],
|
||||||
|
);
|
||||||
|
|
||||||
|
let out = mission_delivery::publish_phase_branch(
|
||||||
|
&repo,
|
||||||
|
remote.to_str().unwrap(),
|
||||||
|
"clawmates/mission-test-aaaaaaaa",
|
||||||
|
mission_delivery::Gate::Always,
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(out.pushed, "push failed: {:?}", out.error);
|
||||||
|
assert_eq!(out.branch, "clawmates/mission-test-aaaaaaaa");
|
||||||
|
|
||||||
|
// The remote genuinely has it, with the content.
|
||||||
|
let refs = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&remote)
|
||||||
|
.args(["for-each-ref", "--format=%(refname:short)"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let refs = String::from_utf8_lossy(&refs.stdout);
|
||||||
|
assert!(
|
||||||
|
refs.contains("clawmates/mission-test-aaaaaaaa"),
|
||||||
|
"refs: {refs}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let show = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&remote)
|
||||||
|
.args(["show", "clawmates/mission-test-aaaaaaaa:work.rs"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
assert!(String::from_utf8_lossy(&show.stdout).contains("fn shipped()"));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A red suite must not block delivery — it must redirect it. The work still
|
||||||
|
/// reaches the forge, on a branch whose name says it is unproven.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_failed_gate_publishes_to_a_wip_branch() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let remote = tmp.path().join("remote.git");
|
||||||
|
std::fs::create_dir_all(&remote).unwrap();
|
||||||
|
Command::new("git")
|
||||||
|
.args(["init", "--bare", "--quiet"])
|
||||||
|
.arg(&remote)
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let mission = Uuid::now_v7();
|
||||||
|
let repo = seed_repo(tmp.path(), mission);
|
||||||
|
std::fs::write(repo.join("half_done.rs"), "fn broken() {}\n").unwrap();
|
||||||
|
git(&repo, &["add", "."]);
|
||||||
|
git(&repo, &["commit", "--quiet", "-m", "wip"]);
|
||||||
|
git(
|
||||||
|
&repo,
|
||||||
|
&["checkout", "-B", "clawmates/mission-test-bbbbbbbb"],
|
||||||
|
);
|
||||||
|
|
||||||
|
let out = mission_delivery::publish_phase_branch(
|
||||||
|
&repo,
|
||||||
|
remote.to_str().unwrap(),
|
||||||
|
"clawmates/mission-test-bbbbbbbb",
|
||||||
|
mission_delivery::Gate::OnGreenTests,
|
||||||
|
Some(false),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(out.pushed, "a failed gate still publishes: {:?}", out.error);
|
||||||
|
assert!(
|
||||||
|
out.branch.ends_with("-wip"),
|
||||||
|
"verdict is in the name: {}",
|
||||||
|
out.branch
|
||||||
|
);
|
||||||
|
|
||||||
|
let refs = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&remote)
|
||||||
|
.args(["for-each-ref", "--format=%(refname:short)"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
let refs = String::from_utf8_lossy(&refs.stdout);
|
||||||
|
assert!(
|
||||||
|
refs.contains("-wip"),
|
||||||
|
"the work reached the forge anyway: {refs}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!refs.contains("clawmates/mission-test-bbbbbbbb\n"),
|
||||||
|
"and did not claim the clean branch name"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An unreachable remote is a degraded success, not a failure: the patch and
|
||||||
|
/// the local branch both still exist.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn an_unreachable_remote_does_not_lose_the_work() {
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let mission = Uuid::now_v7();
|
||||||
|
let repo = seed_repo(tmp.path(), mission);
|
||||||
|
std::fs::write(repo.join("work.rs"), "fn kept() {}\n").unwrap();
|
||||||
|
git(&repo, &["add", "."]);
|
||||||
|
git(&repo, &["commit", "--quiet", "-m", "work"]);
|
||||||
|
git(
|
||||||
|
&repo,
|
||||||
|
&["checkout", "-B", "clawmates/mission-test-cccccccc"],
|
||||||
|
);
|
||||||
|
|
||||||
|
let out = mission_delivery::publish_phase_branch(
|
||||||
|
&repo,
|
||||||
|
&tmp.path().join("does-not-exist.git").display().to_string(),
|
||||||
|
"clawmates/mission-test-cccccccc",
|
||||||
|
mission_delivery::Gate::Always,
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert!(!out.pushed);
|
||||||
|
assert!(
|
||||||
|
out.error.is_some(),
|
||||||
|
"the reason is recorded for the operator"
|
||||||
|
);
|
||||||
|
|
||||||
|
// The commit is still there locally — nothing was rolled back.
|
||||||
|
let show = Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(&repo)
|
||||||
|
.args(["show", "HEAD:work.rs"])
|
||||||
|
.output()
|
||||||
|
.unwrap();
|
||||||
|
assert!(String::from_utf8_lossy(&show.stdout).contains("fn kept()"));
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user