Document the Supabase work that needs project access - #167
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1264b3437
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Both Codex findings were real and are fixed in the latest commit. Thanks — the second one would have sent someone down a wrong path during the urgent step. P1 — row check before the function that writes rows exists. Correct, and it went further than the check itself: the document told the operator that deploying the Edge Function restores error reporting. It does not. The function stores nothing on its own — it calls Split accordingly: reachability at the deploy step (with an explicit note that no row will land yet, and why), storage verification after the migration. The order still recommends the deploy first, but now for the honest reason — it is cheap and unblocked, while the migration is tangled up in the version mismatch. P2 — One more caught while checking: the first draft of this fix used Steps renumbered 1a–1e. |
Three things are pending that a checkout cannot do on its own, and one of them is time-sensitive: PR #164 is merged and deployed, so the production bundle already calls an Edge Function that does not exist yet. Error reports are being dropped, and the anon INSERT grant the PR exists to remove is still in place until the migration is applied. Also records the second half of the preview-check story. PR #166 fixes the is_admin() ordering; the check then fails on public.profiles, which is referenced by five migrations and defined by none. Closing that needs the real table definition dumped from production — reconstructing it from the columns the migrations happen to touch would turn the check green while describing a schema that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Supabase check reports two different failures depending on where it runs, and the runbook only covered one of them. On a pull request it builds a database from the migration files and dies on public.profiles. On main it syncs to production and dies on "Remote migration versions not found in local migrations directory" — which it has been doing since at least 7 August, before any of the recent work. That second failure matters for task 1: nothing has synced to production for weeks, so db push is not going to apply the migration cleanly the way the document originally implied. Added the diagnosis, the migration list step, and the choice between capturing a remote-only version and discarding the record — with a warning against reaching for migration repair to silence the error, since that is how the drift got this far. Also frames the whole document around that drift: the repository and the production database disagree in both directions, and every task here is a symptom of it. Status table refreshed — #165 and #166 are merged, nanoid opened a new high alert today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both from the Codex review on this PR, and both were real. The db commands were written with --project-ref, which they do not accept. `db dump`, `migration list`, `migration repair` and `db push` select their target with --linked / --db-url / --local; only `functions deploy` and `secrets set` take --project-ref. As written, three of the commands would have exited on an unknown flag. Added a `supabase link` step up front, switched the db and migration commands to --linked, and said explicitly which two commands are the exception so the next person does not "fix" them back. The bigger one: the document told the operator to deploy the function to restore error reporting, then confirm a row had landed. Neither holds. The function stores nothing by itself — it calls record_client_error(), which the migration creates — so deploying it alone restores nothing, and the row check would have failed and made the urgent recovery step look broken. Split the verification: reachability at the deploy step, storage after the migration, with the reason stated in both places. The ordering advice stands, but for the honest reason — the deploy is cheap and unblocked while the migration is tangled up in the version mismatch. While checking flags against the CLI reference I found I had reached for `supabase functions logs` in the fix itself; there is no such subcommand, so that now points at the dashboard and says so. Steps renumbered 1a–1e. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No dependabot alerts remain open. What is left in the runbook is now exactly the work that needs Supabase project access, which is worth saying explicitly so the next reader does not re-audit the closed items. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8d8bce5 to
fea1836
Compare
1 participant
Adds dev-plans/supabase-pending-work-2026-08.md — a runbook for the steps that need the Supabase CLI logged in against the production project, so they can be picked up from a machine that has it.
The reason it is worth landing quickly: PR #164 is merged and Vercel has deployed
main, so the production bundle already callsreport-client-error— a function that has not been deployed. Client error reports are being dropped right now (caught and logged, nothing crashes), and theanonINSERT grant that PR exists to remove is still in place until20260816000000is applied. Neither is an emergency; both should be short-lived.The document covers:
ERROR_LOG_IP_SALT, apply the migration, and four SQL checks plus acurlthat proves the direct anon write path is actually closed.profilesgap — why the preview check is still red after Bootstrap the is_admin() stub before anything references it #166, why it needs a dump from production rather than a reconstruction, where the definition should go, and how to verify it locally with the same Docker harness used for Bootstrap the is_admin() stub before anything references it #166.It ends with a status table for the four open/merged PRs and the dismissed alerts.
🤖 Generated with Claude Code