Skip to content

feat: version deprecation and sunset headers - #2224

Merged
epipav merged 10 commits into
mainfrom
feat/IN-1139-deprecation-sunset-headers
Sep 21, 2026
Merged

epipav merged 10 commits into
mainfrom
feat/IN-1139-deprecation-sunset-headers

Conversation

@epipav

@epipav epipav commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Extends the version registry (api/src/versions/registry.ts) so an ApiVersion can carry optional lifecycle metadata: deprecatedAt, sunsetAt, successorPrefix, deprecationDocsUrl. The shipped /v1 entry carries none, so production behavior is unchanged.
  • New api/src/versions/lifecycle.ts validates the metadata at build time (unparseable dates and a sunset earlier than the deprecation reject buildApp) and precomputes the header values once. A scoped onSend hook stamps every response of a deprecated version — success and error replies alike, the version's openapi.json route included — with:
    • Deprecation: @<unix-timestamp> (RFC 9745)
    • Sunset: <IMF-fixdate> (RFC 8594), when a sunset date is planned
    • Link with rel="successor-version" and/or rel="deprecation", when configured
  • The registry loop in app.ts now registers each version inside its own scope; the per-version openapi.json route moved inside that scope (public URL unchanged) so it inherits version-scoped behavior.
  • Docs: new "Deprecation signals" section in docs/site/lifecycle.md for callers; ADR 0003's deprecation-process step updated to the RFC 9745/RFC 8594 header formats with an amendment note (the ADR predates RFC 9745).

Groundwork from IN-1137's registry typing; the promotion-time 410 Gone flow stays with the version-bumping playbook (IN-1140).

Test plan

  • cd api && pnpm test — 85/85 green, including new tests/version-lifecycle.test.ts (18 tests: header formats, error responses, build-time validation, live versions staying unstamped)
  • pnpm tsc-check and pnpm lint clean

Jira: IN-1139

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 17, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Date validation accepts invalid calendar dates, lifecycle headers can overwrite existing links, and architecture documentation remains inconsistent.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds API-version lifecycle metadata and RFC-compliant deprecation signals while leaving /v1 behavior unchanged.

Changes:

  • Adds version-scoped lifecycle headers and validation.
  • Scopes routes and OpenAPI documents per API version.
  • Adds lifecycle tests and documentation.
File summaries
File Description
api/src/app.ts Registers each version in an isolated scope.
api/src/versions/registry.ts Defines lifecycle metadata.
api/src/versions/lifecycle.ts Generates lifecycle headers.
api/tests/version-lifecycle.test.ts Tests lifecycle behavior and validation.
api/docs/site/lifecycle.md Documents deprecation signals.
api/docs/arch/adr/0003-tolerant-reader-versioning.md Updates lifecycle header formats.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread api/src/versions/lifecycle.ts
Comment thread api/src/versions/lifecycle.ts
Comment thread api/docs/arch/adr/0003-tolerant-reader-versioning.md Outdated
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 17, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Date validation currently accepts timestamps and impossible calendar dates despite requiring strict YYYY-MM-DD values.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

