From 96c4f89d45a7371fa3b78a782cdd47ae2ec3737f Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Mon, 7 Sep 2026 10:01:04 +0000 Subject: [PATCH 1/2] nextchanges: Attribute stacked-PR fragments to the PR that added them validate_nextchanges.py required every fragment's trailing PR link to name the current PR. For a gh-stacked PR, the fragments inherited from earlier PRs in the stack are added by ordinary branch commits (no squash "(#N)" subject), so they were mis-attributed to the top PR and CI failed even though each fragment already linked its own PR. For example, #6544 failed because it carried #6448's SDK-bump fragment: "trailing PR link #6448 must include the PR that added this fragment (#6544)". Attribute a fragment to the current PR only when it isn't already present on the PR's base branch; a fragment inherited from an earlier PR in the stack keeps its own link and is no longer re-checked against the top PR. The base branch is BASE_REF (github.event.pull_request.base.ref) in CI, or a best-effort gh lookup locally. Co-authored-by: Isaac --- .github/workflows/changelog-preview.yml | 5 +- tools/validate_nextchanges.py | 88 ++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/.github/workflows/changelog-preview.yml b/.github/workflows/changelog-preview.yml index c250c41faa9..8b1b3aebd36 100644 --- a/.github/workflows/changelog-preview.yml +++ b/.github/workflows/changelog-preview.yml @@ -47,10 +47,13 @@ jobs: # names the right PR. PR_NUMBER is the authoritative event PR (empty on push # to main, where each fragment's PR is inferred from its squash-merge # commit); it must have full history to attribute fragments, hence the - # fetch-depth: 0 checkout above. + # fetch-depth: 0 checkout above. BASE_REF is the PR's base branch, so a + # gh-stacked PR's inherited fragments (which already carry the earlier PR's + # link) aren't attributed to this PR. - name: Validate .nextchanges placement env: PR_NUMBER: ${{ github.event.number }} + BASE_REF: ${{ github.event.pull_request.base.ref }} run: uv run tools/validate_nextchanges.py - name: Render changelog preview diff --git a/tools/validate_nextchanges.py b/tools/validate_nextchanges.py index cca696d7b95..eeee990e046 100755 --- a/tools/validate_nextchanges.py +++ b/tools/validate_nextchanges.py @@ -165,15 +165,18 @@ def pr_link_problem(text, require_pr_link, expected_pr): return None -def infer_expected_pr(path, fallback_pr, root): +def infer_expected_pr(path, fallback_pr, base_ref, root): """Return the PR number that introduced the fragment at ``path``. databricks/cli squash-merges end the commit subject with ``(#N)``, so the commit that most recently added the file names its PR. A fragment not yet on main (added on the current branch, or uncommitted) has no such commit, so - ``fallback_pr`` — the current PR — is used. ``git`` runs in ``root`` so the - repo being validated is queried even when ``--root`` differs from the process - CWD. Requires full git history (the workflow checks out with + ``fallback_pr`` — the current PR — is used, unless the fragment already exists + on the PR's base branch: a gh-stacked PR carries the fragments of the PRs it is + stacked on, which keep their own link, so ``None`` is returned for those and no + specific PR is imposed (see ``fragment_on_base``). ``git`` runs in ``root`` so + the repo being validated is queried even when ``--root`` differs from the + process CWD. Requires full git history (the workflow checks out with ``fetch-depth: 0``); best-effort, so any git failure falls back rather than erroring.""" try: @@ -190,9 +193,37 @@ def infer_expected_pr(path, fallback_pr, root): m = re.search(r"\(#(\d+)\)\s*$", result.stdout.strip()) if m: return m.group(1) + if base_ref and fragment_on_base(path, base_ref, root): + return None return fallback_pr +def fragment_on_base(path, base_ref, root): + """Whether the fragment at ``path`` already exists on the PR's base branch. + + A stacked PR inherits the fragments of the PRs below it; those already live on + the base branch and carry their own PR link, so they are not attributed to the + current PR. The base is looked up as the remote-tracking ``origin/`` + (CI's checkout), falling back to a local branch ````. Best-effort: any + git failure returns ``False``, attributing the fragment to the current PR as + before.""" + rel = path.relative_to(root).as_posix() + for ref in (f"origin/{base_ref}", base_ref): + try: + result = subprocess.run( + ["git", "cat-file", "-e", f"{ref}:{rel}"], + capture_output=True, + text=True, + timeout=10, + cwd=root, + ) + except (OSError, subprocess.SubprocessError): + return False + if result.returncode == 0: + return True + return False + + def is_shallow(root): """Whether ``root`` is a shallow git clone. Best-effort; a git failure (or a non-repo) returns False. On a shallow clone every file looks added at the @@ -225,13 +256,15 @@ def load_sections(root): return tuple(sections) -def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=None, root=None): +def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=None, base_ref=None, root=None): """Return a list of ``(path, message)`` for anything unexpected under ``.nextchanges/``: files that aren't a section fragment or known scaffolding, malformed fragments, a trailing PR link that is missing or names the wrong PR, and a missing/malformed version file. ``require_pr_link`` and ``fallback_pr`` drive the PR-link checks (set in CI / from the branch's PR, - see ``main``); ``root`` is the repo the PR inference queries via git.""" + see ``main``); ``base_ref`` is the PR's base branch, so fragments inherited + from an earlier PR in a stack aren't attributed to the current PR (see + ``infer_expected_pr``). ``root`` is the repo the PR inference queries via git.""" problems = [] known_sections = set(sections) # A shallow clone (e.g. the merge queue's checkout) makes every fragment look @@ -268,7 +301,7 @@ def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=No if problem is None: # Infer the expected PR (a git call) only for a structurally # valid fragment, and only with full history — see `shallow`. - expected_pr = None if shallow else infer_expected_pr(path, fallback_pr, root) + expected_pr = None if shallow else infer_expected_pr(path, fallback_pr, base_ref, root) problem = pr_link_problem(text, require_pr_link, expected_pr) if problem: problems.append((path, problem)) @@ -332,6 +365,43 @@ def detect_current_pr(root): return current_branch_pr(root) +def current_branch_base(root): + """Best-effort base branch of the current branch's PR (via ``gh``), or ``None``. + + A local convenience mirroring ``current_branch_pr``; CI uses the authoritative + event base instead (see ``detect_base_ref``). Any ``gh`` failure returns + ``None`` so local runs never hard-fail on tooling.""" + try: + result = subprocess.run( + ["gh", "pr", "view", "--json", "baseRefName", "-q", ".baseRefName"], + capture_output=True, + text=True, + timeout=10, + cwd=root, + ) + except (OSError, subprocess.SubprocessError) as e: + print(f"gh pr view failed: {e}", file=sys.stderr) + return None + out = result.stdout.strip() + return out if result.returncode == 0 and out else None + + +def detect_base_ref(root): + """Base branch of the PR being validated, or ``None``. + + In CI, the authoritative event base (``BASE_REF``, set from + ``github.event.pull_request.base.ref``); empty on a push to main. Locally, a + best-effort ``gh`` lookup of the branch's PR. Used to spare a stacked PR's + inherited fragments from being attributed to the current PR (see + ``infer_expected_pr``).""" + base = os.environ.get("BASE_REF", "").strip() + if base: + return base + if in_ci(): + return None + return current_branch_base(root) + + def has_fragments(changelog_dir): """Whether any *.md fragment (excluding README.md) exists under a section.""" return any(p.name != README for p in changelog_dir.glob("*/*.md")) @@ -403,11 +473,13 @@ def main(argv=None): # only when there are fragments, to avoid a `gh` call on unrelated runs. require_pr_link = False fallback_pr = None + base_ref = None if has_fragments(changelog_dir): fallback_pr = detect_current_pr(args.root) + base_ref = detect_base_ref(args.root) require_pr_link = in_ci() or fallback_pr is not None - problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, args.root) + problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, base_ref, args.root) if problems: for path, msg in problems: print(f"{path}: {msg}", file=sys.stderr) From 819d6a5ffdcf2ec4d77f8b5c26fe062b25636f8a Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Mon, 7 Sep 2026 12:50:55 +0000 Subject: [PATCH 2/2] Log git cat-file failures in fragment_on_base Print the error to stderr before returning False so a git tooling failure is visible in CI logs, matching current_branch_pr / current_branch_base. Co-authored-by: Isaac --- tools/validate_nextchanges.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/validate_nextchanges.py b/tools/validate_nextchanges.py index eeee990e046..546430e929d 100755 --- a/tools/validate_nextchanges.py +++ b/tools/validate_nextchanges.py @@ -217,7 +217,8 @@ def fragment_on_base(path, base_ref, root): timeout=10, cwd=root, ) - except (OSError, subprocess.SubprocessError): + except (OSError, subprocess.SubprocessError) as e: + print(f"git cat-file failed: {e}", file=sys.stderr) return False if result.returncode == 0: return True