Skip to content

Commit e2178d4

Browse files
committed
fix(fleet): scope the sweep duplicate-check to the paths this round touched
Follow-up to #691. Measured 2026-08-11 in production: PR #692 duplicated #690 (byte-identical diffs, md5-verified) -- the #691 fix's own full-tree comparison missed it, because #690's branch was cut BEFORE #691 itself landed on origin/main, so it legitimately lacks the scripts/ changes #691 added. #692's branch, cut after #691 merged, carries those changes. A raw `diff --quiet <candidate>..HEAD` sees that unrelated drift as "different" and can never again match #690, no matter how many more times the same tag gap gets re-solved identically -- and the same defeat recurs for every future round whenever ANY unrelated commit lands on main while a sweep PR sits open, which given "the fleet publishes, it never merges" is routine. Fix: scope the comparison to `git diff --name-only origin_ref..HEAD` -- the paths THIS round's own squad merges actually touched -- instead of a full-tree diff. That isolates the tag-fix content from incidental history the two branches don't share. New regression test reproduces the exact #692-vs-#690 shape: an unrelated commit lands on main between when the open PR's branch was cut and when the new round's branch is cut, with the identical tag fix on both. Confirmed the test fails against the unversioned (pre-fix) code and passes with it. Full suite: 102 + 240 tests green.
1 parent ccd14f3 commit e2178d4

2 files changed

Lines changed: 81 additions & 2 deletions

File tree

