fix(server): JSON 404/405 for unknown /api paths and wrong methods

[dborup]

Relates to #233

Summary

Before this PR, an unknown /api/* path, or a known one called with the wrong method, fell through to the SPA's index.html with 200:

POST /api/nonexistent  -> 200 text/html
GET  /api/nonexistent  -> 200 text/html
POST /api/packets      -> 200 text/html   (#231: once POST /api/packets was removed,
                                            the bare API router answers 405, but the
                                            production wiring answered 200 anyway)

RegisterRoutes (cmd/server/routes.go) now ends with one explicit catch-all:

router.PathPrefix("/api/").HandlerFunc(apiFallbackHandler(router))

registered as the last route inside RegisterRoutes, strictly after every real /api/* route in that function and strictly before main.go later registers /ws and the SPA PathPrefix("/"). gorilla/mux tries routes in registration order; a route whose path matches but whose method doesn't is a "keep trying", not a final 405 — that's why the fall-through happened. The new route intercepts it before it ever reaches the SPA handler.

apiFallbackHandler (new file cmd/server/api_fallback.go) decides 404 vs 405 by re-walking the router and, for the incoming request's literal path, checking every other /api/* route's own declared method(s) via route.Match with the method swapped — i.e. "would this route's path pattern (including {params}) match this URL at all, regardless of method". This reuses mux's own path/regex matching (the same trick buildOpenAPISpec already uses) instead of reimplementing it. Non-empty set → 405 + Allow: <methods>; empty → 404. Both use the existing writeError JSON shape — no new map[string]interface{}.

Because the fallback lives inside RegisterRoutes, every caller gets it for free: main.go and every test's setupTestServer. cmd/server stays read-only; no SQL touched.

Not touched

• /ws is not under /api/, so it's never reached by the new PathPrefix("/api/").
• Health/metrics/mqtt-status/etc. are themselves registered as /api/... routes before the fallback, so they keep matching their own handlers.
• The SPA fallback for everything else (/, static assets, #/... deep links) is registered later in main.go and is unaffected.
• Fork-guards: unchanged (deploy.yml 9, release-fast-path.yml 1).

One pre-existing test updated

TestPostPacketsRemovedFallsThroughToSPAInProductionRouter (#231) pinned the bug on purpose, noting "tracked in #233". It now asserts 405 with an Allow header instead of the SPA page, since fixing #233 is exactly what flips that assertion. The read-only-DB angle (nothing written) is unchanged.

One other existing test, TestPerfMiddlewareSlowQuery (coverage_test.go), registered an extra /api/test-slow route on the router after calling RegisterRoutes. Since the new catch-all is now the last route RegisterRoutes adds, anything registered after it on the same router is shadowed — exactly the behavior the fix is supposed to produce for truly-unmatched paths. Moved that one registration to before the RegisterRoutes call; no other test in the suite does this pattern.

Tests

New cmd/server/api_fallback_test.go:

• TestAPIFallbackUnknownPathReturns404JSON — bare API router.
• TestAPIFallbackUnknownPathInProductionRouterReturns404JSON — full main.go-style composition (RegisterRoutes + /ws + SPA catch-all).
• TestAPIFallbackPostPacketsReturns405NotSPA — locks POST /api/packets specifically in the production-style router, asserting 405 + Allow: GET, not the SPA page.
• TestAPIFallbackKnownPathWrongMethodReturns405 — DELETE /api/stats → 405 + Allow: GET.
• TestAPIFallbackDoesNotShadowExistingRoutes — walks the live /api/spec route list (95 route entries, ≥20 required) and asserts each one's method+path still matches its own registered route template via router.Match, not the fallback's /api/ template.
• TestAPIFallbackDoesNotAffectWebSocketOrSPA — /ws still routes to the hub; a non-/api deep link still gets the SPA page.

Updated TestPostPacketsRemovedFallsThroughToSPAInProductionRouter as above.

Evidence

[T] = automated test, [A] = automated/live-server check via curl/Playwright against a built binary, [K] = known/accepted gap.

#	Acceptance criterion	Evidence
1	Unknown GET /api/... → JSON 404	[T] TestAPIFallbackUnknownPathReturns404JSON, TestAPIFallbackUnknownPathInProductionRouterReturns404JSON. [A] curl below.
2	Known path, wrong method → 405 + Allow; POST /api/packets locked	[T] TestAPIFallbackKnownPathWrongMethodReturns405, TestAPIFallbackPostPacketsReturns405NotSPA, updated TestPostPacketsRemovedFallsThroughToSPAInProductionRouter. [A] curl below.
3	Existing routes/WS/SPA unchanged; checked against served OpenAPI list	[T] TestAPIFallbackDoesNotShadowExistingRoutes (route-template match against all 95 live /api/spec entries), TestAPIFallbackDoesNotAffectWebSocketOrSPA. [A] Playwright E2E 132/135 passed (3 pre-existing skips, unrelated), test-node-liveness-e2e.js (35 checks, live WS) passed.
4	No new map[string]interface{}; cmd/server read-only	[A] reused writeError/writeJSON; no SQL added — rg "INSERT|UPDATE|DELETE|REPLACE" over the new file is empty.
5	Docs describe 404/405	[A] docs/api-spec.md Error Responses section + openapi.go info.description.
6	Fork-guards unchanged	[A] deploy.yml 9, release-fast-path.yml 1 — grep counts match baseline; neither file touched.

Before vs. after

Before (captured as the "red" test output on this branch's master-based starting point, prior to the fix — an httptest run through the exact same router construction main.go uses):

api_fallback_test.go:64: GET /api/this-path-does-not-exist: want 404, got 200 (body "<html>SPA</html>")
api_fallback_test.go:85: POST /api/packets: want 405, got 200 (body "<html>SPA</html>")

After (live curl against a local build, port 13900, migrated copy of test-fixtures/e2e-fixture.db):

$ curl -i GET http://localhost:13900/api/this-path-does-not-exist
HTTP/1.1 404 Not Found
Content-Type: application/json
{"error":"not found"}

$ curl -i -X POST http://localhost:13900/api/packets -d '{}'
HTTP/1.1 405 Method Not Allowed
Allow: GET
Content-Type: application/json
{"error":"method not allowed"}

$ curl -i -X DELETE http://localhost:13900/api/stats
HTTP/1.1 405 Method Not Allowed
Allow: GET
Content-Type: application/json
{"error":"method not allowed"}

$ curl -i GET http://localhost:13900/api/stats
HTTP/1.1 200 OK
Content-Type: application/json
{...}

$ curl -i GET http://localhost:13900/
HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
<!DOCTYPE html>...

$ curl -i -N -H "Connection: Upgrade" -H "Upgrade: websocket" -H "Sec-WebSocket-Version: 13" \
    -H "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==" http://localhost:13900/ws
HTTP/1.1 101 Switching Protocols
Upgrade: websocket

CI (per job)

Run locally in a dedicated worktree + branch (codex/issue-233-api-json-404), not via the GitHub Actions UI from this session:

• cd cmd/server && go build ./..., go vet ./... — clean.
• gofmt -l on every touched/new file (api_fallback.go, api_fallback_test.go, routes.go, openapi.go, post_packets_removed_223_test.go, coverage_test.go) — no output (all formatted).
• go test ./... in cmd/server — all passing, no regressions.
• sh test-all.sh — 219/219 files passed.
• E2E against a local build (migrated copy of test-fixtures/e2e-fixture.db, freshened + seeded exactly as deploy.yml does, port 13900): test-e2e-playwright.js 132/135 passed (3 skips are pre-existing flaky/fixture-shape skips baked into the test itself, unrelated to this change), test-node-liveness-e2e.js 35/35 checks passed (exercises the live WS path).
• Mutation testing (3 required mutants, each reverted after confirming red):
  1. Fallback call removed from RegisterRoutes → 5 of the new/updated tests failed as expected.
  2. Fallback registered first in RegisterRoutes (wrong placement, shadowing every real route) → TestAPIFallbackDoesNotShadowExistingRoutes failed as expected.
  3. The 405 branch changed to write 200 → 4 of the new/updated tests failed as expected.

GitHub Actions CI on this PR has not been inspected from this session (no gh pr checks run after push); will need a look once it's run, including the known-flaky #256 (Hash Stats-sort) — rerun once if it's the only failure.

Rester (leftovers / out of scope)

• [K] A handful of /api/* handlers (e.g. handleRxCoverage, handleObserverDetail for an unknown id) call stdlib http.NotFound directly instead of the JSON writeError, so a disabled-feature or not-found-resource 404 from inside a matched handler is plain-text, not JSON. This is pre-existing, unrelated to the router-level fallback this PR adds (confirmed via a live-server spec sweep), and out of scope for fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 — not changed here.

[dborup]

Rapport — CS-MacBook PR#266 #233 — head 7439f47

Status: All required work done, local verification + CI both green, no open blockers.

Summary

RegisterRoutes (cmd/server/routes.go) now ends with one explicit catch-all:

router.PathPrefix("/api/").HandlerFunc(apiFallbackHandler(router))

registered as the last route inside RegisterRoutes, strictly after every real /api/* route in that function and strictly before main.go later registers /ws and the SPA PathPrefix("/"). gorilla/mux tries routes in registration order; a route whose path matches but whose method doesn't is a "keep trying", not a final 405 — that's why POST /api/packets (and any unknown /api/* path) used to fall through all the way to the SPA catch-all, which answers any method with 200 index.html. The new route intercepts it before it ever reaches the SPA handler.

apiFallbackHandler (new file cmd/server/api_fallback.go) decides 404 vs 405 by re-walking the router and, for the incoming request's literal path, checking every other /api/* route's own declared method(s) via route.Match with the method swapped — i.e. "would this route's path pattern (including {params}) match this URL at all, regardless of method". This reuses mux's own path/regex matching (the same trick buildOpenAPISpec already uses) instead of reimplementing it. Non-empty set → 405 + Allow: <methods>; empty → 404. Both use the existing writeError JSON shape — no new map[string]interface{}.

Because the fallback lives inside RegisterRoutes, every caller gets it for free: main.go and every test's setupTestServer. cmd/server stays read-only; no SQL touched.

Not touched

• /ws is not under /api/, so it's never reached by the new PathPrefix("/api/").
• Health/metrics/mqtt-status/etc. are themselves registered as /api/... routes before the fallback, so they keep matching their own handlers.
• The SPA fallback for everything else (/, static assets, #/... deep links) is registered later in main.go and is unaffected.
• Fork-guards: unchanged (deploy.yml 9, release-fast-path.yml 1).

Two pre-existing tests touched

• TestPostPacketsRemovedFallsThroughToSPAInProductionRouter (refactor(server): remove the dead POST /api/packets endpoint (#223) #231) pinned the bug on purpose, noting "tracked in fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233". It now asserts 405 with an Allow header instead of the SPA page, since fixing fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 is exactly what flips that assertion. The read-only-DB angle (nothing written) is unchanged.
• TestPerfMiddlewareSlowQuery (coverage_test.go) registered an extra /api/test-slow route on the router after calling RegisterRoutes. Since the new catch-all is now the last route RegisterRoutes adds, anything registered after it on the same router is shadowed — exactly the behavior the fix is supposed to produce for truly-unmatched paths. Moved that one registration to before the RegisterRoutes call; no other test in the suite does this pattern.

Tests

New cmd/server/api_fallback_test.go:

• TestAPIFallbackUnknownPathReturns404JSON — bare API router.
• TestAPIFallbackUnknownPathInProductionRouterReturns404JSON — full main.go-style composition (RegisterRoutes + /ws + SPA catch-all).
• TestAPIFallbackPostPacketsReturns405NotSPA — locks POST /api/packets specifically in the production-style router, asserting 405 + Allow: GET, not the SPA page.
• TestAPIFallbackKnownPathWrongMethodReturns405 — DELETE /api/stats → 405 + Allow: GET.
• TestAPIFallbackDoesNotShadowExistingRoutes — walks the live /api/spec route list (95 route entries, ≥20 required) and asserts each one's method+path still matches its own registered route template via router.Match, not the fallback's /api/ template.
• TestAPIFallbackDoesNotAffectWebSocketOrSPA — /ws still routes to the hub; a non-/api deep link still gets the SPA page.

Plus the updated TestPostPacketsRemovedFallsThroughToSPAInProductionRouter above.

Evidence

[T] = automated test, [A] = automated/live-server check via curl/Playwright/CI against a built binary, [K] = known/accepted gap.

#	Acceptance criterion	Evidence
1	Unknown GET /api/... → JSON 404	[T] TestAPIFallbackUnknownPathReturns404JSON, TestAPIFallbackUnknownPathInProductionRouterReturns404JSON. [A] curl below; CI Go Build & Test green.
2	Known path, wrong method → 405 + Allow; POST /api/packets locked	[T] TestAPIFallbackKnownPathWrongMethodReturns405, TestAPIFallbackPostPacketsReturns405NotSPA, updated TestPostPacketsRemovedFallsThroughToSPAInProductionRouter. [A] curl below.
3	Existing routes/WS/SPA unchanged; checked against served OpenAPI list	[T] TestAPIFallbackDoesNotShadowExistingRoutes (route-template match against all 95 live /api/spec entries), TestAPIFallbackDoesNotAffectWebSocketOrSPA. [A] Local Playwright E2E 132/135 passed (3 pre-existing skips, unrelated), test-node-liveness-e2e.js (35 checks, live WS) passed; CI's own Playwright E2E job green.
4	No new map[string]interface{}; cmd/server read-only	[A] Reused writeError/writeJSON; no SQL added — a search for INSERT/UPDATE/DELETE/REPLACE over the new file is empty.
5	Docs describe 404/405	[A] docs/api-spec.md Error Responses section + openapi.go info.description.
6	Fork-guards unchanged	[A] deploy.yml 9, release-fast-path.yml 1 — grep counts match baseline; neither file touched.

Before vs. after

Before (captured as the "red" test output on this branch's master-based starting point, prior to the fix — an httptest run through the exact same router construction main.go uses):

api_fallback_test.go:64: GET /api/this-path-does-not-exist: want 404, got 200 (body "<html>SPA</html>")
api_fallback_test.go:85: POST /api/packets: want 405, got 200 (body "<html>SPA</html>")

After (live curl against a local build, port 13900, migrated copy of test-fixtures/e2e-fixture.db):

$ curl -i GET http://localhost:13900/api/this-path-does-not-exist
HTTP/1.1 404 Not Found
Content-Type: application/json
{"error":"not found"}

$ curl -i -X POST http://localhost:13900/api/packets -d '{}'
HTTP/1.1 405 Method Not Allowed
Allow: GET
Content-Type: application/json
{"error":"method not allowed"}

$ curl -i -X DELETE http://localhost:13900/api/stats
HTTP/1.1 405 Method Not Allowed
Allow: GET
Content-Type: application/json
{"error":"method not allowed"}

$ curl -i GET http://localhost:13900/api/stats
HTTP/1.1 200 OK
Content-Type: application/json
{...}

$ curl -i GET http://localhost:13900/
HTTP/1.1 200 OK
Content-Type: text/html; charset=utf-8
<!DOCTYPE html>...

$ curl -i -N -H "Connection: Upgrade" -H "Upgrade: websocket" -H "Sec-WebSocket-Version: 13" \
    -H "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==" http://localhost:13900/ws
HTTP/1.1 101 Switching Protocols
Upgrade: websocket

CI (per job)

Local, in a dedicated worktree + branch:

• cd cmd/server && go build ./..., go vet ./... — clean.
• gofmt -l on every touched/new file (api_fallback.go, api_fallback_test.go, routes.go, openapi.go, post_packets_removed_223_test.go, coverage_test.go) — no output (all formatted).
• go test ./... in cmd/server — all passing, no regressions.
• sh test-all.sh — 219/219 files passed.
• E2E against a local build (migrated + freshened + seeded copy of test-fixtures/e2e-fixture.db, port 13900, matching deploy.yml's pipeline exactly): test-e2e-playwright.js 132/135 passed (3 skips are pre-existing flaky/fixture-shape skips baked into the test itself, unrelated to this change), test-node-liveness-e2e.js 35/35 checks passed (exercises the live WS path).
• Mutation testing (3 required mutants, each reverted after confirming red):
  1. Fallback call removed from RegisterRoutes → 5 of the new/updated tests failed as expected.
  2. Fallback registered first in RegisterRoutes (wrong placement, shadowing every real route) → TestAPIFallbackDoesNotShadowExistingRoutes failed as expected.
  3. The 405 branch changed to write 200 → 4 of the new/updated tests failed as expected.

GitHub Actions (CI/CD Pipeline, run 37327732651), per job:

Job	Result	Duration
Go Build & Test	✅ pass	23m3s
🎭 Playwright E2E Tests	✅ pass	24m49s
🏗️ Build & Publish Docker Image	✅ pass	49s
📦 Release Artifacts	skipped (tag-only gate, correct for a branch PR)	—
🚀 Deploy Staging	skipped (push/master-only gate, correct for a PR)	—
📝 Publish Badges & Summary	skipped (push-only gate, correct for a PR)	—

Everything passed on the first run; the known-flaky #256 (Hash Stats-sort) did not surface, so no rerun was needed.

Rester (leftovers / out of scope)

• [K] A handful of /api/* handlers (e.g. handleRxCoverage, handleObserverDetail for an unknown id) call stdlib http.NotFound directly instead of the JSON writeError, so a disabled-feature or not-found-resource 404 from inside a matched handler is plain-text, not JSON. This is pre-existing, unrelated to the router-level fallback this PR adds (confirmed via a live-server spec sweep), and out of scope for fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 — not changed here.

[dborup]

Review — CS-Minimax PR#266 — head 7439f47

Dom: REQUEST CHANGES

Independent, read-only review. Reviewed the merged tree (git merge-tree --write-tree) of head 7439f471 against origin/master — first ae126374, then re-done against c6b356de after master advanced mid-review (both merge cleanly; cmd/server/routes.go and openapi.go moved on master and the fallback is still the last statement of RegisterRoutes after the merge). The three-dot diff is exactly 7 files: api_fallback.go, api_fallback_test.go, routes.go, openapi.go, post_packets_removed_223_test.go, coverage_test.go, docs/api-spec.md.

The core of the change is right, and the stated acceptance criteria hold. One finding blocks: the fallback converts HEAD on every /api/* route from a (wrong but harmless) 200 into a 405, which breaks a shipped operator script in this repo and any HEAD-based liveness probe.

Findings

#	Sev	Where	Finding
1	blocking	cmd/server/api_fallback.go:34-43	HEAD on any known /api/* route now returns 405. gorilla/mux's .Methods("GET") does not match HEAD, so every HEAD /api/... falls into the fallback, finds GET in the allowed set, and gets 405 Allow: GET. Behaviour change outside the PR's purpose, with a concrete in-repo break (below).
2	nit	cmd/server/api_fallback.go:28	PathPrefix("/api/") does not cover bare /api — GET /api still returns 200 text/html (the SPA shell), the exact symptom #233 describes. main.go's API-only banner even points users at /api/.
3	nit	cmd/server/api_fallback_test.go	Own mutant M1 survives the whole suite: changing PathPrefix("/api/") → PathPrefix("/api") passes go test ./... yet flips /api-docs, /apifoo, /apiary, /api from 200 SPA to 404 JSON. Nothing pins that non-API paths sharing the /api prefix still reach the SPA.
4	nit	cmd/server/routes.go:433	The catch-all is now the last route RegisterRoutes adds, so any /api/* route registered on the same router afterwards is silently dead. coverage_test.go was fixed for exactly this; nothing guards the production composition, where a future /api route added in main.go after srv.RegisterRoutes(router) would be shadowed with no test failing. TestAPIFallbackDoesNotShadowExistingRoutes uses the bare router, so it cannot see main.go-added routes.
5	nit	api_fallback_test.go:87,108, post_packets_removed_223_test.go:163	Allow is asserted with strings.Contains(allow, "GET"), which would also accept Allow: GETX. Exact-set comparison would be tighter.

Finding 1 — detail

Two servers built from the same fixture pipeline, master on 13700 and the merged tree on 13701. [A]

METHOD  PATH             master              PR
HEAD    /api/observers   200 text/html       405 application/json  Allow: GET
HEAD    /api/stats       200 text/html       405 application/json  Allow: GET
HEAD    /api/healthz     200 text/html       405 application/json  Allow: GET
OPTIONS /api/stats       200 text/html       405 application/json  Allow: GET

scripts/check-cdn-bypass.sh — a shipped operator tool — issues exactly curl -sSIL <host>/api/observers (HEAD) and hard-fails on any non-2xx: [A]

$ sh scripts/check-cdn-bypass.sh http://127.0.0.1:13700   # master
OK: no CDN caching detected for /api/observers (http=200, cf-cache-status=absent, age=0)   rc=0

$ sh scripts/check-cdn-bypass.sh http://127.0.0.1:13701   # this PR
FAIL: endpoint returned HTTP 405 — cannot verify CDN bypass (check URL / auth / network).  rc=1

Why this is worth fixing rather than accepting:

• RFC 9110 §9.1: a server that supports GET on a resource must support HEAD on it. The new Allow: GET also omits HEAD, which the same response is claiming is unsupported.
• corsMiddleware advertises Access-Control-Allow-Methods: GET, HEAD, OPTIONS for the read-only embed contract (Embed options through a different domain? Kpa-clawbot/CoreScope#1369), so the server simultaneously advertises HEAD and 405s it on every /api path. [A] verified live with CORS_ALLOWED_ORIGINS set.
• HEAD /api/healthz is the standard cheap liveness probe. Operators upgrading past this commit get 405 where they previously got 200. The repo's own docker-compose healthchecks use wget -qO- (GET) and are unaffected, and no frontend code issues HEAD, so the blast radius is the script above plus external monitors — but it is a real status-code flip on the health path.

Master's 200-HTML answer was of course also wrong. The point is that it was non-breaking, and this PR turns it into a hard failure rather than into the correct answer. Suggested direction: in the 405 branch, when the request method is HEAD and the allowed set contains GET, re-dispatch as GET (net/http suppresses the body), or register the real routes as .Methods("GET", "HEAD"). I probed the first option as mutant M3 and the full cmd/server suite stays green, so nothing in the suite pins the current 405-on-HEAD behaviour either way. If you take the re-dispatch route, resolve the matched route and call its handler rather than re-entering router.ServeHTTP, so a path that somehow matches neither cannot loop.

Answers to the review points

1. Unknown paths. [A] live, master vs PR:

GET    /api/this-path-does-not-exist   200 text/html  ->  404 application/json {"error":"not found"}
GET    /api/nonexistent/deep/path      200 text/html  ->  404 application/json
POST   /api/nonexistent                200 text/html  ->  404 application/json
HEAD   /api/this-path-does-not-exist   200 text/html  ->  404 application/json
GET    /api/                           200 text/html  ->  404 application/json
GET    /api/observers/   (trailing /)  200 text/html  ->  404 application/json
GET    /api                            200 text/html  ->  200 text/html      <-- finding 2

Trailing-slash variants of known paths land on 404 rather than 405 or a redirect (StrictSlash is not set anywhere). Defensible — the path genuinely does not exist — noting it only so it is a decision and not an accident. The error shape is the existing writeError → {"error": "..."}; no new response type. [T] TestAPIFallbackUnknownPathReturns404JSON, TestAPIFallbackUnknownPathInProductionRouterReturns404JSON.

2. Wrong method. [A] POST /api/packets → 405 Allow: GET, JSON body, no SPA page — never 200 HTML. Same for DELETE/PUT /api/stats, POST /api/config/client. The only /api path in the spec with more than one method, /api/config/geo-filter (GET+PUT), correctly answers 405 Allow: GET, PUT, so the Walk-and-rematch logic composes methods across route entries rather than reporting just one. [T] TestAPIFallbackPostPacketsReturns405NotSPA, TestAPIFallbackKnownPathWrongMethodReturns405, and the rewritten TestPostPacketsRemovedFallsThroughToSPAInProductionRouter (which also keeps the #223/#231 read-only-DB assertion). POST /api/packets is locked in both the bare and the production-style router.

3. Nothing regresses.

• All existing routes vs the served OpenAPI list — [A] pulled /api/spec from both servers: identical 94-path sets, 95 documented operations on each. Swept every one of the 95 operations ({param} → a placeholder) against master and the PR: 0 status-code differences. Distribution on the PR: 60×200, 12×404, 12×403, 8×400, 2×500, 1×202 — and no 405 anywhere among the documented operations, which is the direct proof of no shadowing. The fallback route itself does not leak into the spec (GetMethods() errors on it, so buildOpenAPISpec skips it) — hence the unchanged 94/95 counts. [T] TestAPIFallbackDoesNotShadowExistingRoutes makes the same assertion at the route-template level.
• WebSocket upgrade (wsOrStatic) — [A] /ws returns 101 Switching Protocols on both; /ws is not under /api/ so PathPrefix("/api/") cannot see it. [T] TestAPIFallbackDoesNotAffectWebSocketOrSPA checks the matched template is /ws.
• Health and metrics — [A] GET /api/healthz and GET /api/health 200 JSON on both; the per-observer metrics endpoints are in the 95-op sweep above. (HEAD /api/healthz is finding 1.)
• pprof — [A] not affected: it is opt-in via ENABLE_PPROF and served by net/http's DefaultServeMux on its own port (main.go:61-70), never through this mux.
• Static files — [A] /favicon.ico 200 image/x-icon on both.
• SPA deep links for non-API paths — [A] /, /index.html, /packets, /api-docs, /apifoo all 200 text/html on both, byte-identical shell.
• CORS preflight — [A] with CORS_ALLOWED_ORIGINS set, OPTIONS + Origin + Access-Control-Request-Method returns 204 on both master and the PR, for a known path and an unknown one alike: corsMiddleware is a router-level Use, so it runs on the fallback route exactly as it previously ran on the SPA route. No preflight regression. Without an allowed origin, OPTIONS /api/stats is now 405 instead of 200 HTML, which is the intended direction.
• Live E2E + WS E2E — see the Tests section; A/B'd against master with identical verdicts.

4. Ordering. Confirmed by reading and by the 95-op sweep: registerAPIFallback(r) is the final statement of RegisterRoutes, strictly after every real /api/* route in that function, and main.go registers /ws and the SPA PathPrefix("/") only after RegisterRoutes returns — so the fallback sits between the real routes and the SPA catch-all, which is the correct slot. Re-verified on the merge against the newer master c6b356de, which itself touched routes.go.

On "can a route registered later be shadowed?" — yes, and that is now load-bearing. It is why coverage_test.go's /api/test-slow had to move above the RegisterRoutes call. I checked the rest of the suite: the other tests that register /api/... handlers (area_filter_test.go, known_channels_cache_test.go, neighbor_api_test.go, node_reach_endpoint_test.go) all build a bare mux.NewRouter() and never call RegisterRoutes, so the PR's "no other test does this pattern" is accurate. There is also no /api route registered outside RegisterRoutes in production code today. The residual risk is finding 4: nothing fails if someone adds one later.

5. curl matrix. 29 method/path pairs, master (13700) vs PR (13701), full matrix captured. Excerpts under points 1, 2 and 3 above; the only cells that changed are unknown /api/ paths, wrong-method known paths, trailing-slash /api/ paths, and the HEAD/OPTIONS rows of finding 1.

Standing checks

• Acceptance criteria red-before / green-after — [T] built a tree of origin/master + only the PR's two test files (no api_fallback.go, unmodified routes.go). 5 of the 7 relevant tests fail there, with the expected messages: want 404, got 200 (body "<html>SPA</html>"), POST /api/packets: want 405, got 200 (body "<html>SPA</html>"), want Allow header containing GET, got "", and got "text/plain; charset=utf-8" (body "404 page not found\n") on the bare router. The two that stay green are TestAPIFallbackDoesNotShadowExistingRoutes and TestAPIFallbackDoesNotAffectWebSocketOrSPA, which are regression guards and correctly pass on both sides. Every box in fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 has a test that goes red before and green after.
• Own mutants — three, all reverted:
  • M1 PathPrefix("/api/") → PathPrefix("/api"): SURVIVED the full cmd/server suite, while live-flipping /api-docs, /apifoo, /apiary, /api from 200 to 404 → finding 3.
  • M2 405 branch downgraded to StatusNotFound, Allow header kept: killed by 4 tests (TestAPIFallbackPostPacketsReturns405NotSPA, TestAPIFallbackKnownPathWrongMethodReturns405, TestPostPacketsRemovedReturns405OnReadOnlyDB, TestPostPacketsRemovedFallsThroughToSPAInProductionRouter). The suite pins 405 specifically, not merely "not 200".
  • M3 HEAD re-dispatched as GET (a candidate fix for finding 1): suite stays green → the current 405-on-HEAD behaviour is unpinned in either direction.
• No behaviour change beyond the purpose — holds except for HEAD/OPTIONS on known /api routes (finding 1).
• cmd/server read-only — [A] no SQL in the new file; the only statements are header writes and writeError. The rewritten POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223 test still asserts the packet tables are byte-identical before and after POST /api/packets.
• No new map[string]interface{} outside tests — [A] clean. The only map added in production code is seen := map[string]bool{}; the openapi.go change is one string inside an existing literal. writeError keeps the existing map[string]string{"error": ...} shape.
• No hardcoded colors; scripts/check-xss-sinks.sh --diff origin/master clean — [A] vacuously: the three-dot diff touches no public/** or CSS file at all.
• Fork-guards — [A] github.repository == '<upstream>' appears 9× in deploy.yml and 1× in release-fast-path.yml, identical to master; neither workflow file is in the diff.
• No closing keywords — [A] commit body and PR body both say "Relates to fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233, POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223, refactor(server): remove the dead POST /api/packets endpoint (#223) #231" only.
• Commit author — [A] matches the required dborup identity (name and email) on both the author and committer fields; single commit.
• Docs — docs/api-spec.md gains the 404/405 lines and openapi.go's info.description states the contract. Accurate for /api/...; it will be slightly off for bare /api until finding 2 is addressed.

Tests I ran

Merged tree against origin/master (ae126374, and re-run against c6b356de):

Suite	Result
cmd/server go build ./..., go vet ./...	clean
gofmt -l on the 6 touched files	clean (46 files are unformatted under Go 1.26 in both master and the PR — pre-existing toolchain drift, not this PR)
cmd/server go test -count=1 ./...	ok, 44s
cmd/server go test -race on the new/updated tests	ok, no warnings
cmd/ingestor go test -count=1 ./...	ok, 107s
cmd/server suite on the merge against newer master c6b356de	ok
sh test-all.sh	219/219 files passed
node test-frontend-helpers.js	707 passed, 0 failed

Live E2E against local Go servers on fixture DBs prepared exactly as deploy.yml does (freshen, Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073 + 199 + 245), instrumented frontend, ports 13710 (PR) and 13711 (master):

E2E	PR	master	Note
test-e2e-playwright.js	131/135 passed, 3 skipped, 1 failed	131/135 passed, 3 skipped, 1 failed	verdict-identical, test for test
test-channels-ws-batch-e2e.js (live WS)	6/6	—
test-channels-ws-race-1498-e2e.js (live WS)	5/5	—
test-node-liveness-e2e.js	35/35 checks	—	own mock server, not the Go WS path

The one Playwright failure is Version info lives on Perf dashboard, not in navbar (#navStats never reaches >5 chars within 10s). It fails identically on master with the same binary pipeline and the same instrumented frontend, and it passes in GitHub Actions, so it is a local-environment artifact, not a regression. To get a full A/B I ran a scratch-only copy of the harness with fail-fast disabled (the PR tree was not modified); the pass/fail/skip name sets are byte-identical between master and the PR. The 3 skips are the fixture-shape skips baked into the test.

GitHub Actions on this head (run 37327732651), per job: Go Build & Test ✅ 23m3s, Playwright E2E ✅ 24m49s, Build & Publish Docker Image ✅ 49s; Release Artifacts, Deploy Staging and Publish Badges correctly skipped by their tag/push gates. No flakes surfaced, so neither of the known-flaky suites needed a rerun. git ls-remote on the head was 7439f471 before and after the review; master advanced from ae126374 to c6b356de mid-review and the branch still merges cleanly and still passes.

What I did not verify

• [K] Nothing on staging or production, and no API key used — local binaries only, per the brief.
• [K] The pprof surface was reasoned about from main.go (separate port, DefaultServeMux, ENABLE_PPROF-gated) rather than exercised with ENABLE_PPROF=true.
• [K] Behaviour behind a real CDN or reverse proxy. A proxy that rewrites or normalises /api paths, or that treats a 405 on a health probe as a hard failure, could see different effects from finding 1 than a direct connection does.
• [K] The 12 documented operations that answer 404 from inside their handler (/api/rx-coverage, /api/nodes/{id}/* for an unknown id, …) still return text/plain 404 page not found, identically on master and on this PR. This is the author's own declared leftover and I confirm it is pre-existing and untouched here, but it means a client cannot assume every /api 404 is JSON even after this PR.
• [K] No load or latency measurement of the fallback. allowedMethodsForPath clones the request once per (route, method) pair that has not yet matched, so an unmatched path walks ~95 routes and allocates ~95 clones per request. Cheap and off the hot path, but it is unbounded work driven by an unauthenticated 404, so it would be worth a glance if /api 404 volume is ever adversarial.

[dborup-agent]

Rapport — CS-pve-agent2 PR#266 runde 2 — head 74dca70

Review feedback addressed (commit 74dca70b)

Commits on top of round 1 (7439f471): 0096767c merge of origin/master (c6b356de), fa48ea9e red tests, c2f6c9b7 fix for findings 1, 2 and 4, 74dca70b fix for finding 3. No rebase, amend or force-push. All commits are by dborup.

1. (blocking) HEAD on known /api routes no longer returns 405. The fallback now resolves the route that a GET to the same URL would reach (router.Match on a GET clone) and runs that route's own handler with its path variables. Because the connection's request is HEAD, net/http drops the body. Middleware is not run a second time, and the fallback cannot re-enter itself: it calls route.GetHandler() directly, never router.ServeHTTP or match.Handler. This one generic fix covers every GET route, with no per-route .Methods("GET", "HEAD"), so the OpenAPI spec is unchanged. HEAD now gets the same status and headers as GET with no body. Wherever GET is allowed, Allow now lists HEAD too, as RFC 9110 requires. HEAD on a path with no GET route still answers 405 with the exact Allow set. [T] [A]
2. Bare /api returns a JSON 404. It has its own exact-path fallback route, router.Path("/api"), next to PathPrefix("/api/"). /api-docs, /apifoo, /apiary, /apis/x and /api.json still reach the SPA. Reviewer mutant M1 (PathPrefix("/api")) now dies. [T] [A]
3. Shadowing is caught. main.go's router composition (RegisterRoutes, /ws, SPA catch-all) moved into newHTTPRouter, which main and the fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 tests now share. The two fallback routes are named, and apiRoutesShadowedByFallback lists every /api route registered after them. TestProductionRouterHasNoShadowedAPIRoutes asserts on newHTTPRouter that the fallback is the last /api route and that nothing is shadowed. TestAPIRoutesShadowedByFallbackReportsLateRoutes tests the guard itself. main also refuses to start (log.Fatalf) if any /api route is shadowed, which covers routes added in main after newHTTPRouter returns. [T] [A]
4. Allow is compared as an exact set. The new assertAllowSet helper splits, trims and sorts the header, so GETX or a missing or extra method fails. It is used in api_fallback_test.go and in both POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223 tests in post_packets_removed_223_test.go; TestPostPacketsRemovedReturns405OnReadOnlyDB had no Allow check before and now has one. [T]

Docs: docs/api-spec.md and the info.description in openapi.go now mention bare /api and HEAD.

Tests

New or changed in cmd/server/api_fallback_test.go:

Test	Locks
TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute	Walks every path in the served /api/spec (94 paths) over a real listener with GET, HEAD, GET. HEAD must have GET's status, Content-Type, Cache-Control and Allow, and send 0 body bytes. Body bytes are read over a raw TCP connection, because http.Client never surfaces a HEAD body; I checked that the probe can fail by sending GET through it (86 failures). Where GET itself is 405, HEAD is 405 with the exact Allow set. Result: 86 of 94 paths serve HEAD like GET; the other 8 have no GET route.
TestAPIFallbackBareAPIReturns404JSON	GET/POST /api returns a JSON 404 on both the API router and the production router.
TestAPIFallbackPrefixSiblingsStillReachSPA	/api-docs, /apifoo, /apiary, /apis/x and /api.json return 200 with the SPA page.
TestAPIFallbackAllowComposesMethodsAcrossRoutes	POST /api/config/geo-filter returns Allow exactly {GET, HEAD, PUT}.
TestProductionRouterHasNoShadowedAPIRoutes	Production router: the fallback is the last /api route, and nothing is shadowed.
TestAPIRoutesShadowedByFallbackReportsLateRoutes	A late /api/late and /api/late/{id} are reported, and the late handler never runs. A late /apiary-late is not reported.
existing 405 tests + both #223 tests	Allow is exactly {GET, HEAD}.

productionStyleRouter in the tests is now newHTTPRouter with a stub index.html, so it can no longer drift from main.go.

Red before green [T]. Run at fa48ea9e (tests only, round-1 code): 7 tests failed.

TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute: 85x "HEAD <path>: got 405 (Allow "GET"), want GET's status ..."
TestAPIFallbackBareAPIReturns404JSON: production router: GET /api: want 404, got 200 (body "<html>SPA</html>")
                                      api router: GET /api: want application/json, got "text/plain; charset=utf-8"
TestAPIFallbackPostPacketsReturns405NotSPA / ...KnownPathWrongMethod / both #223 tests:
                                      Allow: got "GET" (set [GET]), want exactly [GET HEAD]
TestAPIFallbackAllowComposesMethodsAcrossRoutes: Allow: got "GET, PUT", want exactly [GET HEAD PUT]

TestAPIFallbackPrefixSiblingsStillReachSPA passed both before and after, as intended: it guards against M1. The shadow-guard tests were red against a stub guard that returned nil (shadowed routes: got [], want [/api/late /api/late/{id}]).

The HEAD walk test changed between fa48ea9e and c2f6c9b7, and the commit message explains why:

• /api/analytics/distance answers 202 until its background compute finishes, so HEAD is compared with the GET just before and just after it.
• GET /api/packets/observations is served by /api/packets/{hash}, so "no GET route" is now read from GET's own 405 rather than from the spec.

Mutants (one or more per finding; each applied, run, reverted) [T]

Finding	Mutant	Result
1	HEAD dispatch disabled (if r.Method == http.MethodHead && false)	killed: TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute
2	reviewer M1: PathPrefix("/api/") → PathPrefix("/api")	killed: TestAPIFallbackPrefixSiblingsStillReachSPA
2	bare router.Path("/api") route removed	killed: TestAPIFallbackBareAPIReturns404JSON
3	/api/late-mutant registered in newHTTPRouter after RegisterRoutes	killed by TestProductionRouterHasNoShadowedAPIRoutes and TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute. [A] The mutated binary also refused to start, exit 1: /api routes registered after the API fallback are unreachable: [/api/late-mutant] (register them in RegisterRoutes)
4	Allow gets a suffix: "GET, HEAD" → "GET, HEADX" (survives the old strings.Contains)	killed by 6 tests, including both #223 tests

Local runs

Check	Result
cd cmd/server && go vet ./...	[T] clean
gofmt -l on touched files (api_fallback.go, api_fallback_test.go, main.go, openapi.go, post_packets_removed_223_test.go, coverage_test.go, routes.go)	[T] clean. The 46 unformatted files under go1.27 are the same count on master: toolchain drift, not this PR.
cd cmd/server && go test ./...	[T] ok (510 s) with -skip Test1690_BackgroundLoadHonesty, see [K] below
sh test-all.sh	[T] 219/219 files passed
test-e2e-playwright.js, local Go server, e2e-fixture.db prepared as in deploy.yml (freshen, Kpa-clawbot#1486/Kpa-clawbot#1791 seed, corescope-migrate, seeds 2073/199/245)	[A] fail-fast run stopped at Version info lives on Perf dashboard, not in navbar (#navStats timeout). With a scratch copy with fail-fast disabled (repo file untouched), A/B against master: 131/135 passed, 3 skipped, 1 failed on both, with identical pass/skip/fail name sets. This is the same local-only failure the round-1 review saw on master; it passes in GitHub Actions.
test-channels-ws-batch-e2e.js (live WS)	[A] 6/6
test-channels-ws-race-1498-e2e.js (live WS)	[A] 5/5
scripts/check-cdn-bypass.sh (HEAD /api/observers)	[A] OK ... (http=200 ...) rc=0. In round 1 it was FAIL: endpoint returned HTTP 405 rc=1.
Raw-socket HEAD vs GET on /api/stats, /api/healthz, /api/observers, /api/nodes/deadbeef, with and without Accept-Encoding: gzip	[A] same status line, and identical Content-Type, Content-Length, Content-Encoding, Cache-Control, Vary and X-CoreScope-Load-Status. HEAD body is 0 bytes.

Servers were stopped with fuser -k <port>/tcp.

curl matrix (before / after) [A]

Three local servers on the same prepared fixture: master c6b356de; round-1 code merged with master (0096767c, "before"); this head (74dca70b, "after"). 15 paths × GET/HEAD/POST/OPTIONS. Columns: status, content-type, Allow. HEAD uses curl --head.

METHOD  PATH                           master                 | before (round 1)                 | after (this head)
GET     /api                           200 text/html          | 200 text/html                    | 404 json
HEAD    /api                           200 text/html          | 200 text/html                    | 404 json
POST    /api                           200 text/html          | 200 text/html                    | 404 json
OPTIONS /api                           200 text/html          | 200 text/html                    | 404 json
GET     /api/                          200 text/html          | 404 json                         | 404 json
HEAD    /api/                          200 text/html          | 404 json                         | 404 json
GET     /api/stats                     200 json               | 200 json                         | 200 json
HEAD    /api/stats                     200 text/html          | 405 json  Allow: GET             | 200 json
POST    /api/stats                     200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
OPTIONS /api/stats                     200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
GET     /api/healthz                   200 json               | 200 json                         | 200 json
HEAD    /api/healthz                   200 text/html          | 405 json  Allow: GET             | 200 json
POST    /api/healthz                   200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
GET     /api/observers                 200 json               | 200 json                         | 200 json
HEAD    /api/observers                 200 text/html          | 405 json  Allow: GET             | 200 json
POST    /api/observers                 200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
GET     /api/packets                   200 json               | 200 json                         | 200 json
HEAD    /api/packets                   200 text/html          | 405 json  Allow: GET             | 200 json
POST    /api/packets                   200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
OPTIONS /api/packets                   200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
GET     /api/config/geo-filter         200 json               | 200 json                         | 200 json
HEAD    /api/config/geo-filter         200 text/html          | 405 json  Allow: GET, PUT        | 200 json
POST    /api/config/geo-filter         200 text/html          | 405 json  Allow: GET, PUT        | 405 json  Allow: GET, HEAD, PUT
GET     /api/nodes/deadbeef            404 json               | 404 json                         | 404 json
HEAD    /api/nodes/deadbeef            200 text/html          | 405 json  Allow: GET             | 404 json   (= GET)
POST    /api/nodes/deadbeef            200 text/html          | 405 json  Allow: GET             | 405 json  Allow: GET, HEAD
GET     /api/this-path-does-not-exist  200 text/html          | 404 json                         | 404 json
HEAD    /api/this-path-does-not-exist  200 text/html          | 404 json                         | 404 json
POST    /api/this-path-does-not-exist  200 text/html          | 404 json                         | 404 json
GET     /api/observers/                200 text/html          | 404 json                         | 404 json
GET|HEAD|POST|OPTIONS /api-docs        200 text/html (SPA)    | 200 text/html (SPA)              | 200 text/html (SPA)
GET|HEAD|POST|OPTIONS /apifoo          200 text/html (SPA)    | 200 text/html (SPA)              | 200 text/html (SPA)
GET|HEAD|POST|OPTIONS /apiary          200 text/html (SPA)    | 200 text/html (SPA)              | 200 text/html (SPA)
GET|HEAD|POST|OPTIONS /                200 text/html (SPA)    | 200 text/html (SPA)              | 200 text/html (SPA)
GET|HEAD|POST|OPTIONS /packets         200 text/html (SPA)    | 200 text/html (SPA)              | 200 text/html (SPA)

The full 60-row matrix was captured; omitted rows follow the same pattern. Between "before" and "after", the only cells that changed are bare /api (4 rows), HEAD on known routes (now the same as GET), and Allow gaining HEAD. On every known JSON route, the GET body is the same size on all three servers, except /api/healthz: 243 bytes on master and 251 on both branch builds. That difference already exists in round 1, so it does not come from this round; I did not investigate it further.

CI per job (run 37361537909, head 74dca70b)

Job	Result	Duration
Go Build & Test	✅ success (includes Test1690_BackgroundLoadHonesty, skipped locally)	24m08s
🎭 Playwright E2E Tests	✅ success	25m09s
🏗️ Build & Publish Docker Image	✅ success	47s
📦 Release Artifacts	skipped (tag-only gate)	—
🚀 Deploy Staging	skipped (push/master-only gate)	—
📝 Publish Badges & Summary	skipped (push-only gate)	—

Overall: success on the first run. The known flakes (#267, #271) did not show up, so no rerun was needed.

Rester (leftovers)

• [K] Test1690_BackgroundLoadHonesty hangs locally on this host on master too: isolated runs hit the 5 min timeout on both, with ~3 s of CPU, while the test seeds 5000 rows; I observed the ext4 journal thread in D-state at the time. It is an I/O-bound environment problem, unrelated to routing. I skipped it locally; CI ran it (see the CI table).
• [K] The fallback serves HEAD by calling the matched route's handler with mux.SetURLVars, which gorilla/mux documents as intended for tests. It is the only exported way to attach path variables without re-entering router.ServeHTTP, and re-entering would run the middleware twice (double perf counting) and risk a loop. The HEAD walk test pins the behaviour, including routes with {params}.
• [K] perfMiddleware buckets requests by the matched route template, so fallback-served HEAD requests are counted under /api/ (like the 404/405s), not under the GET route's key. Cosmetic; not changed.
• [K] OPTIONS on a known /api path without an allowed CORS origin is still 405 (Allow: GET, HEAD), as in round 1. The reviewer considered that the intended direction. CORS preflight with an allowed origin is handled by corsMiddleware before the fallback and is unchanged.
• [K] noStoreAPIMiddleware only matches the /api/ prefix, so the bare /api JSON 404 carries no Cache-Control: no-store. Harmless for a static 404; not changed, to keep scope.
• [K] A trailing slash on a known path (/api/observers/) is a 404, not a redirect, as in round 1 (StrictSlash is not set).
• [K] The handler-level plain-text http.NotFound 404s noted in round 1 are unchanged and out of scope.
• Nothing on staging or production was touched, and no API key was used.

🤖 Generated with Claude Code

[adminopenclaw8-sketch]

Review — CS-Macmini PR#266 — head 74dca70

Dom: APPROVE med nits

Independent, read-only review of round 2. Reviewed the merged tree (git merge-tree --write-tree origin/master 74dca70b → f64e03c7) against origin/master f91339f2; the merge is clean and the fallback is still the last statement of RegisterRoutes afterwards. Three-dot diff is 8 files: api_fallback.go, api_fallback_test.go, coverage_test.go, main.go, openapi.go, post_packets_removed_223_test.go, routes.go, docs/api-spec.md. git ls-remote on the branch was 74dca70b before and after.

All five of the previous round's findings are fixed and I reproduced each fix on live servers. Nothing blocks. Four nits, two of them test gaps found by my own mutants.

Findings

#	Sev	Where	Finding
N1	nit	cmd/server/api_fallback.go:147, api_fallback_test.go:392	The shadow guard's bare-/api arm is unpinned. Dropping tmpl == "/api" from apiRoutesShadowedByFallback leaves go test ./... fully green (my mutant D). TestAPIRoutesShadowedByFallbackReportsLateRoutes registers /api/late, /api/late/{id} and /apiary-late, but never a late route at exactly /api — which is the case this round deliberately added a fallback route for. Production code is correct; only the test is short one path.
N2	nit	cmd/server/api_fallback.go:123	The self-dispatch guard in getRouteHandler (GetMethods() errors → the matched route is the fallback itself → nil) is unpinned. Removing it leaves the suite green (my mutant L). It is harmless today — the nested call carries Method: GET, so the r.Method == http.MethodHead branch cannot re-enter and there is no loop — but it is the documented loop guard and nothing holds it in place. Related: no test sends HEAD to an unknown /api path at all; the existing 404 tests use GET/POST. [A] live it is 404 application/json, as intended.
N3	nit	cmd/server/post_packets_removed_223_test.go:152	TestPostPacketsRemovedFallsThroughToSPAInProductionRouter no longer falls through to the SPA — it now pins 405 + Allow. The doc comment was rewritten, the name was not, so the test now reads as asserting the behaviour it forbids.
N4	nit	cmd/server/main.go:662	The API-only banner still says "API available at /api/", and /api/ is now a JSON 404 (as is bare /api). Pre-existing text, newly wrong. /api/docs or /api/spec would be an accurate pointer.

None of these change behaviour; all four are safe to leave or fix in a follow-up.

Answers to the review points

1. HEAD (the round-1 blocker). Fixed, verified by matrix rather than by spot checks. I pulled /api/spec from both servers and swept all 94 documented paths ({param} → x) with GET and HEAD on master (13800) and on the merged tree (13801). [A]

• 0 paths answer HEAD with 405 where a GET route exists. On the PR, HEAD status == GET status on every one of the 86 paths that have a GET route; the other 8 are POST-only and answer 405 with the exact Allow set (POST), which is correct.
• The only status delta between master and the PR across the 95 documented operations is the 8 POST-only paths probed with GET: 200 text/html (SPA) on master → 405 application/json Allow: POST on the PR. That is the fix, not a regression.
• /api/analytics/distance is the one path whose GET is 202-until-warm; after the background compute it is a stable 200/200 on GET/HEAD. I re-probed it three times: 200 200, 200 200, 200 200.
• Raw-socket HEAD vs GET on /api/stats, /api/healthz, /api/observers, /api/nodes/deadbeef, /api/spec, /api/docs, /api/packets, with and without Accept-Encoding: gzip: same status line and identical Content-Type, Content-Length, Content-Encoding, Cache-Control, Vary, X-CoreScope-Load-Status; HEAD body is 0 bytes in all 14 probes. [A]
• scripts/check-cdn-bypass.sh (the round-1 break): OK: no CDN caching detected for /api/observers (http=200 …) rc=0 on both master and the PR. [A]
• Authorisation survives the new dispatch path, which was the thing I most wanted to check: requireAPIKey is a per-route wrapper, and getRouteHandler returns route.GetHandler() — the wrapped handler — so HEAD /api/backup, /api/debug/affinity, /api/dropped-packets, /api/admin/channel-proposals, /api/admin/prune-geo-filter/status all answer 403, exactly like GET. On master those same HEADs were 200 SPA pages. No bypass. [A]
• TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute is stable: -count=5 passes 5/5, 86 of 94 paths each time. [T]

2. Bare /api and the prefix siblings. [A] GET/HEAD/POST/OPTIONS /api → 404 application/json on the PR, 200 text/html on master. /api-docs, /apifoo, /apiary, /apis/x, /api.json → 200 text/html (SPA) on both, for all four methods. Mutant M1 (PathPrefix("/api/") → PathPrefix("/api")) is killed by TestAPIFallbackPrefixSiblingsStillReachSPA; the author reported this and the test is present and specific. [T]

3. Shadowing guard. Present and effective. apiRoutesShadowedByFallback walks in registration order, flags every /api route after either named fallback route; main log.Fatalfs on a non-empty result at line 513, after newHTTPRouter (line 368) and before the listener starts — and nothing registers a route in between. I verified it end-to-end with my own mutant: adding /api/late-mutant to newHTTPRouter after RegisterRoutes makes the built binary refuse to start, exit 1, with [server] /api routes registered after the API fallback are unreachable: [/api/late-mutant], and kills two tests. [A][T] The one hole is N1 (bare /api). Also worth recording: the two guard tests were added in the same commit as the guard (74dca70b), so unlike findings 1/2/4 at fa48ea9e there is no red-before commit for them — the mutant evidence stands in for it.

4. Allow as an exact set. assertAllowSet splits, trims, sorts and compares the full set, so GETX, a missing method or an extra one all fail. It is used in all four 405 assertions plus both #223 tests. Composition across route entries is right: POST /api/config/geo-filter → Allow: GET, HEAD, PUT. [T][A] Removing the seen[GET] → seen[HEAD] line (my mutant A) is killed by 5 tests. [T]

5. No regression. 25 paths × GET/HEAD/POST/OPTIONS = 100 cells, master vs PR. [A]

PATH                       METHOD  master              PR
/api/stats                 GET     200 application/json  200 application/json
/api/stats                 HEAD    200 text/html         200 application/json
/api/stats                 POST    200 text/html         405 application/json  Allow: GET, HEAD
/api/stats                 OPTIONS 200 text/html         405 application/json  Allow: GET, HEAD
/api/healthz               GET     200 application/json  200 application/json
/api/healthz               HEAD    200 text/html         200 application/json
/api/config/geo-filter     POST    200 text/html         405 application/json  Allow: GET, HEAD, PUT
/api/nodes/deadbeef        HEAD    200 text/html         404 application/json   (= GET)
/api/this-path-does-not-exist  (all 4)  200 text/html    404 application/json
/api/observers/  (trailing /)  GET  200 text/html        404 application/json
/api  /api/                (all 4)  200 text/html        404 application/json
/api-docs /apifoo /apiary /apis/x /api.json  (all 4)  200 text/html   200 text/html   (unchanged)
/ /index.html /packets     (all 4)  200 text/html        200 text/html   (unchanged)
/favicon.ico               (all 4)  200 image/x-icon     200 image/x-icon (unchanged)
/app.js                    (all 4)  200 text/javascript  200 text/javascript (unchanged)
/ws                        (all 4)  400 text/plain       400 text/plain  (unchanged)

• WebSocket: a real upgrade handshake returns 101 Switching Protocols on both servers. /ws is not under /api/, and TestAPIFallbackDoesNotAffectWebSocketOrSPA pins the matched template. Live WS suites against the PR server: test-channels-ws-batch-e2e.js 6/6, test-channels-ws-race-1498-e2e.js 5/5. [A][T]
• Health/metrics: /api/healthz and /api/health 200 JSON on both, HEAD now also 200 JSON. /api/metrics is not a route and is 404 JSON on the PR (200 SPA on master) — correct.
• Static + SPA deep links: unchanged, byte-identical shell.
• OpenAPI surface: both servers serve the same 94 paths / 95 operations; the fallback does not leak into the spec. The only spec delta is the info.description string. [A]
• Frontend /api references: I extracted every /api/... literal from public/**/*.js (37) and probed each against both servers. Every difference is either a POST-only route probed with GET (now 405, correct) or a base string the frontend concatenates an id onto (/api/nodes/, /api/packets/, /api/channels/). No real frontend request changes status. [A]
• Latency of an unmatched /api path: 200 sequential requests, 4.7 ms/req on master and 4.7 ms/req on the PR — the per-request route walk is not measurable here. [A]

