From 9bdf33b89da32dd166a5cd3e1daa24b101f90de6 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sun, 4 Oct 2026 14:28:01 +0200 Subject: [PATCH] fix(ci): treat an edited update branch as a duplicate of its own bump The update bot recognised an open PR as a duplicate by hashing the blobs of the files its commit changed, read at the PR head. A follow-up commit that edits one of those files changes the hash, so the next run opened a second PR with the identical bump: PR #778 next to PR #775 for web-svc-collabora, after a fix had edited the bumped services.yml. The branch name of a bot PR ends in the first seven characters of the fingerprint of the change the bot proposed. An open PR whose branch ends in the fingerprint of the fresh proposal now counts as a duplicate before its head is compared. The head comparison stays as the second criterion. The test runs pr.sh in a throwaway repository against a stubbed gh. It fails on the previous script with exit 1, because that one goes on to push. Closes #780 Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/github/update/open/pr.sh | 5 + .../python/scripts/github/update/__init__.py | 0 .../scripts/github/update/open/__init__.py | 0 .../scripts/github/update/open/test_pr.py | 112 ++++++++++++++++++ 4 files changed, 117 insertions(+) create mode 100644 tests/unit/python/scripts/github/update/__init__.py create mode 100644 tests/unit/python/scripts/github/update/open/__init__.py create mode 100644 tests/unit/python/scripts/github/update/open/test_pr.py diff --git a/scripts/github/update/open/pr.sh b/scripts/github/update/open/pr.sh index f093369356..83edcfd089 100755 --- a/scripts/github/update/open/pr.sh +++ b/scripts/github/update/open/pr.sh @@ -79,6 +79,11 @@ for entry in "${OPEN_PRS[@]}"; do [[ -z "${entry}" ]] && continue pr_num="${entry%%$'\t'*}" pr_branch="${entry##*$'\t'}" + if [[ "${pr_branch}" == *"-${LOCAL_FP:0:7}" ]]; then + echo " open PR #${pr_num} (${pr_branch}): branch name carries this fingerprint" + DUPLICATE_PR="${pr_num}" + break + fi pr_head="$(gh pr view "${pr_num}" --repo "${REPO}" --json headRefOid --jq .headRefOid 2>/dev/null || true)" if [[ -z "${pr_head}" ]] || ! git fetch --quiet --depth 1 origin "${pr_head}" 2>/dev/null; then echo " open PR #${pr_num} (${pr_branch}): head unfetchable; treating as non-duplicate" diff --git a/tests/unit/python/scripts/github/update/__init__.py b/tests/unit/python/scripts/github/update/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/python/scripts/github/update/open/__init__.py b/tests/unit/python/scripts/github/update/open/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/python/scripts/github/update/open/test_pr.py b/tests/unit/python/scripts/github/update/open/test_pr.py new file mode 100644 index 0000000000..6729c3e1e5 --- /dev/null +++ b/tests/unit/python/scripts/github/update/open/test_pr.py @@ -0,0 +1,112 @@ +from __future__ import annotations + +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + +from utils.cache.files import PROJECT_ROOT + +SCRIPT = PROJECT_ROOT / "scripts" / "github" / "update" / "open" / "pr.sh" +PINNED_FILE = "pin.txt" +GH_STUB = """#!/usr/bin/env bash +if [[ "$1 $2" == "pr list" ]]; then + printf '7\\t%s\\n' "${GH_STUB_BRANCH}" +fi +""" + + +class UpdatePrFixture: + """A throwaway repo with one pending pin bump and a ``gh`` that lists one open PR. + + Args: + tmp: directory that holds the repo and the ``gh`` stub. + """ + + def __init__(self, tmp: str) -> None: + self.repo = Path(tmp) / "repo" + self.repo.mkdir() + stub_dir = Path(tmp) / "bin" + stub_dir.mkdir() + stub = stub_dir / "gh" + stub.write_text(GH_STUB) + stub.chmod(0o755) + inherited = {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + self.env = { + **inherited, + "PATH": f"{stub_dir}{os.pathsep}{os.environ['PATH']}", + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_ALLOW_PROTOCOL": "file", + "GH_TOKEN": "unused", + "UPDATE_BRANCH_PREFIX": "update/pins", + "UPDATE_BRANCH_SUFFIX": "20260101", + "UPDATE_BASE_BRANCH": "main", + "UPDATE_COMMIT_MESSAGE": "update: bump", + "UPDATE_PR_TITLE": "update: bump", + "UPDATE_PR_BODY": "bump", + } + self.git("init", "-b", "main") + self.git("remote", "add", "origin", "https://github.com/acme/widgets.git") + (self.repo / PINNED_FILE).write_text("1.0\n") + self.git("add", "-A") + self.git( + "-c", + "user.name=test", + "-c", + "user.email=test@example.invalid", + "commit", + "-m", + "init", + ) + (self.repo / PINNED_FILE).write_text("1.1\n") + + def git(self, *args: str, stdin: str | None = None) -> str: + return subprocess.run( + ["git", *args], + cwd=self.repo, + env=self.env, + input=stdin, + capture_output=True, + text=True, + check=True, + ).stdout.strip() + + def fingerprint(self) -> str: + blob = self.git("hash-object", PINNED_FILE) + return self.git("hash-object", "--stdin", stdin=f"{blob}\t{PINNED_FILE}\n") + + def open_pr(self, open_branch: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", str(SCRIPT)], + cwd=self.repo, + env={**self.env, "GH_STUB_BRANCH": open_branch}, + capture_output=True, + text=True, + check=False, + ) + + +class TestUpdatePrDedupe(unittest.TestCase): + def test_edited_branch_of_the_same_proposal_is_a_duplicate(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + fixture = UpdatePrFixture(tmp) + branch = f"update/pins-20251231-{fixture.fingerprint()[:7]}" + result = fixture.open_pr(branch) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Open PR #7 already carries this exact change", result.stdout) + self.assertNotIn("Pushing change to branch", result.stdout) + + def test_branch_of_another_proposal_is_not_a_duplicate(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + fixture = UpdatePrFixture(tmp) + result = fixture.open_pr("update/pins-20251231-0000000") + self.assertNotIn("already carries this exact change", result.stdout) + self.assertIn( + f"Pushing change to branch update/pins-20260101-{fixture.fingerprint()[:7]}", + result.stdout, + ) + + +if __name__ == "__main__": + unittest.main()