src/merge_scan.rs
Ref: Size: 20.6 KiB History
//! Record merges as events, from `Patch:` trailers on a patch's base branch.
//!
//! See: docs/superpowers/specs/2026-08-09-merge-recording-design.md
//!
//! `PatchStatus::Merged` used to be derived on every read, by asking whether
//! the patch's head was reachable from its base tip. That fails in ordinary
//! use: the merged branch is routinely deleted, and reachability cannot see a
//! squash merge at all. So a merge is recorded, three ways that degrade into
//! each other:
//!
//! 1. a `commit-msg` hook stamps `Patch: <id>` onto commits — [`crate::hooks`];
//! 2. `sync` scans the base branch for those trailers — [`scan_and_record_merges`];
//! 3. `git-collab patch merge <id>` records it by hand — [`crate::patch::merge`].
//!
//! Layer 3 always works and is the only one that must exist for the design to
//! be correct. Layers 1 and 2 remove the need to remember it.
//!
//! **Recording is never a side effect of a read.** Nothing in this module runs
//! on a display path; [`crate::state::PatchState::looks_merged`] is the only
//! thing a read consults, and it writes nothing.
use std::collections::{HashMap, HashSet};
use git2::{Oid, Repository, Sort};
use crate::dag;
use crate::error::Error;
use crate::event::{Action, Author, Event};
use crate::state::{self, IssueStatus, PatchState, PatchStatus};
use crate::trailer;
const ARCHIVED_PATCH_PREFIX: &str = "refs/collab/archive/patches/";
/// What recording a merge on one patch did.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum MergeOutcome {
/// A `PatchMerge` was appended.
Recorded,
/// The patch was already merged, so nothing was appended.
AlreadyMerged,
}
/// What reconciling a patch's `--fixes` issue did.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CloseOutcome {
/// An `IssueClose` was appended.
Closed,
/// The patch has no `fixes`, or the issue was already closed.
NothingToDo,
/// The issue could not be resolved, or the close failed. Warned about;
/// the merge still stands and the next scan retries.
Failed,
}
/// Append a `PatchMerge` naming the commit that landed the patch, unless the
/// patch is already merged.
///
/// Applying a `PatchMerge` to a patch that is already `Merged` is a no-op in
/// the fold, so a duplicate would be harmless — but appending one anyway would
/// make every sync grow the DAG forever, so the check is here rather than left
/// to the fold.
pub fn record_merge(
repo: &Repository,
events_ref: &str,
patch: &PatchState,
commit: Oid,
author: &Author,
sk: &ed25519_dalek::SigningKey,
) -> Result<MergeOutcome, Error> {
if patch.status == PatchStatus::Merged {
return Ok(MergeOutcome::AlreadyMerged);
}
let event = Event {
timestamp: chrono::Utc::now().to_rfc3339(),
author: author.clone(),
action: Action::PatchMerge {
commit: commit.to_string(),
},
clock: 0,
};
dag::append_event(repo, events_ref, &event, sk)?;
Ok(MergeOutcome::Recorded)
}
/// Close the issue a patch was created to fix, if it is still open.
///
/// `--fixes` is documented as "auto-closes on merge" and this is where that
/// happens: in the same operation that records the merge, never during
/// derivation.
///
/// The two writes go to different refs, so they are not atomic. If this half
/// fails the merge still stands, and the next scan — which sees a merged patch
/// with an open `fixes` issue — retries it. That is why this is driven by the
/// invariant "a merged patch's `fixes` issue is closed" rather than by the
/// merge emission alone: a rule that only fired alongside a freshly-emitted
/// `PatchMerge` could never retry, because on the retry pass the merge is a
/// no-op. Being idempotent is what makes it safe to run every time: an already
/// closed issue is left alone, so repeated syncs append nothing.
///
/// The invariant needs the reopen clause in [`reopened_after_this_patch_closed`]
/// or it is too strong — see there.
pub fn close_fixed_issue(repo: &Repository, patch: &PatchState) -> CloseOutcome {
close_fixed_issue_inner(repo, patch, true)
}
/// The `IssueClose` reason this feature writes, and the marker it reads back to
/// recognize its own close. One function so the writer and the reader cannot
/// drift: if this string changes, a close written by an older version stops
/// being attributable, and the reopen guard below silently stops guarding.
pub fn merge_close_reason(patch_id: &str) -> String {
format!("merged in patch {:.8}", patch_id)
}
/// Whether this patch's close of `issue_ref` already happened and the issue was
/// subsequently reopened.
///
/// Called only when the issue is currently open, which is what makes the test
/// this cheap: if this patch's `IssueClose` is in the DAG *and* the issue reads
/// open, then something reopened it after that close landed. No ordering
/// analysis is needed — the status fold already resolved the ordering, and the
/// answer it reached is the one to respect.
///
/// This is the difference between the two states the retry cannot otherwise
/// tell apart:
///
/// - A genuine half-failure — the merge landed, the close did not — has **no**
/// `IssueClose` attributable to this patch. The close never happened, and
/// restoring it is exactly the retry's job.
/// - A deliberate `issue reopen` has one, followed by a reopen. Reopening a
/// merged patch's issue is ordinary: the fix landed and turned out to be
/// wrong. Without this clause the retry re-closes it on every sync forever,
/// silently, which makes `issue reopen` unusable for any issue a merged patch
/// names — a worse bug than the one the retry fixes.
///
/// The retry restores a close that never happened. It must never override a
/// decision made after one did.
fn reopened_after_this_patch_closed(repo: &Repository, issue_ref: &str, patch_id: &str) -> bool {
let marker = merge_close_reason(patch_id);
let Ok(events) = dag::walk_events(repo, issue_ref) else {
// Unreadable DAG. Treat as "guard applies": declining to close is
// recoverable by hand, re-closing a reopened issue every sync is not.
return true;
};
events.iter().any(|(_oid, event)| {
matches!(&event.action, Action::IssueClose { reason: Some(r) } if *r == marker)
})
}
/// The rule for `warn_unresolvable`: **an unresolvable `fixes` issue is
/// reported once, by the operation that recorded the merge, or by any command
/// the user invoked by hand.** It is never reported by a pass that merely
/// re-observed an already-recorded merge.
///
/// An issue that does not resolve — deleted, or an ambiguous prefix — will not
/// resolve on the next sync either. Both the retry pass and the trailer scan
/// revisit merged patches on every sync (a trailer stays in history forever), so
/// warning from either would repeat until someone edits git history, about
/// something nobody can act on.
///
/// A failure to *append* the close warns unconditionally, because that one is
/// transient, is worth retrying, and is worth hearing about every time.
fn close_fixed_issue_inner(
repo: &Repository,
patch: &PatchState,
warn_unresolvable: bool,
) -> CloseOutcome {
let Some(fixes) = patch.fixes.as_deref() else {
return CloseOutcome::NothingToDo;
};
let (issue_ref, issue_id) = match state::resolve_issue_ref(repo, fixes) {
Ok(v) => v,
Err(e) => {
// resolve_issue_ref distinguishes "no issue found" from "ambiguous
// prefix" in its message.
if warn_unresolvable {
eprintln!(
"warning: patch {:.8}: cannot close fixed issue {:.8}: {}",
patch.id, fixes, e
);
}
return CloseOutcome::Failed;
}
};
let issue = match state::IssueState::from_ref(repo, &issue_ref, &issue_id) {
Ok(i) => i,
Err(e) => {
if warn_unresolvable {
eprintln!(
"warning: patch {:.8}: cannot read fixed issue {:.8}: {}",
patch.id, fixes, e
);
}
return CloseOutcome::Failed;
}
};
if issue.status != IssueStatus::Open {
return CloseOutcome::NothingToDo;
}
if reopened_after_this_patch_closed(repo, &issue_ref, &patch.id) {
return CloseOutcome::NothingToDo;
}
let reason = merge_close_reason(&patch.id);
if let Err(e) = crate::issue::close(repo, &issue_id, Some(&reason)) {
eprintln!(
"warning: patch {:.8}: failed to close fixed issue {:.8}: {} — \
the merge stands; the next scan will retry the close",
patch.id, fixes, e
);
return CloseOutcome::Failed;
}
CloseOutcome::Closed
}
/// A `Patch:` trailer found during the walk, and the commit that carried it.
struct FoundTrailer {
/// The patch id prefix, verbatim from the trailer.
prefix: String,
/// The commit on the base branch that constitutes the merge.
commit: Oid,
}
/// Walk each base branch that has open patches, collect `Patch:` trailers, and
/// emit a `PatchMerge` for every patch whose trailer appears on *its own*
/// base branch.
///
/// **Never breaks sync.** Per-commit and per-patch errors are logged as
/// one-line stderr warnings and iteration continues, mirroring the `Issue:`
/// scan exactly. The only errors that propagate are "couldn't even start"
/// failures; callers treat a returned `Err` as "skip the merge scan for this
/// sync" and proceed.
///
/// Returns the number of `PatchMerge` events actually emitted.
pub fn scan_and_record_merges(
repo: &Repository,
author: &Author,
sk: &ed25519_dalek::SigningKey,
) -> Result<usize, Error> {
// Only open patches are candidates, and a trailer only counts on the
// patch's own base branch — a `Patch:` trailer on some unrelated branch is
// not a merge. So group the open patches by base branch and walk each base
// once, rather than walking every branch as the `Issue:` scan does, or
// walking once per patch.
let all = state::list_patches(repo)?;
let mut by_base: HashMap<String, Vec<PatchState>> = HashMap::new();
for patch in &all {
if patch.status == PatchStatus::Open {
by_base
.entry(patch.base_ref.clone())
.or_default()
.push(patch.clone());
}
}
let mut emitted = 0usize;
// Iterate in a stable order so warnings do not shuffle between runs.
let mut bases: Vec<&String> = by_base.keys().collect();
bases.sort();
for base in bases {
let patches = &by_base[base];
emitted += scan_one_base(repo, base, patches, author, sk);
}
retry_pending_closes(repo, &all);
Ok(emitted)
}
/// Close the `fixes` issue of any patch that was *already* merged when this
/// scan started.
///
/// This is the explicit retry the non-atomicity demands. A merge and the close
/// of the issue it fixes write to different refs, so the pair can half-succeed;
/// when it does, the merge stands and the issue is left open with nothing to
/// finish the job. Recording the merge again is not an option — the second
/// `PatchMerge` would be a no-op, so hanging the close off a freshly-emitted
/// merge could never retry.
///
/// So the rule is the invariant, not the emission: a merged patch's `fixes`
/// issue is closed, *unless it has been closed for this patch once already and
/// since reopened* — see `reopened_after_this_patch_closed` for why the bare
/// invariant is too strong. Cheap, because it only looks at patches that are
/// merged *and* carry a `fixes`, and the DAG walk behind the reopen guard is
/// only reached while the issue is still open, which after one successful close
/// it never is. Idempotent, which is what keeps repeated syncs from appending
/// anything.
///
/// Patches merged during this very scan already had their close attempted
/// inline, in the same operation that emitted the merge. This pass is for the
/// ones merged by some earlier operation — including by `patch merge
/// --no-close`, and including merges recorded on another machine and arriving
/// by sync.
fn retry_pending_closes(repo: &Repository, patches: &[PatchState]) {
for patch in patches {
if patch.status == PatchStatus::Merged && patch.fixes.is_some() {
close_fixed_issue_inner(repo, patch, false);
}
}
}
/// Scan one base branch. Returns the number of merges recorded. Absorbs every
/// error itself, because a base branch that cannot be walked must not stop the
/// other base branches, let alone the sync.
fn scan_one_base(
repo: &Repository,
base: &str,
patches: &[PatchState],
author: &Author,
sk: &ed25519_dalek::SigningKey,
) -> usize {
let base_ref = format!("refs/heads/{}", base);
let Ok(base_tip) = repo.refname_to_id(&base_ref) else {
// The clone does not have this base branch. Detecting merges on a base
// branch we do not have is out of scope, and it is not an error: a
// contributor legitimately syncs without every base branch checked out.
return 0;
};
let trailers = match collect_trailers(repo, base_tip, patches) {
Ok(t) => t,
Err(e) => {
eprintln!(
"warning: cannot scan '{}' for Patch: trailers: {} — skipping",
base, e
);
return 0;
}
};
let mut emitted = 0usize;
for found in trailers {
match record_from_trailer(repo, base, &found, author, sk) {
Ok(MergeOutcome::Recorded) => emitted += 1,
Ok(MergeOutcome::AlreadyMerged) => {}
Err(()) => {}
}
}
emitted
}
/// A commit that is an ancestor of every one of `bases`, or `None` when there
/// is no such commit or the set is empty.
///
/// Not `merge_base_many`, which is the obvious call and the wrong one.
/// libgit2's `git_merge_base_many` is *not* the octopus base: it computes
/// `merge_base(oids[0], merge(oids[1..]))`, so its answer depends on argument
/// order and is an ancestor of only some of its inputs. On a linear chain
/// A-B-C-D-E, `git merge-base A B D` is A but `git merge-base B A D` is B —
/// and B is not an ancestor of A. Using it as a walk bound hides commits
/// between A and B, which is where a merge of the A-based patch lives, so the
/// scan silently records nothing for exactly the case it exists to serve.
///
/// Folding binary `merge_base` is sound by induction: each step returns a
/// common ancestor of the accumulator and the next base, so the result is an
/// ancestor of everything folded in so far. git2 0.19 exposes no octopus
/// equivalent, which would be the other way.
///
/// `None` on the first pair with no common ancestor (disjoint histories) — the
/// safe direction, since an absent bound only costs a longer walk.
pub fn common_ancestor(repo: &Repository, bases: &[Oid]) -> Option<Oid> {
let mut acc = *bases.first()?;
for base in &bases[1..] {
acc = repo.merge_base(acc, *base).ok()?;
}
Some(acc)
}
/// Walk `base_tip` back to a commit older than every open patch's base,
/// collecting `Patch:` trailers.
///
/// The bound matters: nothing older than the oldest open patch's base can have
/// merged a currently-open patch, so walking it is pure cost. It has to be an
/// ancestor of *every* recorded base, or the walk skips the region where some
/// patch's merge actually sits — see `common_ancestor`.
///
/// The first trailer seen for a given prefix wins. The walk is newest-first, so
/// that is the newest commit carrying it: the rebased tip, or the squash commit.
fn collect_trailers(
repo: &Repository,
base_tip: Oid,
patches: &[PatchState],
) -> Result<Vec<FoundTrailer>, Error> {
let mut revwalk = repo.revwalk()?;
revwalk.set_sorting(Sort::TOPOLOGICAL)?;
revwalk.push(base_tip)?;
let bases: Vec<Oid> = patches
.iter()
.filter_map(|p| p.effective_base(repo))
.collect();
if let Some(bound) = common_ancestor(repo, &bases) {
// A failure to hide is not fatal — it only means a longer walk.
let _ = revwalk.hide(bound);
}
let mut found = Vec::new();
let mut seen_prefix: HashSet<String> = HashSet::new();
for oid_result in revwalk {
let oid = match oid_result {
Ok(o) => o,
Err(e) => {
eprintln!("warning: revwalk error, stopping merge scan: {}", e);
break;
}
};
let commit = match repo.find_commit(oid) {
Ok(c) => c,
Err(e) => {
eprintln!("warning: cannot load commit {}: {}", oid, e);
continue;
}
};
let message = commit.message().unwrap_or("");
for prefix in trailer::parse_trailers(message, trailer::PATCH_TOKEN) {
if seen_prefix.insert(prefix.clone()) {
found.push(FoundTrailer {
prefix,
commit: oid,
});
}
}
}
Ok(found)
}
/// Resolve one trailer and record the merge it names. `Err(())` means the
/// trailer was warned about and skipped; it never propagates.
fn record_from_trailer(
repo: &Repository,
base: &str,
found: &FoundTrailer,
author: &Author,
sk: &ed25519_dalek::SigningKey,
) -> Result<MergeOutcome, ()> {
let (events_ref, id) = match state::resolve_patch_ref(repo, &found.prefix) {
Ok(v) => v,
Err(e) => {
// resolve_patch_ref's message already distinguishes "no patch
// found" from "ambiguous prefix".
eprintln!(
"warning: commit {:.8}: Patch: {} — {}, skipping",
found.commit, found.prefix, e
);
return Err(());
}
};
if events_ref.starts_with(ARCHIVED_PATCH_PREFIX) {
eprintln!(
"warning: commit {:.8}: Patch: {} — patch is archived, skipping",
found.commit, found.prefix
);
return Err(());
}
let patch = match PatchState::from_ref(repo, &events_ref, &id) {
Ok(p) => p,
Err(e) => {
eprintln!(
"warning: commit {:.8}: Patch: {} — cannot read patch: {}, skipping",
found.commit, found.prefix, e
);
return Err(());
}
};
// A trailer only records a merge on the patch's *own* base branch. The
// walk was seeded from one branch, but the trailer may name a patch based
// on another, and landing on the wrong branch is not landing.
if patch.base_ref != base {
return Err(());
}
// A patch someone deliberately closed is not resurrected by a trailer.
if patch.status == PatchStatus::Closed {
return Err(());
}
let outcome = match record_merge(repo, &events_ref, &patch, found.commit, author, sk) {
Ok(o) => o,
Err(e) => {
eprintln!(
"warning: commit {:.8}: failed to record merge of patch {:.8}: {}",
found.commit, id, e
);
return Err(());
}
};
// Whether or not the merge was freshly emitted, the `fixes` issue must end
// up closed — see `close_fixed_issue` for why the retry has to be able to
// run on a pass where the merge itself was a no-op.
//
// Warn about an unresolvable issue only on the pass that actually recorded
// the merge. A trailer stays in history forever, so while any other open
// patch keeps this base branch walked, this line is reached on every sync
// for an already-merged patch — and warning there would repeat, every sync,
// about something nobody can act on. That is the same noise
// `retry_pending_closes` is quiet about, arriving by a different route.
let warn = outcome == MergeOutcome::Recorded;
close_fixed_issue_inner(repo, &patch, warn);
Ok(outcome)
}
/// Open patches whose head is reachable from their base tip but which carry no
/// `PatchMerge`. Returned so `sync` can name them; nothing here writes.
pub fn merge_hints(repo: &Repository) -> Result<Vec<PatchState>, Error> {
Ok(state::list_patches(repo)?
.into_iter()
.filter(|p| p.looks_merged(repo))
.collect())
}
/// Print the closing hint `sync` shows for patches that look merged but are
/// recorded nowhere. Knowingly partial — it cannot see a squash — so it is
/// phrased as a suggestion.
pub fn print_merge_hints(repo: &Repository) {
let Ok(hints) = merge_hints(repo) else { return };
if hints.is_empty() {
return;
}
println!(
"\n{} patch(es) look merged but are not recorded:",
hints.len()
);
for p in &hints {
println!(" {:.8} {}", p.id, p.title);
}
println!("Record one with: git-collab patch merge <id>");
}