From 20648036a87aaf992ff235a192aa9c1d5f8a0a25 Mon Sep 17 00:00:00 2001 From: injaneity <44902825+injaneity@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:12:16 +0800 Subject: [PATCH 1/4] fix(ci): ignore clean main sync merges for attribution --- .github/scripts/release_attribution.py | 32 +++++++++-- .github/scripts/tests/test_pr_attribution.py | 58 +++++++++++++++++++- 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/.github/scripts/release_attribution.py b/.github/scripts/release_attribution.py index dc7a1d5a51..f64fdbfba9 100644 --- a/.github/scripts/release_attribution.py +++ b/.github/scripts/release_attribution.py @@ -527,6 +527,7 @@ def validate_pr_attribution( base_config: Mapping[str, Any], identity_changes: Mapping[str, str | None], github: GitHubClient, + main_sha: str = "", ) -> None: """Fail closed when preserved human authorship cannot reach a GitHub login. @@ -611,12 +612,30 @@ def record_unresolved( is_base_sync_merge = len(parents) > 1 and any( parent and parent not in commit_shas for parent in parents[1:] ) - unlinked_author = not author_login and author_email not in ignored - distinct_external_author = bool(author_login) and ( - author_login.lower() != pull_login.lower() - and not is_bot(author_login, bots) - and author_login.lower() not in internal - and author_login.lower() not in opt_out + clean_main_sync = False + if is_base_sync_merge and len(parents) == 2 and main_sha: + try: + ancestry = github.get(f"repos/{repository}/compare/{parents[1]}...{main_sha}") + if ancestry["status"] in {"ahead", "identical"}: + comparisons = [ + github.get(f"repos/{repository}/compare/{parent}...{sha}")["files"] + for parent in parents + ] + clean_main_sync = all(len(files) < 300 for files in comparisons) and { + file["filename"] for file in comparisons[0] + }.isdisjoint(file["filename"] for file in comparisons[1]) + except (ReleaseError, OSError, ValueError, KeyError, TypeError): + pass + unlinked_author = not clean_main_sync and not author_login and author_email not in ignored + distinct_external_author = ( + not clean_main_sync + and bool(author_login) + and ( + author_login.lower() != pull_login.lower() + and not is_bot(author_login, bots) + and author_login.lower() not in internal + and author_login.lower() not in opt_out + ) ) # An ordinary direct PR author is credited from pull-request metadata even # when their commit email is private. A distinct unlinked author committed @@ -1467,6 +1486,7 @@ def validate_pr_command(args: argparse.Namespace) -> None: base_config=base_config, identity_changes=identity_changes, github=client, + main_sha=trusted_sha, ) print(f"contributor attribution is merge-ready for pull request #{number}") diff --git a/.github/scripts/tests/test_pr_attribution.py b/.github/scripts/tests/test_pr_attribution.py index c78c6a6185..8b8dac3e89 100644 --- a/.github/scripts/tests/test_pr_attribution.py +++ b/.github/scripts/tests/test_pr_attribution.py @@ -58,9 +58,16 @@ def commit( class SourceGitHub: - def __init__(self, authors, source_emails): + def __init__(self, authors, source_emails, comparisons=None): self.authors = authors self.source_emails = source_emails + self.comparisons = comparisons or {} + + def get(self, path): + value = self.comparisons[path] + if isinstance(value, Exception): + raise value + return value def pull(self, repository: str, number: int): assert repository == "trycua/cua" @@ -81,6 +88,8 @@ def validate( changes=None, authors=None, source_emails=None, + comparisons=None, + main_sha="", ): base = base or config() validate_pr_attribution( @@ -89,7 +98,8 @@ def validate( commits=commits or [], base_config=base, identity_changes=changes or {}, - github=SourceGitHub(authors or {}, source_emails or {}), + github=SourceGitHub(authors or {}, source_emails or {}, comparisons), + **({"main_sha": main_sha} if main_sha else {}), ) @@ -456,3 +466,47 @@ def test_compare_pages_that_stop_short_are_refused(): client = PagedCompareGitHub(total=250, reported_total=300) with pytest.raises(release_attribution.ReleaseError, match="refusing partial"): client.all_pull_commits("trycua/cua-staging", large_pull()) + + +@pytest.mark.parametrize( + "status, branch_files, main_files, failure, clean", + [ + ("ahead", ["main-change"], ["branch-change"], False, True), + ("identical", ["main-change"], ["branch-change"], False, True), + ("ahead", ["conflict"], ["conflict"], False, False), + ("diverged", ["main-change"], ["branch-change"], False, False), + ("ahead", [str(i) for i in range(300)], ["branch-change"], False, False), + ("ahead", ["main-change"], [str(i) for i in range(300)], False, False), + ("ahead", ["main-change"], ["branch-change"], True, False), + ], +) +def test_main_sync_merge_authorship(status, branch_files, main_files, failure, clean): + comparisons = { + "repos/trycua/cua/compare/main-tip...trusted-main": {"status": status}, + "repos/trycua/cua/compare/feature-tip...sync-merge": { + "files": [{"filename": name} for name in branch_files] + }, + "repos/trycua/cua/compare/main-tip...sync-merge": ( + ReleaseError("API failed") + if failure + else {"files": [{"filename": name} for name in main_files]} + ), + } + values = dict( + commits=[ + commit(sha="feature-tip", login="landing-author"), + commit( + sha="sync-merge", + login="injaneity", + email="44902825+injaneity@users.noreply.github.com", + parent_shas=("feature-tip", "main-tip"), + ), + ], + comparisons=comparisons, + main_sha="trusted-main", + ) + if clean: + validate(**values) + else: + with pytest.raises(ReleaseError, match="base synchronization merge"): + validate(**values) From 346ecbaad9403a60d4aa1c4b55adf7a4278d00d4 Mon Sep 17 00:00:00 2001 From: injaneity <44902825+injaneity@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:19:43 +0800 Subject: [PATCH 2/4] fix(ci): account for rename sources in sync merge attribution --- .github/scripts/release_attribution.py | 26 +++++++++++++------- .github/scripts/tests/test_pr_attribution.py | 7 ++++-- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/.github/scripts/release_attribution.py b/.github/scripts/release_attribution.py index f64fdbfba9..0deccf6553 100644 --- a/.github/scripts/release_attribution.py +++ b/.github/scripts/release_attribution.py @@ -26,7 +26,6 @@ from urllib.parse import quote from urllib.request import Request, urlopen - CONVENTIONAL_RE = re.compile( r"^(?P[a-z][a-z0-9-]*)(?:\((?P[^)]+)\))?(?P!)?:\s+(?P\S.*)$" ) @@ -124,6 +123,7 @@ class ConventionalEntry: PULL_COMMITS_ENDPOINT_CAP = 250 MAX_COMPARE_PAGES = 100 + class GitHubClient: """Small GitHub REST client with explicit, testable endpoints.""" @@ -171,7 +171,9 @@ def pull_commits(self, repository: str, number: int) -> list[dict[str, Any]]: return commits raise ReleaseError(f"pull request #{number} has too many commits to validate safely") - def compare_commits(self, repository: str, base: str, head: str) -> tuple[list[dict[str, Any]], int]: + def compare_commits( + self, repository: str, base: str, head: str + ) -> tuple[list[dict[str, Any]], int]: """Every commit in ``base...head`` and GitHub's total, paging the compare API. The pull request commits endpoint stops at 250 commits; the compare @@ -621,10 +623,18 @@ def record_unresolved( github.get(f"repos/{repository}/compare/{parent}...{sha}")["files"] for parent in parents ] - clean_main_sync = all(len(files) < 300 for files in comparisons) and { - file["filename"] for file in comparisons[0] - }.isdisjoint(file["filename"] for file in comparisons[1]) - except (ReleaseError, OSError, ValueError, KeyError, TypeError): + if all(len(files) < 300 for files in comparisons): + changed_paths = [ + { + path + for file in files + for path in (file["filename"], file.get("previous_filename")) + if path + } + for files in comparisons + ] + clean_main_sync = changed_paths[0].isdisjoint(changed_paths[1]) + except (ReleaseError, OSError, ValueError): pass unlinked_author = not clean_main_sync and not author_login and author_email not in ignored distinct_external_author = ( @@ -1465,9 +1475,7 @@ def validate_pr_command(args: argparse.Namespace) -> None: base_config = json.loads(args.config.read_text()) head_config = client.file_json(head_repository, args.config.as_posix(), head_sha) trusted_sha = run_git(Path.cwd(), "rev-parse", "HEAD").strip() - comparison = client.get( - f"repos/{repository}/compare/{trusted_sha}...{head_sha}?per_page=1" - ) + comparison = client.get(f"repos/{repository}/compare/{trusted_sha}...{head_sha}?per_page=1") ancestor_sha = str((comparison.get("merge_base_commit") or {}).get("sha") or "") if not ancestor_sha: raise ReleaseError("GitHub returned no merge base for attribution validation") diff --git a/.github/scripts/tests/test_pr_attribution.py b/.github/scripts/tests/test_pr_attribution.py index 8b8dac3e89..1e1746413d 100644 --- a/.github/scripts/tests/test_pr_attribution.py +++ b/.github/scripts/tests/test_pr_attribution.py @@ -99,7 +99,7 @@ def validate( base_config=base, identity_changes=changes or {}, github=SourceGitHub(authors or {}, source_emails or {}, comparisons), - **({"main_sha": main_sha} if main_sha else {}), + main_sha=main_sha, ) @@ -474,6 +474,7 @@ def test_compare_pages_that_stop_short_are_refused(): ("ahead", ["main-change"], ["branch-change"], False, True), ("identical", ["main-change"], ["branch-change"], False, True), ("ahead", ["conflict"], ["conflict"], False, False), + ("ahead", [{"filename": "q", "previous_filename": "p"}], ["p"], False, False), ("diverged", ["main-change"], ["branch-change"], False, False), ("ahead", [str(i) for i in range(300)], ["branch-change"], False, False), ("ahead", ["main-change"], [str(i) for i in range(300)], False, False), @@ -484,7 +485,9 @@ def test_main_sync_merge_authorship(status, branch_files, main_files, failure, c comparisons = { "repos/trycua/cua/compare/main-tip...trusted-main": {"status": status}, "repos/trycua/cua/compare/feature-tip...sync-merge": { - "files": [{"filename": name} for name in branch_files] + "files": [ + name if isinstance(name, dict) else {"filename": name} for name in branch_files + ] }, "repos/trycua/cua/compare/main-tip...sync-merge": ( ReleaseError("API failed") From 2a0ca5b7e004b265a54c31b72101ec875446dc79 Mon Sep 17 00:00:00 2001 From: injaneity <44902825+injaneity@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:28:18 +0800 Subject: [PATCH 3/4] fix(ci): simplify clean sync attribution checks --- .github/scripts/release_attribution.py | 73 ++++++++++---------- .github/scripts/tests/test_pr_attribution.py | 17 ++++- 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/.github/scripts/release_attribution.py b/.github/scripts/release_attribution.py index 0deccf6553..37b97cc8ed 100644 --- a/.github/scripts/release_attribution.py +++ b/.github/scripts/release_attribution.py @@ -26,6 +26,7 @@ from urllib.parse import quote from urllib.request import Request, urlopen + CONVENTIONAL_RE = re.compile( r"^(?P[a-z][a-z0-9-]*)(?:\((?P[^)]+)\))?(?P!)?:\s+(?P\S.*)$" ) @@ -123,7 +124,6 @@ class ConventionalEntry: PULL_COMMITS_ENDPOINT_CAP = 250 MAX_COMPARE_PAGES = 100 - class GitHubClient: """Small GitHub REST client with explicit, testable endpoints.""" @@ -171,9 +171,7 @@ def pull_commits(self, repository: str, number: int) -> list[dict[str, Any]]: return commits raise ReleaseError(f"pull request #{number} has too many commits to validate safely") - def compare_commits( - self, repository: str, base: str, head: str - ) -> tuple[list[dict[str, Any]], int]: + def compare_commits(self, repository: str, base: str, head: str) -> tuple[list[dict[str, Any]], int]: """Every commit in ``base...head`` and GitHub's total, paging the compare API. The pull request commits endpoint stops at 250 commits; the compare @@ -521,6 +519,27 @@ def unresolved_coauthor_identities( return sorted(unresolved.values(), key=lambda item: (item["email"], item["sha"])) +def is_clean_main_sync(github, repository, sha, parents, main_sha) -> bool: + if len(parents) != 2 or not main_sha: + return False + try: + ancestry = github.get(f"repos/{repository}/compare/{parents[1]}...{main_sha}") + if ancestry.get("status") not in {"ahead", "identical"}: + return False + changed_paths = [] + for parent in parents: + files = github.get(f"repos/{repository}/compare/{parent}...{sha}").get("files") + if files is None or len(files) >= 300: + return False + changed_paths.append( + {file.get(key) for file in files for key in ("filename", "previous_filename")} + - {None} + ) + return changed_paths[0].isdisjoint(changed_paths[1]) + except (ReleaseError, OSError, ValueError, KeyError, TypeError): + return False + + def validate_pr_attribution( *, repository: str, @@ -614,38 +633,12 @@ def record_unresolved( is_base_sync_merge = len(parents) > 1 and any( parent and parent not in commit_shas for parent in parents[1:] ) - clean_main_sync = False - if is_base_sync_merge and len(parents) == 2 and main_sha: - try: - ancestry = github.get(f"repos/{repository}/compare/{parents[1]}...{main_sha}") - if ancestry["status"] in {"ahead", "identical"}: - comparisons = [ - github.get(f"repos/{repository}/compare/{parent}...{sha}")["files"] - for parent in parents - ] - if all(len(files) < 300 for files in comparisons): - changed_paths = [ - { - path - for file in files - for path in (file["filename"], file.get("previous_filename")) - if path - } - for files in comparisons - ] - clean_main_sync = changed_paths[0].isdisjoint(changed_paths[1]) - except (ReleaseError, OSError, ValueError): - pass - unlinked_author = not clean_main_sync and not author_login and author_email not in ignored - distinct_external_author = ( - not clean_main_sync - and bool(author_login) - and ( - author_login.lower() != pull_login.lower() - and not is_bot(author_login, bots) - and author_login.lower() not in internal - and author_login.lower() not in opt_out - ) + unlinked_author = not author_login and author_email not in ignored + distinct_external_author = bool(author_login) and ( + author_login.lower() != pull_login.lower() + and not is_bot(author_login, bots) + and author_login.lower() not in internal + and author_login.lower() not in opt_out ) # An ordinary direct PR author is credited from pull-request metadata even # when their commit email is private. A distinct unlinked author committed @@ -654,7 +647,9 @@ def record_unresolved( # A base synchronization merge imports a parent outside the PR commit set, # so it is not salvaged contributor work. Its distinct author must still # have explicit coauthor credit elsewhere in the PR. - if is_base_sync_merge and distinct_external_author: + if is_base_sync_merge and is_clean_main_sync(github, repository, sha, parents, main_sha): + pass + elif is_base_sync_merge and distinct_external_author: if author_login.lower() not in credited_coauthors: trailer = f"Co-authored-by: {author_name} <{author_email}>" early_errors.append( @@ -1475,7 +1470,9 @@ def validate_pr_command(args: argparse.Namespace) -> None: base_config = json.loads(args.config.read_text()) head_config = client.file_json(head_repository, args.config.as_posix(), head_sha) trusted_sha = run_git(Path.cwd(), "rev-parse", "HEAD").strip() - comparison = client.get(f"repos/{repository}/compare/{trusted_sha}...{head_sha}?per_page=1") + comparison = client.get( + f"repos/{repository}/compare/{trusted_sha}...{head_sha}?per_page=1" + ) ancestor_sha = str((comparison.get("merge_base_commit") or {}).get("sha") or "") if not ancestor_sha: raise ReleaseError("GitHub returned no merge base for attribution validation") diff --git a/.github/scripts/tests/test_pr_attribution.py b/.github/scripts/tests/test_pr_attribution.py index 1e1746413d..eefe8b9cee 100644 --- a/.github/scripts/tests/test_pr_attribution.py +++ b/.github/scripts/tests/test_pr_attribution.py @@ -479,11 +479,13 @@ def test_compare_pages_that_stop_short_are_refused(): ("ahead", [str(i) for i in range(300)], ["branch-change"], False, False), ("ahead", ["main-change"], [str(i) for i in range(300)], False, False), ("ahead", ["main-change"], ["branch-change"], True, False), + (None, ["main-change"], ["branch-change"], False, False), + ("ahead", ["main-change"], ["branch-change"], "missing-files", False), ], ) def test_main_sync_merge_authorship(status, branch_files, main_files, failure, clean): comparisons = { - "repos/trycua/cua/compare/main-tip...trusted-main": {"status": status}, + "repos/trycua/cua/compare/main-tip...trusted-main": {"status": status} if status else {}, "repos/trycua/cua/compare/feature-tip...sync-merge": { "files": [ name if isinstance(name, dict) else {"filename": name} for name in branch_files @@ -491,8 +493,12 @@ def test_main_sync_merge_authorship(status, branch_files, main_files, failure, c }, "repos/trycua/cua/compare/main-tip...sync-merge": ( ReleaseError("API failed") - if failure - else {"files": [{"filename": name} for name in main_files]} + if failure is True + else ( + {} + if failure == "missing-files" + else {"files": [{"filename": name} for name in main_files]} + ) ), } values = dict( @@ -510,6 +516,11 @@ def test_main_sync_merge_authorship(status, branch_files, main_files, failure, c ) if clean: validate(**values) + values["commits"][1]["commit"][ + "message" + ] += "\n\nCo-authored-by: Pair " + with pytest.raises(ReleaseError, match="has no explicit same-repository source PR"): + validate(**values) else: with pytest.raises(ReleaseError, match="base synchronization merge"): validate(**values) From 6d04b7e1c4bf2663e72086a2caa308c0e1af966a Mon Sep 17 00:00:00 2001 From: injaneity <44902825+injaneity@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:53:20 +0800 Subject: [PATCH 4/4] fix: retain unresolved author validation for clean sync merges --- .github/scripts/release_attribution.py | 10 +++++----- .github/scripts/tests/test_pr_attribution.py | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/scripts/release_attribution.py b/.github/scripts/release_attribution.py index 37b97cc8ed..638ded5e25 100644 --- a/.github/scripts/release_attribution.py +++ b/.github/scripts/release_attribution.py @@ -646,11 +646,11 @@ def record_unresolved( # # A base synchronization merge imports a parent outside the PR commit set, # so it is not salvaged contributor work. Its distinct author must still - # have explicit coauthor credit elsewhere in the PR. - if is_base_sync_merge and is_clean_main_sync(github, repository, sha, parents, main_sha): - pass - elif is_base_sync_merge and distinct_external_author: - if author_login.lower() not in credited_coauthors: + # have explicit coauthor credit elsewhere in the PR unless it is a clean main sync. + if is_base_sync_merge and distinct_external_author: + if author_login.lower() not in credited_coauthors and not is_clean_main_sync( + github, repository, sha, parents, main_sha + ): trailer = f"Co-authored-by: {author_name} <{author_email}>" early_errors.append( f"base synchronization merge {sha} is authored by @{author_login}, " diff --git a/.github/scripts/tests/test_pr_attribution.py b/.github/scripts/tests/test_pr_attribution.py index eefe8b9cee..d3d6d7fc92 100644 --- a/.github/scripts/tests/test_pr_attribution.py +++ b/.github/scripts/tests/test_pr_attribution.py @@ -516,6 +516,13 @@ def test_main_sync_merge_authorship(status, branch_files, main_files, failure, c ) if clean: validate(**values) + unlinked = commit( + sha="sync-merge", + email="unlinked@institution.example", + parent_shas=("feature-tip", "main-tip"), + ) + with pytest.raises(ReleaseError, match="unlinked commit author"): + validate(**{**values, "commits": [values["commits"][0], unlinked]}) values["commits"][1]["commit"][ "message" ] += "\n\nCo-authored-by: Pair "