fix(ops): generated migration manifest, E2E route sweep, and the Add person role fix - #6
Merged
Merged
Conversation
…sweep Migration 0042_site_content_cms was never applied to production, so pages reading tenant site content threw P2022 "column does not exist". The real bug was the safety net: /api/readyz compared applied migrations against a HAND-MAINTAINED list that stopped at 0035 while the repo had grown to 0049, so readiness reported a false green with 14 migrations missing. - Generate EXPECTED_PRODUCTION_MIGRATIONS from prisma/migrations (scripts/generate-migration-manifest.mjs). A generated list cannot go stale; a hand-maintained one did. - Regenerate the manifest in run-build.mjs before next build, so every deploy ships a current list. The build still needs no database access. - CI guard: `npm run migrations:check` fails when a migration was added without regenerating the manifest. - Degrade gracefully: the site-content editor renders an explanatory banner instead of a 500 when the columns are missing. - Playwright suite: load every public/admin/portal route in a real browser and fail on any 5xx, error boundary, or console error — the class of failure unit tests and typecheck cannot see. health.spec.ts asserts migrations.missing === [] against a deployment. Docs: incident writeup, deploy pipeline (migrations run in a release step, before app code), provisioning spec, UI/Paystack fix notes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…picked The Role <select> is controlled, and its onChange deferred setSelectedRole into a setTimeout that read e.target.value lazily, returning a cleanup function React ignores. React restores a controlled select to its current state right after the event, so by the time the timeout ran the DOM value had already snapped back to STAFF — and that is what the form posted. Update state synchronously instead. Same deferred-handler pattern fixed in add-buyer-form's profile toggle. The setTimeout calls inside useEffect are a deliberate pattern here (avoiding set-state-during-render) and are left alone, as are the genuine delays (copy feedback, progress animation). The server action was already correct: it reads `role` from the FormData with no fallback, and provisionCompanyUser rejects a role the actor may not create rather than defaulting. Regression test: e2e/add-person-role.spec.ts picks Finance, waits out any deferred revert, and asserts the submitted server-action payload carries role=FINANCE. It intercepts that request, so the test creates no Clerk account and sends no invite. Verified it fails on the old handler (expected "FINANCE", received "STAFF") and passes on the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…rver noise The suite could never pass in CI. Three harness faults, no app changes: - Public pages resolve their tenant from the HOST, and marketing routes deliberately ignore the DEFAULT_COMPANY_SLUG fallback — only a tenant host counts. Against 127.0.0.1 every tenant page rendered the platform site or 404'd, so the sweep failed on nearly every public route. Drive the seeded demo tenant through <slug>.localhost, which is the supported dev tenant host (resolveTenantSubdomainFromHost). - Windows, and some Linux setups, do not resolve *.localhost, so map it to loopback in the browser itself rather than trusting the OS resolver. The webServer readiness probe keeps using 127.0.0.1, which Node can resolve. - `next dev` logs a hot-reload WebSocket console error in headless CI, and the sweep counts any console error as a defect. It is dev-server noise, not an application fault, so it joins the ignore list. Also raise the per-test timeout to 120s: a route's first request compiles it (30s+ for admin routes on a cold machine), and the old 60s budget made the first visit time out and abort the rest of the file's navigations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… dev The suite ran against `next dev`, which compiles each route on its first request. On a 2-core runner that is 30s+ per admin route, so tests timed out one after another and the job hit its 30-minute limit — 49 failures, none of them real. Build once, then serve that build with NODE_ENV=test. The test value keeps the dev-session bypass available (allowDevBypass in src/lib/config.ts treats "test" as non-production), so the authenticated sweep still runs rather than skipping itself on a production build. Locally the default stays `npm run dev` so the suite still picks up edits; CI overrides it with E2E_SERVER_COMMAND. Measured on the same machine, same suite: next dev 49 failed, 14 passed, 33 min production build 1 failed, 61 passed, 3.8 min (the one failure is this machine's database being behind — exactly what health.spec.ts exists to report; CI migrates a throwaway database first.) Also: accept either submit transport in the Add person regression test. A hydrated page sends a server action (Next-Action header, multipart); before hydration the same form posts natively (url-encoded). The test asserts which role was submitted, so it should not care which transport carried it — it only failed on a production build because hydration timing differs. The *.localhost resolver mapping now applies to external targets too; it only affects that suffix, so it is inert against a deployed environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch was successfully deployed
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
1 participant
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.
Two independent fixes that were sitting uncommitted in the working tree.
1. Migration drift could report a false green
0042_site_content_cmswas never applied to production, so pages reading tenant site content threw P2022 "column does not exist". The real bug was the safety net:/api/readyzcompared applied migrations against a hand-maintained list that stopped at0035while the repo had grown to0049. Readiness reported"missing": []while 14 migrations were absent.EXPECTED_PRODUCTION_MIGRATIONSis now generated fromprisma/migrations(scripts/generate-migration-manifest.mjs). A generated list cannot go stale; the hand-maintained one did.run-build.mjsregenerates it beforenext build, so every deploy ships a current list. The build still requires no database access.npm run migrations:checkfails the build when a migration is added without regenerating the manifest.e2e/): loads every public/admin/portal route in a real browser and fails on any 5xx, error boundary, or console error — exactly the class of failure unit tests and typecheck cannot see.health.spec.tsassertsmigrations.missing === []against a deployment.Docs included: the incident writeup, the deploy pipeline (migrations run in a release step before app code), the provisioning spec, and UI/Paystack fix notes.
2. "Add person" always submitted Front Desk
/admin/users→ Add person postedSTAFFno matter which role was picked.The Role
<select>is controlled, and itsonChangedeferredsetSelectedRoleinto asetTimeoutthat reade.target.valuelazily, then returned a cleanup function React ignores. React restores a controlled select to its current state right after the event, so by the time the timeout ran the DOM value had already snapped back toSTAFF— and that is what got submitted.Fixed by updating state synchronously; same pattern fixed in
add-buyer-form's profile toggle. ThesetTimeoutcalls insideuseEffectare a deliberate pattern in this codebase and are left alone, as are genuine delays (copy feedback, progress animation).The server action was already correct: it reads
rolefrom the FormData with no fallback, andprovisionCompanyUserrejects a role the actor may not create rather than defaulting.Regression test (
e2e/add-person-role.spec.ts): picks Finance, waits out any deferred revert, and asserts the submitted server-action payload carriesrole=FINANCE. It intercepts that request, so no Clerk account is created and no invite is sent. Verified it fails on the old handler (expected"FINANCE", received"STAFF") and passes on the fix.Verified:
npm run checkgreen (535 tests, typecheck, lint, build) andnpm run migrations:checkclean (51 migrations).🤖 Generated with Claude Code