fix(ci): allow accepted protobuf breaks for v0.1.6 prereleases (#4976)

* fix(ci): allow accepted protobuf breaks for one prerelease Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(ci): cover all v0.1.6 prerelease candidates Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * docs(ci): document full rollback of temporary protobuf exception Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(ci): complete protobuf test copyright header Signed-off-by: Taylor Mutch <taylormutch@gmail.com> --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
2026-10-29 07:23:25 +08:00 · 2026-10-27 15:44:53 +00:00
parent dedb089f0b
commit ac7f615cd1
7 changed files with 240 additions and 1 deletions
@@ -42,6 +42,21 @@ tag, or start a second `Pre-release` run for a tag; it has no concurrency guard.
  picks the highest `-pre.N` on `main` whose whole `Pre-release` run passed,
  creates the stable tag on that commit, and publishes without requalifying.

+### Patch-train protobuf exception
+
+All `v0.1.6-pre.N` candidates have a release-captain-approved protobuf compatibility exception
+in `release-qualification.yml`. The check still compiles both protobuf snapshots
+and runs Buf; only detected breaking changes are allowed. Compilation and tooling
+errors still fail. Other versions retain normal version-based enforcement.
+The run summary and qualification JSON record the exception and reason. Review
+all protobuf findings for each candidate and include migration guidance in the
+release notes; the exception does not accept other compatibility-review findings.
+Squash-merge PR #4976 and, after verifying v0.1.6 publication, prepare a cleanup PR
+reverting its entire squash-merge commit. Follow "Remove the v0.1.6 exception" in
+`CI.md`; remove the checker support and metadata along with the workflow exception.
+A new prerelease number
+within this patch train retains the exception; other patch or minor trains do not.
+
 ## Setup

 ```shell
@@ -169,6 +169,8 @@ the shared `check-protobuf-compatibility` action with `nix run .#check-protobuf-
 During `0.x`, a minor train permits compatibility findings
 as warnings; a patch train or no active train rejects them. Compare the current
 train's version with the latest stable release; commit messages are irrelevant.
+For the patch-train `v0.1.6-pre.N` exception, inspect the run summary and the
+qualification JSON's `exceptions` array; see `release-captain` for disposition.
 Compilation, baseline, and tool errors remain fatal. The `protobuf_compatibility` suite participates in
 the `release-tag-v1` qualification profile. Pre-release calls the reusable
 `release-qualification.yml` workflow after building, in parallel with publication. Release
@@ -30,6 +30,10 @@ permissions:

 env:
  RELEASE_TAG: ${{ inputs.release-tag }}
+  # TEMPORARY (#4976): squash-merge and revert after v0.1.6 publication is verified.
+  # See CI.md's "Remove the v0.1.6 exception" procedure.
+  PROTO_COMPATIBILITY_EXCEPTION_TAG: ${{ inputs.is-prerelease && startsWith(inputs.release-tag, 'v0.1.6-pre.') && inputs.release-tag || '' }}
+  PROTO_COMPATIBILITY_EXCEPTION_REASON: Release captain accepted protobuf breaking changes for the v0.1.6 prerelease train; migration guidance must accompany the release.

 defaults:
  run:
@@ -212,6 +216,11 @@ jobs:
            echo "| Docker E2E | ${DOCKER_E2E_RESULT} |"
            echo "| VM E2E | ${VM_E2E_RESULT} |"
            echo
+            if [[ -n "${PROTO_COMPATIBILITY_EXCEPTION_TAG}" ]]; then
+              echo "- Protobuf compatibility exception: \`${PROTO_COMPATIBILITY_EXCEPTION_TAG}\`"
+              echo "- Reason: ${PROTO_COMPATIBILITY_EXCEPTION_REASON}"
+              echo
+            fi
            echo "Advisory compatibility findings do not affect the qualification result."
            echo "See the Compatibility Review job and compatibility-review-run-${GITHUB_RUN_ID}-attempt-${GITHUB_RUN_ATTEMPT} artifact in [this run](${run_url})."
          } >> "${GITHUB_STEP_SUMMARY}"
