From 76fe5e4a72c3dc191adbab5d799bfbffb94ce921 Mon Sep 17 00:00:00 2001 From: ualtinok <94532+ualtinok@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:10:40 +0100 Subject: [PATCH] Landing policy: PR and queue commit ranges follow ancestry, not dates Merge-queue commits carry committer dates out of order. git rev-list base..head walks by date and stopped early, so a queue group counted main's own #984 merge commit as a commit of #1034 and refused it for a missing co-author trailer. Commit ranges are now computed from full ancestry sets, and the queue's first-parent walk stops exactly at the group base. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> --- .../land-landing-policy-exact-ranges.json | 12 +++ scripts/lib/landing_policy.py | 16 +++- scripts/lib/test_merge_queue_workflow.py | 80 +++++++++++++++++++ scripts/merge-queue-search.py | 18 ++++- 4 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 benchmarks/aft-search/slice-descriptors/land-landing-policy-exact-ranges.json diff --git a/benchmarks/aft-search/slice-descriptors/land-landing-policy-exact-ranges.json b/benchmarks/aft-search/slice-descriptors/land-landing-policy-exact-ranges.json new file mode 100644 index 0000000000..8c4da0fda1 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-landing-policy-exact-ranges.json @@ -0,0 +1,12 @@ +{ + "slice_class": "non_ranking", + "targeted_mechanism": "none", + "kind": "harness", + "fixtures": [ + "harness-goldens" + ], + "slices": [ + "landing-policy-exact-ranges" + ], + "note": "CI landing scripts only (scripts/lib/landing_policy.py, scripts/merge-queue-search.py and their test): PR and queue commit ranges follow ancestry instead of committer dates. No aft_search ranking, fusion, lane admission or truncation change." +} diff --git a/scripts/lib/landing_policy.py b/scripts/lib/landing_policy.py index d8d497eee3..8ff0186021 100644 --- a/scripts/lib/landing_policy.py +++ b/scripts/lib/landing_policy.py @@ -26,13 +26,25 @@ def git(*args: str, input: str | None = None) -> str: return subprocess.check_output(["git", *args], input=input, text=True).strip() +def commits_between(base: str, head: str) -> list[str]: + """Commits reachable from head but not from base, newest first. + + Compares full ancestry sets instead of asking `git rev-list base..head`. + Merge-queue commits carry committer dates out of order, and git's + date-based range walk then lists commits that base already contains (a + queue group once reported main's own merge commit as a PR commit). + """ + known = set(git("rev-list", base).splitlines()) + return [sha for sha in git("rev-list", head).splitlines() if sha not in known] + + def check_commits(base: str, head: str, branch: str) -> int: for sha in (base, head): if re.fullmatch(r"[0-9a-f]{40}", sha) is None: raise ValueError("identity policy requires full base/head SHAs") git("cat-file", "-e", f"{sha}^{{commit}}") count = 0 - for sha in git("rev-list", f"{base}..{head}").splitlines(): + for sha in commits_between(base, head): name, author, committer, message = git( "show", "-s", "--format=%an%x00%ae%x00%ce%x00%B", sha ).split("\0", 3) @@ -80,7 +92,7 @@ def check_event(event: str, payload: dict) -> int: base = git("merge-base", "origin/main", head) return check_commits(base, head, payload["ref"].removeprefix("refs/heads/")) base = payload["before"] - commits = git("rev-list", "--first-parent", f"{base}..{head}").splitlines() + commits = queue.first_parent_between(base, head) if commits and all(len(git("show", "-s", "--format=%P", sha).split()) == 2 for sha in commits): members = queue.queue_members(base, head, None, payload["repository"]["full_name"]) return sum(check_commits(base, member["head"], member["branch"]) for member in members) diff --git a/scripts/lib/test_merge_queue_workflow.py b/scripts/lib/test_merge_queue_workflow.py index bfbebae1d1..af7297b61d 100644 --- a/scripts/lib/test_merge_queue_workflow.py +++ b/scripts/lib/test_merge_queue_workflow.py @@ -374,5 +374,85 @@ def test_score_replay_occurs_once_and_other_descriptors_use_that_score(self) -> +class SkewedQueueHistoryTests(unittest.TestCase): + """Merge-queue commits carry committer dates out of order. Ranges must follow + ancestry, not dates: one queue group reported main's own merge commit as a + PR commit because `git rev-list base..head` stopped its walk on dates.""" + + def setUp(self) -> None: + self.temp = tempfile.TemporaryDirectory(prefix="aft-skewed-queue-") + self.addCleanup(self.temp.cleanup) + self.repo = Path(self.temp.name) + self.git("init", "-q", "-b", "main") + self.git("config", "user.name", "Queue fixture") + self.git("config", "user.email", "queue-fixture@example.invalid") + self.git("config", "commit.gpgsign", "false") + self.commit("01:00:00", "root") + m1 = self.commit("02:00:00", "m1") + self.merged_main = self.commit("03:00:00", "merged main commit") + self.pr_base = self.commit("03:10:00", "pr base") + self.git("checkout", "-qb", "pr") + self.pr_head = self.commit("10:00:00", "pr commit") + self.git("checkout", "-qb", "side", m1) + self.commit("02:30:00", "side") + self.git("checkout", "-q", "main") + # Later main commits dated before everything the PR sits on, more of them + # than the five extra commits git walks before ending a date-ordered + # range walk. + for minute in range(1, 9): + self.commit(f"00:1{minute}:00", f"queue {minute}") + self.git("merge", "-q", "--no-ff", "-m", "group base", "side", + env=self.dated("00:19:30")) + self.group_base = self.git("rev-parse", "HEAD") + + def dated(self, clock: str) -> dict: + stamp = f"2026-01-01T{clock}Z" + return {**os.environ, "GIT_AUTHOR_DATE": stamp, "GIT_COMMITTER_DATE": stamp} + + def git(self, *args: str, env: dict | None = None) -> str: + return subprocess.run( + ["git", *args], cwd=self.repo, env=env, text=True, check=True, capture_output=True + ).stdout.strip() + + def commit(self, clock: str, message: str) -> str: + self.git("commit", "-q", "--allow-empty", "-m", message, env=self.dated(clock)) + return self.git("rev-parse", "HEAD") + + def in_repo(self): + previous = Path.cwd() + os.chdir(self.repo) + self.addCleanup(os.chdir, previous) + + def test_pr_commits_exclude_main_commits_the_group_base_contains(self) -> None: + # The naive range lists the PR base and an older main commit too. + naive = self.git("rev-list", f"{self.group_base}..{self.pr_head}").splitlines() + self.assertIn(self.merged_main, naive) + sys.path.insert(0, str(ROOT / "scripts/lib")) + import landing_policy + + self.in_repo() + self.assertEqual(landing_policy.commits_between(self.group_base, self.pr_head), [self.pr_head]) + + def test_first_parent_chain_stops_exactly_at_base(self) -> None: + queue = import_queue_search() + self.in_repo() + chain = queue.first_parent_between(self.pr_base, self.group_base) + self.assertEqual(len(chain), 9) + self.assertEqual(chain[-1], self.group_base) + self.assertNotIn(self.pr_base, chain) + with self.assertRaises(queue.InputFault): + queue.first_parent_between(self.pr_head, self.group_base) + + +def import_queue_search(): + spec = importlib.util.spec_from_file_location( + "merge_queue_search", ROOT / "scripts/merge-queue-search.py" + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + if __name__ == "__main__": unittest.main() diff --git a/scripts/merge-queue-search.py b/scripts/merge-queue-search.py index b342fee877..b1cc817d8b 100644 --- a/scripts/merge-queue-search.py +++ b/scripts/merge-queue-search.py @@ -38,6 +38,22 @@ def paths(base: str, head: str) -> list[str]: return git("diff", "--name-only", "--no-renames", f"{base}...{head}").splitlines() +def first_parent_between(base: str, head: str) -> list[str]: + """Commits on head's first-parent chain after base, oldest first. + + Walks the chain from head until it reaches base instead of asking + `git rev-list base..head`. Merge-queue commits carry committer dates out of + order (a group commit can be dated before the commits under it), and git's + date-based range walk then returns commits that base already contains. + """ + chain = [] + for sha in git("rev-list", "--first-parent", head).splitlines(): + if sha == base: + return list(reversed(chain)) + chain.append(sha) + raise InputFault("queue_base_not_on_first_parent_chain") + + def resolve_git_descriptor(descriptor: dict, changed: list[str], base: str, head: str) -> tuple[dict, bool]: notes: list[str] = [] try: @@ -244,7 +260,7 @@ def queue_members(base: str, head: str, head_ref: str | None, repository: str) - raise InputFault(f"unknown_queue_ref:{head_ref}") members = [] previous = base - commits = git("rev-list", "--first-parent", "--reverse", f"{base}..{head}").splitlines() + commits = first_parent_between(base, head) for index, commit in enumerate(commits): parents = git("show", "-s", "--format=%P", commit).split() # Only two-parent merges bind the synthetic commit to an exact PR head.