docs(releasing): say what actually counts as a breaking change - #1264
Merged
Conversation
Both enums are designed to grow — `Capability` gains an entry whenever the agent gains a power worth gating separately, and `AuditEvent`'s own doc comment says variants arrive alongside the features that emit them. Neither was `#[non_exhaustive]`, so every one of those additions was a breaking change for anyone matching on them. That is not hypothetical. `MemoryRead`, `MemoryWrite`, `RoomPresent`, `RoomOpen` and `AuditEvent::RoomOperation` went out in vti-common 0.16.2 — a PATCH release — so a downstream exhaustive `match` stopped compiling on a routine `cargo update`, with the caret requirement picking it up automatically. The cost inside this workspace is zero: nothing here matches exhaustively on either type. Every reference constructs a variant as a value, and the only `match self` sits in `vti-common` itself, where the attribute has no effect. Checked before writing it, not after. Downstream code now needs a `_ =>` arm, and that is the point rather than the price. A capability a consumer has never heard of is precisely the one it must not silently treat as granted, and an audit event it cannot name still has to be recorded; a wildcard arm forces both decisions to be written down instead of being decided by a compile error at the wrong moment. Deliberately NOT applied to the sixteen `vta-sdk` wire structs that broke the same way when they gained `ext`. `#[non_exhaustive]` on a struct removes literal construction from outside the crate entirely — functional update with `..Default::default()` included, which is the part people assume still works — so all sixteen would need constructors or builders. That is a redesign of the public SDK surface, not cleanup, and the safety half is now covered anyway: a new field forces a breaking bump and #1256's guard makes that stick. Worth doing deliberately, in its own change. Breaking for external consumers, so this needs the minor slot (0.17.0) rather than a patch. The guard added in #1256 will now say so if the release proposes otherwise — which makes this its first live exercise of the failure path. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
Both RELEASING.md and CLAUDE.md tell an author to put `!` on "a breaking change" and then leave the term undefined, as though it were self-evident. It is not. The cases that get missed are the ones that add rather than remove, and they do not feel like breaks while you are writing them. Five went out unmarked here between 2026-08-29 and 2026-09-06: #1234 Capability::{MemoryRead, MemoryWrite} #1247 Capability::RoomPresent #1250 Capability::RoomOpen #1244 AuditEvent::RoomOperation #1231 sixteen vta-sdk wire structs gained `ext` — typed `fix:`, which derives the smallest bump there is Neither enum was `#[non_exhaustive]`, so each variant broke every downstream exhaustive `match`; the struct fields broke every literal. release-plz reads the bump off the type and the `!`, so all of it shipped as patches: vti-common 0.16.2 and vta-sdk 0.32.4, which a caret requirement picks up on a routine `cargo update`. Three of those five are mine, which is the reason to write this down rather than treat it as something careless people do. So RELEASING.md now lists the additive cases explicitly, and says the better answer is usually not the marker at all: a type designed to grow should be `#[non_exhaustive]` once (#1262), rather than depending on every future author remembering. CLAUDE.md gets the short form beside the existing title rule. Both point at the mechanical check, because neither note removes the need for one: `release bump is large enough` compares the versions a Release PR actually proposes against cargo-semver-checks and blocks when the bump is too small. It does not depend on anyone noticing. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
|
🛡️ AI Agentic Security Code Review — all clear. We checked this change and found nothing to report. Keep shipping secure code! Note: for major, breaking, or feature-introducing changes, you can always request an in-depth review from the security team. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RELEASING.mdandCLAUDE.mdboth tell an author to put!on "a breakingchange", then leave the term undefined — as though it were self-evident. It
isn't. The cases that get missed are the ones that add rather than remove, and
they don't feel like breaks while you're writing them.
Five went out unmarked here between 2026-08-29 and 2026-09-06:
Capability::{MemoryRead, MemoryWrite}featCapability::RoomPresentfeatCapability::RoomOpenfeatAuditEvent::RoomOperationfeatvta-sdkwire structs gainedextfix— the smallest bump there isNeither enum was
#[non_exhaustive], so each variant broke every downstreamexhaustive
match; the struct fields broke every literal. release-plz derivesthe bump from the type and the
!, so all of it shipped as patches —vti-common0.16.2 andvta-sdk0.32.4 — which a caret requirement picks up ona routine
cargo update.Three of those five are mine. That is the reason to write this down rather
than file it under carelessness: I diagnosed this problem for several hours
without noticing I had caused most of it, because adding a variant genuinely
does not register as an API break at the time.
What changed
RELEASING.mdgains the additive cases explicitly — new enum variant, newstruct field, new required argument, new trait method without a default,
stricter bound — with the note that this list is not hypothetical and where each
one happened.
It also says the better answer is usually not the marker: a type designed to
grow (a capability list, an event vocabulary) should be
#[non_exhaustive]once,as #1262 did, rather than depending on every future author remembering. Reach for
!when the break is real and intended; reach for#[non_exhaustive]when thetype will keep growing.
CLAUDE.mdgets the short form beside the existing title rule, since that iswhere the convention is actually read.
Why this doesn't replace the guard
Both notes point at
release bump is large enough(#1256), which compares theversions a Release PR actually proposes against cargo-semver-checks and blocks
when the bump is too small. Documentation improves the odds an author notices;
the check does not depend on anyone noticing. This PR is the first half, not a
substitute for the second.