@@ -48,6 +48,14 @@ remains in Actions. This checks
 protobuf compatibility; SDK/configuration compatibility and migration review
 remain separate qualification work.

+The release qualification workflow has a release-captain-approved exception
+for all `v0.1.6-pre.N` candidates, including replacement candidates after a failed
+run. It still runs Buf and fails on compilation or tooling
+errors, but accepts detected protobuf breaking changes for that tag. The run
+summary and qualification JSON's `exceptions` array record the scope and reason.
+Other versions retain the normal version policy. Remove the workflow exception
+after promotion; release notes must describe the accepted changes and migration.
+
 Fetch the target and tags to check committed branch changes locally, replacing
 `origin/main` for another target:

@@ -67,6 +75,23 @@ For an intentional minor-train incompatibility, record the Buf finding,
 linked issue, consumer impact, and migration plan in the PR. Keep the finding
 visible; do not disable the job or add a broad Buf ignore rule.

+### Remove the v0.1.6 exception
+
+Squash-merge [PR #4976](https://github.com/NVIDIA/OpenShell/pull/4976) so the
+exception, checker support, qualification metadata, tests, and temporary guidance
+can be removed together with one revert. After verifying stable v0.1.6 publication,
+open a cleanup PR from current `main` and revert that PR's squash-merge commit:
+
+```shell
+git revert --no-commit <PR-4976-squash-merge-sha>
+git commit --signoff -m "revert(ci): remove temporary v0.1.6 protobuf exception"
+```
+
+Use the final squash-merge SHA shown on the merged PR, not a branch commit SHA.
+Validate the reverted workflow with Actionlint and run the normal protobuf check
+before merging the cleanup. Reverting the whole PR restores normal enforcement
+and removes the exception mechanism; do not leave the bypass support behind.
+
 ### Windows validation

 Native Windows implementation and releases use the
@@ -103,7 +103,8 @@ def compare(candidate: str, baseline: str, allows_breaks: bool) -> int:
        if result.returncode == 100 and allows_breaks:
            print(diagnostics.replace("::error ", "::warning "), end="")
            print(
-                "Breaking changes allowed by the version increment; review migration guidance."
+                "Breaking changes allowed by the version policy or an explicit "
+                "maintainer exception; review migration guidance."
            )
            return 0
        print(diagnostics, end="")
@@ -122,6 +123,15 @@ def main() -> int:
    )
    release = ref.removeprefix("refs/tags/") if ref.startswith("refs/tags/") else None
    stable, train, allows_breaks = train_policy(candidate, release)
+    exception_tag = os.environ.get("PROTO_COMPATIBILITY_EXCEPTION_TAG", "")
+    if exception_tag:
+        if release != exception_tag or not _parse_prerelease_tag(exception_tag):
+            raise ValueError("Protobuf exception must match the exact prerelease tag.")
+        reason = os.environ.get("PROTO_COMPATIBILITY_EXCEPTION_REASON", "").strip()
+        if not reason:
+            raise ValueError("Protobuf exception requires a reason.")
+        print(f"Protobuf compatibility exception for {release}: {reason}", flush=True)
+        allows_breaks = True
    baseline = f"refs/tags/{stable}" if release else candidate
    if not release:
        # merge-tree writes only Git objects, leaving HEAD and the worktree
