fix(ci): release-gate fixes for cache, oasdiff, checksums, docs gate by mchmarny · Pull Request #2673 · NVIDIA/aicr

mchmarny · 2026-09-10T01:07:22Z

Summary

Three independent reliability fixes to the qualification gate, batched into one PR to amortize review: a prefix-fallback Go cache (plus the save-gating that makes the fallback safe), oasdiff installed from a checksummed binary instead of go install, block-scoped checksum refresh scripts, and a docs gate that stops silently dropping pipeline stages.

It also unbreaks main, which Renovate #2678 left red by bumping an image digest without regenerating the artifacts derived from it, and adds two guards so that class of merge stops recurring. Each commit is independently revertable.

Motivation / Context

Two of these come straight out of the v0.21.1 release, which took three attempts to land. Both failures were in the qualification gate and both were on public Go infrastructure, while build-ko sits behind Artifactory and was unaffected.

Fixes: #2663
Fixes: #2658
Fixes: #2655
Related: #2667, #2666, #2670, #2671, #2672

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Refactoring (no functional changes)
Build/CI/tooling

Component(s) Affected

CLI (cmd/aicr, pkg/cli)
API server (cmd/aicrd, pkg/server) - test only, no production code
Recipe engine / data (pkg/recipe)
Bundlers (pkg/bundler, pkg/component/*)
Collectors / snapshotter (pkg/collector, pkg/snapshotter)
Validator (pkg/validator)
Core libraries (pkg/errors, pkg/k8s)
Docs/examples (docs/, examples/)
Other: .github/actions, .github/workflows, .github/renovate.json5, tools/, pkg/bundler/testdata, docs/user/container-images.md

Implementation Notes

`fix(ci)` - #2663, and the oasdiff third of #2667

setup-go restores on an exact hash of go.sum with no prefix fallback. A patch release cut from an older tag carries that tag's go.sum, main has since moved, and GitHub cache scoping lets a tag run read only its own ref and the default branch, so the key it needs no longer exists anywhere it can reach. v0.21.0 hit the cache and finished this job in 9m59s; v0.21.1 missed and was killed at the 15-minute wall after 13.6 minutes of go: downloading stalls. The drift was four modules out of roughly six hundred.

restore-keys caused an incident here before: in install-e2e-tools it prefix-restored stale tool binaries and setup-tools' presence-only guards kept them, pinning E2E to kind v0.31.0 against a v0.33.0 pin. That cannot recur for either cache here, because Go is the consumer and both are content-addressed (GOMODCACHE by module@version, GOCACHE by build ActionID). There is no version-blind presence check to satisfy.

Splitting cache into restore + save is what makes the prefix fallback safe to add. ok-to-test runs on issue_comment, so its github.ref is the default branch and it writes into main's cache scope, while checking out the untrusted PR head that tests / Test then executes via make test. GOCACHE is not re-verified on read. Exact-key-only restore made planting an entry hard to reach by accident; a prefix fallback would not. So restore stays unconditional and save is gated on a new privileged_ci input, which ok-to-test is the sole caller to set false.

Save additionally requires make test to have run, pass or fail. Cache entries are immutable, so whichever run saves first owns that key until go.sum moves. A failing test run is worth banking (modules downloaded, packages compiled); a run that dies at Install Helm or envtest has a warm GOMODCACHE and an empty GOCACHE, and banking that would pin a cache that rebuilds all of ./... under -race on every later run.

oasdiff moved off go install, which builds outside the main module, so go.sum covers none of its dependencies and each is authenticated against sum.golang.org live. That is the mechanism that failed tests / E2E on attempt 2. It now installs from a checksummed release binary via setup-build-tools, which also settles it being installed two different ways after #2665 converted the tools/setup-tools copy. setup-envtest and apidiff still use go install; neither publishes a binary, so they need the Artifactory routing decision in #2667.

`fix(tools)` - #2658

update-chainsaw-checksums rewrote .settings.yaml with a sed anchored on the arch key alone. Four blocks carry the same sub-keys at the same indent, so replace_sha linux_amd64 matched all four; a chainsaw bump would have written chainsaw's digests over the other three tools. The post-check did not catch it because it asked only whether the new value existed somewhere in the file, and after the clobber it existed four times.

Note the issue title says all the refresh scripts share this. Only update-chainsaw-checksums had the live defect: helmfile and helm-diff already scope via awk, with comments saying why, which is how the divergence was found.

All three scripts do share one latent variant, fixed here: the block-exit rule only ended a block on another 2-space key, so if a checksums block were ever the last key of its section, in_block would run to EOF and the first 4-space digest line in a later section would be rewritten. Not reachable today, but it is the same clobber, re-armed by a plain reordering of .settings.yaml.

tools/settings-checksums_test.sh is the standing guard: two independent digests do not collide, so any two blocks sharing a value means something wrote across a boundary. It parses with awk rather than python+yaml, because it runs inside make test and PyYAML is in no documented setup step for this repo, so a contributor without it would get a bare ModuleNotFoundError aborting the whole gate. It cross-checks its own parse against a grep of every *_checksums key at any indent, so a block that moved fails rather than silently shrinking the comparison set.

`test(server)` - #2655

The docs gate stopped at the first curl in a command, so curl … | curl -X POST … -d @- yielded only the GET. The POST leg was discarded inside the parser, before the skip check that exists to report what the gate cannot replay, so it was neither replayed nor logged. The skip lines are what a maintainer reads to know which documented requests are unchecked; a leg that never reaches them is invisible in a way an honest skip is not.

A stage is now replayable only when curl is its command word, which also stops sudo apt-get install -y make git curl pipx in DEVELOPMENT.md from being treated as a curl call that happens to lack a URL. Because tokenizeShell strips $(, that command word may sit behind assignments and reserved words, so those are stepped over; without it, if metrics=$(curl -fsS …/metrics …); then in kubernetes-deployment.md is dropped silently, reintroducing the same bug while fixing it.

A curl token found anywhere else is reported rather than ignored. Replaying it would be wrong (a wrapper such as kubectl exec … -- curl runs curl as a child; a package name is not an invocation), but telling those apart needs to know what each command does with its operands, which is unbounded. This is not hypothetical: it surfaces kubectl run … --image=curlimages/curl -- curl …/health in kubernetes-deployment.md, which the gate had been passing over in silence.

`fix` — unbreaking `main` (not tied to an issue)

main was red when this branch was last updated, independently of this PR.
Renovate #2678 bumped the ubuntu:26.04 digest in
recipes/components/gke-nccl-tcpxo/manifests/nccl-tcpxo-installer.yaml and did
not regenerate the two artifacts derived from it, so
TestStockRenderParityGolden failed on h100-gke-cos-training-kubeflow and
h100-gke-cos-training-slurm — the only two leaves that render that manifest.
The a100 and b200 GKE overlays name gke-nccl-tcpxo only in comments explaining
why it is omitted, which is why their entries did not move.
docs/user/container-images.md still carried the old digest as well; make bom-check would have caught that, but it is opt-in and not part of make qualify.

Established by bisecting main rather than by inspection: 6f2dd9599 passes,
c6d8a5533 fails, and that commit changes exactly one line. The same failure
reproduces byte-for-byte on a clean origin/main checkout locally, with the
same golden and computed hashes CI reported, so it is deterministic rather than
environmental.

`ci` — stopping the next one (#2678 fallout)

Two guards, because the merge and the gate failed independently.

platformAutomerge for the kubernetes manager hands the merge to GitHub,
which merges only once the required gate check passes. #2678 was merged ten
seconds before its own tests / Test even started, so there was no red for a
human to see. Scoped to digest and pin: a digest rotation re-pins the same
tag as upstream rebuilds it (ADR-006), while a tag bump changes what the
component deploys and keeps a human on it.

These PRs still fail their own gate until the derived artifacts are
regenerated. postUpgradeTasks cannot do it — those run inside the Renovate
image, which has neither go nor helm on PATH (verified by running the
pinned digest), while make bom-docs needs both plus network chart pulls. A PR
left open until it is green is the outcome to prefer over one that lands red;
closing that toil needs a workflow that regenerates and commits back to the PR
branch, which is deliberately not in this PR because it needs contents: write and fork gating.

The push: trigger on merge-gate.yaml is a watchdog, not a second PR gate,
and it covers any bad merge rather than only Renovate's. Nothing re-checked
main after a merge landed, so the breakage surfaced on an unrelated author's
PR. Push runs force every check-paths output true, because a merged commit has
no base to diff against and a watchdog scoped to the paths a merge happened to
touch would miss exactly the case it exists for. Both paths-filter steps are
skipped on push: there the action diffs against github.event.before, which is
all-zeros after a history rewrite, and that step error would fail check-paths
and take the whole gate down.

Testing

unset GITLAB_TOKEN
make qualify          # exit 0, no FAIL lines

Beyond the gate, each fix was verified against the failure it claims to fix rather than only against its diagnosis.

#2655, measured against origin/main in a throwaway worktree:

	main	branch
replayed requests	52	52 (byte-identical set)
reported skips	12	17

The replayed set is unchanged, so nothing regressed and nothing new became replayable. The five added skip lines are each a documented curl the gate had not been accounting for: four are the -d @- POST legs of piped bundle examples in api-reference.md, one is the kubectl run … -- curl in kubernetes-deployment.md, and DEVELOPMENT.md:70 changed reason only. This PR buys honesty of reporting, not coverage.

#2658:

Bumping chainsaw to v0.2.14 in a scratch copy changes exactly the four lines of chainsaw_checksums and nothing else.
Re-running all three refresh scripts at their currently pinned versions is a no-op.
The guard fails with exit 1, naming every affected pair, when the unscoped rewrite is simulated across all four blocks.
The guard fails when a block is moved out of the expected shape, and when the file is missing, empty, or garbage.
With a checksums block placed last in its section, a bump leaves the following section's digest untouched. Deleting the new guard lines reproduces the clobber, which is how it was confirmed load-bearing rather than decorative.

#2663: actionlint and yamllint clean on the changed files. Cache semantics were confirmed against actions/cache at the pinned SHA: cache-primary-key is set before the restore attempt (so it is populated on hit, prefix hit, and miss), cache-hit is 'true' only on an exact match, and cache/save uses NullStateProvider so its built-in "exact match, skip" no-op never fires — which is why the explicit condition is required rather than redundant. The composite-abort question (does a step with if: always() still run after an earlier step in the same composite fails?) was settled against a real run rather than by reasoning: run 34377661401's tests / E2E job failed and still executed a later if: failure() step in the same composite, so a failing make test does bank its downloads as intended.

Coverage: no production Go code changed; pkg/server/docs_examples_test.go is test-only and adds no exported functions. make test-coverage passes inside make qualify.

Risk Assessment

Low — Isolated change, well-tested, easy to revert
Medium — Touches multiple components or has broader impact
High — Breaking change, affects critical paths, or complex rollout

Medium because it changes how every tests / Test run gets its Go cache, which is on the release path. Each commit is independently revertable, and a bad cache degrades to a slow run rather than a wrong result.

Rollout notes:

Cache storage. The repo is at ~9.5 GB of the 10 GB Actions cache limit. go-test moves from the setup-go-* key namespace to go-*, and its entry gets larger because it now includes GOCACHE as well as GOMODCACHE. Eight other actions (go-lint, cli-e2e, chainsaw, integration, sbom-and-attest, gpu-cluster-setup, and two workflows) still write setup-go-* keys, so those caches are not orphaned; only go-test's own prior entries go stale and age out on the 7-day unused-eviction policy. Expect some LRU eviction churn during the first few runs after merge.
First run after merge is cold for the new go-* prefix, since no entry matches even the restore-key. The 30-minute timeout covers it.
Residual security exposure, deliberately not closed here. Gating go-test's save does not make the fork path safe on its own: qualification.yaml's lint and e2e jobs are ungated and reach setup-go with cache: true, writing the same two directories into the same scope from the same untrusted checkout. Closing that needs the same restore/save split, because setup-go's cache input cannot separate the halves and disabling it outright would make fork runs pay a cold cache against a 10-minute lint budget. This is pre-existing and is not introduced by this PR; it is tracked in ci: lint and e2e jobs let a fork PR write the Go cache in main's scope #2670 and noted in the code comment so the gate is not mistaken for closed.
Follow-ups filed from the self-review, all pre-existing and none blocking: ci: lint and e2e jobs let a fork PR write the Go cache in main's scope #2670 (lint/e2e cache writes), ci: route setup-build-tools version inputs through env, not inline interpolation #2671 (six setup-build-tools version inputs still interpolated inline rather than routed through env: — a linter-pattern gap, explicitly not a privilege escalation, since the job in question already runs fork code by design), and test(server): docs gate merges curl invocations joined by && into one request #2672 (the docs gate merges curl A && curl B into one fabricated request; latent, no documented source triggers it today).

Checklist

Tests pass locally (make test with -race)
Linter passes (make lint)
I did not skip/disable tests to make CI green
I added/updated tests for new functionality
I updated docs if user-facing behavior changed (.github/actions/README.md input tables; no user-facing behavior changed)
Changes follow existing patterns in the codebase
Commits are cryptographically signed (git commit -S)

Two failures took three attempts to land the v0.21.1 release, both in the qualification gate, both on public Go infrastructure while build-ko sits behind Artifactory. Cache. setup-go restores on an exact hash of go.sum with no prefix fallback. A patch release cut from an older tag carries that tag's go.sum, main has since moved, and GitHub cache scoping lets a tag run read only its own ref and the default branch -- so the key it needs no longer exists anywhere it can reach. v0.21.0 hit the cache and finished this job in 9m59s; v0.21.1 missed and was killed at the 15-minute wall after 13.6 minutes of go: downloading stalls. The drift was four modules out of roughly six hundred. Replace setup-go's built-in cache with actions/cache keyed the same way but with a prefix restore-key, so a near-miss restores the modules that did not change and re-fetches only those that did. restore-keys previously caused an incident here: in install-e2e-tools it prefix-restored stale tool binaries, and setup-tools' presence-only guards kept them, pinning E2E to kind v0.31.0 against a v0.33.0 pin. That cannot recur for either cache here, because Go is the consumer and both are content-addressed: GOMODCACHE by module@version, GOCACHE by build ActionID. There is no version-blind presence check to satisfy. Splitting cache into restore + save is what makes the prefix fallback safe to add. ok-to-test runs on issue_comment, so its github.ref is the default branch and it writes into main's cache scope -- while checking out the untrusted PR head, which tests / Test then executes via make test. GOCACHE is not re-verified on read, so a crafted PR could plant an entry a later trusted run consumes. Exact-key-only restore made that hard to reach by accident; a prefix fallback would not. So restore stays unconditional and save is gated on privileged_ci, which ok-to-test is the sole caller to set false. That gates this job only. qualification.yaml's lint and e2e are ungated and reach setup-go with cache: true, writing the same directories into the same scope from the same untrusted checkout. Closing that needs the same restore/save split, because setup-go's cache input cannot separate the halves and disabling it outright would make fork runs pay a cold cache against a 10-minute lint budget. Pre-existing, not introduced here, and filed as #2670 rather than bolted on. A cache entry is immutable, so whichever run saves first owns that key until go.sum moves. The save is therefore conditioned on `make test` having run, pass or fail: failing still banks the downloads and compiled packages, but a run that dies at Install Helm or envtest has a warm GOMODCACHE and an empty GOCACHE, and banking that would pin a cache that rebuilds all of ./... under -race on every later run. Timeout. 15 minutes was sized for a warm cache. Raise to 30 so a budget that only holds on a cache hit is not the thing standing between a release and a green gate. oasdiff. go install builds outside the main module, so go.sum covers none of its dependencies and each is authenticated against sum.golang.org live -- the mechanism that failed tests / E2E on attempt 2. oasdiff publishes binaries, so install it through setup-build-tools with a checksum instead. That also settles it being installed two different ways after 2665 converted the tools/setup-tools copy. The checksum is fetched from the release rather than pinned in .settings.yaml, matching crane. That catches corruption but not a compromised upstream release. It is a consistency choice, not a security argument -- pinning means a settings key, a refresh script, and a Renovate hook, which belongs with the rest of 2666. Note the token-scope framing cuts the other way from how it first reads: attach-source holds the wider permissions but runs once per release, while tests / Test runs on every PR. setup-envtest and apidiff still go install; neither publishes a binary, so they need the Artifactory routing decision in 2667. Fixes: #2663 Related: #2667, #2670 Signed-off-by: Mark Chmarny <mark@chmarny.com>

update-chainsaw-checksums rewrote .settings.yaml with a sed anchored on the arch key alone. Four blocks -- helm_diff_checksums, helmfile_checksums, chainsaw_checksums, mkcert_checksums -- carry the same sub-keys at the same indent, so `replace_sha linux_amd64` matched all four. A chainsaw bump would have written chainsaw's digests over the other three tools, and the next install of any of them would have failed its checksum verification with no indication of why. The post-check did not catch it because it asked only whether the new value existed somewhere in the file. After the clobber it existed four times, so the check passed. Scope the rewrite with block-tracking awk, matching what update-helmfile-checksums and update-helm-diff-checksums already do -- their comments say why, which is how the divergence was found. Narrow the verification to the same block, so a value landing outside it is a failure rather than a pass. Adopt helm-diff's atomic rename: the replacement file is created next to the target so mv cannot degrade to a copy that truncates .settings.yaml mid-write, and cp -p seeds it so the rename preserves the mode rather than leaving mktemp's 0600. All three scripts also gain a column-0 reset on their block tracking. The existing rule only ends a block on another 2-space key, so if a checksums block were ever the last key of its section, in_block would stay set to EOF and the first 4-space digest line in a later section would be rewritten -- the same cross-block clobber, reachable again by a plain reordering of .settings.yaml. Not triggerable today; the guard is one line and the failure is silent, which is the combination worth pre-empting. Add tools/settings-checksums_test.sh as the standing guard. Two independent digests do not collide, so any two blocks sharing a value means something wrote across a boundary -- which detects this class of defect in the other refresh scripts too, not just the one being fixed here. It runs in make test-shell, so a bad refresh fails the PR that carries it instead of surfacing as a checksum mismatch later. The guard parses with awk rather than python+yaml. It runs inside make test, and PyYAML is in no documented setup step for this repo, so a contributor without it would get ModuleNotFoundError aborting the whole gate, naming neither the package nor the remedy. It also cross-checks its own parse against a grep of every *_checksums key at any indent: the awk only understands the 2-space/4-space shape the refresh scripts write, so a block that moved would otherwise shrink the comparison set and pass while checking less. Verified: bumping chainsaw to v0.2.14 in a scratch copy changes exactly the four lines of chainsaw_checksums and nothing else; re-running all three scripts with their pinned versions is a no-op; the guard fails, naming every affected pair, when the unscoped rewrite is simulated; it fails when a block is moved out of the expected shape; and with a checksums block placed last in its section, a bump leaves the following section's digest untouched. Fixes: #2658 Signed-off-by: Mark Chmarny <mark@chmarny.com>

The docs gate stopped at the first curl in a command. An example shaped `curl … | curl -X POST … -d @-` therefore yielded only the GET: the POST leg was discarded inside the parser, before the skip check that exists to report what the gate cannot replay. It was neither replayed nor logged, so the gate claimed coverage it did not have. That is the failure direction that matters here. The skip lines are what a maintainer reads to know which documented requests are unchecked; a leg that never reaches them is invisible in a way an honest skip is not. Return one result per curl stage and let each be independently replayed or skipped-with-a-reason. parseCurlRequest stays as a first-stage wrapper for the single-invocation table tests. A stage is replayable only when curl is its command word, so `sudo apt-get install -y make git curl pipx` in DEVELOPMENT.md stops being treated as a curl call that happens to lack a URL. Because tokenizeShell strips `$(`, that command word may sit behind assignments and reserved words, so those are stepped over -- without it, `if metrics=$(curl -fsS …/metrics …); then` in kubernetes-deployment.md is dropped, silently, reintroducing this same bug while fixing it. A curl token found anywhere other than the command word is reported rather than ignored. Replaying it would be wrong -- a wrapper such as `kubectl exec … -- curl` runs curl as a child, and a package name is not an invocation at all -- but telling those two apart needs to know what each command does with its operands, which is unbounded. Ignoring them would be the original defect in a smaller costume, so they get a skip line and a maintainer judges. That is not hypothetical: it surfaces `kubectl run … --image=curlimages/curl -- curl …/health` in kubernetes-deployment.md, which the gate had been passing over in silence. Measured against main: the replayed set is byte-identical at 52 requests, so nothing regressed and nothing new became replayable. Skips go 12 -> 17, every one of them a documented curl the gate had not been accounting for. This buys honesty of reporting, not coverage. Fixes: #2655 Signed-off-by: Mark Chmarny <mark@chmarny.com>

github-actions · 2026-09-10T01:20:39Z

Coverage Report ✅

Metric	Value
Coverage	84.3%
Threshold	83%
Status	Pass

Coverage Badge

![Coverage](https://img.shields.io/badge/coverage-84.3%25-brightgreen)

No Go source files changed in this PR.

coderabbitai · 2026-09-10T01:21:45Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: ac71aa6c-1304-48ed-b01d-3af7e0b0b0c0

📥 Commits

Reviewing files that changed from the base of the PR and between 2989653 and 68fd392.

📒 Files selected for processing (2)

docs/user/container-images.md
pkg/bundler/testdata/stock_render_golden.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

Walkthrough

The PR updates Go cache handling and qualification timing. It centralizes verified oasdiff installation and checksum pin updates. Documentation parsing now processes every curl pipeline stage and records skipped stages. Checksum tools validate digests and scope updates to their intended blocks.

Estimated code review effort: 4 (Complex) | ~45 minutes

Severity of issue fixed: High

Suggested reviewers: yuanchen8911

Merge Risk: 🟡 Moderate · up to 68fd3

The checksum updater can still risk truncating .settings.yaml if its temporary replacement crosses filesystems and the final move fails, potentially invalidating tool pins and breaking CI. This failure mode should be fixed or explicitly accepted before merge.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The changes satisfy `#2658` by scoping checksum updates and adding regression checks, and satisfy `#2655` by processing and reporting all curl stages. They partially satisfy `#2663` by improving cache resto…	Complete the remaining `#2663` requirement by moving api-diff and openapi-diff out of the tests / Test job, or otherwise ensure their execution cannot be prevented by make test runtime. Then verify that the qualification gate preserves both c…
Out of Scope Changes check	⚠️ Warning	Most changes support the linked objectives, but docs/user/container-images.md and pkg/bundler/testdata/stock_render_golden.yaml update unrelated image and generated golden digests that are not covered…	Remove the unrelated container-image digest and golden-digest changes, or provide explicit linked-issue requirements and justification showing why they are required for this pull request.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the pull request’s main release-gate fixes, including cache handling, oasdiff, checksums, and the documentation gate.
Description check	✅ Passed	The description directly explains the qualification-gate, tooling, checksum, oasdiff, and documentation-test changes, including motivation, implementation details, testing, and rollout risks.

Full details: Linked Issues check

Explanation

The changes satisfy #2658 by scoping checksum updates and adding regression checks, and satisfy #2655 by processing and reporting all curl stages. They partially satisfy #2663 by improving cache restoration and increasing the timeout, but api-diff and openapi-diff remain coupled to make test and can still be starved by its runtime.

Resolution

Complete the remaining #2663 requirement by moving api-diff and openapi-diff out of the tests / Test job, or otherwise ensure their execution cannot be prevented by make test runtime. Then verify that the qualification gate preserves both checks during a slow or cold-cache test run.

Full details: Out of Scope Changes check

Explanation

Most changes support the linked objectives, but docs/user/container-images.md and pkg/bundler/testdata/stock_render_golden.yaml update unrelated image and generated golden digests that are not covered by #2663, #2658, or #2655.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/qualification-go-cache-and-oasdiff

_{Comment @coderabbitai help to get the list of available commands.}

Three findings from review. Pin the oasdiff archive. The install verified against the checksums.txt served beside the tarball, which comes from the same release -- so it catches corruption in transit but not a compromise of the release itself. That matters here more than the "matches crane" framing the earlier comment leaned on, because this PR moved oasdiff off go install, which authenticated every dependency against sum.golang.org, a public transparency log. Trading that for an unpinned binary would have been a regression in integrity, not a move between equivalents. So pin it the way oras already is: a flat key in .settings.yaml, a refresh script wired to Renovate postUpgradeTasks, threaded through load-versions to the action. A missing or malformed pin fails the step rather than falling back to the release's own manifest -- a silent fallback would make the pin's absence indistinguishable from its presence. The existing RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS regex already admits update-[a-z-]+-checksums, so no allowlist change. Split curl stages on &&. tokenizeShell emits & as its own token, which covers && because the character appears twice. Previously `curl A && curl -X POST B` was a single stage and parseCurlSegment kept A's URL with B's method and body, so the gate replayed a request neither documented command issues. Fabricating one is worse than dropping one, because the result is reported as a pass or a failure either way. A quoted & is a query-string separator and never reaches the tokenizer's switch; an unquoted one would end the command in a real shell too, which is why every documented curl already quotes its URL. Verified across the eight gated sources. Closes the case filed as 2672. Validate extracted digests in the three checksum refresh scripts, as update-oras-checksums already does. extract_sha returned the first whitespace field with no format check, and that value is interpolated into the awk regex used to verify the rewrite. A digest of `.*` would therefore have matched any line, so verification would have passed vacuously while writing a non-digest into .settings.yaml for tools/setup-tools to pin against. Reproduced with a stubbed upstream: the script now exits 1 and leaves the file untouched. Replayed request set is unchanged at 52 and byte-identical to main; the & tokenizer change touches every query string, so that was the check that mattered. Fixes: #2672 Signed-off-by: Mark Chmarny <mark@chmarny.com>

The previous commit threaded oasdiff_sha256 from qualification.yaml into the action's `with:` block but never declared it under `inputs:`, so `${{ inputs.oasdiff_sha256 }}` resolved to empty and the install would have failed closed on its own pin check. Caught by actionlint, which is a merge-gate job and not part of `make qualify` -- so a green local gate said nothing about it. Same shape as the lychee docs-link check being CI-only. Verified by running the gate's exact invocation, `actionlint -shellcheck=`: the branch now reports the identical single finding as origin/main (a pre-existing YAML alias under `paths:` in sigstore-scaffolding-e2e.yaml, which the pinned 1.7.11 does not flag). Signed-off-by: Mark Chmarny <mark@chmarny.com>

The release-workflow example omitted setup_envtest_version, which go-test guards at the top of its envtest install step, so a workflow copied from it fails at run time rather than skipping a step. required: true on a composite action input is documentation only; GitHub does not reject an empty value, so the explicit guards are what enforce this. The cross-repo example had the same gap plus oasdiff_version and oasdiff_sha256, and its helm_version literal had gone stale at v4.2.3 against the v4.2.4 pin in .settings.yaml. That block has no load-versions to read the pins, so nothing fails when its literals drift -- noted alongside it, including that oasdiff_sha256 must be the digest for the adjacent oasdiff_version. Signed-off-by: Mark Chmarny <mark@chmarny.com>

PR #2678 bumped the ubuntu:26.04 digest in recipes/components/gke-nccl-tcpxo/manifests/nccl-tcpxo-installer.yaml but did not regenerate the two artifacts derived from it, leaving main red. That manifest renders into exactly two leaf bundles, h100-gke-cos-training- kubeflow and h100-gke-cos-training-slurm, so TestStockRenderParityGolden failed on both. The a100 and b200 GKE overlays reference gke-nccl-tcpxo only in comments explaining why it is omitted, which is why their entries are unchanged. docs/user/container-images.md still carried the old digest; make bom-check would have caught it, but it is opt-in and not part of make qualify. Bisected across main: 6f2dd95 passes, c6d8a55 fails, and that commit changes one line. Regenerated with AICR_UPDATE_GOLDEN=1 and make bom-docs; both produce exactly the expected lines and a clean re-run passes. Signed-off-by: Mark Chmarny <mark@chmarny.com>

github-actions · 2026-09-10T11:28:27Z

🌿 Preview your docs: https://nvidia-preview-fix-qualification-go-cache-and-oasdiff.docs.buildwithfern.com/aicr

github-actions · 2026-09-10T11:29:42Z

Recipe evidence check

No leaf overlays affected by this PR.

This gate is warning-only and never blocks merge.

Two independent guards against the failure mode that broke main in #2678: a Renovate digest bump was merged ten seconds before its own gate started, and nothing re-checked main afterward, so the breakage surfaced on an unrelated PR's gate rather than on the PR that caused it. platformAutomerge hands the merge to GitHub, which merges only once the required gate check passes. Scoped to digest and pin updates: a digest rotation re-pins the same tag as upstream rebuilds it (ADR-006), while a tag bump changes what the component deploys and keeps a human on it. These PRs still fail their own gate until the derived artifacts are regenerated -- postUpgradeTasks cannot do it, because they run inside the Renovate image, which has neither go nor helm on PATH (verified against the pinned digest), while make bom-docs needs both plus network chart pulls. A PR that sits open until it is green is the outcome to prefer over one that lands red. The push trigger is a watchdog rather than a second PR gate, and it covers any bad merge, not only Renovate's. Push runs force every check-paths output true because a merged commit has no base to diff against, and a watchdog scoped to the paths a merge happened to touch would miss the case it exists for. The two paths-filter steps are skipped on push for the same reason they are unnecessary there: on a push event the action diffs against github.event.before, which is all-zeros after a history rewrite, and the resulting step error would fail check-paths and take the gate down. Signed-off-by: Mark Chmarny <mark@chmarny.com>

ok-to-test runs on issue_comment, so its github.ref is the default branch and every cache write lands in main's scope while the checkout is an untrusted PR head. #2673 closed that for go-test; the rest of qualification still wrote. lint and e2e are now restore-only against go-test's key rather than taking a privileged_ci-gated save as the issue proposed. Entries are immutable, so a save from either could claim the key with a GOCACHE holding none of the -race test objects go-test gates its own save on, reintroducing the rebuild cost #2663 fixed. Separate prefixes avoid the collision but add multi-GB entries to a scope already over its ceiling and evicting. Two write paths the issue does not enumerate are gated here too. golangci-lint-action keeps its own cache and saves by default. install-e2e-tools saved seven executables from /usr/local/bin -- a worse payload than a content-addressed Go cache, and one whose overwrite-before-use defence does not actually hold: setup-tools:229 runs yq (https://github.com/mikefarah/yq/) version v4.53.6 on the restored binary before the --upgrade reinstall at :238, and a failed download only calls log_error, which returns without exiting. Split into restore plus a gated save. Closes #2679. cli-e2e keeps setup-go's cache off for storage rather than trust -- it is already skipped on the fork path -- because the duplicate setup-go-* entry it banked competes for eviction with the single go- entry lint and e2e now depend on entirely. Scope of the gate, recorded in the code and the PR: on the ok-to-test path these action files are themselves checked out from the fork, so this suppresses the writes an ordinary fork run makes by default but is not a boundary against a crafted PR that edits the gate away. Job-level skipping in qualification.yaml, as cli-e2e and security-scan use, is the control that holds there. Fixes: #2670 Fixes: #2679 Signed-off-by: Mark Chmarny <mark@chmarny.com>

Two regressions from the push trigger added in #2673, plus the false premise it was justified with. The premise was wrong. on-push.yaml has always run the full qualification on every push to main, with cancel-in-progress: false and a comment saying never to cancel main pushes. It caught this morning's breakage: its run on 5b0e372 failed on tests / Test at 10:40:59Z. Nothing acted on that for four hours, which is the actual gap -- detection existed, response did not. Duplication: this workflow called the same reusable qualification on push, so every merge qualified twice. The tests job is now skipped on push and tests-skip runs instead, leaving this workflow to contribute only what on-push.yaml does not run -- actionlint, verify-licenses, verify-renovate, docs-mdx, malware-scan, CodeQL, the freshness gates. Those still matter on push because main's ruleset is bypassable and bypassing is routine: a direct push reports "Bypassed rule violations ... 2 of 2 required status checks are expected", so none of them ran before the commit landed. Cancellation: the concurrency group keyed push runs on github.ref with cancel-in-progress: true, so two merges close together cancelled the earlier run -- the exact failure mode on-push.yaml avoids deliberately. c6d8a55 was never qualified for this reason; its run was cancelled 36 seconds later when 5b0e372 landed, and the failure was attributed to the following commit. Push runs now key on github.sha and never cancel, so every merged commit gets its own run and blame lands on the right commit. Verified: exactly one of tests/tests-skip runs for every event and path combination, and the gate treats a skipped job as satisfied when its inverse ran. actionlint and yamllint clean. Signed-off-by: Mark Chmarny <mark@chmarny.com>

mchmarny added 3 commits September 9, 2026 18:05

mchmarny added the theme/ci-dx CI pipelines, developer experience, and build tooling label Sep 10, 2026

mchmarny self-assigned this Sep 10, 2026

github-actions Bot added area/ci area/api size/XL labels Sep 10, 2026

This comment was marked as resolved.

Sign in to view

mchmarny and others added 3 commits September 9, 2026 20:19

Merge branch 'main' into fix/qualification-go-cache-and-oasdiff

a3ea530

mchmarny marked this pull request as ready for review September 10, 2026 10:28

mchmarny requested review from a team as code owners September 10, 2026 10:28

Merge branch 'main' into fix/qualification-go-cache-and-oasdiff

2989653

lalitadithya previously approved these changes Sep 10, 2026

View reviewed changes

mchmarny enabled auto-merge (squash) September 10, 2026 10:46

mchmarny dismissed lalitadithya’s stale review via 68fd392 September 10, 2026 11:26

github-actions Bot added area/docs area/bundler labels Sep 10, 2026

lalitadithya approved these changes Sep 10, 2026

View reviewed changes

mchmarny merged commit 5849fb4 into main Sep 10, 2026
77 checks passed

mchmarny deleted the fix/qualification-go-cache-and-oasdiff branch September 10, 2026 11:55

mchmarny mentioned this pull request Sep 10, 2026

test(server): docs gate merges curl invocations joined by && into one request #2672

Closed

mchmarny mentioned this pull request Sep 10, 2026

fix(ci): stop fork runs writing caches from lint, e2e and cli-e2e #2680

Merged

11 of 25 tasks

This was referenced Sep 10, 2026

ci: go install in composite actions still exposes the gate to sumdb outages #2667

Closed

fix(ci): remove go install from the qualification gate #2713

Merged

Navigation Menu

fix(ci): release-gate fixes for cache, oasdiff, checksums, docs gate - #2673

fix(ci): release-gate fixes for cache, oasdiff, checksums, docs gate#2673
mchmarny merged 10 commits into
mainfrom
fix/qualification-go-cache-and-oasdiff

mchmarny commented Sep 10, 2026 •

edited

Loading

github-actions Bot commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Reviews paused

Walkthrough

❌ Failed checks (2 warnings)

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions Bot commented Sep 10, 2026

github-actions Bot commented Sep 10, 2026

Uh oh!

Labels

2 participants

Conversation

mchmarny commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation / Context

Type of Change

Component(s) Affected

Implementation Notes

fix(ci) - #2663, and the oasdiff third of #2667

fix(tools) - #2658

test(server) - #2655

fix — unbreaking main (not tied to an issue)

ci — stopping the next one (#2678 fallout)

Testing

Risk Assessment

Checklist

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report ✅

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

❌ Failed checks (2 warnings)

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions Bot commented Sep 10, 2026

github-actions Bot commented Sep 10, 2026

Recipe evidence check

Uh oh!

Labels

2 participants

mchmarny commented Sep 10, 2026 •

edited

Loading

`fix(ci)` - #2663, and the oasdiff third of #2667

`fix(tools)` - #2658

`test(server)` - #2655

`fix` — unbreaking `main` (not tied to an issue)

`ci` — stopping the next one (#2678 fallout)

github-actions Bot commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading