diff --git a/scripts/overlord_sweep.py b/scripts/overlord_sweep.py index f44277459..6efbc2c1c 100644 --- a/scripts/overlord_sweep.py +++ b/scripts/overlord_sweep.py @@ -1691,7 +1691,23 @@ def clear_parks(squads_to_clear): # open_sweep_prs_fn is optional (None in every existing test and any # caller that predates this check) so this is additive: skip # entirely rather than fail a sweep over a `gh` hiccup. + # + # Scoped to the PATHS this round's own squad merges touched, not a raw + # full-tree diff. Measured 2026-08-11: the raw full-tree version of this + # check missed a real duplicate (PR #692 vs #690, since confirmed + # byte-identical by md5) because THIS branch was cut from an origin/main + # that had since gained an unrelated commit (this very fix, #691) that + # PR #690's branch -- opened before #691 merged -- does not carry. Every + # future round inherits any commit that lands on origin/main in the + # meantime, so a full-tree compare against an older open PR sees THAT + # unrelated drift as "different" and never again matches, no matter how + # many times the same gap gets re-solved identically. Comparing only the + # paths this round actually changed isolates the tag-fix content from + # incidental history the two branches don't share. if open_sweep_prs_fn is not None: + changed_rc, changed_out, _changed_err = run_git( + ["diff", "--name-only", f"{origin_ref}..HEAD"], repo_root) + changed_paths = [p for p in changed_out.splitlines() if p.strip()] if changed_rc == 0 else [] # origin_ref is "/" in production (ORIGIN_MAIN = # "origin/main") and a bare local branch name ("main") in tests # that want no real remote at all -- same split every other @@ -1703,7 +1719,7 @@ def clear_parks(squads_to_clear): open_prs = open_sweep_prs_fn() or [] for pr in open_prs: head = pr.get("headRefName") if isinstance(pr, dict) else None - if not head or head == branch: + if not head or head == branch or not changed_paths: continue candidate_ref = head if remote: @@ -1716,7 +1732,8 @@ def clear_parks(squads_to_clear): # checking that candidate, never the whole duplicate scan. continue candidate_ref = f"refs/remotes/{remote}/{head}" - cmp_rc, _out2, _err2 = run_git(["diff", "--quiet", f"{candidate_ref}..HEAD"], repo_root) + cmp_rc, _out2, _err2 = run_git( + ["diff", "--quiet", f"{candidate_ref}..HEAD", "--", *changed_paths], repo_root) if cmp_rc == 0: pr_ref = pr.get("url") or pr.get("number") or head log_fn(f"{branch} is tree-identical to already-open {pr_ref} ({head}) -- skipping " diff --git a/scripts/test_overlord_sweep.py b/scripts/test_overlord_sweep.py index c727841ad..a12a2256c 100644 --- a/scripts/test_overlord_sweep.py +++ b/scripts/test_overlord_sweep.py @@ -1701,6 +1701,68 @@ def test_content_identical_to_an_already_open_pr_is_a_duplicate_not_a_second_pr( # re-collecting the stamp forever would spin every round. self.assertIn("canon", cursor["squads"]) + def test_an_unrelated_commit_landing_on_main_meanwhile_does_not_defeat_the_duplicate_check(self): + """Measured 2026-08-11: PR #692 duplicated #690 (byte-identical + diffs, md5-verified) and the FIRST version of this fix -- a raw + full-tree compare -- missed it. #690's branch was cut before this + very fix (#691) landed on main; #692's branch was cut after, so it + legitimately carries #691's scripts/ changes that #690's branch + does not. A full-tree diff sees that as "different" forever, no + matter how many times the SAME gap gets re-solved identically -- + every future round inherits whatever unrelated commits landed on + main in the meantime. The fix: scope the comparison to the paths + THIS round's own squad merges touched, so incidental history the + two branches don't share can never mask a real duplicate. + """ + repo = self.make_repo() + # An earlier round already published this exact fix as an open PR, + # cut from main BEFORE the unrelated commit below landed. + 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") + + # A completely unrelated PR (infra, docs, anything) lands on main in + # between -- e.g. this very fix. + self.commit_file(repo, "scripts/unrelated.py", "# unrelated infra change\n", "infra: unrelated fix") + + # THIS round's squad branch is cut from the NEW main tip (so it + # carries the unrelated commit #690 never saw) and produces the + # identical fix under a fresh sha. + 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