@@ -0,0 +1,163 @@
+# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+import json
+import os
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).resolve().parent))
+
+import check_proto_compatibility as compatibility
+
+TAG = "v0.1.6-pre.7"
+REASON = "Release captain accepted the protobuf changes."
+
+
+@pytest.mark.parametrize(
+    ("ref", "exception_tag", "reason", "expected"),
+    [
+        (f"refs/tags/{TAG}", TAG, REASON, True),
+        ("refs/tags/v0.1.6-pre.8", "v0.1.6-pre.8", REASON, True),
+        ("refs/tags/v0.1.6-pre.12", "v0.1.6-pre.12", REASON, True),
+        ("refs/tags/v0.1.6-pre.8", "", "", False),
+        ("refs/tags/v0.1.7-pre.1", "", "", False),
+        ("refs/tags/v0.2.0-pre.1", "", "", False),
+        (f"refs/tags/{TAG}", "", "", False),
+        ("refs/tags/v0.1.6-pre.8", TAG, REASON, ValueError),
+        ("refs/tags/v0.1.6", "v0.1.6", REASON, ValueError),
+        ("refs/heads/main", TAG, REASON, ValueError),
+        (f"refs/tags/{TAG}", TAG, " ", ValueError),
+    ],
+)
+def test_exception_scope(monkeypatch, ref, exception_tag, reason, expected):
+    monkeypatch.setenv("PROTO_COMPATIBILITY_EXCEPTION_TAG", exception_tag)
+    monkeypatch.setenv("PROTO_COMPATIBILITY_EXCEPTION_REASON", reason)
+    monkeypatch.setattr(sys, "argv", ["check", ref])
+    with (
+        patch.object(compatibility, "git", side_effect=[ref, "candidate-sha"]),
+        patch.object(
+            compatibility, "train_policy", return_value=("v0.1.5", "v0.1.6", False)
+        ),
+        patch.object(compatibility, "compare", return_value=0) as compare,
+    ):
+        if expected is ValueError:
+            with pytest.raises(ValueError):
+                compatibility.main()
+            compare.assert_not_called()
+        else:
+            assert compatibility.main() == 0
+            assert compare.call_args.args[2] is expected
+
+
+@pytest.mark.parametrize(
+    ("allows_breaks", "exit_code", "expected"),
+    [(False, 100, 100), (True, 100, 0), (True, 1, 1), (True, 0, 0)],
+)
+def test_exception_only_accepts_breaking_diagnostics(
+    allows_breaks, exit_code, expected
+):
+    with (
+        patch.object(compatibility, "export_proto"),
+        patch.object(
+            compatibility.subprocess,
+            "run",
+            side_effect=[
+                subprocess.CompletedProcess([], 0),
+                subprocess.CompletedProcess([], 0),
+                subprocess.CompletedProcess([], exit_code, stdout="", stderr=""),
+            ],
+        ) as run,
+    ):
+        assert compatibility.compare("candidate", "baseline", allows_breaks) == expected
+        assert [call.args[0][1] for call in run.call_args_list] == [
+            "build",
+            "build",
+            "breaking",
+        ]
+
+
+@pytest.mark.parametrize("failed_snapshot", [0, 1])
+def test_exception_does_not_accept_invalid_protobufs(failed_snapshot):
+    results = [subprocess.CompletedProcess([], 0)] * failed_snapshot
+    results.append(subprocess.CalledProcessError(100, ["buf", "build"]))
+    with (
+        patch.object(compatibility, "export_proto"),
+        patch.object(compatibility.subprocess, "run", side_effect=results) as run,
+        pytest.raises(subprocess.CalledProcessError),
+    ):
+        compatibility.compare("candidate", "baseline", True)
+    assert run.call_count == failed_snapshot + 1
+
+
+@pytest.mark.skipif(shutil.which("buf") is None, reason="Buf is not installed")
+@pytest.mark.parametrize("allows_breaks", [False, True])
+def test_real_buf_field_rename(monkeypatch, allows_breaks):
+    def export_fixture(ref, destination):
+        (destination / "proto").mkdir(parents=True)
+        (destination / "buf.yaml").write_text(
+            "version: v2\nmodules:\n  - path: proto\nbreaking:\n  use:\n    - FILE\n"
+        )
+        field = "old_name" if ref == "baseline" else "new_name"
+        (destination / "proto" / "test.proto").write_text(
+            'syntax = "proto3";\npackage test.v1;\n'
+            f"message Request {{ bool {field} = 3; }}\n"
+        )
+
+    monkeypatch.setattr(compatibility, "export_proto", export_fixture)
+    assert compatibility.compare("candidate", "baseline", allows_breaks) == (
+        0 if allows_breaks else 100
+    )
+
+
+@pytest.mark.parametrize("exception", [False, True])
+@pytest.mark.parametrize("proto_result", ["success", "failure", "skipped"])
+@pytest.mark.parametrize("tag", [TAG, "v0.1.6-pre.8"])
+def test_qualification_records_exception_without_bypassing_results(
+    tmp_path, exception, proto_result, tag
+):
+    output = tmp_path / "qualification.json"
+    env = {
+        **os.environ,
+        "RELEASE_TAG": tag,
+        "SOURCE_SHA": "candidate-sha",
+        "IS_PRERELEASE": "true",
+        "GITHUB_RUN_ID": "123",
+        "GITHUB_RUN_ATTEMPT": "1",
+        "GITHUB_SERVER_URL": "https://github.com",
+        "GITHUB_REPOSITORY": "NVIDIA/OpenShell",
+        "PROTO_COMPATIBILITY_RESULT": proto_result,
+        "PROTO_COMPATIBILITY_EXCEPTION_TAG": tag if exception else "",
+        "PROTO_COMPATIBILITY_EXCEPTION_REASON": REASON,
+    }
+    for suite in (
+        "SECURITY",
+        "CONFORMANCE",
+        "FEATURE_INTEGRATION",
+        "DOCKER_E2E",
+        "VM_E2E",
+    ):
+        env[f"{suite}_RESULT"] = "success"
+    subprocess.run(
+        [
+            "bash",
+            str(Path(__file__).with_name("generate-qualification-summary.sh")),
+            str(output),
+        ],
+        env=env,
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    summary = json.loads(output.read_text())
+    assert summary["current_profile_passed"] is (proto_result == "success")
+    assert summary["exceptions"] == (
+        [{"suite": "protobuf_compatibility", "tag": tag, "reason": REASON}]
+        if exception
+        else []
+    )
@@ -22,12 +22,20 @@ require_env() {
 [[ $# -eq 1 ]] || die "usage: $0 OUTPUT"
 output=$1
 current_profile_passed=true
+proto_exception_tag=${PROTO_COMPATIBILITY_EXCEPTION_TAG:-}
+proto_exception_reason=${PROTO_COMPATIBILITY_EXCEPTION_REASON:-}

 require_env \
  RELEASE_TAG SOURCE_SHA IS_PRERELEASE GITHUB_RUN_ID GITHUB_RUN_ATTEMPT \
  GITHUB_SERVER_URL GITHUB_REPOSITORY SECURITY_RESULT CONFORMANCE_RESULT \
  FEATURE_INTEGRATION_RESULT DOCKER_E2E_RESULT VM_E2E_RESULT PROTO_COMPATIBILITY_RESULT

+if [[ -n "${proto_exception_tag}" ]]; then
+  [[ "${IS_PRERELEASE}" == true && "${proto_exception_tag}" == "${RELEASE_TAG}" ]] ||
+    die "protobuf exception must match the prerelease tag"
+  [[ -n "${proto_exception_reason//[[:space:]]/}" ]] || die "protobuf exception requires a reason"
+fi
+
 for result in \
  "${PROTO_COMPATIBILITY_RESULT}" \
  "${SECURITY_RESULT}" \
@@ -53,6 +61,8 @@ jq -n \
  --arg generated_at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \
  --arg security "${SECURITY_RESULT}" \
  --arg protobuf_compatibility "${PROTO_COMPATIBILITY_RESULT}" \
+  --arg proto_exception_tag "${proto_exception_tag}" \
+  --arg proto_exception_reason "${proto_exception_reason}" \
  --arg conformance "${CONFORMANCE_RESULT}" \
  --arg feature_integration "${FEATURE_INTEGRATION_RESULT}" \
  --arg docker_e2e "${DOCKER_E2E_RESULT}" \
@@ -72,6 +82,11 @@ jq -n \
      ]
    },
    current_profile_passed: $current_profile_passed,
+    exceptions: (if $proto_exception_tag == "" then [] else [{
+      suite: "protobuf_compatibility",
+      tag: $proto_exception_tag,
+      reason: $proto_exception_reason
+    }] end),
    run: {
      id: $run_id,
      attempt: $run_attempt,