timothy/ersatztv
1
0
Fork 0
Code Issues 57 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

#197 C1 [BLOCKER]: no API versioning — mount /api/v1 before the contract freezes #286

New Issue
Closed
opened 2026-07-11 23:21:16 +02:00 by timothy · 3 comments
timothy commented 2026-07-11 23:21:16 +02:00
Owner
Copy Link
Copy Source

From the #197 cold API contract review. Phase 2 contract-freeze gate — must decide/land before declaring v1 frozen (can't be added compatibly after).

Finding (BLOCKER): /api/* is completely unversioned (no path/header/media-type; info.version is cosmetic). The first breaking change silently breaks the SPA and the MCP client with no negotiation path.

Fix (Phase-2 #7): mechanical route sweep /api/...→/api/v1/... + an internal in-pipeline rewrite /api/{nonversioned}→/api/v1/... (rewrite, not redirect — bodies/auth survive; MCP/curl safe) carrying Deprecation: true+Sunset headers; regen v1.json + v1.d.ts + sweep the SPA literal paths; MCP regenerates. Then /api/v1 is additive-only (append to decisions.md). Owner decision needed: sunset window for the legacy rewrite (recommend 2 releases). Full rationale + rejected alternatives (Asp.Versioning, header/media-type) in the #197 roadmap. Refs #197.

Done-when

  • All /api/* routes swept to /api/v1 (controllers, Location headers, scanner callback), uniform across auth/scan/scripted; ApiVersionRewriteMiddleware rewrites legacy /api/*→/api/v1/* in-pipeline with Deprecation/Sunset headers
  • Route standardization enforced by ApiRouteVersioningTests (^/api/v\d+/ over the whole Controllers.Api surface)
  • .NET build + full test suite green; SPA typecheck + vitest + check:api + eslint green
  • OpenAPI artifacts regenerated (v1.json all /api/v1/…, endpoint-index.md, v1.d.ts)
  • docs updated same-PR (decisions.md versioning entry, api-conventions.md §1/§9, route/parity docs)
  • live-E2E: SPA drives /api/v1/* end-to-end AND a legacy /api/* call is rewritten (200 + Deprecation header)
  • adversarial review passed (cold fork + Codex + fix-commit re-review → Review-verdict: MERGEABLE @ 682dceec)
  • CI green on the PR head sha (682dceec)
From the #197 cold API contract review. **Phase 2 contract-freeze gate — must decide/land before declaring v1 frozen (can't be added compatibly after).** **Finding (BLOCKER):** `/api/*` is completely unversioned (no path/header/media-type; `info.version` is cosmetic). The first breaking change silently breaks the SPA and the MCP client with no negotiation path. **Fix (Phase-2 #7):** mechanical route sweep `/api/...`→`/api/v1/...` + an internal in-pipeline rewrite `/api/{nonversioned}`→`/api/v1/...` (rewrite, **not** redirect — bodies/auth survive; MCP/curl safe) carrying `Deprecation: true`+`Sunset` headers; regen `v1.json` + `v1.d.ts` + sweep the SPA literal paths; MCP regenerates. Then `/api/v1` is **additive-only** (append to `decisions.md`). **Owner decision needed:** sunset window for the legacy rewrite (recommend 2 releases). Full rationale + rejected alternatives (Asp.Versioning, header/media-type) in the #197 roadmap. Refs #197. ## Done-when - [x] All `/api/*` routes swept to `/api/v1` (controllers, Location headers, scanner callback), uniform across auth/scan/scripted; `ApiVersionRewriteMiddleware` rewrites legacy `/api/*`→`/api/v1/*` in-pipeline with `Deprecation`/`Sunset` headers - [x] Route standardization enforced by `ApiRouteVersioningTests` (`^/api/v\d+/` over the whole `Controllers.Api` surface) - [x] `.NET` build + full test suite green; SPA typecheck + vitest + check:api + eslint green - [x] OpenAPI artifacts regenerated (`v1.json` all `/api/v1/…`, `endpoint-index.md`, `v1.d.ts`) - [x] docs updated same-PR (`decisions.md` versioning entry, `api-conventions.md` §1/§9, route/parity docs) - [x] live-E2E: SPA drives `/api/v1/*` end-to-end AND a legacy `/api/*` call is rewritten (200 + `Deprecation` header) - [x] adversarial review passed (cold fork + Codex + fix-commit re-review → `Review-verdict: MERGEABLE @ 682dceec`) - [x] CI green on the PR head sha (`682dceec`)
timothy added the apipriority: highreview labels 2026-07-11 23:21:16 +02:00
timothy commented 2026-07-11 23:29:01 +02:00
Author
Owner
Copy Link
Copy Source

Bundling — own session, land LAST. The /api/v1 route sweep rewrites every controller route and regenerates all artifacts, so it conflicts with controller-attribute changes (Bundle A: #280/#281/#282/#284/#285) and spec changes (Bundle C: #287/#288). Sequence it after Bundle A + Bundle C merge, then regenerate once. Phase-2 freeze gate. See #197 roadmap.

**Bundling — own session, land LAST.** The `/api/v1` route sweep rewrites **every** controller route and regenerates all artifacts, so it conflicts with controller-attribute changes (Bundle A: #280/#281/#282/#284/#285) and spec changes (Bundle C: #287/#288). Sequence it **after** Bundle A + Bundle C merge, then regenerate once. Phase-2 freeze gate. See #197 roadmap.
timothy added the in-progress label 2026-07-12 23:46:57 +02:00
timothy commented 2026-07-12 23:47:18 +02:00
Author
Owner
Copy Link
Copy Source

claiming — session started 2026-07-12 (night). This is the #197 frontier BLOCKER; PR #76 (MCP) just closed as superseded so the fresh MCP builds against the frozen /api/v1 this lands.

Owner decision (sunset window): going with the issue's recommendation — 2 releases for the legacy /api/* → /api/v1/* in-pipeline rewrite before the compat shim is removed (emit Deprecation: true + Sunset headers meanwhile). Will confirm approach details after reading the #197 roadmap.

claiming — session started 2026-07-12 (night). This is the #197 frontier BLOCKER; PR #76 (MCP) just closed as superseded so the fresh MCP builds against the frozen `/api/v1` this lands. **Owner decision (sunset window)**: going with the issue's recommendation — **2 releases** for the legacy `/api/*` → `/api/v1/*` in-pipeline rewrite before the compat shim is removed (emit `Deprecation: true` + `Sunset` headers meanwhile). Will confirm approach details after reading the #197 roadmap.
timothy commented 2026-07-13 00:53:15 +02:00
Author
Owner
Copy Link
Copy Source

Closed — merged to main (PR #326, 682dceec, fast-forward)

What was done. Versioned the entire REST API to /api/v1. The whole /api surface — machine API, browser-session auth (/api/v1/auth/*), scanner loopback (/api/v1/scan/*), scripted-build (/api/v1/scripted/*) — now lives under /api/v1, uniformly, so there's no unversioned corner. Legacy unversioned /api/* callers are handled by ApiVersionRewriteMiddleware, an in-pipeline rewrite (not redirect) that preserves method/body/auth/query and advertises Deprecation: true + a Link: rel="deprecation" (+ Sunset when Api:LegacyRoutesSunset is set).

Root cause. n/a — not a bug; a pre-ossification contract gate. /api was unversioned from the original REST build, which was acceptable while the SPA + server shipped lockstep, but becomes a one-way door once external/MCP consumers depend on the contract.

Owner decisions taken. Sunset window = 2 releases (the compat shim removal is a tracked Phase-3 follow-up, not this PR). Uniform versioning (incl. internal/auth surfaces) rather than a carve-out, so the rewrite needs no exclusion list. Route-convention standardization requested by the owner mid-session: leading-slash absolute route per method (class-[Route] retained only for the two shared-{id}-prefix controllers).

Files changed (218). 251 controller route attributes + ~24 Location headers + CallLibraryScannerHandler.GetBaseUrl + the Startup request-log literal; new ErsatzTV/Middleware/ApiVersionRewriteMiddleware.cs + registration; new ErsatzTV.Tests/Controllers/ApiRouteVersioningTests.cs (enforces ^/api/v\d+/ + a KnownNonApiRoutes allowlist); regenerated v1.json (160 paths) / endpoint-index.md / v1.d.ts; 945 SPA request literals + test mocks; docs (decisions.md, api-conventions.md §1/§9, rest-api/spa-conventions/blazor-route-parity/e2e-local/domain-model).

Review. Cold fork (MERGEABLE-WITH-NITS) + independent Codex (BLOCKED → 2 findings: test silent-skip of non-/api routes; deprecation Link under ETV_BASE_URL) → both fixed in 682dceec → fix-commit re-review MERGEABLE. Live-E2E confirmed versioned routes, the legacy rewrite header, /api/v2 passthrough, no auth bypass, and SPA boot with 0 console errors.

/api/v1 is now frozen: additive-only. Renames/removals/retypes require /api/v2. The fresh read+write MCP (#58) builds against this frozen surface.

Deferred / follow-ups. Phase-3: sunset + remove the legacy-rewrite shim after 2 releases. No new issues filed. Cosmetic review nits deliberately not filed (the Link rel is a draft relation; harmless).

Docs updated: all in-PR (see files above).

## Closed — merged to main (PR #326, `682dceec`, fast-forward) **What was done.** Versioned the entire REST API to `/api/v1`. The whole `/api` surface — machine API, browser-session auth (`/api/v1/auth/*`), scanner loopback (`/api/v1/scan/*`), scripted-build (`/api/v1/scripted/*`) — now lives under `/api/v1`, uniformly, so there's no unversioned corner. Legacy unversioned `/api/*` callers are handled by `ApiVersionRewriteMiddleware`, an in-pipeline **rewrite** (not redirect) that preserves method/body/auth/query and advertises `Deprecation: true` + a `Link: rel="deprecation"` (+ `Sunset` when `Api:LegacyRoutesSunset` is set). **Root cause.** n/a — not a bug; a pre-ossification contract gate. `/api` was unversioned from the original REST build, which was acceptable while the SPA + server shipped lockstep, but becomes a one-way door once external/MCP consumers depend on the contract. **Owner decisions taken.** Sunset window = **2 releases** (the compat shim removal is a tracked Phase-3 follow-up, not this PR). Uniform versioning (incl. internal/auth surfaces) rather than a carve-out, so the rewrite needs no exclusion list. Route-convention standardization requested by the owner mid-session: leading-slash absolute route per method (class-`[Route]` retained only for the two shared-`{id}`-prefix controllers). **Files changed (218).** 251 controller route attributes + ~24 `Location` headers + `CallLibraryScannerHandler.GetBaseUrl` + the `Startup` request-log literal; new `ErsatzTV/Middleware/ApiVersionRewriteMiddleware.cs` + registration; new `ErsatzTV.Tests/Controllers/ApiRouteVersioningTests.cs` (enforces `^/api/v\d+/` + a `KnownNonApiRoutes` allowlist); regenerated `v1.json` (160 paths) / `endpoint-index.md` / `v1.d.ts`; 945 SPA request literals + test mocks; docs (`decisions.md`, `api-conventions.md` §1/§9, `rest-api`/`spa-conventions`/`blazor-route-parity`/`e2e-local`/`domain-model`). **Review.** Cold fork (MERGEABLE-WITH-NITS) + independent Codex (BLOCKED → 2 findings: test silent-skip of non-`/api` routes; deprecation `Link` under `ETV_BASE_URL`) → both fixed in `682dceec` → fix-commit re-review MERGEABLE. Live-E2E confirmed versioned routes, the legacy rewrite header, `/api/v2` passthrough, no auth bypass, and SPA boot with 0 console errors. **`/api/v1` is now frozen: additive-only.** Renames/removals/retypes require `/api/v2`. The fresh read+write MCP (#58) builds against this frozen surface. **Deferred / follow-ups.** Phase-3: sunset + remove the legacy-rewrite shim after 2 releases. No new issues filed. Cosmetic review nits deliberately not filed (the `Link` `rel` is a draft relation; harmless). **Docs updated:** all in-PR (see files above).
timothy removed the in-progress label 2026-07-13 00:53:18 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels api priority: high review
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#286