Fix production: every route except the homepage was 404, and every API function was 500 - #2
Merged
Conversation
Two independent faults, both invisible locally because `npm run build` and the dev server exercise neither. The project's Vercel build command is overridden in the dashboard to `vite build`, so generate-seo-pages.mjs never ran in CI. None of the diagram pages, route shells or sitemap.xml existed in the deployment. Set `buildCommand` in vercel.json, where it is version controlled and beats the dashboard, instead of fixing it by hand in a UI nobody diffs. The SPA fallback rewrote everything to `/index.html`, which cannot work with `cleanUrls: true`. Compiled, cleanUrls emits a 308 from `/index.html` to `/` ahead of the filesystem, and rewrites carry `check: true`, so the rewritten path re-entered the table, hit that redirect and resolved to nothing. Every route below `/` returned NOT_FOUND, taking share links, /settings and /editor with it. The rewrites now name the SPA's routes and target `/`. Enumerating them rather than catching everything means anything else falls through to a real 404 instead of answering with the app and a 200, which would tell crawlers every mistyped URL is a page. Also: a 404 page, generated into dist/404.html and noindexed, which Vercel serves with a 404 status. And a Diagrams link in the landing nav and footer, without which the prerendered pages were reachable only from search results and nothing on the site linked to them. package-lock.json picks up the license field it should have got when the project moved from MIT to AGPL.
Every serverless function has been returning 500 in production: ERR_MODULE_NOT_FOUND: Cannot find module '/var/task/api/_lib/supabaseAdmin' imported from /var/task/api/usage.js package.json sets "type": "module" and tsconfig uses moduleResolution "bundler", so extensionless relative imports typecheck and run locally. Vercel transpiles api/*.ts to ESM without rewriting specifiers, and Node's ESM loader does not guess extensions, so every function crashed on import. Hosted AI, usage metering, checkout, the billing portal and account deletion were all dead in production. It never showed up locally because the dev server loads these through Vite's ssrLoadModule, whose resolver maps a .js specifier onto the .ts file and also tolerates the bare one. Unrelated to the routing commit before it: the function bundles in the deployment that first exposed this are byte for byte the same size as the ones in the deployment before, and nothing here touched api/.
The equilibrium annotation sat on top of the line it was annotating.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideFixes production routing and serverless function failures by enforcing the correct Vercel build command, adding a static 404 page and diagrams navigation, and making all serverless imports Node-ESM compatible so routes and APIs work in production. Flow diagram for updated Vercel routing and static 404 handlingflowchart TD
Request["Incoming HTTP request (path)"]
Request --> CheckFile{"Static file exists in dist?"}
CheckFile -- Yes --> StaticFile["Serve static file (e.g. /diagrams/*, /404.html)"]
CheckFile -- No --> CheckSpaRoute{"Path matches SPA route (vercel.json rewrites)?"}
CheckSpaRoute -- Yes --> SpaRewrite["Rewrite to / (SPA index shell)"]
SpaRewrite --> SpaHandler["SPA handles client-side route"]
CheckSpaRoute -- No --> NotFound["Serve dist/404.html with 404 status\nrobots: noindex, follow"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates API imports for ESM resolution, centralizes client route matching, refines checkout interval parsing, narrows Vercel rewrites, generates a noindex 404 page, validates route coverage, and adds ChangesApplication routing and runtime
Deployment and SEO output
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoRestore Vercel routing, SEO builds, and serverless APIs
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The SPA routes now exist both in
SPA_ROUTES(for shell generation) and invercel.json(for rewrites); consider centralizing them in a single source of truth (or generating one from the other) to avoid future drift when routes change. - The 404 page’s large inline SVG and HTML string inside
render404Pagemake the generator script quite dense; consider extracting this markup into a separate template/module or at least breaking it into smaller helpers to keepgenerate-seo-pages.mjseasier to scan and modify.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The SPA routes now exist both in `SPA_ROUTES` (for shell generation) and in `vercel.json` (for rewrites); consider centralizing them in a single source of truth (or generating one from the other) to avoid future drift when routes change.
- The 404 page’s large inline SVG and HTML string inside `render404Page` make the generator script quite dense; consider extracting this markup into a separate template/module or at least breaking it into smaller helpers to keep `generate-seo-pages.mjs` easier to scan and modify.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Restores correct production behavior on Vercel by ensuring the full build pipeline runs (including SEO/static page generation), SPA routes resolve without the cleanUrls redirect trap, and serverless functions work under Node ESM by using explicit .js specifiers.
Changes:
- Configure Vercel to run
npm run buildand replace the SPA catch-all rewrite with explicit SPA route rewrites. - Enhance the SEO/static generator to support configurable
robotsand to emit a customdist/404.html. - Fix Vercel serverless function imports by adding explicit
.jsextensions for ESM module resolution; add “Diagrams” navigation entry points.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vercel.json | Ensures the full build runs on Vercel and rewrites only known SPA routes to /, allowing unknown paths to fall through to a real 404. |
| scripts/generate-seo-pages.mjs | Documents deployment behavior, adds configurable robots meta, and generates dist/404.html alongside sitemap + route shells. |
| package-lock.json | Updates lock metadata to reflect AGPL licensing. |
| components/LandingPage.tsx | Adds direct navigation links to the prerendered /diagrams hub. |
| api/webhooks/polar.ts | Adds explicit .js extensions for internal ESM imports. |
| api/usage.ts | Adds explicit .js extension for supabase admin import to fix ESM resolution on Vercel. |
| api/portal.ts | Adds explicit .js extensions for internal ESM imports. |
| api/generate.ts | Adds explicit .js extensions for internal ESM imports. |
| api/delete-account.ts | Adds explicit .js extensions for internal ESM imports. |
| api/checkout.ts | Adds explicit .js extensions for internal ESM imports. |
| api/_lib/supabaseAdmin.ts | Adds explicit .js extension for entitlement import to fix ESM resolution on Vercel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
/s/:slug matched any single path segment, but parsePath only treats
[A-Za-z0-9_-]{6,64} as a share route. Anything else, /s/x or /s/foo.bar,
was rewritten to the app and rendered as the landing page with a 200,
where it should have reached the 404 page.
The pattern mirrors parsePath rather than the narrower 24-hex format
shares are actually minted in. If the two disagreed, the edge would reject
links the client can still open, and the client is what decides whether a
share resolves.
Serving the SPA's routes by name is what lets an unknown URL return a real 404, but it means a route added to parsePath and to neither SPA_ROUTES nor vercel.json gets no shell and no rewrite. It then works perfectly in dev and 404s in production, which is the same shape as the build command override this branch started with: invisible locally, total in production. The generator now reads the routes out of parsePath and checks each is covered by a shell or a rewrite, and that some /s/ rewrite exists for share links. It refuses to pass vacuously: if the regex stops finding the routes, that is itself a failure rather than an empty list quietly satisfying the check. Verified by removing the /settings rewrite and separately by renaming the /terms shell; each fails the build naming the route, and both pass again once restored. Raised by review as SPA_ROUTES and vercel.json duplicating each other. They do not overlap: one holds the routes with prerendered shells, the other those without. The risk is not drift between them but a route in neither, which is what this checks. The review also suggested extracting the 404 markup; every other page in this file is built the same way inline, so moving one of them out would make the file less consistent, not more.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Both fair. The guard matched `pathname === 'literal'` in App.tsx, so a
route written any other way was silently absent from a check whose whole
job is catching absent routes, and it leaned on a hard floor of five
routes to notice when the scrape stopped working, which couples build
validity to how many routes the app happens to have.
Both problems are the regex. routes.mjs now holds the route table and the
share pattern; App.tsx routes with it and the generator imports it. There
is nothing left to extract, so nothing to miscount, and a route cannot be
spelled in a way the guard cannot see.
While the table was moving, the share pattern became checkable too: the
guard compares the character class in routes.mjs against the one in the
vercel.json rewrite. A stricter edge rejects links the client can open, a
looser one hands the app URLs it will not route, and those render as the
landing page rather than a 404. That agreement was a comment before.
Verified by breaking each branch in turn: a route with nothing serving it,
a removed rewrite, an edge pattern narrowed to {24,24}, and no /s/ rewrite
at all. Each fails the build naming the specific fault, and the build
passes again once restored.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@App.tsx`:
- Around line 84-94: Update parsePath to handle the root pathname explicitly as
the landing view, while preserving configured routes and SHARE_PATH matches. For
every other unmatched pathname, return the 404 state instead of defaulting to
landing, using the existing ViewType value or 404 rendering mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7073e17a-302a-4f46-92e3-e7dd66221e64
📒 Files selected for processing (4)
App.tsxroutes.mjsscripts/generate-seo-pages.mjsvercel.json
🚧 Files skipped from review as they are similar to previous changes (1)
- vercel.json
navigateToView took any ViewType and pushed `/${newView}`, which assumes both
that a view's name is its path segment and that every view has a path. 'shared'
breaks the second: you reach it by opening a share link, there is no /shared
rewrite, so pushing it would work until the first reload and then 404 at the
edge. No caller does that today, but nothing stopped one.
Read the path out of the shared route table instead, and exclude 'shared' from
the parameter type so navigating somewhere unservable is a compile error rather
than a production-only 404.
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The previous commit's comment claimed excluding 'shared' made an unservable navigation a compile error. That holds for 'shared' alone: RoutableView subtracts from ViewType, so a view added later joins it automatically, and the `?? '/'` fallback would then push / while showing the new view, a URL and view mismatch that hides the missing route until someone reloads. Resolve the path before committing to the view, and refuse to navigate if there is none. Staying put is poor, but it is a visible fault rather than a URL that 404s on the next reload. That is the runtime backstop. The build guard now does the real work, checking ViewType against the route table so the case cannot ship: it previously only checked table -> deployment, which cannot see a view the table never mentions.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
Two gaps in yesterday's guard, both found by review. Reading only single-quoted members meant a double-quoted ViewType would parse to an empty list and satisfy the check vacuously, which is the same failure the "could not find the alias" branch exists to prevent. Quote style is formatting, so accept either. The view -> table check also only reported views with no route. Retyping an existing view's name is caught by that, since the real view then looks stranded, but adding an entry that names a view which does not exist leaves coverage intact and passed: `'/studio': 'studo'` built cleanly, would have been served, and would have rendered as an unknown view rather than a 404. Compare the other way as well so the two sets have to agree exactly.
A production-only bugfix release: the deployed site served a 404 for every page but the homepage, and none of the API functions could start. Both were invisible locally, so 1.1.0 shipped with them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Production was serving only
/. Found while spot-checking the live site after the 1.1.0 merge.Three faults
1. SEO output never built. The Vercel project's build command is overridden in the dashboard to
vite build, soscripts/generate-seo-pages.mjsnever ran in CI. No diagram pages, no route shells, nositemap.xml. Fixed by settingbuildCommandinvercel.json, which is version controlled and takes precedence over the dashboard.2. The SPA fallback could not work with
cleanUrls. The rewrite sent everything to/index.html. Compiled,cleanUrlsemits a 308 from/index.htmlto/ahead of the filesystem, and rewrites carrycheck: true, so the rewritten path re-entered the route table, hit that redirect and resolved to nothing. Share links,/settingsand/editorwere all dead.The rewrites now name the SPA's routes and target
/. Enumerating them rather than catching everything means anything else reaches a real 404 instead of answering with the app and a 200, which would tell crawlers every mistyped URL is a page.3. Every serverless function returned 500, pre-existing and unrelated to the above:
package.jsonsets"type": "module"and tsconfig usesmoduleResolution: bundler, so extensionless relative imports typecheck and run locally. Vercel transpilesapi/*.tsto ESM without rewriting specifiers, and Node's ESM loader does not guess extensions. Hosted AI, usage metering, checkout, the billing portal and account deletion were all dead. Hidden locally because the dev server loads these through Vite'sssrLoadModule, whose resolver maps.jsonto.ts.Also
dist/404.html, noindexed, served by Vercel with a real 404 status.package-lock.jsonpicks up the license field it should have got when the project moved from MIT to AGPL.Verified on production
All 18 sitemap URLs return 200. All 12 links on the 404 page return 200.
/nopeand/diagrams/nopereturn 404 with the new page./api/usagereturns 401{"error":"Not signed in."}and the POST-only endpoints return 405, none 500.Important
Production is currently running a CLI deploy from this branch, so it is fixed right now but ahead of
main. Any deploy triggered frommainbefore this merges will put the outage back.Summary by Sourcery
Ensure production serves all SPA routes, static SEO pages, and API endpoints correctly instead of returning 404/500 errors.
New Features:
Bug Fixes:
Enhancements:
Build:
Summary by CodeRabbit
/404page.