Uh oh!
There was an error while loading. Please reload this page.
fix(cli): repair remote db pull with pg-delta and per-connection role step-down - #5895
Conversation
… step-down Remote `db pull --linked` with the pg-delta engine reported "No schema changes found" on every hosted project, and the migra fallback failed at the final migration-history write (CLI-1919, #5826). Three stacked causes: - pg-delta (<= alpha.31) extracted user mappings from the superuser-only `pg_catalog.pg_user_mapping` catalog, so extraction as the temp `cli_login_postgres` role failed with SQLSTATE 42501. Fixed upstream in 1.0.0-alpha.32 (world-readable `pg_user_mappings` view); bump the default pinned version in Go and TS. - The pg-delta Deno templates force the edge-runtime worker to exit by throwing on both success and failure, and both runners suppress any non-zero exit whose stderr contains "main worker has been destroyed" — so a script crash was indistinguishable from an empty diff. The catch blocks now print a PGDELTA_SCRIPT_ERROR sentinel and both runners treat it as a hard failure that surfaces the collected stderr. - alpha.32 also moved plan statements into execution-aware `units`; the diff template's `result?.plan.statements ?? []` silently yielded an empty diff. Use `flattenPlanStatements(result.plan)` instead. - The TS shell ran `SET SESSION ROLE postgres` once per session, but `PgClient.make` leaves node-postgres' default 10s idle timeout, so the pool silently replaced the stepped-down connection during the long shadow diff and the final `CREATE SCHEMA supabase_migrations` executed as the bare login role (42501). The primary connection now uses a self-managed pg.Pool (`PgClient.fromPool`) with idle reaping disabled and a pg-pool `verify` hook that re-runs the step-down on every new physical connection, matching Go's per-connection `AfterConnect`. Verified end-to-end against staging: initial and incremental `db pull --linked` produce correct migrations (including FDW server and user mapping without credential leak) and update the remote migration history as the stepped-down role. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@d72d355b4705d5b3199e12e48ff4f741ab1770dcPreview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6ffa72156e
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…diff output by default Follow-up to the PR review: flattening the pg-delta plan discarded its transaction-boundary units, so plans like `ALTER TYPE ... ADD VALUE` followed by a statement using the new value produced a single migration that fails on `db push`/`reset` (both CLIs apply a migration file as one transaction). - The diff template now emits a JSON envelope of `renderPlanFiles` output (one entry per execution-aware unit, `includeTransactions: false` since the appliers provide per-file transactionality). - `db pull` writes one ordered migration per unit — a single-unit plan keeps today's `<ts>_<name>.sql` name; multi-unit plans get strictly increasing timestamps and a `_<unit>` suffix — and records every version in the migration history. - `db diff` and declarative sync join the rendered units into one script with boundary header comments (unchanged single-file behavior). - Diff/pull SQL is now formatted by default with the same settings as the declarative export (`maxWidth: 180`, uppercase keywords, formatter defaults); `[experimental.pgdelta] format_options` still merges on top, and `format_options = "null"` opts out entirely. - pg-delta routing in `DiffDatabase` goes through a stubable seam (`diffPgDeltaRefDetailed`) so the pgtest-based diff suites keep their injection point. Verified against staging: an enum `ADD VALUE` plus a table defaulting to the new value pulls as two ordered migration files (`schema_changes`, `after_enum_values`) with both versions recorded; a plain schema pull stays a single formatted file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:4407659e51
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…igrations in machine output Review follow-ups on the pg-delta db pull changes: - The primary connection's pg.Pool was created before its acquireRelease finalizer, so a failed or timed-out SELECT 1 probe (bad primary host in the multi-host fallback chain, black-holed host) leaked the pool and its in-flight dial. Pool construction is now the acquireRelease resource itself (legacyAcquireProbedPool), and each fallback attempt runs in its own scope forked from the session scope so a failed attempt's pool closes before the next host is dialed. - The structured `db pull` result now carries `schemaFiles` with every written plan-unit migration path in order; `schemaWritten` stays the first path for released consumers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:70f171722b
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:797c43bf9f
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…story atomically Review follow-ups on the pg-delta multi-file work: - `db diff -f` (and the TS `--output` write) now materializes one migration file per pg-delta plan unit, reusing db pull's naming and timestamp scheme, so a boundary-crossing plan saved as a migration no longer fails when `db push`/`reset` applies it as one transaction. Single-unit plans and the migra/pgadmin engines keep the byte-identical single-file write; stdout mode keeps the joined review script. The per-unit writer is hoisted from `pull` into `diff` (`WritePgDeltaMigrations`) and shared by both commands. - The TS pull's migration-history upserts now run in one explicit transaction with rollback on any failure, mirroring Go's single `pgx.Batch` (implicitly transactional): a mid-loop failure no longer leaves partial remote history that breaks the next pull's sync check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:555063e4bf
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…sted names, and partial writes Review follow-ups on the per-unit migration writer: - Generated versions are collision-checked as a full set against existing migration files (base bumped forward until every filename is free, bounded), and files open with O_CREATE|O_EXCL instead of O_TRUNC, so a rapid successive pull/diff can never silently overwrite a migration or mint a duplicate version. The base only moves forward: backdating could sort a new file before pre-existing migrations, and the ≤N−1s future-dating is inherent to second-granularity versions. - The writer creates each generated path's parent directory again, fixing the nested-name regression (`db diff -f snapshots/remote`) in both Go and TS; the TS logic is hoisted into a shared `legacyWritePgDeltaMigrations` used by diff and pull. - A mid-loop open/write failure now removes every file this invocation already wrote (best-effort, never masking the original error), so a partial multi-file plan can't strand extra local migrations that break the next pull's history sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Remote
db pull --linkedwith the pg-delta engine reported "No schema changes found" on every hosted project, and the migra fallback failed at the final migration-history write withpermission denied for database postgres(Slack report, #5826). Three stacked causes, all addressed here:1. pg-delta required superuser to extract (fixed upstream, version bump here). Up to alpha.31, extraction read
pg_catalog.pg_user_mapping(superuser-only), so extracting as the tempcli_login_postgresrole failed with SQLSTATE 42501 on every hosted project. It only worked locally because local connections usesupabase_admin.@supabase/pg-delta@1.0.0-alpha.32reads the world-readablepg_user_mappingsview instead; this PR bumps the default pinned version (GoDefaultPgDeltaNpmVersion+ TSLEGACY_DEFAULT_PG_DELTA_NPM_VERSION).2. pg-delta script crashes were swallowed as an empty diff. The Deno templates force the edge-runtime worker to exit by throwing on both the success and failure paths, and both runners (Go
RunEdgeRuntimeScript, TSlegacy-edge-runtime-script.layer.ts) suppress any non-zero exit whose stderr contains "main worker has been destroyed" — making a crash indistinguishable from a genuinely empty diff. The template catch blocks now print aPGDELTA_SCRIPT_ERRORsentinel to stderr, and both runners treat its presence as a hard failure that surfaces the collected stderr (so users see the real error instead of "No schema changes found").3. alpha.32 changed the plan API. Plan statements moved into execution-aware
units(+sessionStatements); the diff template'sresult?.plan.statements ?? []silently produced an empty diff. The template now usesflattenPlanStatements(result.plan). TS template embeds regenerated from the Go sources (byte-equality test unchanged).4. The TS role step-down was lost mid-command (the migra-path failure). The legacy shell ran
SET SESSION ROLE postgresonce per session, butPgClient.makeleaves node-postgres' default 10s idle timeout — during the minutes-long shadow diff the pool silently reaped and redialed the stepped-down connection, so the finalCREATE SCHEMA IF NOT EXISTS supabase_migrationsexecuted as the bare login role (42501). The primary connection now uses a self-managedpg.PoolviaPgClient.fromPoolwith idle reaping disabled (idleTimeoutMillis: 0,max: 1) and a pg-poolverifyhook that re-runs the step-down on every new physical connection — matching Go's per-connectionAfterConnect(connect.go:337-362).verifyruns before the checkout resolves, so it cannot race the caller's first query (apool.on("connect")client.query()hits node-postgres' concurrent-query deprecation, removed in pg@9).Verification against staging
link(login-role path, noSUPABASE_DB_PASSWORD) →db pull --linked("engine":"pg-delta"): migration contains the table with PK and grants, andRepaired migration history: [...] => appliedsucceeds — exercising the step-down after a long-idle diff, exactly where 2.109.1 failed.CREATE USER MAPPINGcarries no password option (CLI-1467 handling intact).Linked issue
Closes#5826
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
fix(cli): …).pnpm check:allandpnpm testpass for the workspace(s) I touched.🤖 Generated with Claude Code