‎scripts/overlord_sweep.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,7 +1691,23 @@ def clear_parks(squads_to_clear):
16911691
# open_sweep_prs_fn is optional (None in every existing test and any
16921692
# caller that predates this check) so this is additive: skip
16931693
# entirely rather than fail a sweep over a `gh` hiccup.
1694+
#
1695+
# Scoped to the PATHS this round's own squad merges touched, not a raw
1696+
# full-tree diff. Measured 2026-08-11: the raw full-tree version of this
1697+
# check missed a real duplicate (PR #692 vs #690, since confirmed
1698+
# byte-identical by md5) because THIS branch was cut from an origin/main
1699+
# that had since gained an unrelated commit (this very fix, #691) that
1700+
# PR #690's branch -- opened before #691 merged -- does not carry. Every
1701+
# future round inherits any commit that lands on origin/main in the
1702+
# meantime, so a full-tree compare against an older open PR sees THAT
1703+
# unrelated drift as "different" and never again matches, no matter how
1704+
# many times the same gap gets re-solved identically. Comparing only the
1705+
# paths this round actually changed isolates the tag-fix content from
1706+
# incidental history the two branches don't share.
16941707
if open_sweep_prs_fn is not None:
1708+
changed_rc, changed_out, _changed_err = run_git(
1709+
["diff", "--name-only", f"{origin_ref}..HEAD"], repo_root)
1710+
changed_paths = [p for p in changed_out.splitlines() if p.strip()] if changed_rc == 0 else []
16951711
# origin_ref is "<remote>/<branch>" in production (ORIGIN_MAIN =
16961712
# "origin/main") and a bare local branch name ("main") in tests
16971713
# that want no real remote at all -- same split every other
@@ -1703,7 +1719,7 @@ def clear_parks(squads_to_clear):
17031719
open_prs = open_sweep_prs_fn() or []
17041720
for pr in open_prs:
17051721
head = pr.get("headRefName") if isinstance(pr, dict) else None
1706-
if not head or head == branch:
1722+
if not head or head == branch or not changed_paths:
17071723
continue
17081724
candidate_ref = head
17091725
if remote:
@@ -1716,7 +1732,8 @@ def clear_parks(squads_to_clear):
17161732
# checking that candidate, never the whole duplicate scan.
17171733
continue
17181734
candidate_ref = f"refs/remotes/{remote}/{head}"
1719-
cmp_rc, _out2, _err2 = run_git(["diff", "--quiet", f"{candidate_ref}..HEAD"], repo_root)
1735+
cmp_rc, _out2, _err2 = run_git(
1736+
["diff", "--quiet", f"{candidate_ref}..HEAD", "--", *changed_paths], repo_root)
17201737
if cmp_rc == 0:
17211738
pr_ref = pr.get("url") or pr.get("number") or head
17221739
log_fn(f"{branch} is tree-identical to already-open {pr_ref} ({head}) -- skipping "

‎scripts/test_overlord_sweep.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,6 +1701,68 @@ def test_content_identical_to_an_already_open_pr_is_a_duplicate_not_a_second_pr(
17011701
# re-collecting the stamp forever would spin every round.
17021702
self.assertIn("canon", cursor["squads"])
17031703

1704+
def test_an_unrelated_commit_landing_on_main_meanwhile_does_not_defeat_the_duplicate_check(self):
1705+
"""Measured 2026-08-11: PR #692 duplicated #690 (byte-identical
1706+
diffs, md5-verified) and the FIRST version of this fix -- a raw
1707+
full-tree compare -- missed it. #690's branch was cut before this
1708+
very fix (#691) landed on main; #692's branch was cut after, so it
1709+
legitimately carries #691's scripts/ changes that #690's branch
1710+
does not. A full-tree diff sees that as "different" forever, no
1711+
matter how many times the SAME gap gets re-solved identically --
1712+
every future round inherits whatever unrelated commits landed on
1713+
main in the meantime. The fix: scope the comparison to the paths
1714+
THIS round's own squad merges touched, so incidental history the
1715+
two branches don't share can never mask a real duplicate.
1716+
"""
1717+
repo = self.make_repo()
1718+
# An earlier round already published this exact fix as an open PR,
1719+
# cut from main BEFORE the unrelated commit below landed.
1720+
git(repo, "branch", "sweep/tags-earlier", "main")
1721+
git(repo, "checkout", "-q", "sweep/tags-earlier")
1722+
self.commit_file(repo, "src/a.rs", "fn a() {}\n", "sweep: fix JPEG:Foo")
1723+
git(repo, "checkout", "-q", "main")
1724+
1725+
# A completely unrelated PR (infra, docs, anything) lands on main in
1726+
# between -- e.g. this very fix.
1727+
self.commit_file(repo, "scripts/unrelated.py", "# unrelated infra change\n", "infra: unrelated fix")
1728+
1729+
# THIS round's squad branch is cut from the NEW main tip (so it
1730+
# carries the unrelated commit #690 never saw) and produces the
1731+
# identical fix under a fresh sha.
1732+
git(repo, "branch", "squad/canon", "main")
1733+
git(repo, "checkout", "-q", "squad/canon")
1734+
canon_sha = self.commit_file(
1735+
repo, "src/a.rs", "fn a() {}\n", "fix JPEG:Foo",
1736+
trailers=[("Format", "JPEG"), ("Tag", "MakerNotes:Foo")],
1737+
)
1738+
git(repo, "checkout", "-q", "main")
1739+
1740+
with tempfile.TemporaryDirectory() as tmpdir:
1741+
home = Path(tmpdir) / "home"
1742+
config_toml = self._config_toml(Path(tmpdir), ["canon"])
1743+
squad_merge_loop.record_head(
1744+
squad_merge_loop.squad_status_file(home, "canon"), "workerhead", status="consumed",
1745+
patch_id="p1", format_name="JPEG", squad_sha=canon_sha, now_fn=lambda: 100,
1746+
)
1747+
tested, pushed, prs = [], [], []
1748+
result = overlord_sweep.run_sweep(
1749+
repo_root=repo, home=home, cache_dir="/unused",
1750+
comparison_fn=self._passing_comparison_fn, checkout_fn=self._checkout_fn,
1751+
config_path=config_toml, sweep_state_path=home / "sweep-state.json", origin_ref="main",
1752+
dispatcher_lock_path=home / "logs" / "dispatcher.lock",
1753+
cargo_test_workspace_fn=lambda repo_root: tested.append(1) or (True, "ok"),
1754+
push_branch_fn=lambda repo_root, branch: pushed.append(branch) or (True, "pushed"),
1755+
create_pr_fn=lambda *a, **kw: prs.append(a) or {"ok": True, "url": "u"},
1756+
fmt_fn=self._reformatting_fmt_fn, lint_fn=lambda repo_root: (True, ""), log_fn=lambda *a: None,
1757+
open_sweep_prs_fn=lambda: [
1758+
{"headRefName": "sweep/tags-earlier", "number": 42, "url": "https://example/pull/42"},
1759+
],
1760+
)
1761+
self.assertEqual(result["status"], "duplicate_of_open_pr")
1762+
self.assertEqual(tested, [])
1763+
self.assertEqual(pushed, [])
1764+
self.assertEqual(prs, [])
1765+
17041766
def test_a_content_DIFFERENT_open_pr_does_not_veto_a_genuinely_new_fix(self):
17051767
"""The duplicate gate must not become a second, accidental
17061768
zero_delta check: an open sweep PR fixing a DIFFERENT tag must

0 commit comments

Comments
 (0)