Skip to content

fix(supabase): wrap REVOKE+GRANT in DO block to fix prod migration push#41

Merged
fyoosh merged 1 commit intomainfrom
fix/migrations-parser-workaround-do-block
Apr 30, 2026
Merged

fix(supabase): wrap REVOKE+GRANT in DO block to fix prod migration push#41
fyoosh merged 1 commit intomainfrom
fix/migrations-parser-workaround-do-block

Conversation

@fyoosh
Copy link
Copy Markdown
Contributor

@fyoosh fyoosh commented Apr 30, 2026

Summary

Production hotfix for PR #40. supabase db push against production failed at migration 20260430000002_grant_claim_pairing_atomic.sql with Postgres SQLSTATE 42601 ("cannot insert multiple commands into a prepared statement"). The Supabase CLI v2.90 prepared-statement parser sometimes sends multi-statement migration files as a single Parse message; branch-review's addition of three REVOKE statements to the GRANT file pushed it from 1 to 4 statements, retriggering the bug.

Fix: wrap REVOKE+GRANT in DO $$ ... $$ blocks. Block is one statement from the parser's perspective. Functional semantics unchanged.

Test plan

  • supabase db reset --local applies all 4 of the 0001-0004 migrations cleanly (previously died at 0002).
  • \df+ public.claim_pairing_atomic confirms postgres + service_role have EXECUTE; PUBLIC, anon, authenticated revoked.
  • npx vitest run tests/lib/pairing.test.ts — 17/17 pass.
  • Smoke harness against real local Supabase + real Upstash: 10x concurrent same-user same-code → 10/10 success, 1 device row, Redis token hash matches devices.api_token_hash.
  • Post-merge: supabase db push against production succeeds; \df+ public.claim_pairing_atomic against production confirms ACLs.

