Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions scripts/overlord_sweep.py
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,33 @@ def clear_parks(squads_to_clear):
"bisection": bisection_result, "failed_squads": failed_squads, "preflight": health,
}

# Normalize formatting BEFORE either idempotency check below. Both
# checks are commit-to-commit diffs (`ref..HEAD`), which never see
# uncommitted working-tree changes -- so this has to actually commit,
# not just reformat the working tree. Reuses format_sweep_branch
# itself (idempotent: the LATER call, further down, finds nothing left
# to do and reports "already cargo-fmt clean"). Committing here cannot
# leak an extra row into the evidence table or judgment queue below:
# both are built from all_shas, and commits_contributed resolves each
# squad's contribution from the merge boundaries recorded in
# merge_infos (computed earlier this round, before any fmt call), not
# from "whatever sits on HEAD now" -- see commits_contributed's own
# docstring.
#
# Measured 2026-08-11: PR #694 duplicated #692 (byte-identical diffs,
# md5-verified) despite the path-scoped duplicate check from #693 --
# the ONLY difference the scoped diff found was rustfmt line-wrapping
# (e.g. a 3-line `match` arm collapsed to one line), because #692's
# branch had already been through format_sweep_branch's fmt-and-commit
# step while THIS round's comparison ran on raw, pre-fmt worker output.
# Both idempotency checks below compare tree content; unformatted
# content can never match a PR that already went through fmt, no
# matter how many times the same gap gets re-solved with functionally
# identical code. A fmt failure here is not fatal -- format_sweep_branch
# itself logs and reports it, and this degrades to exactly today's
# pre-fix behavior (comparing unformatted content), never worse.
format_sweep_branch(repo_root, run_git, fmt_fn=fmt_fn, log_fn=log_fn)

# The repo's DURABLE idempotency rule: compare the TREE, not the SHA.
# A cherry-pick or a squash gives identical content a fresh sha (fresh
# committer timestamp), so a stamp whose whole contribution is already
Expand Down Expand Up @@ -1779,14 +1806,18 @@ def clear_parks(squads_to_clear):
body = build_pr_body(evidence_rows=evidence_rows, judgment_entries=judgment_entries, branch=branch)
title = build_sweep_pr_title(branch, all_shas, merge_infos)

# Formatting is deliberately the LAST thing to touch the branch.
# Usually a no-op by now: the idempotency-check section above already
# ran format_sweep_branch once, before either the origin_ref or the
# open-PR duplicate check, so both compare formatted content. Kept
# here (rather than relying solely on that earlier call) because
# all_shas / the evidence table / the judgment queue above are the
# TAG-FIX commits, and the fmt commit is not one of them: it carries
# no trailers, closes no gap, and must not show up as a row in the
# PR's evidence table or as an entry in the judgment queue. Running
# it after cargo_test_workspace_fn is also deliberate -- rustfmt only
# moves whitespace, so re-running a multi-minute workspace suite
# afterwards would double the sweep's wall clock for no semantic gain.
# TAG-FIX commits, and the fmt commit must never show up as a row in
# the PR's evidence table or as an entry in the judgment queue --
# calling it again after they're built, not before, is what keeps that
# true regardless of whether the earlier call already committed
# everything or the fmt-and-commit step still needs to happen
# (fmt_fn=None in a caller that never wired the earlier normalization
# in, or a fmt failure there that a retry here might still recover).
fmt_result = format_sweep_branch(repo_root, run_git, fmt_fn=fmt_fn, log_fn=log_fn)

