You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds the maintainer-facing version-bump playbook at api/docs/arch/version-bump-playbook.md: the step-by-step procedure for introducing /v2 of an endpoint while /v1 keeps serving identical responses. It covers when a bump is required, implementing the sparse v2 plugin, registering the prefix (including the registry-guard test updates that come with it), the v1 stability bar, the deferred deprecation-signaling step per ADR-0021, communication, and a verification checklist.
Docs-only change: the shipped registry and all server code are untouched.
References
JIRA: IN-1140 (closes out the IN-1136 API versioning epic's playbook subtask)
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The lifecycle guidance could deprecate or remove unbumped /v1 endpoints when /v2 is sparse.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a maintainer playbook for introducing sparse API major versions while preserving /v1.
Changes:
Documents version-bump implementation and registry updates.
Covers testing, deprecation, communication, and retirement.
File summaries
File
Description
api/docs/arch/version-bump-playbook.md
Adds the version-bump procedure and verification checklist.
Review details
Suppressed comments (2)
api/docs/arch/version-bump-playbook.md:3
The architecture terminology guide says to describe this stage as /v1 with the full contract and to avoid “stable” (architecture-review/03-context.md:141-144). Use contract-preservation wording here to avoid reintroducing that ambiguity.
How to introduce `/v2` of an endpoint while `/v1` stays stable. The registry, routing,
api/docs/arch/version-bump-playbook.md:90
Only Deprecation is unconditional. Sunset and Link are omitted when their optional lifecycle fields are unset (src/versions/lifecycle.ts:26-46), so describe them as conditional rather than promising all three for every valid lifecycle entry.
Every `/v1` response then carries `Deprecation`, `Sunset`, and `Link` headers in the
[ADR-0021](adr/0021-rfc-deprecation-sunset-header-formats.md) wire formats. Headers
are additive: bodies and status codes stay exactly as they were, and
`tests/version-lifecycle.test.ts` pins that behavior. Invalid dates fail the build at
startup, so a bad `lifecycle` entry cannot reach production silently.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The playbook needs actionable retirement guidance, accurate lifecycle claims, and public /v2 reference coverage.
Review details
Suppressed comments (10)
Previously missed (5) — in code that hasn't changed since the last review.
api/docs/arch/version-bump-playbook.md:78
Lifecycle metadata always emits Deprecation, but Sunset and Link are conditional on their optional fields (api/src/versions/registry.ts:6-12, api/src/versions/lifecycle.ts:26-45). Clarify this generic description so maintainers do not expect all three headers from every lifecycle entry. api/docs/arch/version-bump-playbook.md:111
This suite checks lifecycle headers and response status codes, but it never compares response bodies before and after lifecycle metadata is added. Narrow the claim to what the implementation and tests actually establish, or add a before/after body assertion. api/docs/arch/version-bump-playbook.md:132
The lifecycle hook does not generate 410 responses, and removing /v1 from the registry immediately produces 404. Make the grace-period state actionable: keep /v1 registered with explicit 410 stubs, then remove it after the grace window. api/docs/arch/version-bump-playbook.md:140
The checklist covers shared versioning infrastructure but does not require behavior coverage for each new /v2 endpoint and its changed contract. Add endpoint integration tests to the verification step so a bump cannot pass with routing and schema checks alone. api/docs/arch/version-bump-playbook.md:144
The public Scalar reference is hard-coded to /v1/openapi.json (api/docs/site/.vitepress/theme/ScalarReference.vue:18-20). Add a checklist step to expose /v2; otherwise maintainers can follow this playbook while the public reference still omits every bumped route.
api/docs/arch/version-bump-playbook.md:3
Repository terminology names this stage /v1 and explicitly says to avoid “stable” (api/docs/arch/CONTEXT.md:75-77). Replace both “stays stable” here and “prove v1 stable” at line 64 with contract-preservation wording.
How to introduce `/v2` of an endpoint while `/v1` stays stable. The registry, routing,
api/docs/arch/version-bump-playbook.md:119
The linked lifecycle page defines header semantics but promises no minimum sunset window; ADR-0003:48 explicitly leaves it TBD and only calls six months conventional. This instruction is therefore unverifiable. Define an approved minimum or state that the bump must establish one before deprecation.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
api/docs/arch/version-bump-playbook.md:119
lifecycle.md does not promise a minimum window for whole-version deprecation; it only says the configured Sunset is the earliest removal date. The six-month value is currently marked TBD in ADR-0003, so this instruction is not actionable. Cite that ADR or define the minimum before referring to a promised window.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
api/docs/arch/version-bump-playbook.md:119
Endpoint lifecycle does not promise a minimum /v1 deprecation window: its two-week period applies only to the post-promotion /v1-alpha 410 response (lines 38-44), while ADR-0003 says the stable-version minimum is still TBD (line 48). This instruction therefore gives maintainers no valid duration to follow; either state the intended /v1 policy here or link to the source that defines it.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
api/docs/arch/version-bump-playbook.md:38
These instructions refer to a v1Mapper that does not exist: src/versions/v1/index.ts currently exports only an empty plugin. As written, a maintainer cannot follow the prescribed mapper layout for the first endpoint; phrase this conditionally or describe introducing both version-specific mappers around shared upstream logic.
Keep queries and business logic shared upstream of both version plugins; isolate the
contract difference itself in a version-specific mapper, with the v2 handler importing
a `v2Mapper` next to the `v1Mapper` the v1 handler already uses. Duplicating logic
instead of sharing it upstream is how the two versions drift apart.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Several rollout, lifecycle, retirement, and verification instructions remain incomplete or inaccurate.
Review details
Suppressed comments (14)
Previously missed (8) — in code that hasn't changed since the last review.
api/docs/arch/version-bump-playbook.md:33
The rollout procedure omits required endpoint artifacts. api/docs/arch/PUBLIC_API_PLAN.md:239-241 requires each endpoint to ship with its TypeBox schema, integration test, OpenAPI tag, and documentation; include these so maintainers do not implement only the plugin and mapper. api/docs/arch/version-bump-playbook.md:79
VersionLifecycle always emits Deprecation, but Sunset and Link depend on optional fields (api/src/versions/registry.ts:6-12). This generic wording promises all three headers whenever lifecycle is set; describe the conditional headers explicitly. api/docs/arch/version-bump-playbook.md:87
ADR-0003 also requires each endpoint, parameter, or field scheduled for retirement to have a TypeBox description beginning with DEPRECATED: so OpenAPI consumers see the change. Add this schema step before the registry-wide lifecycle metadata. api/docs/arch/version-bump-playbook.md:112
tests/version-lifecycle.test.ts checks status codes and lifecycle headers, but it does not compare response bodies or pre-existing headers with an unstamped control. Narrow the claim so this suite is not presented as pinning behavior it does not assert. api/docs/arch/version-bump-playbook.md:119
The linked lifecycle page defines a two-week window only for /v1-alpha; it does not define a whole-version /v1 sunset window. Also separate the initial sparse /v2 announcement from the later prefix-wide sunset communication so maintainers do not announce migration before /v2 covers every supported route. api/docs/arch/version-bump-playbook.md:132
Removing /v1 from versionRegistry immediately removes its routes; lifecycle metadata only adds headers and cannot generate 410 responses. Keep the prefix registered with tested temporary 410 handlers during the grace window, then remove the registry entry. api/docs/arch/version-bump-playbook.md:142
The checklist does not include the before/after /v1 response and OpenAPI comparison required by Step 3. Untouched handlers and green shared tests can still miss changes introduced by shared helpers or generated-spec metadata, so require a concrete baseline comparison. api/docs/arch/version-bump-playbook.md:144
The public Scalar reference currently loads only /v1/openapi.json (api/docs/site/.vitepress/theme/ScalarReference.vue:18-20). Reviewing /v2/openapi.json is not enough for callers to discover the bumped contract; require the reference page to expose or link to the /v2 spec.
api/docs/arch/version-bump-playbook.md:14
The additive-only guarantee applies to promoted prefixes such as /v1; the linked page explicitly allows breaking changes under /v1-alpha. Narrow this statement so maintainers do not treat alpha contract changes as major-version bumps.
[Endpoint lifecycle](../site/lifecycle.md) promises callers that a version only ever
changes additively: new fields, new optional params, new endpoints, new error codes,
and expanded accepted enum input values. A change breaks that promise, and therefore
requires a new major version, when it matches the committed Breaking Change definition
in [CONTEXT.md](CONTEXT.md):
api/docs/arch/version-bump-playbook.md:3
The terminology guide lists stable as an avoided synonym for the /v1 stage (api/docs/arch/CONTEXT.md:75-77). Replace it in both the opening sentence and the Step 3 heading with wording about preserving the /v1 contract.
How to introduce `/v2` of an endpoint while `/v1` stays stable. The registry, routing,
api/docs/arch/version-bump-playbook.md:119
The linked lifecycle page defines a two-week grace period only for /v1-alpha; it does not promise a minimum window for retiring /v1. ADR-0003 leaves the whole-version minimum as TBD (with six months conventional), so this instruction does not define an actionable sunset policy.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
api/docs/arch/version-bump-playbook.md:38
src/versions/v1/index.ts is currently empty and no v1Mapper exists, so saying the v1 handler already uses one is not actionable for the first endpoint. Make mapper creation conditional on the two contracts needing different representations.
Keep queries and business logic shared upstream of both version plugins; isolate the
contract difference itself in a version-specific mapper, with the v2 handler importing
a `v2Mapper` next to the `v1Mapper` the v1 handler already uses. Duplicating logic
instead of sharing it upstream is how the two versions drift apart.
api/docs/arch/version-bump-playbook.md:119
lifecycle.md defines only the /v1-alpha two-week grace period; it does not promise a minimum window for deprecating the whole /v1 prefix, and ADR-0003 leaves that value TBD. This instruction therefore gives maintainers no valid duration to follow.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
api/docs/arch/version-bump-playbook.md:119
Neither docs/site/lifecycle.md nor ADR-0003 commits to a minimum notice window for retiring /v1: the lifecycle page only specifies two weeks for /v1-alpha, while ADR-0003 calls six months conventional/TBD. The instruction to give callers the window “promised” by lifecycle is therefore not actionable; define a whole-version minimum or link to the decision that sets it.
- State the sunset window in both; give callers at least the window promised in
[Endpoint lifecycle](../site/lifecycle.md).
Include new success status codes in additive-change rules
api/docs/arch/version-bump-playbook.md:12
The additive-change list omits new success status codes, although PUBLIC_API_PLAN.md:384 explicitly permits them within an existing version. Because this is the decision point for creating a major version, the omission could cause an unnecessary bump.
Document required endpoint rollout artifacts and OpenAPI registration
api/docs/arch/version-bump-playbook.md:33
Step 1 omits required endpoint rollout artifacts. PUBLIC_API_PLAN.md:239-241 requires TypeBox schemas, an integration test, an OpenAPI tag, and documentation. Also, docs/site/.vitepress/theme/ScalarReference.vue:19 currently loads only /v1/openapi.json, so registering /v2 alone would leave the customer-facing reference unaware of the new contract.
Define a concrete version-level tombstone for the grace window
api/docs/arch/version-bump-playbook.md:139
The documented grace window cannot be implemented by removing the registry entry: app.ts mounts /v1 only while that entry exists. The alpha example uses explicit route-level 410 handlers, but there is no equivalent whole-prefix tombstone mechanism. Define and test a version-level tombstone plugin or another concrete way to keep /v1 mounted and return 410 responses during the grace period before presenting retirement as an executable procedure.
Explain when paired version mappers must be introduced
api/docs/arch/version-bump-playbook.md:38
The repository does not yet contain a v1Mapper; src/versions/v1/index.ts is currently an empty plugin. The first version bump cannot assume that mapper already exists, so explain that paired mappers must be introduced when necessary.
The reason will be displayed to describe this comment to others. Learn more.
re-checked the step 4 pins against tests/version-lifecycle.test.ts - both are named correctly now, and the step 5 sunset wording matches ADR-0003. one question on the docs reference embed
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
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.
Summary
Adds the maintainer-facing version-bump playbook at
api/docs/arch/version-bump-playbook.md: the step-by-step procedure for introducing/v2of an endpoint while/v1keeps serving identical responses. It covers when a bump is required, implementing the sparse v2 plugin, registering the prefix (including the registry-guard test updates that come with it), the v1 stability bar, the deferred deprecation-signaling step per ADR-0021, communication, and a verification checklist.Docs-only change: the shipped registry and all server code are untouched.
References
mainonce feat: version deprecation and sunset headers #2224 merges; only the single docs commit belongs to this PR.main