From 7d91dd6862c1b7b200651ae7d37769c01f1bee4d Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sat, 8 Aug 2026 18:56:05 -0700 Subject: [PATCH] fix(ci): resolve backport_branch from the top-level key only, and honour an empty value `load_backport_branch` reads `backport_branch` out of ci/versions.yml with a regex, then falls back to GITHUB_REF_NAME. Two things go wrong. 1. The value capture is `[^"'\s#]+`, requiring at least one character. An explicitly empty `backport_branch: ""` therefore matches nothing, which is indistinguishable from an absent key, so control falls through to the environment: backport_branch: "" + GITHUB_REF_NAME=12.9.x -> '12.9.x' A maintainer who blanks the setting to disable backport gating gets it silently re-enabled from whatever branch the workflow ran on. 2. The line is `.strip()`ed before matching, which erases indentation. The regex is anchored at column 0 precisely so it matches a top-level key, but after stripping any nesting level matches, and the first one in file order wins: cuda: legacy: backport_branch: "11.8.x" backport_branch: "12.9.x" -> '11.8.x' Match the raw line so only a column-0 key counts, and let the capture be empty so an explicit empty value returns None without consulting the environment. A commented-out `# backport_branch:` was already ignored correctly; a test now pins that too. --- ci/tools/check_release_notes.py | 16 +++++-- ci/tools/tests/test_check_release_notes.py | 51 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/ci/tools/check_release_notes.py b/ci/tools/check_release_notes.py index 75d2c9871f0..cac5f0ad9ff 100644 --- a/ci/tools/check_release_notes.py +++ b/ci/tools/check_release_notes.py @@ -41,7 +41,10 @@ BACKPORT_PLANNING_COMPONENTS = frozenset({"cuda-bindings", "cuda-python"}) BACKPORT_NOT_PLANNED = "not planned" -BACKPORT_BRANCH_RE = re.compile(r"""^backport_branch:\s*["']?(?P[^"'\s#]+)""") +# The value may legitimately be empty ("no backport branch configured"), so the +# capture is `*` rather than `+` -- otherwise an explicitly empty value looks +# exactly like an absent key. +BACKPORT_BRANCH_RE = re.compile(r"""^backport_branch:\s*["']?(?P[^"'\s#]*)""") BACKPORT_BRANCH_NAME_RE = re.compile(r"^\d+\.\d+\.x$") @@ -67,9 +70,16 @@ def load_backport_branch(repo_root: str = ".") -> str | None: try: with open(path, encoding="utf-8") as f: for line in f: - m = BACKPORT_BRANCH_RE.match(line.strip()) + # Match the raw line: the regex is anchored at column 0, so a + # `backport_branch:` nested under some other key does not count. + # Stripping first erased the indentation and let a nested key + # win over the real top-level one. + m = BACKPORT_BRANCH_RE.match(line) if m: - return m.group("branch") + # An explicitly empty value means "no backport branch is + # configured". Falling through to GITHUB_REF_NAME here let + # the environment silently override that decision. + return m.group("branch") or None except FileNotFoundError: pass github_ref_name = os.environ.get("GITHUB_REF_NAME", "") diff --git a/ci/tools/tests/test_check_release_notes.py b/ci/tools/tests/test_check_release_notes.py index 4f65404eed5..01279c1dc46 100644 --- a/ci/tools/tests/test_check_release_notes.py +++ b/ci/tools/tests/test_check_release_notes.py @@ -6,6 +6,8 @@ import os import sys +import pytest + sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from check_release_notes import ( check_release_notes, @@ -145,6 +147,55 @@ def test_ignores_non_backport_github_ref_name(self, tmp_path, monkeypatch): assert load_backport_branch(str(tmp_path)) is None + @pytest.mark.agent_authored(model="claude-opus-5") + def test_explicitly_empty_value_is_not_overridden_by_the_environment(self, tmp_path, monkeypatch): + """`backport_branch: ""` is a decision, not a missing key. + + The value capture required at least one character, so an empty value + matched nothing and was indistinguishable from an absent key -- control + fell through to GITHUB_REF_NAME, and a maintainer who blanked the + setting to disable backport gating got it re-enabled from whatever + branch the workflow happened to run on. + """ + d = tmp_path / "ci" + d.mkdir(parents=True) + (d / "versions.yml").write_text('backport_branch: ""\n') + monkeypatch.setenv("GITHUB_REF_NAME", "12.9.x") + + assert load_backport_branch(str(tmp_path)) is None + + @pytest.mark.agent_authored(model="claude-opus-5") + def test_configured_value_wins_over_the_environment(self, tmp_path, monkeypatch): + d = tmp_path / "ci" + d.mkdir(parents=True) + (d / "versions.yml").write_text('backport_branch: "12.9.x"\n') + monkeypatch.setenv("GITHUB_REF_NAME", "11.8.x") + + assert load_backport_branch(str(tmp_path)) == "12.9.x" + + @pytest.mark.agent_authored(model="claude-opus-5") + def test_nested_key_does_not_shadow_the_top_level_one(self, tmp_path, monkeypatch): + """Only a column-0 `backport_branch:` is the real setting. + + The line was stripped before matching, so indentation was erased and a + `backport_branch:` nested under any other key won on line order. + """ + monkeypatch.delenv("GITHUB_REF_NAME", raising=False) + d = tmp_path / "ci" + d.mkdir(parents=True) + (d / "versions.yml").write_text('cuda:\n legacy:\n backport_branch: "11.8.x"\nbackport_branch: "12.9.x"\n') + + assert load_backport_branch(str(tmp_path)) == "12.9.x" + + @pytest.mark.agent_authored(model="claude-opus-5") + def test_commented_out_key_is_ignored(self, tmp_path, monkeypatch): + monkeypatch.delenv("GITHUB_REF_NAME", raising=False) + d = tmp_path / "ci" + d.mkdir(parents=True) + (d / "versions.yml").write_text('# backport_branch: "9.9.x"\nbackport_branch: "12.9.x"\n') + + assert load_backport_branch(str(tmp_path)) == "12.9.x" + class TestMain: def _make_notes(self, tmp_path, pkg, version, content="Release notes."):