fix(cli): remove db schema declarative __catalog seam for migrations mode (CLI-1959) - #6061
Merged
Coly010 merged 3 commits intoAug 6, 2026
Conversation
…mode via native pg-delta export (CLI-1959) db diff's explicit --from/--to migrations, and db schema declarative sync's migrations-catalog diff source, resolved through the hidden Go seam db schema declarative __catalog --mode migrations. Replace both with native TypeScript resolution: the existing db __shadow --mode diff seam (unchanged, CLI-1956 scope) provisions the shadow, and the already-native legacyExportCatalogPgDelta exports its catalog, cached with the same byte-stable Go cache-key scheme pg-delta caching already uses elsewhere. db diff and db schema declarative sync use two different, intentionally distinct Go cache-key schemes here (bare migrations hash vs. a setup-inputs- folded hash) — legacyResolveMigrationsCatalogRef and legacyGetMigrationsCatalogRef mirror Go's explicit.go/declarative.go functions respectively; see their doc comments. "baseline" and "declarative" catalog modes (db schema declarative generate/sync) still go through the Go seam: they need a shadow provisioned with only the platform baseline, or with declarative files applied, neither of which has a native TS port yet (tracked by CLI-1956/CLI-1823). LegacyCatalogMode is narrowed to drop "migrations" accordingly.
… (CLI-1959) Three reviewers converged on the same gaps in the prior commit's native migrations-catalog resolution: - Cover the dominant real-world code path (project with local migrations, cache enabled) for `legacyGetMigrationsCatalogRef`, which was previously only exercised via the zero-migrations branch. Also pin the stderr asymmetry between `db diff --from/--to migrations` (no "Creating shadow database..." line) and `db schema declarative sync` (prints it) that this port's parity fix depends on. - Deduplicate the two catalog-path resolvers' scan loop into a shared `resolveLatestByFamily` helper, and normalize `legacyResolveDeclarativeCatalogPath` to the same Go-faithful `(hash, prefix)` parameter order as its sibling `legacyResolveMigrationCatalogPath` (the two previously took the opposite order, a silent-cache-miss footgun on a swap). Extract the shared shadow-provision/export/persist/cleanup mechanics behind `legacyResolveMigrationsCatalogRef` and `legacyGetMigrationsCatalogRef` into `exportViaShadowCatalog`. - Export `LegacyBaselineTomlConfig` from `legacy-db-config.toml-read.ts` instead of re-declaring its shape inline in `legacyResolveSetupInputs`, and update `legacy-pgdelta.cache.ts`'s module header to reflect that it now also owns shadow provisioning and a stderr side effect.
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@4feb890a791b2cabd89a00a1e8e7c1c188f00694Preview package for commit |
jgoux
approved these changes
Aug 5, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Aug 5, 2026
pull Bot
pushed a commit
to oogalieboogalie/cli
that referenced
this pull request
Aug 5, 2026
…audit (CLI-1967) (supabase#6074) ## What Doc/comment-accuracy sweep from `apps/cli/docs/go-parity-audit-2026-07-24.md` §6 (untracked locally, not part of this repo). No runtime behavior changes — every item below was independently re-verified against current Go (`apps/cli-go/`) and TS source before fixing, since the audit is 12 days old and several items had drifted further in that time (in both directions). ## Fixed - **`docs/go-cli-porting-status.md`** — `functions list` legacy-shell status `wrapped` → `ported` (verified: zero `LegacyGoProxy` refs). `functions download` status corrected the other way, `ported` → `wrapped` (its handler still delegates wholesale via `LegacyGoProxy` for the default `--use-docker` path; only `--use-api` is native — noted as a hybrid in the row). Added a `ported` definition to the legacy-status legend (it only defined `wrapped`/`missing`, despite `ported` being ~90% of rows) and retitled that section from "Legacy Shell Wrapping Status" to "Legacy Shell Command Status" to match. Fixed the adjacent `functions delete/deploy/list/new/serve` legacy-shell notes, which all still said "Wrapped in legacy shell" despite being natively ported. - **`legacy-pgdelta.seam.service.ts`** — fixed two stale doc comments (`execInherit`, `ensureLocalDatabaseStarted`) that referenced things as "not yet ported" when they now are (`db reset`, `start`/`db start`). Did **not** touch `exportCatalog`'s doc comment / `LegacyCatalogMode`, which the audit also flagged — that exact hunk is already being rewritten more completely by the in-flight CLI-1959 PR (supabase#6061, open); fixing it here would guarantee a conflict with a strictly better version. `legacy-db-bootstrap.seam.service.ts` (the audit's other named file) was checked and found already accurate — CLI-1954/1955 (native `db start`/`db reset --local`) are still unmerged, so its "not yet ported" claim is currently true. - **`network-restrictions/{get,update}/SIDE_EFFECTS.md`** — the `-o {json,yaml,toml,env}` sections previously implied Go itself produces distinct byte-identical output per format. Verified against Go source: `restrictions/get`/`update` never read `OutputFormat` at all — they always print the same 3-line `fmt.Printf` template regardless of `-o`. Corrected both docs to state this plainly, documented that TS's format-specific output here is a deliberate TS-only enhancement with no real Go behavior to match (including no Go casing convention, since TS uses the map-shaped encoders rather than CLI-1975's struct-spec ones), and trimmed the resulting repetition. - **`inspect/report/SIDE_EFFECTS.md`** — added the empty/no-file divergence on `COPY` failure: Go's `copyToCSV` opens the output file with `O_TRUNC` before running the query, so a failing query still leaves a file (empty or partial); TS buffers in memory and only writes on success, leaving no file on a fresh run — and leaving the *previous* run's stale CSV in place on a same-day re-run (the more consequential case). Cross-referenced from `legacy-db-connection.errors.ts`'s `LegacyDbCopyError` doc comment, which already covered the message-text angle of the same divergence. - **`domains.cname.ts`** — the comment describing Go's CNAME "failed to locate" error dump wrongly implied Go embeds readable JSON. Verified against Go source and empirically (compiled the equivalent locally): Go JSON-marshals the answers to a `[]byte`, then formats that `[]byte` with `%+v`, which Go's `fmt` renders as an uncapped decimal byte-value array, not the JSON text — a `%+v`-on-`[]byte` footgun, not an intended format. Fixed both the function's JSDoc and the inline comment (they'd contradicted each other after an earlier pass), and cross-referenced the divergence from `domains/SIDE_EFFECTS.md`. - **`branches/orgs/projects/secrets` SIDE_EFFECTS `-o toml`/`-o yaml` claims**, **`functions deploy`'s `NPM_AUTH_TOKEN` env table entry**, and **`update-root-key.handler.ts`'s color comment** — all already fixed by CLI-1975 (supabase#6002), CLI-1985 (supabase#6005), and CLI-1990 (supabase#5978) respectively, which merged after the audit ran. Verified current state matches; no changes needed. - **`start/SIDE_EFFECTS.md`'s `--ignore-health-check` ruling** — already fully handled by CLI-1987 (supabase#6007, merged), whose own description explicitly says CLI-1967 should not re-document it. Left untouched. ## Left as noted, not fixed (out of scope for a docs-only pass) - The `network-restrictions get`/`update` TS-only `-o` support is a real, pre-existing behavioral divergence from Go (Go has no such behavior at all for these two commands) — documented accurately here, but whether it should be *removed* to enforce strict parity is a ruling this PR doesn't make. - `docs/go-cli-porting-status.md`'s "Functions" section (next/-shell table) has a larger, pre-existing inaccuracy discovered while fixing the adjacent legacy-shell notes: it claims there's "still no dedicated `functions` CLI surface" in `next/`, but `next/commands/functions/` already exists (list/delete/deploy/download/new/dev, registered in `next/cli/root.ts`). Added command-path links and flagged the section as needing its own flag-by-flag parity audit rather than silently reclassifying rows without one. Fixes CLI-1967
…9-remove-the-db-schema-declarative-__catalog-seam-via-native # Conflicts: # apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md
![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: 4feb890a79
ℹ️ 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
deleted the
columferry/cli-1959-remove-the-db-schema-declarative-__catalog-seam-via-native
branch
August 6, 2026 10:49
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.
What changed
Replaces the hidden Go seam
db schema declarative __catalog --mode migrations— used bydb diff --from/--to migrationsanddb schema declarative sync's migrations-catalog diff source — with native TypeScript catalog export, invoking pg-delta directly via the already-shippedlegacyExportCatalogPgDeltaedge-runtime path instead of proxying through the Go binary.baseline/declarativecatalog modes remain seam-backed (out of scope here — they need native shadow-provisioning primitives that are CLI-1956's territory, a separate in-progress issue).LegacyCatalogModeis narrowed to"baseline" | "declarative", making the remaining Go-backed surface exhaustive/compile-checked.Bundled parity fix: the old code routed
db diff --from migrationsthrough the wrong Go-equivalent function — one shaped fordeclarative.go's semantics (setup-token-folded cache key, zero-migrations→baseline reuse, aCreating shadow database...stderr line) — when Go's actualdb diffpath usesexplicit.go'sresolveMigrationsCatalogRef(bare migration-hash cache key, no stderr line, always consults cache). This meantdb diff --from migrationscould never reuse a cache warmed bydb push, and printed a spurious progress line. Both are fixed. The generated SQL was unaffected (a zero-migration catalog and the platform baseline are the same snapshot by Go's own logic) — this was a caching-efficiency and stderr-parity bug, not a correctness bug in the diff output.Why
Part of the M9 "Go removal" milestone. This is the last piece of
legacy-pgdelta.seam.layer.ts'sexportCatalogsurface that doesn't need shadow-provisioning primitives from CLI-1956.Review notes
Reviewed independently by go-parity-auditor, engineer-reviewer, and architect-reviewer (all approved after a follow-up commit). Findings addressed:
syncwith local migrations present, cache hit/miss,--no-cache), including theCreating shadow database...stderr-parity assertions — all verified load-bearing via mutation checks.LegacyBaselineTomlConfig(was duplicated as an inline structural type) and corrected the module header, which had gone stale (the file now also owns shadow-provisioning orchestration, not just pure cache-key derivation).Fixes CLI-1959