fix(cli): repair remote db pull with pg-delta and per-connection role step-down#5895
Merged
Merged
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>
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@d72d355b4705d5b3199e12e48ff4f741ab1770dcPreview package for commit |
![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: 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".
…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".
Coly010
approved these changes
Jul 20, 2026
…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".
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".
…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".
…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>
avallete
commented
Jul 21, 2026
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
2 participants
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.
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