Standing checks

• Red before / green after — [T] I rebuilt the tree at the tests-only commit fa48ea9e (round-2 tests on round-1 code) and ran it: 7 tests fail, with HEAD /api/observers: got 405 (Allow "GET"), want GET's status 200 ×85, GET /api: want 404, got 200 (body "<html>SPA</html>"), and Allow: got "GET" (set [GET]), want exactly [GET HEAD]. Green on the head. Every acceptance box in fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 has a test that is red before and green after (the fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 criteria were also red against plain master in round 1).
• Own mutants — five, each applied, run and reverted:
  • A seen[GET] → seen[HEAD] line removed: killed by 5 tests.
  • D shadow guard drops the tmpl == "/api" arm: SURVIVED the full suite → N1.
  • E mux.SetURLVars(getReq, match.Vars) → plain getReq (HEAD dispatch loses path vars): killed by TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute.
  • L self-dispatch guard removed from getRouteHandler: SURVIVED the full suite → N2. No recursion results, since the nested call runs as GET.
  • M /api/late-mutant registered in newHTTPRouter after RegisterRoutes: killed by TestProductionRouterHasNoShadowedAPIRoutes and TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute, and the built binary exits 1 at startup.
• No behaviour change beyond the purpose — holds. The only deltas versus master are unmatched /api paths, wrong-method known paths, bare /api, trailing-slash /api paths, and HEAD/OPTIONS, all of which are the stated purpose. HEAD now runs the real handler, so it costs what GET costs; auth, Cache-Control and Vary all come out identical.
• cmd/server read-only — no SQL in the new file; the only writes are headers and writeError. The rewritten POST /api/packets INSERTs on the server's read-only handle and always returns 500 #223 test still asserts the packet tables are unchanged after POST /api/packets.
• No new map[string]interface{} outside tests — clean; the non-test diff adds none (writeError keeps the existing map[string]string, and the one new map is seen := map[string]bool{}).
• No hardcoded colors, scripts/check-xss-sinks.sh --diff origin/master clean — vacuous: the diff touches no public/**, CSS or HTML file (0 files under public/).
• Fork-guards — 9 in deploy.yml, 1 in release-fast-path.yml; neither file is in the diff.
• No closing keywords — the five commit bodies and the PR body say "Relates to" only.
• Commit author — all five commits on top of master are authored and committed by the required dborup identity.
• gofmt — clean on all seven touched files.

Tests I ran

Merged tree (origin/master f91339f2 + head), Go 1.27.0, darwin/arm64:

Suite	Result
cmd/server go build ./..., go vet ./...	clean
cmd/server go test -count=1 ./...	ok, 38.5 s — no skips needed on this host
cmd/ingestor go test -count=1 ./...	ok, 103.9 s
sh test-all.sh	220/220 files passed
node test-frontend-helpers.js	707 passed, 0 failed
go test -count=5 -run TestAPIFallbackHeadMatchesGetForEveryOpenAPIRoute	5/5, no flake

Live E2E against local Go servers on fixture DBs prepared exactly as deploy.yml does (freshen, Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073/199), instrumented frontend, ports 13800 (master), 13801 (PR), 13802 (PR + instrumented), 13803 (master + instrumented):

E2E	PR	master
test-e2e-playwright.js	131/135 passed, 3 skipped, 1 failed	see below
test-channels-ws-batch-e2e.js (live WS)	6/6	—
test-channels-ws-race-1498-e2e.js (live WS)	5/5	—

The one Playwright failure is Version info lives on Perf dashboard, not in navbar (#navStats never exceeds 5 chars within 10 s). I ran the same harness against a master-based server on the same fixture pipeline: master is also 131/135 passed, 3 skipped, 1 failed, and the pass/fail/skip name sets are byte-identical between master and the PR. It is a local-environment artifact, not a regression, and the Playwright E2E job on this head is green in CI. (The A/B run used a scratch copy of the harness with fail-fast disabled; the repo file was not modified.) The 3 skips are the fixture-shape skips baked into the tests.

Neither known flake (#256 Hash Stats sort, #267 backfill write-hold) surfaced; Analytics Hash Stats tab renders content passed on both servers.

CI per job (run 37361537909, head 74dca70b)

Job	Result	Duration
✅ Go Build & Test	success	24m08s
🎭 Playwright E2E Tests	success	25m09s
🏗️ Build & Publish Docker Image	success	47s
📦 Release Artifacts	skipped (tag gate)	—
🚀 Deploy Staging	skipped (push/master gate)	—
📝 Publish Badges & Summary	skipped (push gate)	—

Overall success on the first run; no rerun needed. Checked per job myself, not just the roll-up.

What I did not verify

• [K] Nothing on staging or production, no API key used, no writes to the PR or to upstream. Local binaries only.
• [K] main's log.Fatalf shadow guard: I verified it [A] with a mutated binary (exit 1), but no automated test covers main() itself, so a regression in that one call site would only be caught by the newHTTPRouter-level test.
• [K] Behaviour behind a real CDN or reverse proxy. A proxy that rewrites or normalises /api paths, or that previously got a 200 for bare /api, could see different effects than a direct connection.
• [K] gzipMiddleware was not exercised with gzip actually enabled; Accept-Encoding: gzip probes ran against the default (gzip-off) config, where HEAD and GET headers matched exactly.
• [K] perfMiddleware buckets a fallback-served HEAD under the /api/ template rather than the GET route's key, so /api/perf under-counts those endpoints by exactly the HEAD traffic. Cosmetic, the author's declared leftover, unchanged here.
• [K] noStoreAPIMiddleware matches only the /api/ prefix, so the bare /api JSON 404 carries no Cache-Control: no-store. Declared leftover.
• [K] Handler-level http.NotFound 404s (/api/rx-coverage, /api/rx-leaderboard, /api/packets/observations, …) are still text/plain on both master and this head — pre-existing and out of scope, but it means a client still cannot assume every /api 404 is JSON.
• [K] Trailing slash on a known path (/api/observers/) is a 404 rather than a redirect; StrictSlash is not set anywhere. Decision, not accident.
• [K] No adversarial load test of the 404 path. The sequential measurement above showed no difference, but I did not drive concurrent unmatched-path traffic.