Stand up Supabase locally, with an empty database - #32
Conversation
Adds the local half of the Supabase stack and proves it end to end: a migration, a client, generated types, and a page that reads a row and renders it. A connection nobody has queried is not a connection. The CLI is a devDependency rather than a global install, for the same reason the Vercel CLI is: in the lockfile it comes under the 7-day release floor and each bump is a reviewable diff, and a tool that writes migrations should not be whatever version a machine happens to have. It needs an onlyBuiltDependencies entry — its postinstall downloads the platform binary, and blocked, the install reports success and the failure surfaces much later at pnpm exec. Two things found by running it rather than by reading about it: A table created in a migration is unreadable through the API until it is granted, and the failure reads as a broken policy: Postgres checks the privilege before it evaluates RLS, so a correct policy with no grant returns 42501 and the policy is never consulted. The public schema carries two sets of default privileges, and migrations run as postgres, which is the set that grants anon no read at all. Dashboard-created tables land under the other one, which is why most advice never mentions grants. This is the same privilege layer the verified rule depends on, so it is documented rather than just fixed. NEXT_PUBLIC_ variables are inlined into the client bundle at build time, but a server read during dynamic rendering still comes from the runtime environment. So the build stays environment-free — verified by building with none set — while the deployment genuinely needs them. That stops being true the moment a client component reads the key, which is where auth is heading. /connection-check is dynamic via connection(), which keeps it out of prerendering and is the case AGENTS.md reserves that call for. Everything else stays static. ESLint now ignores supabase/.temp: the CLI writes a bundled Deno entrypoint there while the stack runs, and ESLint does not read nested .gitignore files. Both README.md and AGENTS.md claimed the build needs no environment variables. Half of that is still true and the other half was load-bearing, so both are corrected rather than deleted. The practitioners table, its policies and the verified write protection are deliberately not here — see #9 and #14. So is the hosted project. Refs #31
The file asserted the local keys are identical on every machine. They are, but the reason was worth checking rather than assuming: they are string constants compiled into the CLI binary, verified by grepping it for the publishable key, the secret key and the demo JWT secret. All three are embedded. That makes the pin in pnpm-lock.yaml the thing keeping it true, which is another argument for the CLI being a devDependency rather than a global install.
Adds `supabase` and `supabase-postgres-best-practices` from [supabase/agent-skills](https://github.com/supabase/agent-skills), so guidance on RLS, migrations and Postgres schema work is in the repo rather than in whatever each contributor happens to have installed locally. Split out of #32 to keep that PR to the walking skeleton. No dependency between them; either can merge first. ## Why these two - **`supabase`** — client libraries and `@supabase/ssr`, auth and sessions, RLS, migrations, Edge Functions, and debugging workflows for when something returns a status nobody expected. - **`supabase-postgres-best-practices`** — Postgres rather than Supabase specifically: column types, indexes, RLS policies and the tests that prove them, and migration patterns that avoid taking a lock on a production table. The second is the more directly useful of the two here. The `verified` column rule under Database in `AGENTS.md` — column privileges plus a `before update` trigger, because RLS has no `OLD` to compare against — is exactly the class of problem it covers, and #9 and #14 are heading straight into it. ## How they are stored Not as ordinary directories, which is the one thing worth a reviewer's attention. The real files live in `.agents/skills/<name>/`; `.claude/skills/<name>` is a **symlink** into it. That lets one copy serve any agent tool that reads its own directory instead of duplicating ~200 KB per tool. Git stores the symlinks natively as mode `120000` — worth confirming they resolve on your machine when you pull. On Windows this needs `core.symlinks` enabled. The Node section already points Windows contributors at WSL for unrelated reasons, so this is one more, and `AGENTS.md` now says so. `skills-lock.json` pins each skill by **content hash**, not version. Do not trust the `version` field inside a `SKILL.md`: upstream leaves it stale — `supabase` declares `0.1.2` in frontmatter against a changelog whose newest entry is `0.1.7`. The hash is the real pin. ## Review note 44 files, but only two are ours to review: `AGENTS.md` and `skills-lock.json`. Everything under `.agents/` is vendored upstream content, and `AGENTS.md` now states that it is not ours to edit — a local change there is reverted by the next update with no diff left to explain it.
davidtaing
left a comment
davidtaing
left a comment
There was a problem hiding this comment.
Automated review — pass run by Codex (codex CLI), verified and posted by Claude Code (Opus 5). Every finding was checked against the source before posting. Caveat worth stating: this PR was authored in the same session, so the areas reported clean are weaker evidence than an independent review would be.
The load-bearing claims hold. I reproduced the environment-free build, the write refusals at the privilege layer, and the generated types matching the pinned CLI, independently of the PR description.
Codex's pass returned no findings. Both below came out of the verification step.
1 major, 1 minor.
Most useful next step: fix the db:types redirect before merge. It is the one thing here that destroys a committed file, and it does it on the ordinary mistake of forgetting pnpm db:start — with an error message that never mentions the file it just overwrote.
Two findings from the review on #32. db:types redirected straight into src/lib/database.types.ts, so the shell truncated the file before the CLI ran. The CLI writes its failure to stdout, so a run with the stack down did not merely empty the file — it replaced it with a JSON error object, and the only console output was ELIFECYCLE with no mention of the file. The next build then failed with type errors in a file nobody had edited. Forgetting db:start first is the ordinary case, not an unusual one. gen types has no --output flag, so the script writes to a sibling and moves it into place only on success, removing the temporary file otherwise. Verified both ways: with the stack down the committed file is byte-identical afterwards and no stray file is left; with it up the regenerated output matches what is committed. The README now says both db:reset and db:types need the stack running. The shared client took auth-js's defaults, which assume a browser where one client belongs to one person. This one is cached at module scope and serves every request in the process, so a session stored on it would be visible to all of them. Nothing signs in through it today; the options are set now because the next ticket adds auth and the path of least resistance is to reuse the client that already exists. Signed-in reads get their own cookie-backed client.
Review threads resolved — 6ade0a7Both findings held, both fixed, both threads replied to and resolved. Nothing deferred, nothing rejected.
Verified both directions rather than the happy path alone: stack down leaves the committed file byte-identical with no stray Client inheriting auth-js browser defaults (minor). Now explicitly
Still open on this PRUnchanged by the above, and not blockers for reviewing the code:
|
… rule The connection_check migration is deliberately not pushed to the hosted project. A health-check table that gets dropped once the practitioners table lands would sit in the production migration history permanently, and since nothing has been applied there yet, the migration can simply be deleted when that happens rather than needing a second migration to undo it. It stays a local fixture, rebuilt by db:reset. That makes the page's two states wrong. Against the hosted project PostgREST returns PGRST205, which means it answered and accepted the key and only the table is absent — a pass for everything this page tests. Reporting it as "Not connected" would call a working deployment broken, which matters now that the Vercel variables are set. Three states now: read, reachable, unreachable. Verified against all three, including the real hosted project: local reads the row, hosted reports reachable with PGRST205, a bad host reports unreachable with fetch failed. Correcting a claim this PR previously made in AGENTS.md and README, which was wrong in a way that would mislead. NEXT_PUBLIC_ variables are not client-only at build time — Next inlines them by literal substitution wherever they are defined, the server bundle included. Confirmed by grepping .next/server after a build and finding the URL and key as string literals. A built artifact therefore ignores the environment it is later started with. The earlier claim came from observing that a build made with no variables does read them at run time. That is true, but it is the exception rather than the rule: only a variable absent at build time survives as a runtime lookup. Testing a build against a different project means rebuilding, and on Vercel the values must exist before the build, which vercel pull already ensures.
The migration is deliberately not being pushed — 70c22a3Raised in review and it holds: a health-check table does not belong in the hosted migration history. It would sit there permanently to describe a table dropped a fortnight later, when the practitioners schema lands. What makes this cheap: nothing has been applied to the hosted project yet — Which made the page wrongAgainst the hosted project PostgREST returns Three states now, all verified end to end rather than reasoned about:
So #31's hosted half is now provable without putting anything in the production schema: a preview deployment showing "Reachable" is the honest claim that URL and key are right. The full read arrives with the first real table. Correcting something this PR got wrongEarlier commits claimed, in both Next inlines these by literal substitution wherever they are defined at build time, server bundle included — confirmed by grepping The original claim came from observing that a build made with no variables does read them at run time. True, but that is the exception, not the rule: only a variable absent at build time survives as a runtime lookup. I only caught it because testing the three states above by restarting one build with different variables produced three identical results. Consequences, now written down: testing a build against a different project means rebuilding, and on Vercel the values must exist before the build — which |
The connection_check table was invented to give the skeleton something to read. It was never going to survive: the schema starts with practitioners, and a health-check table would have sat in the migration history permanently, describing a table dropped a fortnight later, to prove something the first real query proves for free. It is out before it was ever pushed, so the hosted history will start at the first real migration with nothing to undo. Removing it took the page's justification with it. Without a table, the check could only ask whether Supabase answered at all, and doing that honestly needed a probe distinguishing four outcomes, a raw fetch to sidestep a typed query builder that will not name a table it does not know, and a caveat that the local gateway does not validate the key. That is a lot of apparatus to prove a URL resolves, and every line of it would have been deleted by the first real read. So the database ships empty. What remains is the part with a life beyond this PR: the CLI under the release floor, the local stack, the lazy client and its constraints, type generation, and the environment wiring. database.types.ts describes no tables, which is the honest output of an empty schema rather than a placeholder. #31 asked for a Server Component reading something trivial, on the assumption the fixture would be committed. That assumption is gone, so the requirement goes with it rather than being satisfied by machinery built for the purpose. The first query is the practitioners read.
davidtaing
changed the title
Two files, salvaged from #38 — which is closed, because the `keystones` skill belongs in a personal skills repository rather than this one. These two do not. ## The invariants `AGENTS.md` gains an **Invariants beat scope** section. It is the durable output of building the Supabase skeleton, and it holds regardless of where the skill that produces invariants ends up living. Scope is hard to set upfront, because the constraints that matter usually only become visible once the work has started — so "scope harder next time" is the wrong lesson to draw from work that sprawled. Invariants do not have that problem: they say what is unacceptable rather than what to build, so they can be stated before anyone knows the shape of the answer. The line carrying the most weight is the last one: > When a premise is removed, revisit the requirement that rested on it rather than building machinery to keep satisfying it. #32 is cited as the worked example — a `connection_check` table added to give a walking skeleton something to read, and when it was cut for not belonging in migration history, the page that read it grew a four-state probe rather than being cut too. Both went in the end. A rule with an incident attached is easier to apply than one without. ## The ignore rule Unrelated, but found the same way. `.claude/*` with `!.claude/skills/`. Only `.claude/skills` is tracked and nothing else in there is shared: local settings, session state, and worktrees — which carry their own `.git` file, so `git add -A` stages one as an embedded repository and would commit a broken pointer with none of the contents. Found by doing exactly that while preparing #38. One detail worth not undoing: it is `.claude/*`, **not** `.claude/`. A trailing-slash ignore stops git descending into the directory at all, and a negation cannot rescue what git never looked at — so the version most people write first silently ignores the skills too. Verified with `git check-ignore` rather than by reading it: ``` .claude/skills/code-tour/SKILL.md visible .claude/skills/supabase visible .claude/worktrees/ci-skip-draft-prs IGNORED .claude/settings.json IGNORED ``` Also confirmed nothing already tracked became ignored — the failure that stays invisible until someone deletes a file and finds git no longer cares. Only `.claude/skills` is tracked today, so this changes nothing now and prevents the accident later. If a settings file with a permissions allowlist becomes worth sharing, add another `!` exception rather than loosening the rule. ## Checks `pnpm lint` passes. No application code touched.
1 participant
Adds the local Supabase stack and the wiring the app needs to talk to it. The database ships empty — there are no migrations, nothing queries anything, and that is the intended end state of this PR rather than an unfinished one.
Refs #31.
What is here
supabaseas a devDependency, not a global install, for the same reason the Vercel CLI is one: in the lockfile it comes under the 7-day release floor and each bump is a reviewable diff, and a tool that writes migrations should not be whatever version a machine happens to have. It needs anonlyBuiltDependenciesentry — its postinstall downloads the platform binary, and blocked, the install reports success while the failure surfaces much later atpnpm exec.supabase/config.tomland the local stack, withpnpm db:start/db:stop/db:reset/db:types.src/lib/supabase.ts— a lazygetClient()cached at module scope, perAGENTS.md, withpersistSessionandautoRefreshTokenoff because it is anonymous and shared across every request in the process.src/lib/database.types.ts, generated bypnpm db:types. It describes no tables, which is the honest output of an empty schema..env.example, and a README setup section written from what was actually run — including the Docker prerequisite and why it differs by platform.Why there is no table
An earlier revision of this branch had a
connection_checktable and a/connection-checkpage that read from it. Both are gone, and the reasoning is worth recording because it will come up again.A health-check table would have to live in the migration history permanently, describing a table dropped a fortnight later, to prove something the first real query proves for free. Once it stopped being committed, the page could only ask whether Supabase answered at all — and doing that honestly needed a four-state probe, a raw
fetchto sidestep a typed query builder that will not name a table it does not know, and a caveat that the local gateway does not validate the publishable key. A lot of apparatus to prove a URL resolves, all of it deleted by the first real read.#31's "a Server Component reads something trivial from the local database" was written assuming the fixture would be committed. That assumption is gone, so the requirement goes with it rather than being met by machinery built for the purpose.
Two things found by running it rather than reading about it
A table created in a migration is unreadable through the API until it is granted, and the failure reads as a broken policy. Postgres checks the privilege before it evaluates RLS, so a correct policy with no grant returns
42501 permission deniedand the policy is never consulted. Thepublicschema carries two sets of default privileges:Migrations run as
postgres. Dashboard-created tables land under the other set, which is why most advice never mentions grants. This is the same privilege layer theverifiedrule depends on, so it is inAGENTS.mdeven though the migration that taught it is gone.NEXT_PUBLIC_*is inlined at build time into the server bundle too, not just the browser one. Confirmed by grepping.next/serverafter a build and finding the URL and key as string literals — so a built artifact ignores the environment it is later started with. The exception, and the thing that misled an earlier commit on this branch: a variable absent at build time does survive as a runtime lookup, which is whynext buildwith nothing set still produces something that works when given values at run time. Consequence: pointing a build at a different project means rebuilding, and on Vercel the values must exist beforevercel build— whichvercel pullalready ensures.Checks
pnpm lintandpnpm buildpass.next buildsucceeds with no environment variables set — the point of the lazy client.pnpm db:startfrom clean pulls the stack and comes up;db:resetanddb:typeswork against it.ESLint now ignores
supabase/.temp, where the CLI writes a bundled Deno entrypoint while the stack runs — ESLint does not read nested.gitignorefiles, and without thispnpm lintfails with ~150 errors in a file nobody wrote.Not here
The
practitionerstable, its RLS policies and theverifiedwrite protection — #9 owns the model and #14 the submission flow. Nothing has been pushed to the hosted project, so its migration history will start at that first real migration.