api/src/versions/lifecycle.ts:9

  • Date.parse accepts full timestamps and normalizes impossible dates (2026-02-30 becomes March 2), so invalid registry metadata can emit headers for a different instant. Enforce exact YYYY-MM-DD syntax and calendar validity.
  const ms = Date.parse(value);
  if (Number.isNaN(ms)) {
    throw new Error(`lifecycle ${field} must be a valid YYYY-MM-DD date: "${value}"`);
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/src/versions/lifecycle.ts
@epipav

epipav commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 17, 2026 21:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation is scoped, standards-compliant, and comprehensively tested; the remaining comment is non-blocking.

Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread api/tests/version-lifecycle.test.ts
…ion-sunset-headers

Signed-off-by: anilb <epipav@gmail.com>

# Conflicts:
#	api/docs/arch/adr/README.md
Copilot AI review requested due to automatic review settings September 17, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation is scoped, standards-aligned, and comprehensively tested with no unresolved issues found.

Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@epipav epipav self-assigned this Sep 18, 2026

@gaspergrom gaspergrom left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified /v1 stays fully unstamped, the openapi.json route keeps its exact pre-PR URL through the scope move, and the Sunset header is genuine IMF-fixdate rather than an ISO date. Build-time validation runs before buildApp() resolves, confirmed by the reject-on-bad-config tests. Nice work here, approving.

@themarolt themarolt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - one question on the 404 handler, not blocking

Comment thread api/src/versions/lifecycle.ts Outdated
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 21, 2026 08:34
Signed-off-by: anilb <epipav@gmail.com>
@epipav
epipav requested a review from themarolt September 21, 2026 08:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

It includes unrelated stacked-PR content and an incorrect ADR chronology.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Correct the ADR-0003 and RFC 9745 chronology

api/​docs/​arch/​adr/​0021-rfc-deprecation-sunset-header-formats.md:8

ADR-0003 was added on 2026-09-08, but RFC 9745 was published in March 2025, so “after ADR-0003 was written” reverses the chronology. Remove that claim and describe the older syntax without the incorrect timeline.

Comment thread api/docs/arch/version-bump-playbook.md
Copilot AI review requested due to automatic review settings September 21, 2026 08:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation is scoped, standards-aligned, documented, and comprehensively tested without changing current /v1 behavior.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Copilot AI review requested due to automatic review settings September 21, 2026 08:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The ADR chronology is incorrect, and the unresolved stacked-playbook change expands the PR beyond its stated scope.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)

Comment thread api/docs/arch/adr/0021-rfc-deprecation-sunset-header-formats.md Outdated
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 21, 2026 08:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The public documentation currently presents optional Sunset and Link headers as unconditional.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Low severity Document Sunset and Link headers as optional

api/​docs/​site/​lifecycle.md:59

sunsetAt, successorPrefix, and deprecationDocsUrl are optional, so deprecated versions may emit only Deprecation. Qualify Sunset and Link as optional; otherwise callers are told to expect headers that the implementation deliberately omits.

@themarolt themarolt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - the shared 404 handler answers the question I left last time. one thing on it also taking over the root 404

Comment thread api/src/app.ts

// Deprecated version scopes register this same handler, so the two 404 bodies can't drift
// (see src/errors/not-found.ts).
app.setNotFoundHandler(notFoundHandler);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this takes over the root 404 for the whole app, not just the deprecated scopes - /v1/does-not-exist and /docsomething go through it now too. the body is byte-identical to fastify's default, but I think the default calls .type('application/json') while this one falls through to fastify's object default, application/json; charset=utf-8, so the content-type shifts on every 404 in the API. tests/docs-static-serving.test.ts:76 is also still named "returns Fastify's own default 404" and only matches /application\/json/, so it wouldn't catch that. is owning the root 404 the intent here, or would keeping it in the version scopes be enough?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, owning root is the intent, it keeps root and the deprecated scopes on one body. fastify 5.12's basic404 never calls .type(), so both already send application/json; charset=utf-8 -- 461c9bab renames the stale test and pins that exact value

Signed-off-by: anilb <epipav@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The successor-version link currently targets a version prefix instead of the corresponding successor resource.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Build successor-version links from the request URL

api/​src/​versions/​lifecycle.ts:40

successor-version identifies a successor of the current resource, but this sends the same bare prefix for every endpoint (for example, /v1/projects/x?cursor=y points to /v2, which may itself 404). Build this relation from the request URL by replacing the current prefix and preserving the path/query; the promotion example in version-bump-playbook.md:173-174 already uses that pattern. Update the corresponding tests and lifecycle docs as well.

@epipav

epipav commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

re copilot's successor-version note on lifecycle.ts:40: the prefix target is on purpose, v2 can rename routes or change cursor encoding, so rewriting /v1/x?cursor=y to /v2/x?cursor=y could point at nothing. The request.url rewrite only fits alpha promotion, where the path stays the same

@themarolt themarolt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - one question on the promotion section pointing at files that aren't in the repo

Comment thread api/docs/arch/version-bump-playbook.md Outdated
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 21, 2026 09:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation, validation, scoped routing, tests, and documentation consistently satisfy the stated lifecycle-header requirements.

Review effort: Balanced
Findings: None

@epipav
epipav merged commit 863ac3f into main Sep 21, 2026
11 checks passed
@epipav
epipav deleted the feat/IN-1139-deprecation-sunset-headers branch September 21, 2026 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants