fix(release): correct pending changeset bump classification#5774
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
Request changes. The scope-gate exemption is content-blind, and it's being used in the same PR to drive a protocol-scoped reclassification the gate's own policy forbids.
The block
scripts/check-changeset-protocol-scope.cjs — the new isChangesetEditOnlyMaintenance early-return at the top of findChangesetProtocolScopeViolations never inspects changeset content. It exempts the PR whenever every path is an M changeset (plus policy code). changesetTargetsProtocol is never consulted on this path.
Failure mode, in one sentence: a PR whose entire diff is M .changeset/foo.md editing the frontmatter to "adcontextprotocol": major — with zero protocol-source change — now passes the scope gate, where main blocks it with "Protocol changesets are only allowed when the PR also changes protocol schemas...".
Traced both ways:
main:{M .changeset/foo.md}, no protocol-scoped files →changesetFiles=[foo],protocolScopedFiles=[]→ violation. Blocked.- This PR: same diff →
isChangesetEditOnlyMaintenancereturns true →findChangesetProtocolScopeViolationsreturns[]. Passes.
The exemption can't tell a downgrade/removal (this PR's actual, legitimate use) from an escalation (minor→major with no source). That asymmetry is the defect. And the new assertion at the diff's L86-90 uses protocolChangeset (which targets adcontextprotocol) with a single M and no protocol source, asserting violations === [] — the test codifies the hole as sanctioned behavior. code-reviewer returned Blocker on exactly this.
Fix is small: edited changesets that still targetsProtocol while protocolScopedFiles.length === 0 must stay rejected. Let them fall through to the existing content check, or have the exemption read content and refuse to exempt a protocol-targeting edit with no protocol source. That preserves the downgrade/remove use case while keeping escalation closed. Add a negative test: "edit introduces a protocol bump, no protocol source" must fail.
The second blocking thread — the registry reclassification is unsound
ad-tech-protocol-expert: unsound. static/schemas/source/core/registry-feed-response.json is registered in static/schemas/source/index.json and ships inside the versioned adcontextprotocol schema bundle. The repo's own PROTOCOL_SCOPED_PATHS lists ^static\/registry\// and ^docs\/reference\// as protocol-scoped — registry changes are defined by this script as requiring an adcontextprotocol bump. Dropping the bump on .changeset/registry-feed-stream.md directly contradicts that policy. The freshness object (now required on the protocol-versioned response) and tmpx_providers are both additive wire changes — textbook minor under docs/reference/versioning.mdx ("new fields, new capabilities" = a Release; patch = "introduces no new capabilities"). Shipping them as 3.1.1 mislabels two minor wire additions as a no-new-capabilities patch.
The two threads are one story: the PR loosens the gate, then uses the gap to remove a protocol-scoped bump the gate was built to require. If the real position is that the registry HTTP transport is separately versioned (static/openapi/registry.yaml is 1.0.0), then reconcile it — drop ^static\/registry\// from PROTOCOL_SCOPED_PATHS and document it in versioning.mdx. Don't leave the script asserting registry-is-protocol while the changeset asserts the opposite.
Things I checked
- Full read of
scripts/check-changeset-protocol-scope.cjs(base) and traced the new function againstfindChangesetProtocolScopeViolationsL128-159 and the gate step in.github/workflows/changeset-check.yml. tmpxdeprecation is non-breaking now — "still emitted by routers... removed in 4.0." No hidden breaking change in either changeset. That part is semver-clean.- No interaction bug with
isChangesetDeleteOnlyCleanup/isChangesetStatusExemptMaintenance:Dand empty-diff both return false from the new function. Renames (R) fall through to the content check — correct, but untested.
Follow-ups (non-blocking — file as issues)
- An empty-frontmatter changeset that's kept (registry-feed-stream) produces no changelog entry on
changeset version— the registry SSE prose disappears from release notes. If registry is versioned elsewhere, fine; confirm the feature isn't shipping silently. isChangesetMaintenancePathduplicates the inline path checks inisChangesetDeleteOnlyCleanup(L89) andisChangesetStatusExemptMaintenance(L111). Consolidate once the block is resolved.- Add a test pinning that a renamed-but-edited protocol changeset is still content-checked.
Test-plan note
The validation block confirms changeset status resolves to 3.1.1 — that's the version math, not the version correctness. It doesn't validate that 3.1.1 is the right classification for two additive wire changes. That's the open question, not a settled result.
Unblock with: content-aware exemption that still rejects escalation, the negative test, and a one-line statement reconciling registry's protocol-scoped status with the dropped bump.
00b3bc2 to
cfac355
Compare
There was a problem hiding this comment.
Correct release reclassification — 3.2.0 was the accidental bump (#5769), this lands it at 3.1.1. Right call on the architectural principle: the registry mirror transport and experimental surfaces version independently of the stable adcontextprotocol wire contract, and the gate now encodes that instead of treating every static/ touch as a protocol bump.
Things I checked
- Rank math is correct.
PROTOCOL_BUMP_RANKnull=0 < patch=1 < minor=2 < major=3;protocolBumpRank(bump ?? null) ?? 0is robust. Downgrade/escalation/same-level-prose all classify as intended (check-changeset-protocol-scope.cjs:7-14). - The default
readFileAtBase = () => ''fails closed. With no base reader, every head bump reads as an escalation fromnull, soisChangesetClassificationMaintenancereturnsfalseand the maintenance bypass never applies — backward-compatible with the existing 2-arg callers. Fail-closed beats fail-open. - Registry carve-out doesn't leak a protocol change.
REGISTRY_RELEASE_SCOPED_PATHSis checked before the broadstatic/schemas/source/rule (isProtocolScopedPath, L68-72) and is anchored to exactlyregistry-(event|feed-response).json.ad-tech-protocol-expertconfirmedadagents.jsonand thecore/*authorization*schemas — the real interop contract — stay protocol-gated. - This PR's own check passes. It also edits
docs/reference/versioning.mdx, which is still protocol-scoped, soprotocolScopedFiles.length > 0short-circuits and the new classification branch never runs on this diff. - tmpx_providers minor→patch is sound. The
trusted-matchsurface carriesx-status: experimentalat the schema root (identity-match-response.json:15) andtrusted_match.coreis a declared experimental feature; experimental-only schema changes are patch-level under the new policy. - UTF-8 fix is non-breaking.
get-adcp-capabilities-response.json:216is a—→-change inside thegovernance_awaredescription string only — no field/type/enum/required/default change, 44 em-dashes left untouched elsewhere, and no reference-doc copy of that text, so zero drift.
Follow-ups (non-blocking — file as issues)
- Latent crash in
isChangesetClassificationMaintenanceon deleted/renamed changesets (check-changeset-protocol-scope.cjs:120-152). The new loop iterates rawchange.pathsand callsgit show HEAD:<path>/git show <base>:<path>for any changeset path, butisChangesetEditOnlyMaintenanceonly short-circuits on statusA— it toleratesDandR. A[{M, a.md}, {D, b.md}]set reachesreadFileAtHead('b.md')on a file absent at HEAD,git()(L161-163) has no try/catch, and the whole check dies with status 128. That is exactly the delete+downgrade shape the next release-classification PR produces. It's gated out here by theversioning.mdxedit, and a crash fails the gate closed (spurious CI failure, not a silent pass), so it's safe today — but guard the reads (skip non-Mstatuses, or wrapgit()to return''on missing paths) before the next changeset-maintenance PR hits it. - The new branch ships covered only by stubs. All tests use the
readFilesstub returning''for unknown paths, which masks the crash above. AddM+DandM+Rcases throughfindChangesetProtocolScopeViolationsto prove it doesn't throw. - Registry payload-shape changes have no automated backstop.
registry-event.jsoncarries buyer/seller-facing authorization state (authorization.granted/revoked/modified,publisher.adagents_changedwithverified_specialisms,badgeRole). The new prose hedges with "unless they also change a compliance artifact or normative protocol rule," but the regex unconditionally exempts the file — a wire-breaking reshape of those event payloads would now escape theadcontextprotocolbump on author discipline alone. Worth a narrower backstop than file-level exemption. versioning.mdxstability table (L178-185) reads as unconditional. "Fields: Never removed / Enums: Only additive" still presents as a flat 3.x guarantee; the experimental subsection (L187) references break-with-notice but not the new patch-level reclassification. Cross-link it so a reader who stops at the table learns experimental schema changes can land in a patch.
Minor nits (non-blocking)
static/openapi/registry.yamlexclusion is dead code.static/openapi/was never inPROTOCOL_SCOPED_PATHS, so excluding it (check-changeset-protocol-scope.cjs:16) removes something never included. The new test assertion passes identically without the regex. Drop it or note the intent.- The tmpx changeset prose undersells the change and cites a wrong path. It describes "introduce
tmpx_providers... aprovider_id → token map," but perad-tech-protocol-expertthe schema documents a breaking reshape (Map<id,string>→Map<id,{macros:[…]}>) that shipped in #5689 — patch-eligible on an experimental surface, but breaking means the 6-week-notice obligation applies, which a patch changeset entry doesn't satisfy on its own. The referenced pathstatic/schemas/source/tmp/identity-match-response.jsonis also wrong — the file lives attrusted-match/. Cosmetic, but it weakens the audit trail.
A changeset-classification PR whose new classification branch never executes — because the PR also touches a protocol-scoped doc — is a notable way to ship a feature against stubs. Tighten the test coverage when you fix the crash guard.
LGTM. Follow-ups noted below.
cfac355 to
80bb2f6
Compare
There was a problem hiding this comment.
Blocking on versioning governance. The minor→patch downgrade of the tmpx_providers reshape contradicts AdCP's own documented precedent and the page's own patch contract — ad-tech-protocol-expert returned unsound with cited spec divergence. The script and tests are clean; the classification is not.
Things I checked
- Gate logic fails closed.
code-reviewertraced the newisChangesetClassificationMaintenance/isChangesetEditOnlyMaintenancepath (scripts/check-changeset-protocol-scope.cjs:108-160): escalations are blocked (null→patchreads as escalation, denied), new changesets (statusA) are blocked, renames (R*) fall through and stay content-checked,D-status changesets are never read, and thereadFileAtBase = () => ''default is fail-closed. The new bypass is correct. - This PR passes its own gate via the normal branch, not the bypass —
docs/reference/versioning.mdxand the schema file are both protocol-scoped, soprotocolScopedFiles.length > 0. Self-consistent. - The UTF-8 fix is non-breaking.
get-adcp-capabilities-response.json:215— em-dash → ASCII hyphen inside thegovernance_awaredescription. Norequired/enum/const/type touched. Not spec drift. Clean.
MUST FIX (blocking)
-
minor→patch on a breaking experimental reshape contradicts established precedent.
release-notes.mdx:652: the SIcontext→intentrename (#2774) — a breaking reshape of anx-status: experimentalsurface — was "Treated as a minor change under thex-status: experimental+ 6-week notice policy." Experimental status relaxes the notice window (6 weeks vs 6 months) and permits the break in-cycle; it does not reclassify the break as non-breaking.tmpx_providersgoing fromMap<provider_id, string>(#5689) toMap<provider_id, {macros:[...]}>is the same class of change. Patch is the wrong tier. What breaks for adopters: a router that adopted the v1tmpx_providersshape silently breaks on a 3.1.0→3.1.1 upgrade that the patch contract promises is "always safe." -
The new patch text contradicts the page's own guarantees.
versioning.mdx:192now folds "experimental-only surfaces" into the patch definition, whileversioning.mdx:17still defines patch as introducing "no new capabilities" and the page elsewhere promises patch upgrades are "always safe."docs-expertflagged both as HIGH internal-consistency breaks. A consumer reading top-down gets "patch = always safe" and then "patch can ship breaking experimental changes" with no reconciling clause. Ratify the policy change and reconcile lines 17/186/192 before shipping it as the rule that justifies this downgrade. -
core/registry-schema carve-out overreaches.scripts/check-changeset-protocol-scope.cjsmovesstatic/schemas/source/core/registry-(event|feed-response).jsonout of protocol scope by directory pattern. Those are wire-shape JSON Schemas understatic/schemas/source/—versioning.mdx:150defines exactly that as "what an agent implements on the wire." Exempting them means a future breaking change toregistry-feed-response.jsonships with no changeset gate. The registry REST surface having an independentversion: 1.0.0(static/openapi/registry.yaml) justifies exempting the OpenAPI doc and policy files — it does not justify exempting core wire schemas. Narrow this to the REST surface, or justify per-schema that no SDK validates those payloads.
Follow-ups (non-blocking)
- Duplicate changeset.
.changeset/tmp-tmpx-macros.md(alreadypatch) describes the identical reshape and carries the actual schema adds (tmpx_macros,tmpx_providersreshape). Two changesets narrate one reshape. Resolve the duplicate rather than reclassifying around it — the 3.1.1 math rests on which one counts. - Decision table is missing an experimental row.
versioning.mdx:156-164enumerates conformance tiers but has no "experimental-only schema change → Patch" row. That table is where an implementer decides whether to upgrade; the rule should live there, not only in prose. - Section heading drift.
versioning.mdx:148still reads "Spec changes vs. conformance-suite changes" but the section now describes four surfaces (spec / registry / conformance / experimental).docs-expertHIGH — broaden the heading and the closer at line 166. - Downgrade-edit + registry-file interaction.
code-reviewerMedium: a PR that downgrades an existing changeset and touches a now-non-protocol registry file fails edit-only maintenance and emits a false violation. Fails closed, but it's a usability trap given this PR's premise.
Minor nits (non-blocking)
- PR title.
.agents/playbook.md:353wants conventional-commits. "Fix release bump classification" →fix(release): correct pending changeset bump classification.
What flips this to approve: either keep the tmpx_providers reshape at minor to match the #2774 precedent, or land the experimental-is-patch policy as a ratified change with versioning.mdx lines 17/186/192 reconciled so "patch is always safe" survives — and narrow the core/ registry carve-out off wire-shape schemas. The gate code itself is solid; confirm node tests/changeset-protocol-scope.test.cjs is green in CI.
Requesting changes.
80bb2f6 to
5b458dc
Compare
There was a problem hiding this comment.
Correct release-hygiene fix: drops the pending release from 3.2.0 to 3.1.1 by removing two unjustified minor bumps, and codifies the policy that justifies it. Right principle — registry transport and experimental surfaces are versioned on their own clocks, and the stable adcontextprotocol package should not inherit their motion.
Things I checked
- The math lands at 3.1.1. On
mainthe only twominorbumps areregistry-feed-stream.mdandtmp-tmpx-providers.md; both pulled the release to 3.2.0. This PR strips the first and deletes the second, leavingpatchas the ceiling across the remaining changesets. PR validation confirmschangeset statusreports 3.1.0 → 3.1.1. - The deleted tmpx changeset is not unversioned.
tmp-tmpx-providers.md(minor, the#5689v1Map<id,string>shape) is superseded inside the same release by the survivingtmp-tmpx-macros.md(patch), which reshapes the same experimental surface and carries the migration note plus the legacy-tmpxtransitional alias.ad-tech-protocol-expertconfirms the reshape lives on ax-status: experimentalschema (trusted-match/identity-match-response.json) whose contract explicitly sanctions type and required↔optional breaks within 3.x — patch classification is consistent. - De-scoping
static/registry/does not weaken the gate. The protocol schemas that define registry shapes (static/schemas/source/core/registry-feed-response.json) still match^static/schemas/source/and remain protocol-scoped — test attests/changeset-protocol-scope.test.cjsasserts that path returnstrue. Only the independently-versioned data artifacts understatic/registry/and the registry OpenAPI/docs move out of scope. - The downgrade exemption can't smuggle anything.
isChangesetEditOnlyMaintenance(scripts/check-changeset-protocol-scope.cjs:99) returns false for anyAchangeset, any non-M/Dstatus (renamesRexcluded), and any path outside.changeset/*plus the three policy-code files.isChangesetClassificationMaintenanceblocks escalations and prose-only edits of protocol changesets, and requires at least one genuine downgrade/removal to fire. No app/schema change rides along. - Deleted changesets don't trigger a missing-HEAD read.
if (change.status === 'D') continue;skips before any read; thereadFilesStricttest confirms the deleted path is never read. readFileAtBasedefault fails closed. The() => ''default ranks as no-bump, which can only make a real downgrade look like an escalation (blocked) — never the reverse.run()always passes the real base reader.- Schema edit is cosmetic.
get-adcp-capabilities-response.json:216— singledescriptionstring,—→-. Notype/enum/required/default/property-name touched. Satisfiesnormalize-schema-utf8.mjs --check. Zero wire impact. - versioning.mdx is consistent. The rewrite keeps the 6-week experimental notice window and the frozen stable-surface guarantees; it assigns a release tier to movement the experimental contract already permits.
Follow-ups (non-blocking — file as issues)
- Abbreviated-notice precedent.
tmp-tmpx-macros.mdadmits the literal 6-week experimental notice window "can't apply" becausetmpx_providersshipped days ago, and waives the dual-shape alias. Defensible under the contract's "where feasible" softeners, but worth a one-line WG sign-off so this isn't later cited to rush other experimental reshapes. - Cosmetic schema fix has no own changeset. The em-dash normalization touches
static/schemas/source/**but adds no.changeset. The scope gate passes it (protocol-scoped files present → no violation) and the change is wire-neutral, so it lands cleanly in the existing 3.1.1 patch release — but if release notes want to attribute the lint fix, a one-line patch changeset would carry it.
Minor nits (non-blocking)
- Changeset prose path drift.
tmp-tmpx-macros.mdstill referencestmp/identity-match-response.json/tmp/provider-registration.json; the files were renamed tostatic/schemas/source/trusted-match/by the separatetrusted-match-schema-namespace.mdpatch. Cosmetic, not a defect in this PR. - Redundant guard.
findChangesetProtocolScopeViolationschecksprotocolScopedFiles.length > 0in both the new exemption guard and the following early-return. Functionally correct, collapsible for readability.
LGTM. Follow-ups noted below.
Summary
Corrects pending changeset release classification so the next release resolves to
3.1.1, not3.2.0.Changes
adcontextprotocol; registry changes do not drive ADCP package versioning.tmpx_providerschangeset is removed; the survivingtmp-tmpx-macrospatch changeset carries the experimental notice/migration details.adcontextprotocolbumps without protocol-source changes.adcontextprotocol.get-adcp-capabilities-response.json.Validation
rg -n '"adcontextprotocol": minor' .changeset || truenode tests/changeset-protocol-scope.test.cjsnode scripts/check-changeset-protocol-scope.cjs origin/mainnpx changeset status --output .context/changeset-status.jsongit diff --checknode scripts/normalize-schema-utf8.mjs --checknpm run test:unitnpm run test:server-unitnpm run test:openapinpm run typecheckChangesets now reports
adcontextprotocol3.1.0->3.1.1.