`capture_phase_diff_at` parsed `git diff --stat` down to three integers and
threw the filenames away. Nothing downstream could name a single file a coding
phase changed: the World can draw a coding station but nothing underneath it,
and an operator reading a mission sees "11 files" with no way to learn which.
A second `--name-status` call now records the paths into the code_diff metadata
and into `names.txt` beside `diffstat.txt`, so raw evidence survives
independently of the JSONB.
Three ways this could have been wrong, each guarded:
- Different revision or excludes from the `--stat` call would make
`files_changed` and the path list describe different diffs, with no way to
tell which lied. A source-walk test pins both to the same `base_sha` and
the same `excludes`.
- Running after `git reset --quiet` would drop newly CREATED files, since
`--intent-to-add` is what makes them visible to diff at all — and the stat
would still count them, so the list would look merely incomplete rather
than wrong. A test asserts the ordering.
- A rename is `R100\told\tnew` — three fields. Taking field two records where
the file USED to be, naming a path nobody can open, and the bug is
invisible in any repo where nothing was renamed. `changed_paths` is now
shared with auto_merge (which had the same parse) and takes the NEW path,
with tests for renames and copies.
The list is capped at 500 paths with `files_truncated` beside it: a cap that
silently clips is worse than no cap, because "touched 12 files" and "touched at
least 500" would look identical.
Co-Authored-By: Claude Opus 5 <[email protected]>
443 lines
16 KiB
Rust
443 lines
16 KiB
Rust
//! Merging a delivered branch into the base, when that is provably safe.
|
|
//!
|
|
//! Every mission type delivers to a branch and never to `main`. For most that
|
|
//! is where it should stop — a human reads the code and merges. But some
|
|
//! missions only ever *add* files in a folder they own: a paper catalogue, a
|
|
//! benchmark record. Those branches carry no judgement call, and leaving them
|
|
//! to pile up unmerged means the work is done but not actually in the vault.
|
|
//!
|
|
//! # Additive-only is a property, not a preference
|
|
//!
|
|
//! The gate is not "is this mission type trusted". It is measured from the
|
|
//! diff: if the branch modifies or deletes anything that already existed, it
|
|
//! does not qualify, whatever its template says. A research harvest that
|
|
//! somehow rewrote a hand-written note would be refused by the same check
|
|
//! that lets its new notes through.
|
|
//!
|
|
//! Three conditions, all required:
|
|
//!
|
|
//! 1. the mission type declares [`MergePolicy::AdditiveOnly`]
|
|
//! 2. verification passed — a run that did not prove its work does not merge
|
|
//! 3. the diff against the base contains only additions
|
|
//!
|
|
//! Anything else lands as a branch for a human, which is the existing
|
|
//! behaviour and the safe default.
|
|
|
|
use std::path::Path;
|
|
|
|
/// What a mission type is allowed to do with its own branch.
|
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
|
pub enum MergePolicy {
|
|
/// Always leave the branch for a human. Correct for anything that touches
|
|
/// code: `refactor`, `research_and_code`, security patches.
|
|
Never,
|
|
/// Merge automatically when the diff is provably additive and the run
|
|
/// verified. Correct for catalogues and recorded measurements.
|
|
AdditiveOnly,
|
|
}
|
|
|
|
impl MergePolicy {
|
|
/// Parse a template's `merge_policy`. Unknown values fall back to `Never`
|
|
/// and say so: a typo must not silently grant auto-merge.
|
|
pub fn parse(raw: Option<&str>) -> MergePolicy {
|
|
match raw.map(str::trim) {
|
|
Some("additive_only") => MergePolicy::AdditiveOnly,
|
|
Some("never") | None => MergePolicy::Never,
|
|
Some(other) => {
|
|
eprintln!(
|
|
"auto_merge: unknown merge_policy {other:?} — refusing to auto-merge"
|
|
);
|
|
MergePolicy::Never
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Why a branch was or was not merged. The reason is always recorded: a
|
|
/// branch that silently did not merge is indistinguishable from one that was
|
|
/// never delivered.
|
|
#[derive(Debug, Clone)]
|
|
pub struct MergeOutcome {
|
|
pub merged: bool,
|
|
pub reason: String,
|
|
}
|
|
|
|
impl MergeOutcome {
|
|
fn refused(reason: impl Into<String>) -> MergeOutcome {
|
|
MergeOutcome {
|
|
merged: false,
|
|
reason: reason.into(),
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Every path in a `git diff --name-status` body, with its status letter.
|
|
///
|
|
/// The World draws a file orb per changed path, and `mission_delivery` records
|
|
/// the list — both need the same parse, so it lives in one place.
|
|
///
|
|
/// **Renames are three fields**: `R100\told\tnew`. The path that changed is the
|
|
/// NEW one; splitting on the first tab and taking field two records where the
|
|
/// file used to be, which then matches nothing anyone can open. Copies (`C###`)
|
|
/// have the same shape.
|
|
pub fn changed_paths(name_status: &str) -> Vec<(char, String)> {
|
|
name_status
|
|
.lines()
|
|
.filter(|l| !l.trim().is_empty())
|
|
.filter_map(|l| {
|
|
let mut fields = l.split('\t');
|
|
let status = fields.next()?.trim();
|
|
let letter = status.chars().next()?;
|
|
let first = fields.next()?.trim();
|
|
// R/C carry old THEN new; everything else has a single path.
|
|
let path = match letter {
|
|
'R' | 'C' => fields.next().map(str::trim).unwrap_or(first),
|
|
_ => first,
|
|
};
|
|
if path.is_empty() {
|
|
return None;
|
|
}
|
|
Some((letter, path.to_string()))
|
|
})
|
|
.collect()
|
|
}
|
|
|
|
/// Classify a `git diff --name-status` body.
|
|
///
|
|
/// Returns the offending entries, empty when every change is an addition.
|
|
/// Built on `changed_paths` so the two cannot disagree about what a line means.
|
|
pub fn non_additive_changes(name_status: &str) -> Vec<String> {
|
|
changed_paths(name_status)
|
|
.into_iter()
|
|
.filter(|(letter, _)| *letter != 'A')
|
|
.map(|(letter, path)| format!("{letter}\t{path}"))
|
|
.collect()
|
|
}
|
|
|
|
async fn git(repo: &Path, args: &[&str]) -> Result<String, String> {
|
|
let out = tokio::process::Command::new("git")
|
|
.arg("-C")
|
|
.arg(repo)
|
|
.args(["-c", &format!("safe.directory={}", repo.display())])
|
|
.args(args)
|
|
.env("GIT_AUTHOR_NAME", crate::mission_delivery::commit_identity().0)
|
|
.env("GIT_AUTHOR_EMAIL", crate::mission_delivery::commit_identity().1)
|
|
.env(
|
|
"GIT_COMMITTER_NAME",
|
|
crate::mission_delivery::commit_identity().0,
|
|
)
|
|
.env(
|
|
"GIT_COMMITTER_EMAIL",
|
|
crate::mission_delivery::commit_identity().1,
|
|
)
|
|
.output()
|
|
.await
|
|
.map_err(|e| format!("spawn git: {e}"))?;
|
|
if !out.status.success() {
|
|
return Err(format!(
|
|
"git {} → {}: {}",
|
|
args.first().copied().unwrap_or("?"),
|
|
out.status,
|
|
crate::mission_workspace::redact_token(&String::from_utf8_lossy(&out.stderr))
|
|
.chars()
|
|
.take(300)
|
|
.collect::<String>()
|
|
));
|
|
}
|
|
Ok(String::from_utf8_lossy(&out.stdout).into_owned())
|
|
}
|
|
|
|
/// Merge `branch` into `base` and push, if all three conditions hold.
|
|
///
|
|
/// Never returns `Err` for a refusal — a refusal is a normal outcome with a
|
|
/// reason. `Err` is reserved for the merge itself going wrong after we decided
|
|
/// to attempt it.
|
|
pub async fn try_merge(
|
|
repo: &Path,
|
|
push_url: &str,
|
|
branch: &str,
|
|
base: &str,
|
|
policy: MergePolicy,
|
|
verified: bool,
|
|
) -> Result<MergeOutcome, String> {
|
|
if policy != MergePolicy::AdditiveOnly {
|
|
return Ok(MergeOutcome::refused(
|
|
"merge_policy is not additive_only; left for a human",
|
|
));
|
|
}
|
|
if !verified {
|
|
return Ok(MergeOutcome::refused(
|
|
"run did not verify; refusing to merge unproven work",
|
|
));
|
|
}
|
|
|
|
// Compare against the base as the REMOTE has it, not a local ref that may
|
|
// be stale. `...` gives changes on the branch since it diverged, so an
|
|
// unrelated commit landing on main meanwhile is not misread as ours.
|
|
git(repo, &["fetch", push_url, base]).await?;
|
|
let diff = git(
|
|
repo,
|
|
&["diff", "--name-status", &format!("FETCH_HEAD...{branch}")],
|
|
)
|
|
.await?;
|
|
|
|
let offending = non_additive_changes(&diff);
|
|
if !offending.is_empty() {
|
|
return Ok(MergeOutcome::refused(format!(
|
|
"diff is not additive ({} non-add change(s), first: {}); left for a human",
|
|
offending.len(),
|
|
offending.first().map(String::as_str).unwrap_or("?")
|
|
)));
|
|
}
|
|
if diff.trim().is_empty() {
|
|
return Ok(MergeOutcome::refused("branch adds nothing"));
|
|
}
|
|
|
|
merge_and_push(repo, push_url, branch, base, "auto-merge")
|
|
.await
|
|
.map(|o| match o.merged {
|
|
true => MergeOutcome {
|
|
merged: true,
|
|
reason: format!("additive-only and verified; merged into {base}"),
|
|
},
|
|
false => o,
|
|
})
|
|
}
|
|
|
|
/// The git half of a merge, with no policy in it.
|
|
///
|
|
/// Split out so an OPERATOR-approved merge runs exactly the same commands as an
|
|
/// automatic one — fetch the base as the remote has it, merge onto that, push.
|
|
/// The gates differ; the mechanics must not, or the rarely-taken path is the one
|
|
/// that breaks.
|
|
async fn merge_and_push(
|
|
repo: &Path,
|
|
push_url: &str,
|
|
branch: &str,
|
|
base: &str,
|
|
label: &str,
|
|
) -> Result<MergeOutcome, String> {
|
|
// Merge onto the freshly fetched base rather than a local branch.
|
|
git(repo, &["checkout", "-B", base, "FETCH_HEAD"]).await?;
|
|
if let Err(e) = git(
|
|
repo,
|
|
&["merge", "--no-ff", "-m", &format!("{label} {branch}"), branch],
|
|
)
|
|
.await
|
|
{
|
|
// Leave the repo clean so the next run is not fighting a wedged merge.
|
|
let _ = git(repo, &["merge", "--abort"]).await;
|
|
return Ok(MergeOutcome::refused(format!(
|
|
"merge conflicted ({e}); left for a human"
|
|
)));
|
|
}
|
|
|
|
git(repo, &["push", push_url, &format!("HEAD:refs/heads/{base}")]).await?;
|
|
Ok(MergeOutcome {
|
|
merged: true,
|
|
reason: format!("merged into {base}"),
|
|
})
|
|
}
|
|
|
|
/// Merge a delivered branch because an OPERATOR asked for it.
|
|
///
|
|
/// `MergePolicy::Never` means "do not merge on your own" — it defers to a human,
|
|
/// and this is that human. So the additive-only test does not apply: an operator
|
|
/// looking at a code change is exactly the judgement the policy was holding out
|
|
/// for.
|
|
///
|
|
/// What is NOT waived:
|
|
///
|
|
/// - the branch must exist on the remote and differ from the base, so the button
|
|
/// cannot report success for a merge of nothing;
|
|
/// - a conflict refuses and leaves the repo clean, rather than forcing;
|
|
/// - the work happens in a FRESH CLONE, never the mission checkout — that
|
|
/// directory is reaped on a timer after the mission ends, so a merge that
|
|
/// depended on it would work right after a run and mysteriously fail later.
|
|
pub async fn merge_on_operator_approval(
|
|
workdir: &Path,
|
|
push_url: &str,
|
|
branch: &str,
|
|
base: &str,
|
|
) -> Result<MergeOutcome, String> {
|
|
git(workdir, &["fetch", push_url, base]).await?;
|
|
git(workdir, &["fetch", push_url, branch]).await?;
|
|
git(workdir, &["branch", "-f", branch, "FETCH_HEAD"]).await?;
|
|
git(workdir, &["fetch", push_url, base]).await?;
|
|
|
|
let diff = git(
|
|
workdir,
|
|
&["diff", "--name-status", &format!("FETCH_HEAD...{branch}")],
|
|
)
|
|
.await?;
|
|
if diff.trim().is_empty() {
|
|
return Ok(MergeOutcome::refused(
|
|
"branch has nothing the base does not already have",
|
|
));
|
|
}
|
|
|
|
merge_locally(workdir, branch, base, "merge mission branch").await
|
|
}
|
|
|
|
/// Merge onto the fetched base WITHOUT publishing it.
|
|
///
|
|
/// Split from the push so a caller can run the project's tests against the
|
|
/// merged tree first. Verifying BEFORE publishing rather than reverting after is
|
|
/// the difference between "main was never broken" and "main was broken for as
|
|
/// long as it took us to notice".
|
|
pub async fn merge_locally(
|
|
repo: &Path,
|
|
branch: &str,
|
|
base: &str,
|
|
label: &str,
|
|
) -> Result<MergeOutcome, String> {
|
|
git(repo, &["checkout", "-B", base, "FETCH_HEAD"]).await?;
|
|
if let Err(e) = git(
|
|
repo,
|
|
&["merge", "--no-ff", "-m", &format!("{label} {branch}"), branch],
|
|
)
|
|
.await
|
|
{
|
|
// Leave the repo clean so the next attempt is not fighting a wedged merge.
|
|
let _ = git(repo, &["merge", "--abort"]).await;
|
|
return Ok(MergeOutcome::refused(format!(
|
|
"merge conflicted ({e}); left for a human"
|
|
)));
|
|
}
|
|
Ok(MergeOutcome {
|
|
merged: true,
|
|
reason: format!("merged into {base} locally, not yet published"),
|
|
})
|
|
}
|
|
|
|
/// Publish an already-merged base.
|
|
pub async fn push_merged(repo: &Path, push_url: &str, base: &str) -> Result<(), String> {
|
|
git(repo, &["push", push_url, &format!("HEAD:refs/heads/{base}")])
|
|
.await
|
|
.map(|_| ())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
/// Publication must be gated on the merged tree, and refusal must not push.
|
|
///
|
|
/// The two halves are separate functions precisely so a caller can run tests
|
|
/// BETWEEN them. If `merge_locally` ever pushed, verification would be
|
|
/// after-the-fact and `main` would be broken for as long as it took to
|
|
/// notice — which is the failure mode this whole thing exists to avoid.
|
|
#[test]
|
|
fn merging_locally_never_publishes() {
|
|
let src = include_str!("auto_merge.rs");
|
|
let body = src
|
|
.split("pub async fn merge_locally")
|
|
.nth(1)
|
|
.and_then(|s| s.split("\n}").next())
|
|
.unwrap_or("");
|
|
assert!(!body.is_empty(), "merge_locally not found");
|
|
assert!(
|
|
!body.contains("\"push\""),
|
|
"merge_locally must not push — publication is the caller's decision \
|
|
after it has verified the result"
|
|
);
|
|
// And the push half must exist separately, or the caller cannot publish.
|
|
assert!(src.contains("pub async fn push_merged"), "push_merged missing");
|
|
}
|
|
|
|
/// An operator merge and an automatic one must run the SAME git commands.
|
|
///
|
|
/// The gates differ — that is the whole point — but if the mechanics
|
|
/// diverged, the rarely-taken path would be the untested one. Both go
|
|
/// through `merge_and_push`.
|
|
#[test]
|
|
fn both_merge_paths_share_the_same_mechanics() {
|
|
let src = include_str!("auto_merge.rs");
|
|
let calls = src.matches("merge_and_push(").count();
|
|
// one definition + one call from each path
|
|
assert!(
|
|
calls >= 3,
|
|
"expected try_merge and merge_on_operator_approval to both call \
|
|
merge_and_push, found {calls} mention(s)"
|
|
);
|
|
// And the operator path must NOT re-implement the policy gate it exists
|
|
// to bypass — if this string appears there, the button is a no-op.
|
|
let op = src
|
|
.split("pub async fn merge_on_operator_approval")
|
|
.nth(1)
|
|
.unwrap_or("");
|
|
let body = op.split("\n}").next().unwrap_or("");
|
|
assert!(
|
|
!body.contains("MergePolicy::AdditiveOnly"),
|
|
"the operator path must not apply the additive-only gate"
|
|
);
|
|
// It must still refuse an empty branch: a button that reports success
|
|
// for merging nothing is worse than no button.
|
|
assert!(
|
|
body.contains("nothing the base does not already have"),
|
|
"the operator path must refuse an empty branch"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn only_pure_additions_qualify() {
|
|
assert!(non_additive_changes("A\t60 Papers/a.md\nA\t60 Papers/b.md\n").is_empty());
|
|
|
|
// A modification disqualifies the whole branch.
|
|
let m = non_additive_changes("A\t60 Papers/a.md\nM\tREADME.md\n");
|
|
assert_eq!(m.len(), 1);
|
|
assert!(m[0].contains("README.md"));
|
|
|
|
// So do deletes and renames — a rename is a delete plus an add, and
|
|
// the delete half can destroy hand-written work.
|
|
assert_eq!(non_additive_changes("D\tnotes/old.md\n").len(), 1);
|
|
assert_eq!(non_additive_changes("R100\ta.md\tb.md\n").len(), 1);
|
|
}
|
|
|
|
/// A rename records the NEW path.
|
|
///
|
|
/// `R100\told\tnew` is three fields. Reading field two — which is what a
|
|
/// split-on-first-tab gives you — records where the file USED to be, so the
|
|
/// World would draw an orb for a path that no longer exists and the
|
|
/// delivered file list would name something nobody can open. The bug is
|
|
/// invisible in any repo where nothing was renamed.
|
|
#[test]
|
|
fn a_rename_records_where_the_file_ended_up() {
|
|
let paths = changed_paths("R100\tsrc/old.rs\tsrc/new.rs\n");
|
|
assert_eq!(paths, vec![('R', "src/new.rs".to_string())]);
|
|
|
|
let copied = changed_paths("C075\tsrc/a.rs\tsrc/b.rs\n");
|
|
assert_eq!(copied, vec![('C', "src/b.rs".to_string())]);
|
|
|
|
// Ordinary two-field lines are unaffected.
|
|
assert_eq!(
|
|
changed_paths("A\tone.md\nM\ttwo.md\nD\tthree.md\n"),
|
|
vec![
|
|
('A', "one.md".to_string()),
|
|
('M', "two.md".to_string()),
|
|
('D', "three.md".to_string()),
|
|
]
|
|
);
|
|
}
|
|
|
|
/// `files_changed` and the path list must agree, or nobody can tell which
|
|
/// one lied. git counts a rename as ONE changed file; so must we.
|
|
#[test]
|
|
fn a_rename_counts_once() {
|
|
assert_eq!(changed_paths("R100\ta.rs\tb.rs\n").len(), 1);
|
|
}
|
|
|
|
#[test]
|
|
fn an_unknown_policy_never_grants_auto_merge() {
|
|
assert_eq!(MergePolicy::parse(None), MergePolicy::Never);
|
|
assert_eq!(MergePolicy::parse(Some("never")), MergePolicy::Never);
|
|
assert_eq!(
|
|
MergePolicy::parse(Some("additive_only")),
|
|
MergePolicy::AdditiveOnly
|
|
);
|
|
// A typo must fail closed, not open.
|
|
assert_eq!(MergePolicy::parse(Some("aditive_only")), MergePolicy::Never);
|
|
assert_eq!(MergePolicy::parse(Some("always")), MergePolicy::Never);
|
|
}
|
|
}
|