Skip to content

Commit 5698c4f

Browse files
SDAChessmrunalp
andauthored
fix(ci): qualify protobuf compatibility by release train (#4049)
* feat(ci): detect breaking protobuf changes Compare the proto module against the PR or merge-group base and report Buf violations in Branch Checks. Add local reproduction and fixture coverage. Closes #3794 Signed-off-by: Mrunal Patel <mrunalp@gmail.com> * fix(ci): pin protobuf check container image Signed-off-by: Mrunal Patel <mrunalp@gmail.com> * fix(ci): qualify protobuf compatibility by release train Signed-off-by: Simon Scatton <sscatton@nvidia.com> * refactor(ci): reuse protobuf compatibility action Signed-off-by: Simon Scatton <sscatton@nvidia.com> * refactor(ci): run protobuf checks as a Nix app with one ref Signed-off-by: Simon Scatton <sscatton@nvidia.com> --------- Signed-off-by: Mrunal Patel <mrunalp@gmail.com> Signed-off-by: Simon Scatton <sscatton@nvidia.com> Co-authored-by: Mrunal Patel <mrunalp@gmail.com>
1 parent 0e8d9e5 commit 5698c4f

10 files changed

Lines changed: 297 additions & 4 deletions

File tree

‎.agents/skills/watch-github-actions/SKILL.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ not substitute the current `main` tip or the event's older PR base SHA. Merge
132132
groups and manual runs use their explicit baseline. Findings are reported by
133133
`Reject new high or critical findings`; distinguish those from scanner failures.
134134

135+
For `Protobuf Compatibility`, check the logged train and comparison baseline.
136+
Branch Checks compares the prospective merge tree with its target; Release Tag
137+
compares the tagged candidate with the previous stable release. Both use
138+
the shared `check-protobuf-compatibility` action with `nix run .#check-protobuf-compatibility -- <ref>`.
139+
During `0.x`, a minor train permits compatibility findings
140+
as warnings; a patch train or no active train rejects them. Compare the current
141+
train's version with the latest stable release; commit messages are irrelevant.
142+
Compilation, baseline, and tool errors remain fatal. The `protobuf_compatibility` suite participates in
143+
the `release-tag-v1` qualification profile. Failed qualification prevents stable
144+
publication but still allows pre-release artifacts to publish with the failure
145+
recorded.
146+
135147
View logs for a specific run:
136148

137149
```bash
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
name: Check Protobuf Compatibility
5+
description: Check protobuf compatibility against the target or latest stable release using the release train policy. Requires a checkout with full history and tags.
6+
7+
inputs:
8+
ref:
9+
description: PR or merge-group target branch/commit, or release tag to qualify.
10+
required: true
11+
12+
runs:
13+
using: composite
14+
steps:
15+
- uses: ./.github/actions/setup-nix
16+
17+
- name: Check protobuf compatibility
18+
shell: bash
19+
env:
20+
CHECK_REF: ${{ inputs.ref }}
21+
run: |
22+
nix run .#check-protobuf-compatibility --no-write-lock-file -- "$CHECK_REF"

‎.github/actions/pr-gate/action.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ outputs:
1616
should_run:
1717
description: "true if the workflow should proceed, false otherwise"
1818
value: ${{ steps.gate.outputs.should_run }}
19+
base_sha:
20+
description: "Target branch commit SHA for a mirrored pull request, or empty for other events"
21+
value: ${{ steps.gate.outputs.base_sha }}
1922
labels_json:
2023
description: "JSON array of PR label names for push-triggered mirror runs, or [] otherwise"
2124
value: ${{ steps.gate.outputs.labels_json }}
@@ -40,16 +43,19 @@ runs:
4043
if [ "$EVENT_NAME" != "push" ]; then
4144
echo "labels_json=[]" >> "$GITHUB_OUTPUT"
4245
echo "should_run=true" >> "$GITHUB_OUTPUT"
46+
echo "base_sha=" >> "$GITHUB_OUTPUT"
4347
exit 0
4448
fi
4549
4650
if [ "$GET_PR_INFO_OUTCOME" != "success" ]; then
4751
echo "labels_json=[]" >> "$GITHUB_OUTPUT"
4852
echo "should_run=false" >> "$GITHUB_OUTPUT"
53+
echo "base_sha=" >> "$GITHUB_OUTPUT"
4954
exit 0
5055
fi
5156
5257
head_sha="$(jq -r '.head.sha' <<< "$PR_INFO")"
58+
base_sha="$(jq -r '.base.sha // empty' <<< "$PR_INFO")"
5359
labels_json="$(jq -c '[.labels[].name]' <<< "$PR_INFO")"
5460
if [ -z "$REQUIRED_LABEL" ]; then
5561
has_label=true
@@ -67,3 +73,4 @@ runs:
6773
6874
echo "labels_json=$labels_json" >> "$GITHUB_OUTPUT"
6975
echo "should_run=$should_run" >> "$GITHUB_OUTPUT"
76+
echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT"

‎.github/workflows/branch-checks.yml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,33 @@ jobs:
3030
pull-requests: read
3131
outputs:
3232
should_run: ${{ steps.gate.outputs.should_run }}
33+
base_sha: ${{ steps.gate.outputs.base_sha }}
3334
steps:
3435
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
3536

3637
- id: gate
3738
uses: ./.github/actions/pr-gate
3839

40+
protobuf-compatibility:
41+
name: Protobuf Compatibility
42+
needs: pr_metadata
43+
if: needs.pr_metadata.outputs.should_run == 'true'
44+
runs-on: linux-amd64-cpu8
45+
timeout-minutes: 15
46+
steps:
47+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
48+
with:
49+
fetch-depth: 0
50+
persist-credentials: false
51+
52+
- uses: ./.github/actions/check-protobuf-compatibility
53+
with:
54+
ref: >-
55+
${{ github.event_name == 'workflow_dispatch'
56+
&& format('refs/remotes/origin/{0}', github.event.repository.default_branch)
57+
|| github.event.merge_group.base_sha
58+
|| needs.pr_metadata.outputs.base_sha }}
59+
3960
mise-lockfile:
4061
name: mise Lockfile
4162
needs: pr_metadata

‎.github/workflows/release-tag.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,28 @@ jobs:
215215
checkout-ref: ${{ needs.compute-versions.outputs.source_sha }}
216216
conformance-artifact-prefix: openshell-conformance
217217

218+
protobuf-compatibility:
219+
name: Protobuf Compatibility
220+
needs: compute-versions
221+
runs-on: linux-amd64-cpu8
222+
timeout-minutes: 15
223+
steps:
224+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
225+
with:
226+
fetch-depth: 0
227+
persist-credentials: false
228+
ref: ${{ needs.compute-versions.outputs.source_sha }}
229+
230+
- uses: ./.github/actions/check-protobuf-compatibility
231+
with:
232+
ref: refs/tags/${{ env.RELEASE_TAG }}
233+
218234
qualification-result:
219235
name: Release Qualification
220236
if: always()
221237
needs:
222238
- compute-versions
239+
- protobuf-compatibility
223240
- security
224241
- conformance-integration
225242
- feature-specific-integration
@@ -245,6 +262,7 @@ jobs:
245262
DOCKER_E2E_RESULT: ${{ needs.docker-e2e.result }}
246263
FEATURE_INTEGRATION_RESULT: ${{ needs.feature-specific-integration.result }}
247264
IS_PRERELEASE: ${{ needs.compute-versions.outputs.is_prerelease }}
265+
PROTO_COMPATIBILITY_RESULT: ${{ needs.protobuf-compatibility.result }}
248266
SECURITY_RESULT: ${{ needs.security.result }}
249267
SOURCE_SHA: ${{ needs.compute-versions.outputs.source_sha }}
250268
VM_E2E_RESULT: ${{ needs.vm-e2e.result }}
@@ -269,6 +287,7 @@ jobs:
269287
echo
270288
echo "| Suite | Result |"
271289
echo "| --- | --- |"
290+
echo "| Protobuf API compatibility | ${PROTO_COMPATIBILITY_RESULT} |"
272291
echo "| Security | ${SECURITY_RESULT} |"
273292
echo "| Conformance integration | ${CONFORMANCE_RESULT} |"
274293
echo "| Feature integration | ${FEATURE_INTEGRATION_RESULT} |"

‎CI.md‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,55 @@ Manual admission does not change the bot's automatic trust policy for ready PRs.
1717

1818
Merge queue validation is a second integration gate for `main`. After a PR has passed the required PR-head statuses, a maintainer adds it to the merge queue. GitHub creates a temporary merge-group branch that combines the latest `main`, the queued PR, and any earlier queued PRs. The same required `OpenShell / ...` status contexts are then published against the merge-group SHA before GitHub merges it.
1919

20+
### Protobuf API compatibility
21+
22+
`Protobuf Compatibility` runs in Branch Checks and Release Tag through the shared
23+
`check-protobuf-compatibility` action and Nix app. The app supplies Python, Buf,
24+
and Git from the flake lockfile.
25+
It uses Buf's `FILE` policy for the `proto/`
26+
module, covering SDK descriptors and extension contracts. Storage-only protobufs
27+
remain subject to their separate durability checks.
28+
29+
Branch Checks compares the prospective merge tree with the PR target commit,
30+
so additions on the target do not look like deletions in an outdated PR. Merge
31+
queues use their base commit. Both paths resolve the train from tags reachable
32+
from that target, preventing a PR from selecting its own train. The checkout
33+
must contain full history and tags; the checker leaves HEAD and working files
34+
unchanged.
35+
36+
The current train's version is compared with the latest stable release. During
37+
`0.x`, a minor increment permits breaking changes: for example, `0.2.0-pre.N`
38+
after stable `0.1.2`. A patch increment such as `0.1.3-pre.N` rejects them.
39+
Commit messages do not affect this decision. No active train also rejects
40+
breaking changes. Allowed findings remain visible as warnings; schema errors,
41+
missing baselines, merge conflicts, and tool failures always fail the check.
42+
43+
Release Tag compares the tagged candidate cumulatively against the previous
44+
stable release, using the same minor-versus-patch policy. Its result is part
45+
of the qualification profile: failure blocks stable publication, while a
46+
pre-release can still publish with failed qualification recorded. This checks
47+
protobuf compatibility; SDK/configuration compatibility and migration review
48+
remain separate qualification work.
49+
50+
Fetch the target and tags to check committed branch changes locally, replacing
51+
`origin/main` for another target:
52+
53+
```shell
54+
git fetch origin main --tags
55+
nix run .#check-protobuf-compatibility -- origin/main
56+
```
57+
58+
The same command accepts a release tag to qualify it against the previous stable
59+
release. Branch names and commit SHAs select the branch comparison instead:
60+
61+
```shell
62+
nix run .#check-protobuf-compatibility -- refs/tags/v0.2.0-pre.1
63+
```
64+
65+
For an intentional minor-train incompatibility, record the Buf finding,
66+
linked issue, consumer impact, and migration plan in the PR. Keep the finding
67+
visible; do not disable the job or add a broad Buf ignore rule.
68+
2069
Windows PR checks are opt-in: add `test:windows`, then select **Re-run all jobs**
2170
on the current Windows MSVC run. Subsequent mirrored commits run them automatically.
2271
Windows checks are not required for merging and do not run in merge queues.
@@ -172,7 +221,7 @@ jobs:
172221
```
173222
174223
Set `needs: security` on a downstream promotion job to require successful scans.
175-
The tagged release workflow records security and integration outcomes in a
224+
The tagged release workflow records protobuf, security, and integration outcomes in a
176225
qualification job after publishing its commit-addressed OCI images. A failed
177226
check remains visible in the workflow, but pre-release artifact assembly and
178227
publication continue. Stable publication currently requires the implemented
@@ -431,7 +480,7 @@ These workflows run after merge to publish dev/tagged artifacts and verify them.
431480
| File | Role |
432481
|---|---|
433482
| `.github/workflows/release-dev.yml` | Publishes the rolling `dev` build on every push to `main`. Builds gateway, sandbox, and supervisor images and binaries, packages, wheels, and pushes the Helm chart as `oci://ghcr.io/nvidia/openshell/helm-chart:0.0.0-dev` (plus an immutable `0.0.0-dev.<sha>` pin). Also dispatchable manually. |
434-
| `.github/workflows/release-tag.yml` | Publishes tagged stable releases and manually dispatched pre-releases. Its automatic tag trigger excludes `-pre.*`. Security and integration failures do not block pre-release artifact publication. Stable publication requires the currently implemented qualification profile to pass; the summary identifies the remaining RFC 0014 coverage. |
483+
| `.github/workflows/release-tag.yml` | Publishes tagged stable releases and manually dispatched pre-releases. Its automatic tag trigger excludes `-pre.*`. Protobuf, security, and integration failures do not block pre-release artifact publication. Stable publication requires the currently implemented qualification profile to pass; the summary identifies the remaining RFC 0014 coverage. |
435484
| `.github/workflows/release-canary.yml` | Smoke-tests published dev artifacts in the `macos`, `ubuntu-deb`, `ubuntu-snap-system-docker`, `fedora`, and `kubernetes` (kind + Helm) jobs. Each job reaches its gateway and creates, exercises, and deletes a sandbox. The Snap lanes verify a compatible system Docker lifecycle and `ubuntu-snap-docker-preflight` tests fail-fast behavior when Docker is absent or supplied by the Docker snap. It runs automatically after `Release Dev` succeeds and supports manual dispatch (`gh workflow run release-canary.yml --ref <branch>`). See the `test-release-canary` skill for the playbook and local kind reproduction. |
436485

437486
## Required status contexts

‎flake.nix‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,23 @@
125125
firmwarePkgs = tmachineRuntimePkgs;
126126
};
127127
artifacts = pkgs.callPackage ./tests/artifacts.nix { inherit rustToolchain toolchains; };
128+
checkProtobufCompatibility = pkgs.writeShellApplication {
129+
name = "check-protobuf-compatibility";
130+
runtimeInputs = [
131+
pkgs.buf
132+
pkgs.git
133+
];
134+
text = ''
135+
exec ${pkgs.python3}/bin/python3 ${./tasks/scripts}/check_proto_compatibility.py "$@"
136+
'';
137+
};
128138
in
129139
{
130140
apps = {
141+
check-protobuf-compatibility = {
142+
type = "app";
143+
program = "${checkProtobufCompatibility}/bin/check-protobuf-compatibility";
144+
};
131145
build-artifacts = {
132146
type = "app";
133147
program = "${artifacts.all}/bin/build-artifacts";
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
"""Check protobuf compatibility under the current, tag-defined release train."""
5+
6+
import argparse
7+
import io
8+
import os
9+
import subprocess
10+
import sys
11+
import tarfile
12+
import tempfile
13+
from pathlib import Path
14+
15+
from release import _parse_prerelease_tag, _parse_semver_tag
16+
17+
18+
def git(*args: str) -> str:
19+
return subprocess.run(
20+
["git", *args], check=True, capture_output=True, text=True
21+
).stdout.strip()
22+
23+
24+
def train_policy(ref: str, release: str | None) -> tuple[str, str, bool]:
25+
tags = git("tag", "--merged", ref, "--list", "v*").splitlines()
26+
stable = sorted(
27+
(version, tag)
28+
for tag in tags
29+
if tag != release and (version := _parse_semver_tag(tag))
30+
)
31+
if release is None:
32+
prereleases = sorted(
33+
(version, tag) for tag in tags if (version := _parse_prerelease_tag(tag))
34+
)
35+
if not prereleases or (stable and prereleases[-1][0][:3] <= stable[-1][0]):
36+
return "", "none", False
37+
release = prereleases[-1][1]
38+
39+
if not stable:
40+
raise ValueError("No previous stable release baseline is available.")
41+
previous, baseline = stable[-1]
42+
prerelease = _parse_prerelease_tag(release)
43+
version = prerelease[:3] if prerelease else _parse_semver_tag(release)
44+
if version is None or not release.startswith("v"):
45+
raise ValueError(f"Invalid release tag: {release}")
46+
if version <= previous:
47+
raise ValueError(f"Release {release} must be newer than stable {baseline}.")
48+
train = "v" + ".".join(map(str, version))
49+
allows_breaks = version[:2] > previous[:2]
50+
return baseline, f"{train} (latest stable: {baseline})", allows_breaks
51+
52+
53+
def export_proto(ref: str, destination: Path) -> None:
54+
archive = subprocess.check_output(
55+
["git", "archive", ref, "--", "buf.yaml", "proto"]
56+
)
57+
with tarfile.open(fileobj=io.BytesIO(archive)) as snapshot:
58+
snapshot.extractall(destination, filter="data")
59+
60+
61+
def compare(candidate: str, baseline: str, allows_breaks: bool) -> int:
62+
with tempfile.TemporaryDirectory(prefix="openshell-proto-") as directory:
63+
root = Path(directory)
64+
export_proto(baseline, root / "before")
65+
export_proto(candidate, root / "after")
66+
error_format = (
67+
"github-actions" if os.environ.get("GITHUB_ACTIONS") == "true" else "text"
68+
)
69+
# Buf also returns 100 for compiler diagnostics. Validate both snapshots
70+
# before treating that status from `breaking` as an allowed API change.
71+
for snapshot in ("before", "after"):
72+
subprocess.run(
73+
[
74+
"buf",
75+
"build",
76+
"proto",
77+
"--error-format",
78+
error_format,
79+
"-o",
80+
os.devnull,
81+
],
82+
cwd=root / snapshot,
83+
check=True,
84+
)
85+
result = subprocess.run(
86+
[
87+
"buf",
88+
"breaking",
89+
"proto",
90+
"--against",
91+
str(root / "before" / "proto"),
92+
"--config",
93+
"buf.yaml",
94+
"--error-format",
95+
error_format,
96+
],
97+
cwd=root / "after",
98+
capture_output=True,
99+
text=True,
100+
check=False,
101+
)
102+
diagnostics = result.stdout + result.stderr
103+
if result.returncode == 100 and allows_breaks:
104+
print(diagnostics.replace("::error ", "::warning "), end="")
105+
print(
106+
"Breaking changes allowed by the version increment; review migration guidance."
107+
)
108+
return 0
109+
print(diagnostics, end="")
110+
return result.returncode
111+
112+
113+
def main() -> int:
114+
parser = argparse.ArgumentParser(description=__doc__)
115+
parser.add_argument("ref", help="Target branch/commit, or release tag to qualify")
116+
args = parser.parse_args()
117+
ref = git(
118+
"rev-parse", "--symbolic-full-name", "--verify", "--end-of-options", args.ref
119+
)
120+
candidate = git(
121+
"rev-parse", "--verify", "--end-of-options", f"{args.ref}^{{commit}}"
122+
)
123+
release = ref.removeprefix("refs/tags/") if ref.startswith("refs/tags/") else None
124+
stable, train, allows_breaks = train_policy(candidate, release)
125+
baseline = f"refs/tags/{stable}" if release else candidate
126+
if not release:
127+
# merge-tree writes only Git objects, leaving HEAD and the worktree
128+
# intact. Target-only additions are therefore not mistaken for deletions.
129+
candidate = git("merge-tree", "--write-tree", baseline, "HEAD").splitlines()[0]
130+
print(
131+
f"Train: {train}; breaking changes {'allowed' if allows_breaks else 'forbidden'}"
132+
)
133+
print(f"Comparing {candidate} against {baseline}", flush=True)
134+
return compare(candidate, baseline, allows_breaks)
135+
136+
137+
if __name__ == "__main__":
138+
try:
139+
sys.exit(main())
140+
except subprocess.CalledProcessError as error:
141+
print(error.stderr or error.stdout or str(error), file=sys.stderr)
142+
sys.exit(1)
143+
except (OSError, ValueError, tarfile.TarError) as error:
144+
print(error, file=sys.stderr)
145+
sys.exit(1)

0 commit comments

Comments
 (0)