chore(api): remove 4.13 grace-period shims — v1 root paths + /api/upgrade 410s (closes #4117)

[Yeraze]

Summary

Removes the two deprecation shims that shipped in 4.13 with a documented one-release grace period. Docs and the announcement blog promised both would be removed in 4.14; this does that cleanly.

Part 1 — v1 root-path API shim (removed)

• Deleted src/server/routes/v1/deprecatedShim.ts (the deprecationShim middleware that stamped the Warning: 299 header) and its test.
• Removed the legacy root mounts in v1/index.ts (/api/v1/{nodes,messages,channels,telemetry,traceroutes,packets,network,status} + root position-history). These now 404.
• Simplified the getScopedSourceId helpers across the sub-routers (and the inline resolver in status.ts) to read the :sourceId path param only — the ?sourceId= / body fallbacks existed solely to serve the removed root mounts.
• Kept the canonical /api/v1/sources/{sourceId}/... mounts, attachSource, the default alias, and the global /api/v1/solar + /api/v1/channel-database endpoints.
• openapi.yaml: dropped the deprecated /nodes root path block and all Warning: 299 framing.

Part 2 — /api/upgrade/* 410 FEATURE_RETIRED endpoints (removed)

• Deleted src/server/routes/upgradeRoutes.ts (the 410 stubs from feat!: retire in-app auto-upgrade, replace with update notifications #4108) and its server.ts mount. These paths now 404 like any unknown route (Auto-Upgrade Retirement Plan, Phase 3).

Docs

• REST_API.md, API_REFERENCE.md, development/api-reference.md: warning banners rewritten to past tense ("removed in 4.14 — now return 404"); migration table retained for reference.
• Historical blog post left as-is (it documents what 4.13 did). docs/api/API.md already marked outdated (no change). FAQ auto-upgrade stubs left (they reference the retirement, not the 410 endpoints).
• Docs build (npm run docs:build, vitepress) is green.

Tests

• Converted the legacy-shape tests (v1-api.test.ts, messages.search.test.ts, messages.permissions.test.ts) to the per-source shape.
• Added assertions: GET /api/v1/nodes (root) → 404, GET /api/v1/messages (root) → 404, canonical /api/v1/sources/default/nodes still works; /api/upgrade/* → 404 (not 410).

Acceptance criteria

• GET /api/v1/nodes (root shape) → 404; GET /api/v1/sources/default/nodes still works.
• deprecatedShim.ts deleted; no Warning: 299 emitted anywhere (no code sets a Warning header; remaining mentions are past-tense docs/comments + the historical blog).
• /api/upgrade/trigger → 404 (not 410).
• Docs updated to past tense; docs build green.
• Server typecheck clean; npm run lint:ci exits 0.
• Full Vitest suite: 9525 passed; the only remaining failures are pre-existing/environmental in this worktree (missing ts-overlapping-marker-spiderfier-leaflet dep + the [BUG] MeshCore: Virtual Node functionality still does not work correctly #4094 fork-dependent meshcoreCompanionCodec test) — all confirmed to fail identically on the clean baseline, and they pass in CI. My changes introduce zero new failures.

Note on the default alias

While converting tests I found a pre-existing latent issue (not introduced here, out of scope): attachSource normalizes req.params.sourceId to the resolved source id, but Express re-derives req.params for the mergeParams sub-router, so handlers reading req.params.sourceId (the codebase convention, incl. actions.ts) see the raw URL literal "default" rather than the resolved id. Route resolution for default still works (returns 200); deeper DB scoping by the literal "default" is the latent bug. Kept helpers param-only per the issue's guidance; used concrete source ids where a test asserts real scoping. Flagging for a follow-up.

⚠️ MERGE TIMING

This breaks the documented grace period if it ships in a 4.13.x release. It should only be merged once 4.14 is the active target. Do not merge before then.

🤖 Generated with Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: chore(api): remove 4.13 grace-period shims

Tasks

• Gather context and understand the PR
• Read changed files
• Review code quality, bugs, security, test coverage
• Post final review

---

Overall Assessment

This is a clean, well-scoped removal PR. The diff is net-negative (−211 lines), the intent is clear, and the approach is sound. A few observations below, ranging from blocking to informational.

---

Issues Worth Addressing

1. Duplicate test in v1-api.test.ts (minor bug)

v1-api.test.ts:1394–1405 has an exact duplicate of the ?before=2500 position-history test at lines 1381–1393. Both describe blocks have the same title 'should filter positions by before parameter' and identical bodies. One of them should be removed or renamed (e.g. to test an after or since parameter).

2. void getScopedSourceId(req) in network.ts:65 is a smell

network.ts:65:

void getScopedSourceId(req);

The call is discarded to silence a lint warning, with a TODO referencing getDirectNeighborStatsAsync not yet accepting sourceId. This pattern is fine as a placeholder, but it means the /direct-neighbors endpoint silently ignores the :sourceId path param — a caller with a scoped token expecting source isolation will get cross-source data without any indication. Consider returning a 501 Not Implemented or a response note until the repo is extended, rather than silently ignoring the param.

3. nodes.ts single-node lookup fetches all nodes (pre-existing, worth flagging)

nodes.ts:143–144:

const sourceNodes = (await databaseService.nodes.getAllNodes(sourceId ?? ALL_SOURCES)) as unknown as DbNode[];
const node = sourceNodes.find(n => n.nodeId === nodeId);

Fetching all nodes and filtering in-process is inefficient when only one node is needed. This is pre-existing code, not introduced here, but since this file was touched it's a good opportunity to note it. If there's a getNode(nodeId, sourceId) repo method, it should be used instead.

---

Positive Observations

Good: getScopedSourceId simplification is correct. Removing the ?sourceId= / body fallbacks that existed solely for the removed root mounts keeps each helper focused. The remaining one-liner return typeof req.params.sourceId === 'string' ? req.params.sourceId : undefined; is uniform across all sub-routers.

Good: Upgrade routes test conversion to createRouteTestApp. upgradeRoutes.test.ts correctly uses the real harness (per CLAUDE.md requirements) instead of the deprecated monkey-patch pattern.

Good: messages.permissions.test.ts now uses real SQL. The conversion from vi.mock('../../services/database.js', ...) to createRouteTestApp with vi.spyOn passthroughs is exactly the pattern described in CLAUDE.md. The source-isolation assertions (sourceA grants → 200, sourceB → count 0) provide meaningful regression coverage.

Good: The default alias latent issue is documented but left out of scope. The note in the PR description about req.params.sourceId being re-derived by Express after attachSource normalizes it is accurate and appropriate to flag for a follow-up rather than blocking this cleanup PR.

Good: Test coverage of the removal. The V1 legacy root paths removed in 4.14 describe block explicitly asserts 404 on /api/v1/nodes, /api/v1/messages, and confirms the canonical /api/v1/sources/default/nodes still returns 200 with no Warning header.

Good: Server typecheck and lint:ci are confirmed green.

---

Security / Permissions

No regressions observed. Permission checks remain in place on all retained routes. The removal of root mounts closes the surface area where a caller could bypass the per-source attachSource middleware by hitting the old shape.

---

Summary

Two items worth fixing before merge:

1. Remove the duplicate position-history test (v1-api.test.ts ~line 1394).
2. Decide whether /direct-neighbors should explicitly signal that sourceId is not yet honored, or leave the TODO as-is with a comment (not silently void-discarding the parsed param).

Neither is a blocker in the strictest sense, but the duplicate test is a clear bug-in-tests and should be cleaned up. The void discard is a pre-existing pattern and acceptable to leave as a follow-up.

Merge timing note is well-called — this should only land once 4.14 is the active target.