Skip to content

server: serve SPA index.html for /users, /agents, /forgot-password#137

Open
nickhodaly-ud wants to merge 1 commit intoInfisical:mainfrom
nickhodaly-ud:fix/spa-fallback-routes
Open

server: serve SPA index.html for /users, /agents, /forgot-password#137
nickhodaly-ud wants to merge 1 commit intoInfisical:mainfrom
nickhodaly-ud:fix/spa-fallback-routes

Conversation

@nickhodaly-ud
Copy link
Copy Markdown

Summary

  • The SPA fallback in internal/server/server.go is an explicit allow-list of frontend routes, not a true catch-all. Three top-level routes (/users, /agents, /forgot-password) were missing, so a hard refresh on those pages 404'd while client-side navigation worked.
  • Add the three missing mux.HandleFunc("GET /...", s.handleSPA) entries. No behavior change for any path that already worked; unknown paths still 404 (the allow-list stays tight).
  • Add TestSPACatchAllRoutes to lock the contract: every top-level route in web/src/router.tsx is hit through the real mux and asserted non-404. The next missing route fails CI rather than shipping silently.

Why this snuck in

/users and /agents are sibling tabs to / (vaults list) under the home layout, and /forgot-password is a sibling to /login. They were the only top-level frontend routes whose path didn't include a wildcard segment matched by an existing fallback (e.g. /account/{path...}, /vaults/{name...}), and client-side navigation hides the gap because the server is never asked.

Test plan

  • go build ./... — clean
  • go test -race -count=1 ./internal/server/... — green, including new TestSPACatchAllRoutes (13 path subtests + an unknown-path 404 sanity subtest)
  • go vet ./internal/server/... — clean
  • Verified the new test catches the original bug: stashing the three new SPA registrations causes exactly /users, /agents, /forgot-password to fail
  • Manual smoke against a running server — /users /agents /forgot-password return 200; /definitely-not-a-route returns 404
  • Reviewer: refresh /users in a deployed instance to confirm the original repro is resolved

Out of scope

  • /change-password is in the SPA allow-list but no longer exists in web/src/router.tsx. Stale-but-harmless entry; not removed in this PR.
  • Bare /account and /manage (no trailing path) would still 404 because /{path...} requires ≥1 segment. The frontend redirects out of those paths immediately on mount, so user-impact is near-zero. Pre-existing.

Test note: why /invite/{token} is excluded from the test

The API handler GET /invite/{token} (server.go:627) is registered alongside the SPA fallback GET /invite/{token...} (server.go:746). Go's ServeMux prefers the single-segment pattern, so single-segment /invite/abc requests route to handleInviteRedeem (which 404s on an unknown token in the mock store), not handleSPA. A 404 there wouldn't tell us anything about the SPA fallback being broken, so the test skips it with an inline comment.

🤖 Generated with Claude Code

The SPA fallback in server.go is an explicit allow-list, not a true
catch-all. Three top-level frontend routes (/users, /agents,
/forgot-password) were missing from the list, so a hard refresh on
those pages 404'd while client-side navigation worked.

Add a regression test that hits every top-level route in
web/src/router.tsx through the real mux. /invite/{token} is excluded
because the more-specific API handler at GET /invite/{token} wins
over the SPA fallback at /invite/{token...}.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nickhodaly-ud nickhodaly-ud marked this pull request as ready for review April 27, 2026 19:49
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

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.

1 participant