# The lint gate CI will apply, applied BEFORE the push rather than after.
Expand Down
67 changes: 66 additions & 1 deletion scripts/test_overlord_sweep.py
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,13 @@ def fake_push(repo_root, branch):
# branch ref -- the only thing `git push origin <branch>` and
# `gh pr create --head <branch>` ever see -- stays behind.
branch = result["branch"]
self.assertTrue(result["fmt"]["committed"])
# result["fmt"] reflects the LATE format_sweep_branch call -- by
# design usually a no-op now, since the idempotency-check section
# runs the same helper once, earlier, so both the origin_ref and
# the open-PR duplicate check compare already-formatted content
# (see the comment above that first call). The commit itself is
# what matters here, not which of the two calls made it.
self.assertFalse(result["fmt"]["committed"])
self.assertEqual(git_out(repo, "rev-parse", "HEAD").strip(),
git_out(repo, "rev-parse", branch).strip())
self.assertIn("style: cargo fmt --all (sweep publish)",
Expand Down Expand Up @@ -1763,6 +1769,65 @@ def test_an_unrelated_commit_landing_on_main_meanwhile_does_not_defeat_the_dupli
self.assertEqual(pushed, [])
self.assertEqual(prs, [])

def test_formatting_alone_does_not_defeat_the_duplicate_check(self):
"""Measured 2026-08-11: PR #694 duplicated #692 (byte-identical
diffs, md5-verified) despite the path-scoped fix from #693. The
ONLY difference the scoped diff found was rustfmt whitespace: #692's
branch had already been through format_sweep_branch's fmt-and-commit
step (it published in an earlier round), while THIS round's
comparison ran on raw, pre-fmt worker output -- since both
idempotency checks are commit-to-commit diffs, unformatted content
can never tree-match a PR that already went through fmt, no matter
how many times the same gap gets re-solved with functionally
identical code. Fix: run format_sweep_branch once, early, before
either check, so both compare already-formatted content.
"""
repo = self.make_repo()
# An earlier round already published this fix, POST-fmt (the "( )"
# -> "()" normalization _reformatting_fmt_fn performs, matching
# what format_sweep_branch already committed for that round).
git(repo, "branch", "sweep/tags-earlier", "main")
git(repo, "checkout", "-q", "sweep/tags-earlier")
self.commit_file(repo, "src/a.rs", "fn a() {}\n", "sweep: fix JPEG:Foo")
git(repo, "checkout", "-q", "main")

# THIS round's squad branch contributes the identical fix, but as
# raw worker output -- PRE-fmt, exactly like every real worker
# patch before format_sweep_branch ever touches it.
git(repo, "branch", "squad/canon", "main")
git(repo, "checkout", "-q", "squad/canon")
canon_sha = self.commit_file(
repo, "src/a.rs", "fn a( ) {}\n", "fix JPEG:Foo",
trailers=[("Format", "JPEG"), ("Tag", "MakerNotes:Foo")],
)
git(repo, "checkout", "-q", "main")

with tempfile.TemporaryDirectory() as tmpdir:
home = Path(tmpdir) / "home"
config_toml = self._config_toml(Path(tmpdir), ["canon"])
squad_merge_loop.record_head(
squad_merge_loop.squad_status_file(home, "canon"), "workerhead", status="consumed",
patch_id="p1", format_name="JPEG", squad_sha=canon_sha, now_fn=lambda: 100,
)
tested, pushed, prs = [], [], []
result = overlord_sweep.run_sweep(
repo_root=repo, home=home, cache_dir="/unused",
comparison_fn=self._passing_comparison_fn, checkout_fn=self._checkout_fn,
config_path=config_toml, sweep_state_path=home / "sweep-state.json", origin_ref="main",
dispatcher_lock_path=home / "logs" / "dispatcher.lock",
cargo_test_workspace_fn=lambda repo_root: tested.append(1) or (True, "ok"),
push_branch_fn=lambda repo_root, branch: pushed.append(branch) or (True, "pushed"),
create_pr_fn=lambda *a, **kw: prs.append(a) or {"ok": True, "url": "u"},
fmt_fn=self._reformatting_fmt_fn, lint_fn=lambda repo_root: (True, ""), log_fn=lambda *a: None,
open_sweep_prs_fn=lambda: [
{"headRefName": "sweep/tags-earlier", "number": 42, "url": "https://example/pull/42"},
],
)
self.assertEqual(result["status"], "duplicate_of_open_pr")
self.assertEqual(tested, [])
self.assertEqual(pushed, [])
self.assertEqual(prs, [])

def test_a_content_DIFFERENT_open_pr_does_not_veto_a_genuinely_new_fix(self):
"""The duplicate gate must not become a second, accidental
zero_delta check: an open sweep PR fixing a DIFFERENT tag must
Expand Down
Loading