feat(api): deprecation channel, CLI surface gate, ADR-022 emit guards - #2436
Conversation
mchmarny
added
the
theme/ci-dx
Recipe evidence checkNo leaf overlays affected by this PR. This gate is warning-only and never blocks merge. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe changes define deprecation policy and documentation for frozen public surfaces. They add a CLI surface golden test and baseline. They add structured deprecation notices and REST response headers for configured routes. They make direct Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Coverage Report ✅
Coverage BadgeMerging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
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. |
f217428 to
fb2c0a3
Compare
This comment was marked as resolved.
This comment was marked as resolved.
fb2c0a3 to
cec7c19
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
b235299 to
2290267
Compare
This comment was marked as resolved.
This comment was marked as resolved.
2290267 to
77299a7
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Closes #2421: validateRecipeInputAPIVersion short-circuited on an empty apiVersion before it inspected the kind, so a headerless RecipeMetadata was rejected by the catalog scanner but silently hydrated when passed with -r. The empty-value tolerance is now scoped to RecipeResult inputs, which genuinely predate the field; ADR-022 3 retires that at N+2 (#2417). Closes #2423: adds the emit-site half of the ADR-022 contract. adr022_map_test.go pins that each track constant routes to the right gate and target, but cannot see which constant a given emit site chose -- stable and authoring carry the same string until the v0.22 switch. adr022_emit_test.go asserts the observed apiVersion on a real artifact against its track constant, so a mislabeled site diverges at the switch instead of shipping a wrong-version artifact. Covers the five code emitters plus the 115 committed catalog documents. Signed-off-by: Mark Chmarny <mark@chmarny.com>
Closes #2115 except its scheduling criterion. RELEASING.md gains a per-surface deprecation policy: what counts as breaking and additive on each of the four frozen surfaces, and the notice a removal owes -- two minor releases before v1.0, the next vMAJOR after. Artifact apiVersion keeps its maturity-scoped window and takes precedence for that surface. New pkg/deprecation carries the runtime half. Recorder.Warn emits one slog warning per distinct subject, so a loader walking 100 deprecated files does not emit 100 identical lines. SetHTTPHeaders sets Sunset (RFC 8594), Deprecation, and Link rel=deprecation. The Go SDK needs no runtime support: a // Deprecated: marker reaches consumers through staticcheck at their build time, which is documented in the integrator guide. pkg/server gains WithDeprecatedRoutes and a deprecationMiddleware ordered outside rateLimitMiddleware, so a throttled client still learns the endpoint is going away. No route is marked yet -- the /v1 disposition is #2112, which is also the exercise of record the channel owes before v1.0.0. docs/user/deprecations.md is the durable register, seeded with the two active ADR-022 artifact deprecations. Signed-off-by: Mark Chmarny <mark@chmarny.com>
Closes #2111. pkg/cli/testdata/cli-surface.golden pins all 21 commands and 205 flags with their aliases, types, defaults, required/hidden state, and env vars. TestCLISurface walks the live urfave/cli tree and fails on drift. It runs under make test, so it is already inside the merge gate. The gate classifies its own failure rather than reporting every diff identically. An added command or flag is additive and the message says to regenerate; a removed or renamed one is reported as BREAKING and points at the deprecation policy in RELEASING.md. Reporting both the same way would train everyone to run -update reflexively, which is the reflex that lets a rename reach main. Aliases are pinned in urfave's order, so promoting --gpu over --accelerator registers as a change rather than a reordering. Usage strings are deliberately excluded: they are prose and would make the gate fail on every wording fix. TestCLISurfaceIsNotEmpty guards the walker itself, so a refactor that collapsed the tree cannot pass by regenerating an empty golden. Signed-off-by: Mark Chmarny <mark@chmarny.com>
main's 1743160 replaced ROADMAP's "### 2. Stability" with "## 1. Defensible API stability", which broke every inbound ROADMAP.md#2-stability anchor. Two of the four were pre-existing in ADR-022 and are fixed here too, since lychee runs on any PR touching docs/** and would otherwise fail this one for main's breakage. The rewrite also dropped the "deprecation channel exercised at least once in practice before tagging v1.0.0" acceptance criterion. RELEASING.md no longer claims ROADMAP requires a rehearsal; it names the two real deprecations already scheduled (#2112 and the ADR-022 alpha migration) as the exercise, which is also why #2115 could close without one. Signed-off-by: Mark Chmarny <mark@chmarny.com>
6472065 to
bb23fcc
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes.
The RFC 9745 correction is correct. The synthetic alias test closes the coverage gap that an all-empty production baseline could not detect. The SDK documentation now correctly describes its build-time deprecation channel. I accept the established same-line 🐇 ✅ ✏️ Learnings added
🧠 Learnings usedYou are interacting with an AI system. |
njhensley
left a comment
njhensley
left a comment
There was a problem hiding this comment.
Multi-persona review — Approve with comments
Method: four independent persona reviewers (Correctness · API-contract/ADR-022 · Security-Operability/CI-DX · Docs) fanned out in parallel, then every finding adjudicated against the resolved code by an adversarial senior pass, with the crux claims independently reproduced.
Legend: 🔴 Blocker · 🟠 Major · 🟡 Minor · 🔵 Nitpick
Overall assessment
High-quality, unusually well-documented PR. The load-bearing correctness claims all hold up under independent verification:
- The #2421 loader fix is correct. Moving the
apiVersion == ""short-circuit below theRecipeMetadatabranch (pkg/recipe/loader.go) makes an empty header on aRecipeMetadatafall through to rejection (IsSupportedAuthoringAPIVersion("")andIsSupportedProfileAPIVersion("")both returnfalse). The reordered gate is now behavior-identical to the catalog scannerclassifyRecipeMetadataCatalogHeaderacross the empty/alpha/target/unknown matrix, andTestRecipeMetadataHeaderGatesAgreeproves that directly, error codes included. The fail-open seam is genuinely closed. - The ADR-022 emit/map tripwire is armed, not vacuous. The "passes trivially today" behavior is deliberately disclosed (stable/authoring constants share the alpha string until the v0.22 switch); expectations are written as constants that retarget in lockstep. The "no emit site for
RecipeCriteriaAPIVersion/config.APIVersion" finding for #2416 checks out — both appear only in read/validation gates. - The deprecation runtime half is concurrency- and RFC-correct.
Recorder.Warn'sseenmap is read/written entirely under the mutex with theslogI/O outside the lock;SetHTTPHeadersemits RFC 9745@unixseconds/ RFC 8594 IMF-fixdate, omits on zero-time, and usesAdd(notSet) for the multi-valuedLink. No attacker-controlled string reaches any header —Subject/Replacementgo only to the log message. - The CLI freeze gate is reproducible. No flag default reads env/
$HOME/cwd/time; the one version-derived default (--image→:latest) is guarded byTestCLISurfaceDefaultsAreBuildIndependent.
No blocker or major issues survived adjudication. What remains is doc polish (2 minor) and two forward-looking nitpicks, left inline.
Confirmed non-issues (examined, no change needed)
WithDeprecatedRoutesstores the caller's map by reference — matches the existingWithHandlerpattern; construction-time config, read-only at request time.AICR_LOG_LEVEL=errorsuppresses CLI deprecation warnings — disclosed and deliberate; the HTTP and Go-SDK arms are log-independent, and release notes + the durable page are the log-independent channels.deprecationMiddleware/WithDeprecatedRoutesunused by any live route — exported (noU1000), and the populated path is covered byTestDeprecationMiddleware,TestDeprecationHeadersSurviveRateLimitRejection, andTestWithDeprecatedRoutes. Scaffold for #2112.- No HTTP header-injection vector in
SetHTTPHeaders— only theDocsURLconst and formatted dates are written.
One note for the PR description (not a code change)
The description's framing of the Deprecation header ("the widely-implemented earlier form is the literal true… we emit a date when we have one and true when we do not") does not match the shipped code, which emits only RFC 9745 @unixseconds or omits the header entirely — never true. The code and all three docs are internally consistent and correct; only the description is stale. Worth a one-line edit so a future reader doesn't trust the narrative over the artifact.
Summary
🔴 Blocker 0 | 🟠 Major 0 | 🟡 Minor 2 | 🔵 Nitpick 2 Recommendation: Approve with comments
| another: release notes are read once, the durable page is read later by someone | ||
| debugging, and the runtime warning reaches the user who never read either. | ||
|
|
||
| 1. A `## Deprecations` section in the release notes for the release that |
There was a problem hiding this comment.
🟡 Minor — RELEASING.md says release notes carry a ## Deprecations (h2) section, but the aicr-release-notes skill requires ### Deprecations (h3)
This item says "A ## Deprecations section in the release notes," but the authoritative generator (.agents/skills/aicr-release-notes/SKILL.md:108) adds ### Deprecations as an h3 sibling of ### Highlights. Someone following RELEASING.md literally would emit an h2 that breaks the heading hierarchy the skill enforces.
Fix: Change this to ### Deprecations to match the skill's h3 level.
| ### Empty `apiVersion` on artifacts | ||
|
|
||
| **Surface:** bundle and artifact schemas · | ||
| **Deprecated in:** v0.22 · **Removed in:** v0.23 |
There was a problem hiding this comment.
🟡 Minor — "Empty apiVersion" entry header (Deprecated v0.22 / Removed v0.23) is contradicted by its own body, which says the RecipeMetadata narrowing landed in v0.21
The header labels this Deprecated in v0.22 / Removed in v0.23, but the body two lines down (lines 71-76) says that for the RecipeMetadata direct-overlay path the empty-header tolerance was already removed in v0.21 (#2421 — what this PR ships); only the RecipeResult tolerance survives to v0.23. The single-line summary contradicts its own paragraph and reads as forward-dated for the change it describes.
Fix: Qualify the header, e.g. Removed in: v0.23 (RecipeResult); v0.21 for RecipeMetadata overlays, mirroring the clean scoping already in api-reference.md:604 ("The tolerance is scoped to RecipeResult").
| // contract change, not a reordering. | ||
| rendered := make([]string, 0, len(names)) | ||
| for i, n := range names { | ||
| if i == 0 || len(n) > 1 { |
There was a problem hiding this comment.
🔵 Nitpick — flagFacts renders a single-char primary flag as --x rather than -x
flagFacts prefixes names[0] with -- unconditionally; only aliases (i>0) get the single-char - treatment. A flag whose primary name is a single character would be recorded as --x. No such flag exists in the tree today, so this is latent and cosmetic — the golden stays deterministic and internally consistent.
Fix: Apply the same len(n) > 1 check to i == 0 to future-proof the rendering.
| // made here. | ||
| func (s *Server) deprecationMiddleware(next http.HandlerFunc) http.HandlerFunc { | ||
| return func(w http.ResponseWriter, r *http.Request) { | ||
| if notice, ok := s.config.DeprecatedRoutes[r.URL.Path]; ok { |
There was a problem hiding this comment.
🔵 Nitpick — Deprecation route matching is exact-path only
s.config.DeprecatedRoutes[r.URL.Path] is an exact-path lookup, consistent with AICR's exact-path mux registration and correct today (no route is deprecated). When #2112 deprecates a /v1/* family, exact-match won't cover sub-paths.
Fix: Revisit prefix semantics when #2112 lands. Flagging only so it is a conscious choice, not a defect here.
Three findings from the approving review on #2436, all landing before v0.21 cuts so none of them reach a release. RELEASING.md advertised a "## Deprecations" release-notes section while the generator that actually emits it (.agents/skills/aicr-release-notes) uses "### Deprecations" as an h3 sibling of "### Highlights". Following the prose literally would have broken the heading hierarchy the skill enforces. The deprecations ledger entry for the empty-apiVersion tolerance was headed "Deprecated in v0.22, Removed in v0.23" while its own body disclosed that RecipeMetadata overlays lost the tolerance in v0.21. The header now scopes both dates by kind, mirroring the scoping already used in docs/user/api-reference.md. flagFacts chose the dash prefix by position rather than name length, so a flag whose *primary* name was a single character would have been recorded as "--x" instead of "-x". No flag in the tree has one, which is exactly why the golden could never catch it: a positional implementation produces a byte-identical baseline forever. The rendering is now keyed on length and TestFlagFactsRendersDashPrefixByNameLength pins it against a synthetic flag. Regenerating the golden after the change is a no-op, confirming no current flag is affected. Signed-off-by: Mark Chmarny <mark@chmarny.com>
yuanchen8911
left a comment
yuanchen8911
left a comment
There was a problem hiding this comment.
Post-merge review of this PR. Verified against current main at b6b242026; findings already fixed by that commit are excluded. Five findings, four of them mechanical.
1. Migration guidance names a command that does not exist. Four published-doc locations and one test comment tell users to pass an overlay with aicr recipe -r. The recipe command has no --recipe/-r flag — the CLI surface baseline committed here shows --snapshot,-s and no --recipe under aicr recipe — and a live probe returns flag provided but not defined: -r. The flag exists on bundle, validate, mirror list, and evidence digest. This is the remediation path for exactly the users this change breaks. Replace with aicr bundle -r and aicr validate -r.
2. A required flag added to an existing command is reported as compatible. diffLines routes every new baseline line into the additive bucket, including lines carrying required=true, and the failure text reads "Additions are compatible. Regenerate the golden." Adding a required flag to an existing command makes previously valid invocations fail, which the policy table in RELEASING.md classifies as breaking — only a new flag whose default preserves behavior is additive. A required flag on an entirely new command remains additive, so the check must be scoped to commands already present in the baseline.
3. The baseline omits framework-injected surface. renderSurface walks RootCommand() before urfave/cli v3.11.0 performs setup, so none of the injected surface reaches the golden: the completion command and its bash, zsh, fish, and pwsh subcommands, --help across the command tree, and root --version. root.go sets EnableShellCompletion: true and then uses ConfigureShellCompletionCommand to un-hide the completion command and give it a category, so it is deliberately public surface. Removing any of it would pass the gate silently. Render from a post-setup tree; enumerating specific injected entries would close the gap only partly.
4. The empty-subject assertion cannot detect the regression it guards. The condition requires both that the malformed message is present and that --real-flag is absent, but the preceding call always logs --real-flag, so that branch is unreachable. The newline anchor is a second problem: slog's text handler does not place a newline immediately after the message, because structured attributes intervene. The assertion below still catches a suppressed real warning, so the test is not entirely inert — but the regression it is named for would pass. The production guard is correct.
5. Reconcile the early closure with the central policy. Rejecting a headerless RecipeMetadata on the direct-input path is reachable for externally authored overlays, while the deprecation policy added in the same commit requires two minor releases of notice before a pre-1.0 breaking removal. The decision is documented — ADR-022 records the early closure and its rationale, and the deprecations ledger calls it an early narrowing — so this is not undisclosed behavior. What is missing is reconciliation: RELEASING.md should classify it explicitly as the one-time v0.21 exception the ADR and ledger already imply.
Suggested split: one mechanical follow-up for 1–4, and a separate policy item for 5. Findings 2 and 3 are gaps in a gate added to prevent this exact class of drift, so they are worth closing before the gate accumulates trust it has not earned. Nothing here reaches a released artifact.
| empty. That tolerance retires alongside the alpha values. | ||
|
|
||
| One narrowing landed earlier than the rest: as of v0.21, a `RecipeMetadata` | ||
| overlay passed directly (`aicr recipe -r overlay.yaml`, |
There was a problem hiding this comment.
aicr recipe -r overlay.yaml is not a valid invocation — the recipe command has no --recipe/-r flag, and running it returns flag provided but not defined: -r. This is the remediation path for the users this change breaks, so it needs to be right. Use aicr bundle -r overlay.yaml and aicr validate -r overlay.yaml.
| `RecipeResult` inputs through v0.22, and v0.23 stops admitting it along with the | ||
| alpha values. The tolerance is scoped to `RecipeResult`, which predates the | ||
| field: a `RecipeMetadata` overlay is a catalog document however it arrives, so | ||
| `aicr recipe -r` and `aicr bundle -r` reject a headerless one exactly as a |
There was a problem hiding this comment.
Same invalid command: aicr recipe -r does not exist. Should read aicr bundle -r and aicr validate -r.
| outside this contract. | ||
|
|
||
| This gate follows the document, not the entry point. Passing a single overlay | ||
| directly — `aicr recipe -r overlay.yaml`, `aicr bundle -r overlay.yaml` — |
There was a problem hiding this comment.
Same invalid command. aicr recipe -r overlay.yaml should be aicr validate -r overlay.yaml, alongside the aicr bundle -r already cited here.
|
|
||
| **A catalog kind is governed by §8 on every path it can arrive by.** The | ||
| tolerance above is scoped by wire kind, not by entry point. A `RecipeMetadata` | ||
| reaching AICR as a direct recipe input (`aicr recipe -r overlay.yaml`, |
There was a problem hiding this comment.
Same invalid command in the clause defining which paths the gate governs. The direct-input paths are aicr bundle -r and aicr validate -r.
| // TestRecipeMetadataHeaderGatesAgree pins the #2421 invariant: the same | ||
| // RecipeMetadata header is accepted or rejected identically whether the | ||
| // document reaches AICR through the catalog scanner (`--data`) or through the | ||
| // direct recipe input path (`aicr recipe -r`, `aicr bundle -r`). |
There was a problem hiding this comment.
This comment cites aicr recipe -r as a direct recipe input path. That flag does not exist; the paths are aicr bundle -r and aicr validate -r. Worth correcting so the test's stated scope matches the code it guards.
| "intentional and the window has passed, regenerate the golden.\n\n") | ||
| } | ||
|
|
||
| if len(added) > 0 { |
There was a problem hiding this comment.
Every added line lands here and is reported as additive, including a line with required=true. Adding a required flag to a command that already exists breaks previously valid invocations, which the policy table in RELEASING.md classifies as breaking. Suggest splitting added lines: those with required=true whose command already appears in the baseline belong in the BREAKING section. A required flag on a brand-new command is genuinely additive and should stay where it is.
|
|
||
| func renderSurface() string { | ||
| var lines []string | ||
| collectSurface(RootCommand(), "", &lines) |
There was a problem hiding this comment.
This walks the pre-setup command tree, so urfave-injected surface never reaches the baseline. In v3.11.0 that is the completion command plus its bash, zsh, fish, and pwsh subcommands, --help across the command tree, and root --version — none appear in the golden. root.go enables shell completion and explicitly un-hides the completion command through ConfigureShellCompletionCommand, so it is intentional public surface, and Version is set. Disabling any of it would not fail this gate. Prefer rendering from a post-setup tree over asserting a fixed list of injected entries, which would leave the gap only partly closed.
| r.Warn(Notice{RemovedIn: "v0.25"}) | ||
| r.Warn(Notice{Subject: "--real-flag", RemovedIn: "v0.25"}) | ||
|
|
||
| if strings.Contains(buf.String(), "is deprecated and will be removed in v0.25\n") && |
There was a problem hiding this comment.
The empty-subject branch is unreachable: the call on the line above always logs --real-flag, so the negated second operand is always false. The newline anchor is a second problem — slog's text handler does not place a newline immediately after the message, because structured attributes intervene. The assertion below still catches a suppressed real warning, so the test is not entirely inert, but the regression it is named for would pass. Assert that the message appears exactly once instead.
4 participants
Summary
Closes the four ADR-022 / API-freeze follow-ups scheduled for v0.21: aligns the two
RecipeMetadataheader gates, adds an emit-site guard before the v0.22 emitter switch, defines and implements the deprecation channel, and baselines the CLI surface. Three focused commits, reviewable independently.Motivation / Context
With ADR-022 merged, v0.21 is Release N of its three-release migration (readers accept both tracks, emitters still write alpha). Two of these issues have to land before the v0.22 emitter switch (#2416) to be useful at all; the other two are v1 API-freeze prerequisites that do not depend on it.
One sequencing note worth flagging for the epics: epic #2370 lists the three ungated-surface baselines (#2111 CLI, #2112 REST, #2113 bundle/schemas) as Phase 1, ahead of #2114/#2115. For REST and bundle/schemas that ordering is inverted — ADR-022 deliberately breaks the artifact surface at N+1 and N+2, and #2417 removes alpha values from every
apiVersionenum inserver.yaml. A REST or schema baseline committed before v0.23 would fire on our own planned removal. The CLI is the one surface ADR-022 does not touch, which is why #2111 is in this PR and #2112/#2113 are not.Fixes: #2421
Fixes: #2423
Fixes: #2111
#2115 and #2114 were closed manually alongside this PR. #2115's five implementable criteria all ship here; its sixth ("exercise the channel once on a real deprecation before
v1.0.0") was judged theater and will be satisfied for real by #2112 / #2416 / #2417. #2114's decision was already recorded in ADR-022; its remaining execution is #2416 and #2417.Related: #2416, #2417, #2112, #2113, #2370, #1812
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/server)pkg/recipe)docs/,examples/)pkg/deprecation;RELEASING.md;pkg/headertestsImplementation Notes
1.
fix(recipe)— closes #2421 and #2423#2421 —
validateRecipeInputAPIVersionshort-circuited on an emptyapiVersionbefore it inspected the kind, so the same headerlessRecipeMetadatawas rejected by the catalog scanner and silently hydrated when passed with-r. The empty-value tolerance is now scoped toRecipeResultinputs, which genuinely predate the field.The issue offered three options and leaned toward deferring to v0.22 with a deprecation warning. We took option 1 (close it now) because the blast radius is nil: no committed
RecipeMetadatain the tree lacks anapiVersion, verified by scanning everykind: RecipeMetadatafile. Deferring would have left the two paths disagreeing across the exact release where #2416 rewrites every committed header. ADR-022 §3 records which clause governs a catalog kind arriving via the direct-input path, so the next reader does not re-derive it.#2423 —
adr022_map_test.gopins that each track constant routes to the right gate and target, but cannot see which constant an individual emit site chose.StableGroupVersionandAuthoringGroupVersioncarry the same string until v0.22, so a catalog emitter reaching for the stable constant is invisible today and would quietly emitaicr.run/v1on an authoring document at the switch.adr022_emit_test.gocloses that: it asserts the observedapiVersionon a real artifact against its track's constant. Expectations are written as constants, never literals, so they retarget in lockstep at the switch. Chained with the map, the contract is covered end to end:emit site → track constant → §2 target → read gate.Coverage is five code emitters (Snapshot via
NodeSnapshotter.Measure, default/profile/configuration-bearingRecipeResult,BundleProvenanceviaWriteProvenance) plus a scan of all 115 committed catalog documents, which directly guards #2416's "no committed artifact carries an alpha apiVersion" criterion.2.
feat(server)— closes #2115 except its scheduling criterionRELEASING.mdgains a per-surface policy: breaking vs. additive for CLI, REST, Go SDK, and bundle/schemas, plus the notice owed — two minor releases before v1.0, the nextvMAJORafter. ArtifactapiVersionkeeps its maturity-scoped window and takes precedence for that surface.New
pkg/deprecationcarries the runtime half.Recorder.Warnemits oneslogwarning per distinct subject, so a loader walking 100 deprecated files does not emit 100 identical lines.SetHTTPHeaderssetsSunset(RFC 8594),Deprecation, andLink rel="deprecation".Three deliberate calls worth review:
slog, not raw stderr. This honorsAICR_LOG_LEVEL,NO_COLOR, and TTY detection like every other diagnostic. The tradeoff is real:AICR_LOG_LEVEL=errorsuppresses deprecation warnings. Silencing warnings is treated as an explicit opt-out, and the release notes plus the docs page do not depend on log level.Deprecationis not RFC 8594 — it is RFC 9745. OnlySunsetis RFC 8594.Deprecationis RFC 9745 (Standards Track, March 2025), whose §2.1 requires a Structured Field Date:@<unix-seconds>. The shipped code emits only that form, and omits the header entirely when no date is recorded — it never emitstrue. RFC 9745 admits no placeholder, and an unparseableDeprecationis worse than an absent one. (An earlier revision of this description described atruefallback; that reflected a pre-review draft of the code, not what merged.)DocsURLpoints at GitHub, not docs.nvidia.com. The published URL is derived by Fern fromdocs/index.ymland versioned per release, so it is neither stable nor verifiable from this repo. A dead link inside a deprecation warning defeats the warning.deprecationMiddlewareis ordered outsiderateLimitMiddleware, which writes a 429 and returns without callingnext. A client backing off a deprecated endpoint is exactly the one that needs to know it is going away.No route is marked deprecated. The
/v1/*disposition is #2112. Per the policy added here,/v1retirement is also named as the exercise of record the channel owes before v1.0.0 — the ADR-022 alpha migration warns and removes across v0.22/v0.23, but alpha owes no window under §4, so it cannot alone demonstrate the channel honoring an obligation it actually had.3.
test(cli)— closes #2111cli-surface.goldenpins all 21 commands and 206 flags with aliases, types, defaults, required/hidden state, and env vars. Aliases are pinned in urfave's order, so promoting--gpuover--acceleratorregisters as a change rather than a reordering.The gate classifies its own failure: an added command or flag reports as additive with "regenerate"; a removed or renamed one reports as
BREAKINGand points at the deprecation policy. Reporting both identically would train everyone to run-updatereflexively, which is the reflex that lets a rename reachmain.Usage strings are deliberately excluded — they are prose, and pinning them would make the gate fail on every wording fix.
Testing
Every behavioral change was mutation-tested, not just asserted:
apiVersion == ""short-circuit inloader.gorecipes/overlays/aks.yamltov1alpha2(wrong track)deprecationMiddlewareinsiderateLimitMiddlewareBREAKING, cites the deprecation policyAdditive, says regenerateAn earlier version of the middleware ordering test used a panicking handler; that passes either way, because the panic happens after the inner layers have already run. It was rewritten against a zero-token rate limiter, which is the short-circuit that actually discriminates.
Coverage:
pkg/deprecation97%+ (new package);pkg/server83.9% → 84.0%. Every new exported func is at 100% (WithDeprecatedRoutes,deprecationMiddleware,Warn,SetHTTPHeaders,Message).Risk Assessment
Rollout notes: The one behavior change is #2421: a
RecipeMetadataoverlay with a missing or emptyapiVersionpassed viaaicr recipe -r/aicr bundle -ris now rejected instead of silently hydrated. No in-tree file is affected (all 115 committed catalog documents carry a header). External users authoring headerless overlays outside a catalog tree are affected; this is documented indocs/integrator/data-extension.md,docs/user/api-reference.md, anddocs/user/deprecations.md. HydratedRecipeResultinputs keep the empty tolerance until v0.23 (#2417).Everything else is additive: a new package, a new opt-in server option, a new test, and docs.
Checklist
make testwith-race)make lint) —golangci-lint0 issues; see themake qualifycaveat abovegit commit -S)