Deploy / migration notes

  • After merge: supabase migration list should show 0001 already applied (production), 0002/0003/0004 pending. supabase db push applies the remaining three. The fixed 0002 will apply cleanly this time.
  • In-place edit of merged migration files is justified here because the broken content was NEVER successfully applied in any environment (push died at 0002, developers' local resets also died at 0002 after branch-review). No one's database has the broken-shape applied, so there's no immutability constraint to violate.
  • Rollback plan: revert PR + DROP FUNCTION public.claim_pairing_atomic(uuid, uuid, text); DROP FUNCTION public.rollback_claim_pairing(uuid, uuid); against production.

Reviewer notes

  • The parser bug is a known Supabase CLI v2.90 quirk (we saw it earlier in this audit cycle when the original claim_pairing_atomic migration with CREATE+GRANT+COMMENT in one file failed; the DO-block pattern is now applied to both 0002 and 0004 to make all four migrations bulletproof against the parser).
  • Follow-up issue P7 tracked locally in the audit doc: wire supabase db reset --local into pre-commit or CI so this class of regression hits in branch-review instead of production.

🤖 Generated with Claude Code

…ession

The Supabase CLI v2.90 prepared-statement parser sometimes sends
multi-statement migration files as a single Parse message, hitting
Postgres SQLSTATE 42601 ("cannot insert multiple commands into a
prepared statement"). The trigger is inconsistent — files with the
exact same shape sometimes parse cleanly, sometimes not.

PR #40 (B-atomic) shipped four migrations that survived local
`supabase db reset` but production `supabase db push` died on
20260430000002. Branch-review had added three REVOKE statements to the
GRANT file, pushing it from 1 statement (single GRANT) to 4 statements;
the parser bug triggered on the multi-statement form during push even
though local tests had passed when the file was single-GRANT only. The
regression is invisible to vitest (unit tests don't apply migrations
against a fresh DB) and to CI (no migration smoke step). First
exercise of the new file shape was production push.

Fix: wrap REVOKE+GRANT in DO $$ ... $$ blocks. The block is one
statement from the parser's perspective, so multi-statement files
collapse to single-statement files at the protocol layer. Functional
semantics unchanged — REVOKE/GRANT inside a plpgsql DO block execute
identically to bare REVOKE/GRANT. Applied to both:

  - 20260430000002_grant_claim_pairing_atomic.sql (3 REVOKE + 1 GRANT
    → 1 DO block).
  - 20260430000004_create_rollback_claim_pairing_fn.sql (CREATE +
    3 REVOKE + 1 GRANT + 1 COMMENT → CREATE + 1 DO block + COMMENT,
    matching the 3-statement shape that was already known to parse).

Verified locally:
  - `supabase db reset --local` applies all 4 of the 0001-0004
    migrations cleanly (previously died at 0002).
  - `\df+ public.claim_pairing_atomic` confirms postgres + service_role
    have EXECUTE; PUBLIC, anon, authenticated revoked.
  - 17/17 pairing tests pass.
  - Smoke harness against real local Supabase + real Upstash:
    10x concurrent same-user same-code → 10/10 success, 1 device row,
    Redis hash matches DB hash.

In-place edit of merged migration files is justified here because the
content has NEVER successfully applied in any environment (production
push died at 0002, every developer's local push died at 0002 after
branch-review). No one's database has the broken-shape migration
applied, so there's no immutability constraint to violate.

Follow-up audit issue (P7) tracks adding a CI guard: `supabase db
reset --local` against a fresh local Supabase pre-merge would have
caught this class of regression in branch-review, not in production.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
web Ready Ready Preview, Comment Apr 30, 2026 2:37pm

@fyoosh fyoosh merged commit 3df1b61 into main Apr 30, 2026
2 checks passed
@fyoosh fyoosh deleted the fix/migrations-parser-workaround-do-block branch April 30, 2026 14:39
fyoosh added a commit that referenced this pull request Apr 30, 2026
* fix(supabase): wrap REVOKE+GRANT in DO block to dodge CLI parser regression

The Supabase CLI v2.90 prepared-statement parser sometimes sends
multi-statement migration files as a single Parse message, hitting
Postgres SQLSTATE 42601 ("cannot insert multiple commands into a
prepared statement"). The trigger is inconsistent — files with the
exact same shape sometimes parse cleanly, sometimes not.

PR #40 (B-atomic) shipped four migrations that survived local
`supabase db reset` but production `supabase db push` died on
20260430000002. Branch-review had added three REVOKE statements to the
GRANT file, pushing it from 1 statement (single GRANT) to 4 statements;
the parser bug triggered on the multi-statement form during push even
though local tests had passed when the file was single-GRANT only. The
regression is invisible to vitest (unit tests don't apply migrations
against a fresh DB) and to CI (no migration smoke step). First
exercise of the new file shape was production push.

Fix: wrap REVOKE+GRANT in DO $$ ... $$ blocks. The block is one
statement from the parser's perspective, so multi-statement files
collapse to single-statement files at the protocol layer. Functional
semantics unchanged — REVOKE/GRANT inside a plpgsql DO block execute
identically to bare REVOKE/GRANT. Applied to both:

  - 20260430000002_grant_claim_pairing_atomic.sql (3 REVOKE + 1 GRANT
    → 1 DO block).
  - 20260430000004_create_rollback_claim_pairing_fn.sql (CREATE +
    3 REVOKE + 1 GRANT + 1 COMMENT → CREATE + 1 DO block + COMMENT,
    matching the 3-statement shape that was already known to parse).

Verified locally:
  - `supabase db reset --local` applies all 4 of the 0001-0004
    migrations cleanly (previously died at 0002).
  - `\df+ public.claim_pairing_atomic` confirms postgres + service_role
    have EXECUTE; PUBLIC, anon, authenticated revoked.
  - 17/17 pairing tests pass.
  - Smoke harness against real local Supabase + real Upstash:
    10x concurrent same-user same-code → 10/10 success, 1 device row,
    Redis hash matches DB hash.

In-place edit of merged migration files is justified here because the
content has NEVER successfully applied in any environment (production
push died at 0002, every developer's local push died at 0002 after
branch-review). No one's database has the broken-shape migration
applied, so there's no immutability constraint to violate.

Follow-up audit issue (P7) tracks adding a CI guard: `supabase db
reset --local` against a fresh local Supabase pre-merge would have
caught this class of regression in branch-review, not in production.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(migrations): gate PRs on `supabase db reset --local` at pinned CLI

Add `.github/workflows/migration-smoke.yml`. On every PR (and `main`
push) that touches `supabase/migrations/**`, `supabase/seed.sql`, or
`supabase/config.toml`, spin up local Supabase via `supabase/setup-cli`,
run `supabase db reset --local`, and fail the build on a non-zero exit.

Pin the CLI to `2.90.0` — the same version used for local dev and for
production `supabase db push`. This is deliberate: PR #41 shipped a
hotfix for a v2.90 prepared-statement parser bug that batches multi-
statement migrations into a single Parse message and trips Postgres
SQLSTATE 42601. The whole point of the gate is to surface that class
of regression at PR time, so the gate has to run at the same parser
version production runs at. Bumping the pin requires a coordinated
local-CLI bump across contributors.

The incident path was: branch-review added `REVOKE`+`GRANT` statements
to a migration that had only ever been parsed at one-statement shape
locally; vitest does not apply migrations against a fresh DB; merge
to main happened green; production `supabase db push` was the FIRST
parse of the new shape and hit 42601. This workflow closes that gap
structurally — the CLAUDE.md "if you edit a migration, re-run db
reset --local" rule is no longer the only signal.

Update CLAUDE.md "Build Commands" to document the gate, the pin
rationale, and the reminder that local `db reset --local` after
mid-review migration edits is still the primary signal — CI is the
safety net.

Closes audit issue P7. Subsumed by F1 (integration test harness via
Toxiproxy) when that lands; until then, this is a standalone gate.

Refs: docs/audits/2026-04-29-server-helpers.md (P7), PR #41

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fyoosh added a commit that referenced this pull request Apr 30, 2026
…karound (#43)

Two changes that belong together because the second is only safe given the first:

1. **Bump `.github/workflows/migration-smoke.yml` pin from `2.90.0` to `2.95.4`.** PR #42's squash-merge carried only the first commit on its branch (the original 2.90.0 ship) and dropped the in-branch bump to 2.95.4, leaving the CI pin at 2.90.0 while the local Homebrew install and the laptop running `supabase db push` had already moved to 2.95.4. This commit reconciles the three contexts; CLI parser behaviour is now uniform across local / CI / prod.

2. **Drop the DO-block REVOKE+GRANT wrappers from migrations 20260430000002 and 20260430000004**, restoring the bare-statement form. Header comments rewritten as a historical note (the CLI bug, the substring "atomic" trigger, the fix in v2.91.1 `supabase/cli#5064`, why the wrapper is no longer needed, why the >= 2.91.1 pin is now load-bearing).

## Why the workaround is dead weight

The workaround was added in PR #41 against a CLI v2.90 parser bug that hit production via PR #40. The audit (`docs/audits/2026-04-29-server-helpers.md` issue P7) hypothesised an "inconsistent" parser trigger and treated the DO-block as defence-in-depth.

Investigation against `supabase/cli#5064` shows the trigger was actually **deterministic**: the SQL splitter treated any occurrence of the substring `atomic` inside an identifier as the start of a `BEGIN ATOMIC ... END;` block, then bundled subsequent statements into a single prepared-statement Parse message and tripped Postgres SQLSTATE 42601. Our `claim_pairing_atomic` function name is the trigger; the "inconsistency" was actually whether a multi-statement file was present, which only became true after branch-review added REVOKE statements to the originally single-GRANT file.

Verification at CLI 2.95.4 against current main migrations: `supabase db reset --local` run 5x consecutively after unwrapping both DO blocks. All 5 runs applied 38/38 migrations cleanly with no 42601. Combined with the explicit upstream fix in v2.91.1, this is conclusive — the workaround has zero remaining value at the pinned CLI version.

## Why combine with the pin bump

The DO-block drop is only safe at CLI >= 2.91.1. Shipping the unwrap without also pinning >= 2.91.1 would re-open the production-failure path the moment a contributor on a stale CLI runs the migrations. Pinning the bumped version in the same commit makes the dependency unambiguous and keeps `git log` clean: one diff, one rationale.

## Test plan

- [x] `supabase db reset --local` at CLI 2.95.4 — 5/5 clean runs after unwrap, no 42601.
- [x] YAML lint of workflow file (`python3 -c "import yaml; yaml.safe_load(...)"`).
- [ ] CI run on this PR will exercise the bumped pin against the unwrapped migrations against the current full migration history.

## CLI version downgrade hazard

If a contributor downgrades local CLI below 2.91.1 (or a future workflow refactor accidentally bumps the pin DOWN), migrations 02 and 04 will start failing again with 42601. The header comments call this out explicitly, and CLAUDE.md "Build Commands" now says "pin must stay >= v2.91.1". Worth a future hook that asserts CLI >= 2.91.1 at `supabase` invocation time, but out of scope here.

Refs: supabase/cli#5064, PR #41 (workaround), PR #42 (initial CI gate)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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