Skip to content

feat(validate): warn on aicr version skew across inputs#1386

Merged
mchmarny merged 2 commits into
mainfrom
feat/validate-version-skew-warning
Jun 16, 2026
Merged

feat(validate): warn on aicr version skew across inputs#1386
mchmarny merged 2 commits into
mainfrom
feat/validate-version-skew-warning

Conversation

@mchmarny

@mchmarny mchmarny commented Jun 16, 2026

Copy link
Copy Markdown
Member

Summary

aicr validate now emits a single advisory warning when the running binary, the recipe-producing binary, and the snapshot-producing binary report two or more distinct release versions.

Motivation / Context

Snapshots and recipes already embed the aicr version that produced them (Metadata["version"] / Metadata.Version), but validate never compared them against each other or the running binary. Mixing artifacts from different versions can surface as confusing validation failures with no obvious root cause. This adds a low-cost breadcrumb pointing at version skew.

The embedded versions are unsigned, advisory metadata, so this deliberately does not fail the command and ignores dev/-next builds to avoid noise. A stronger, schema-versioned compatibility gate (consolidating apiVersion, enforcing it on load) is tracked separately.

Fixes: N/A
Related: #1385

Type of Change

  • New feature (non-breaking change that adds functionality)
  • Documentation update

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • Docs/examples (docs/, examples/)

Implementation Notes

  • New pkg/cli/version_skew.go: isReleaseVersion (ignores empty/dev/-next), versionSkewDetected (≥2 distinct release versions), and warnVersionSkew (single slog.Warn naming all three).
  • Wired into the validate action after the snapshot loads (both the file and agent-capture paths populate snap), before phase warnings.
  • Pure decision functions are table-tested; the log wrapper is verified via a captured slog handler.

Testing

make test   # pass, total coverage 77.2% (>= 70% floor)
golangci-lint run -c .golangci.yaml ./pkg/cli/...   # 0 issues

Risk Assessment

  • Low — Isolated, advisory-only CLI log line; no control-flow or exit-code change; easy to revert.

Rollout notes: N/A — no flags, no behavior change beyond an additional warning log.

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
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S)

@mchmarny
feat(validate): warn on aicr version skew across inputs
eb61028 
Snapshots and recipes already embed the aicr version that produced
them, but validate never compared them against each other or the
running binary. Mixed-version artifacts can surface as confusing
validation failures with no obvious cause.

Add an advisory check at the start of the validate run: when the
recipe, snapshot, and binary report two or more distinct release
versions, emit a single slog.Warn naming all three. Dev and -next
builds are ignored to avoid noise. The embedded versions are
unsigned/advisory, so this does not fail the command.

A stronger, schema-versioned compatibility gate is tracked as
follow-up work.
@mchmarny mchmarny requested a review from a team as a code owner June 16, 2026 14:22
@mchmarny mchmarny added the theme/validation Constraint evaluation, health checks, and conformance evidence label Jun 16, 2026
@mchmarny mchmarny self-assigned this Jun 16, 2026
@mchmarny mchmarny mentioned this pull request Jun 16, 2026
Closed
4 tasks
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

🌿 Preview your docs: https://nvidia-preview-feat-validate-version-skew-warning.docs.buildwithfern.com/aicr

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

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: bdd9aefc-b8dc-4a83-8926-17f192646399

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd2268 and eb61028.

📒 Files selected for processing (4)
  • docs/user/cli-reference.md
  • pkg/cli/validate.go
  • pkg/cli/version_skew.go
  • pkg/cli/version_skew_test.go
📝 Walkthrough

Walkthrough

A new version_skew.go file is added to the pkg/cli package implementing four internal functions: isReleaseVersion (excludes empty, dev, and -next builds), versionSkewDetected (returns true when two or more distinct release versions are present), warnVersionSkew (emits a single slog.Warn with binary, recipe, and snapshot version fields when skew is detected), and displayVersion (substitutes "unknown" for empty strings). The validate command is updated to call warnVersionSkew with the running binary version, the recipe metadata version, and the snapshot metadata version. Table-driven unit tests cover all three classifying and detection functions. The CLI reference documentation gains a short note describing this advisory behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(validate): warn on aicr version skew across inputs' clearly and specifically summarizes the main change: adding version skew detection warnings to the validate command.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, covering the feature's purpose, implementation details, testing, and risk assessment.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/validate-version-skew-warning

Comment @coderabbitai help to get the list of available commands and usage tips.

@mchmarny mchmarny enabled auto-merge (squash) June 16, 2026 14:26
@github-actions

github-actions Bot commented Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Coverage Report ✅

Metric Value
Coverage 77.1%
Threshold 75%
Status Pass
Coverage Badge 
![Coverage](https://img.shields.io/badge/coverage-77.1%25-green)

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/NVIDIA/aicr/pkg/cli 69.02% (+0.18%) 👍
Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/NVIDIA/aicr/pkg/cli/validate.go 35.23% (-0.20%) 176 (+1) 62 114 (+1) 👎
github.com/NVIDIA/aicr/pkg/cli/version_skew.go 100.00% (+100.00%) 12 (+12) 12 (+12) 0 🌟

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@mchmarny mchmarny merged commit 68eee88 into main Jun 16, 2026
34 checks passed
@mchmarny mchmarny deleted the feat/validate-version-skew-warning branch June 16, 2026 14:37
@mchmarny mchmarny mentioned this pull request Jun 16, 2026
Merged
15 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Jun 16, 2026
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dims dims dims approved these changes

Assignees

@mchmarny mchmarny

Labels

area/cli area/docs size/L theme/validation Constraint evaluation, health checks, and conformance evidence

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mchmarny @dims