feat(cli): port db diff and db pull to native TypeScript (CLI-1313)#5620
feat(cli): port db diff and db pull to native TypeScript (CLI-1313)#5620Coly010 wants to merge 25 commits into
Conversation
Replace the Phase-0 Go proxy stubs for `db diff` and `db pull` with native Effect handlers in the legacy shell. - Promote the pg-delta engine (adapter, seam, cache, deno-templates, write, errors) from db/schema/declarative/ to db/shared/legacy-pgdelta.*; update the generate/sync call sites. Hoist findDropStatements to legacy-sql-split. - Add a live-shadow Go seam: hidden `db __shadow` command (+ PrepareShadowSource / PrepareRawShadow, refactored from DiffDatabase / pullDeclarativePgDelta) provisions a shadow Postgres, prints its URL + container, and leaves it running so the native handler runs migra/pg-delta against it then tears it down. Both engines now mirror Go's differ(shadowConfig, config). - db diff: native pg-delta / migra (edge-runtime) + explicit --from/--to mode; --use-pgadmin / --use-pg-schema delegate to the Go binary. - db pull: native pg-delta / migra migration + --declarative pg-delta export; reconciles schema_migrations and updates remote history. --experimental dump and initial-pull pg_dump (migra) delegate to the Go binary. - Shared: legacy-diff-engine (pure resolvers), legacy-migra (free fn over context, OOM->bash fallback), byte-exact migra deno-templates, migration-file helpers, pull.sync reconciliation. - Add --output-format json/stream-json envelopes; unit + integration + e2e tests; SIDE_EFFECTS.md; flip both commands to `ported` in the porting status.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@a016ddc5d770ea74da380798beb4b1e4cac1dee3Preview package for commit |
…CLI driver behaviour (ci: e2e shard 2/3) db pull --local connects via the native LegacyDbConnection sql-pg layer, so its connection-failure stderr diverges from Go's pgx output by driver — the same un-normalizable divergence already documented for db lint --local and test db --local, which deliberately omit testParity. The newly-added testParity(["db","pull","--local"]) was inconsistent with that established pattern; the user-observable contract (non-zero exit + connect error on stderr) remains covered by the behaviour test.
CodeQL flagged clear-text logging (CWE-312) on the hidden `db __shadow` seam: it printed the shadow Postgres connection URLs — password included — to stdout for the native-TS db diff/pull caller to capture. Emit the URLs without the password via a new ToPostgresURLWithoutPassword (ToPostgresURL is unchanged; its many env-var callers still need the password). The shadow always uses the local Postgres password, which the TS seam already resolves from config.toml, so it re-injects the password into the captured URLs before handing them to the differ / sql-pg connection. No credential is ever written to stdout. Fixes CodeQL alerts #27/#28.
|
@codex review |
![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
cli/apps/cli/src/legacy/commands/db/diff/diff.command.ts
Lines 40 to 43 in 4866659
Avoid reusing the global --output flag for paths
For db diff --from ... --to ... --output out.sql, this local flag has the same name and -o alias as the legacy root LegacyOutputFlag, whose parser only accepts the Go machine formats (env|pretty|json|toml|yaml|table|csv). Since global flags are registered for the whole legacy command tree, an arbitrary file path is consumed/rejected as the global output format before the handler can receive it, so the explicit diff file-output mode added here is not usable through the CLI.
ℹ️ 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".
… output for db pull Two Go-parity fixes in the native db pull handler, both flagged in review: - --experimental: Go gates the structured-dump branch on viper's EXPERIMENTAL, which resolves from the --experimental pflag OR SUPABASE_EXPERIMENTAL env (cmd/root.go:318-320,327,334). The handler only checked the env var, so `supabase --experimental db pull` slipped past and ran the native diff path. Now reads LegacyExperimentalFlag and ORs it with the env check. - Machine-output prompt: json/stream-json layers fail every prompt as non-interactive, so the history-update confirm failed the command before the structured payload was emitted. Go's PromptYesNo returns the default (yes) on --yes / non-interactive / error (console.go:74-82); now machine mode and any prompt error fall through to the default, matching Go. Adds regression tests for the --experimental flag form and machine-mode-in-TTY.
Go's shadow cleanup routes through DockerRemove with RemoveOptions{RemoveVolumes:
true, Force: true} (internal/utils/docker.go:330). The native seam ran
`docker rm -f` without -v, leaving the Postgres anonymous data volume dangling
after every db diff / db pull. Add -v to match. Flagged in review.
Automated-review findings, each grounded in Go behaviour: - Preserve connect_timeout from --db-url: the connToUrl helpers (diff + pull) dropped conn.connectTimeoutSeconds before serializing, so it defaulted to 10s; Go's ToPostgresURL serializes the parsed ConnectTimeout (connect.go). - Don't double-print the --use-pg-schema experimental warning: the delegated Go child prints it in its RunE (cmd/db.go), so the TS pre-print was a duplicate. - Print the migration repair confirmation: Go's repair.UpdateMigrationTable emits 'Repaired migration history: [<version>] => applied' to stderr (repair.go); the native pull updated the table silently. - Detect the git branch from the resolved --workdir, not the caller's CWD: Go chdirs into --workdir before GetGitBranch (cmd/root.go). detectGitBranch now takes an optional startDir (default = runtime CWD, so branches callers are unchanged) and db diff passes cliConfig.workdir. Updates the pg-schema delegate test and adds a detectGitBranch startDir test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6f886107f
ℹ️ 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".
| yield* fs | ||
| .makeDirectory(path.dirname(migrationPath), { recursive: true }) | ||
| .pipe(Effect.mapError((cause) => new LegacyDbPullWriteError({ message: cause.message }))); | ||
| yield* fs.writeFileString(migrationPath, out).pipe( |
There was a problem hiding this comment.
If the generated migration path already exists, this overwrites the file with the new diff SQL. Go opens the same path with O_APPEND (apps/cli-go/internal/db/pull/pull.go:189-196) so it preserves existing content, which matters for timestamp/name collisions and for the initial-pull flow that appends diff output after a dump; the native path can silently discard migration content before repairing the remote history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Rejecting this one — the native write here can't reach the case Go's O_APPEND is for. Go opens with O_APPEND (internal/db/pull/pull.go:191), but its own comment (:189-190) explains it preserves content only in the dump-then-diff sequence (errMissing && !usePgDeltaDiff): dumpRemoteSchema truncates with O_TRUNC (:150), then diffRemoteSchema appends a second pass. In the TS port that exact branch is delegated to the Go binary (pull.handler.ts, the sync.kind === "missing" && !usePgDeltaDiff → proxy.exec path), so the native write never runs after a dump. Every native flow that reaches this write corresponds to a Go path where diffRemoteSchema is the sole writer into a freshly-O_CREATEd file (pg-delta initial pull; in-sync delta path with a brand-new timestamped path), where append and truncate are byte-identical. The only residual gap is a same-second + same---name collision (seconds-resolution timestamps), which isn't what Go's append targets and would concatenate unrelated diffs. Leaving the truncate as-is; worth revisiting if the migra initial-pull branch is ever ported natively (then the append becomes load-bearing).
…xplicit diff output Two Go-parity fixes from review: - db __shadow never loaded config.toml: the hidden seam carries no db-url/local/linked target flag, so the root ParseDatabaseConfig skips LoadConfig (db_url.go:46-90) and the shadow was provisioned with default [db] settings. With a custom [db].password the shadow was created with the default password while the native-TS caller injects the config password into the seam URLs (the CWE-312 fix), so the diff/export connection failed. Load config explicitly in the __shadow RunE so the shadow uses the project's password, shadow_port, version, and baseline. - Explicit db diff --output to a nested path failed: the native handler called writeFileString without creating parent dirs; Go's writeOutput -> utils.WriteFile creates them first (explicit.go, misc.go). Make the parent dir first.
|
🤖 pr-autopilot: the |
… in db pull Two Go-parity fixes from review: - Project .env for the pg-delta engine flag: Go's LoadConfig loads supabase/.env via godotenv before viper reads EXPERIMENTAL_PG_DELTA (config.go:624,1055-1096), so a project enabling pg-delta in supabase/.env takes the pg-delta path. The handlers read SUPABASE_EXPERIMENTAL_PG_DELTA (and SUPABASE_EXPERIMENTAL) only from process.env. Route them through the project-env-aware resolver already loaded by legacyReadDbToml (exposed as toml.envLookup), matching Go's shell-then-.env precedence. - Local-target shadow for db pull: Go's pull runs through DiffDatabase, which derives targetLocal from utils.IsLocalDatabase and substitutes the declarative contrib_regression target override (diff.go:190,196-197). The native pull hard-coded targetLocal:false and ignored shadow.targetUrlOverride, so db pull --local with declarative schemas diffed the live local DB. Now passes targetLocal: resolved.isLocal and uses shadow.targetUrlOverride ?? targetUrl, mirroring the native db diff handler. Adds pull integration tests for the .env-pg-delta and local-target paths.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d81ee15435
ℹ️ 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".
…ad of an empty repair Go drives the migration-history repair from the generated timestamp and globs supabase/migrations/<timestamp>_*.sql, failing with os.ErrNotExist when nothing matches (repair.GetMigrationFile, internal/migration/repair/repair.go). A name with a path separator (supabase db pull foo/bar) writes a nested file <ts>_foo/bar.sql whose basename doesn't match, so Go fails the repair. The native legacyUpdateMigrationHistory re-parsed the basename and fell back to version="", silently upserting an empty-version migration-history row. Now it fails with Go's glob/file-does-not-exist message before any upsert. Adds a pull integration test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 923d6a437d
ℹ️ 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".
…reat empty --output as stdout Four Go-parity fixes from review: - Forward explicit false target flags (--linked=false/--local=false) to delegated db diff/pull children. Go's ParseDatabaseConfig selects the target by flag.Changed before the value (internal/utils/flags/db_url.go), so a Changed-but-false flag still selects that target. Dropping Some(false) made the child fall through to a different default (diff: local; pull: linked) than the native path resolved. New pushTarget forwards linked/local whenever Some. - Disable telemetry for the hidden db __shadow and __catalog seam subprocesses (SUPABASE_TELEMETRY_DISABLED=1), matching the explicit LegacyGoProxy delegates, so native db diff/pull doesn't record an extra internal cli_command_executed. - Treat an empty --output value as stdout in explicit diff mode: Go gates the file write on len(outputPath) > 0 (explicit.go), so --output="" prints to stdout instead of failing to write SQL into the project directory. Adds diff/pull integration tests for the delegate target flag and empty --output.
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/legacy/commands/db/diff/diff.command.ts
Lines 40 to 43 in d3f2787
Route explicit diff output around the global parser
This command-local --output/-o will not receive normal file paths because the legacy root already registers a global LegacyOutputFlag with the same name and alias, and the repo documents that Effect extracts global flags before the leaf parse and validates them against the fixed output-format enum. As a result, supabase db diff --from local --to linked --output tmp/diff.sql is rejected as an invalid global output value, while a path like --output json is treated as machine-output mode instead of an explicit diff file, so the advertised Go-compatible explicit-output workflow is broken before legacyDbDiff runs.
ℹ️ 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".
…shadow seam Two Go-parity fixes from review: - Empty --file -> stdout: Go's SaveDiff gates the file write on len(file) > 0 (internal/db/diff/pgadmin.go), so an empty --file="" (unset shell var) prints the diff to stdout instead of writing a nameless <timestamp>_.sql migration. The explicit-mode file branch now requires a non-empty value. - Forward --profile into the hidden seam subprocesses: the Go root loads the profile before config and applies profile-specific overrides (cmd/root.go, config_path.go), but a flag-only --profile snap was never forwarded to the __shadow / __catalog / db start children (only SUPABASE_PROFILE env was inherited). Forward the raw --profile flag token when it isn't the default 'supabase', mirroring the existing --network-id forwarding, so the shadow baseline is built under the selected profile. Adds a diff test for the empty --file case.
There was a problem hiding this comment.
💡 Codex Review
Avoid reusing the global --output flag
The legacy root already registers --output/-o globally via LegacyOutputFlag, and the repo notes that Effect CLI hoists global flags into a single tree-wide registry. Adding another --output here does not faithfully shadow Go's command-local diff output flag: real invocations such as supabase db diff --from local --to linked --output tmp/diff.sql can be parsed by the global enum (env|pretty|json|...) before this handler sees the path, while the new tests call the handler directly. That makes explicit diff file output unusable for arbitrary paths from the actual CLI.
ℹ️ 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".
…d db pull Go loads the project ref before LoadConfig on the linked path (internal/utils/flags/db_url.go:87-97), so a matching [remotes.<ref>] block merges before experimental.pgdelta.enabled, format_options, deno_version, and declarative paths are read. The native handlers read config once without the ref, ignoring remote overrides. Pass the resolved linked ref into legacyReadDbToml; for db diff, reorder so the linked ref resolves before the merged read and engine decision, and thread the merged config through the explicit --from/--to cascade (Go's resolveExplicitDatabaseRef LoadConfig).
…he IPv4 pooler Go wraps both the migration-style diff (diffRemoteSchema) and the declarative export (pullDeclarativePgDelta) with dump.PoolerFallbackConfig and retries against the IPv4 transaction pooler when the direct host is unreachable over IPv6 from inside the edge-runtime container (internal/db/pull/pull.go). The native handler ran each once against the direct targetUrl with no fallback. Add a withPoolerFallback helper mirroring the ported db dump (step 7b): classify the differ/export error message via legacyIsIPv6ConnectivityError (the edge-runtime/migra errors embed the container stderr Go classifies), gate to the linked path with a direct db.<ref>.<host> connection, resolve the pooler with orElseSucceed(None) so a resolution failure preserves the ORIGINAL error (Go's ok=false), emit the byte-exact IPv6 warning, and retry once against the pooler reusing the same shadow source.
Go's WriteDeclarativeSchemas points [db.migrations] schema_paths at the declarative dir after a declarative pull, but only when pg-delta is disabled in config (declarative.go:260-268, gated on IsPgDeltaEnabled). generate/sync force-enable pg-delta so the branch is unreachable for them, but db pull --declarative does not (cmd/db.go:180-182), so without this a pull with pg-delta disabled leaves db reset/db diff reading supabase/migrations and ignoring the pulled files. Port updateDeclarativeSchemaPathsConfig as a raw-text replace-or-append (byte-matching Go's regex and literal block, not a TOML re-serialize) and invoke it from the pull declarative path when config pg-delta is disabled, keeping the shared declarative writer a pure file-materializer for generate/sync.
…l paths The pgAdmin/pg-schema delegate (db diff) and the initial-migra / EXPERIMENTAL delegate (db pull) forwarded to the Go binary with inherited stdout and returned without output.success, so in --output-format json|stream-json mode scripted callers got the child's raw human/SQL output on stdout instead of a structured envelope (broken JSON.parse, the CLI-1546 'stdout is payload-only in machine mode' invariant). Add LegacyGoProxy.execCapture, which pipes the child's stdout (capturing it as a string) while keeping stdin/stderr inherited and still propagating the exit code. In machine mode the delegate paths now capture the child stdout and wrap it in output.success with the native envelope shape; text mode keeps the inheriting exec so the child's output streams straight through. Go has no --output-format concept, so the envelope is a TS-layer concern with no Go template; the delegated child owns any file write, so those fields are reported as null/best-effort.
…GDELTA_DEBUG When migration-style db pull uses pg-delta and PGDELTA_DEBUG is set, Go saves a debug bundle on an empty diff and embeds its path in the in-sync error (internal/db/pull/pull.go:176-185, pgdelta_pull_debug.go). The native path discarded the capture and returned the plain error. Port saveEmptyPgDeltaPullDebug: capture the shadow source catalog + pg-delta stderr during the diff run (Go's DiffDatabase), export the remote catalog at save time (warn-and-continue), write the bundle (source/target catalog, stderr, redacted connection.txt, error.txt), print the catalog summary + issue-report message, and fail with 'No schema changes found (debug bundle: <dir>)'; a save failure falls through to the plain in-sync error. Hoist the declarative debug-bundle helper to db/shared/legacy-debug-bundle.ts (now used by declarative + pull), extend it with inline catalog strings and connection.txt (Go's full DebugBundle), and hoist the debug-id formatter. Add pure helpers (URL redaction, catalog summary, byte-size) with unit coverage and an integration test for the empty-diff bundle.
…t, explicit preflight) Three Go-parity fixes from review of the db diff/pull port: - pull.sync.ts: reject migration versions above Number.MAX_SAFE_INTEGER in parseVersion, mirroring Go's strconv.Atoi range error (skip). A crafted 16+ digit version could exceed the exhausted-side sentinel and stall the two-pointer scan in legacyReconcileMigrations indefinitely. - legacy-migra.ts: the migra OOM bash fallback now derives extraHosts (Linux host.docker.internal mapping) and the network from --network-id, matching Go's DockerStart (internal/utils/docker.go:266-271). Previously it always ran host networking with no extra hosts, so the fallback failed on custom-network / host.docker.internal setups the primary path handles. - diff.handler.ts: explicit --from/--to mode now runs the target-flag preflight (resolver.resolve) when a target flag is changed, matching Go running ParseDatabaseConfig in PersistentPreRunE before RunExplicit (cmd/root.go:118). A bad --db-url / unreachable --linked is surfaced instead of silently ignored.
There was a problem hiding this comment.
💡 Codex Review
Forward --profile through execInherit
When db schema declarative generate or sync invokes the reset recovery path under a flag-only profile such as --profile snap, this child runs supabase-go db reset --local without the selected profile, unlike the other seam subprocesses that append profileArgs. Go's original path runs reset in-process after loading the root profile, so profile-specific config and auth overrides are applied; here the reset can target the default supabase profile instead of the user's selected profile.
ℹ️ 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".
The native db diff/pull handlers merge a linked [remotes.<ref>] override for the target/engine/format, but the shadow database is provisioned by the hidden Go 'db __shadow' command, which ran flags.LoadConfig with no project ref — so the shadow baseline (db.major_version, service enables, vault) was built from the base config, not the remote-merged config the Go monolith uses. That can produce spurious or missing migration SQL on the linked path. cli-go: db __shadow gains a --project-ref flag; when set it seeds flags.ProjectRef before LoadConfig so the matching [remotes.<ref>] block merges (pkg/config/config.go). Omitted on local/db-url shadows, which the monolith never remote-merges, so base config is used exactly as before. TS: provisionShadow gains an optional projectRef forwarded as --project-ref (flag, not env — channel parity, and avoids over-merging on local shadows via inherited env). The diff native path and both pull shadow paths pass the resolved ref only on the linked path.
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/legacy/commands/db/diff/diff.command.ts
Lines 40 to 42 in 457e7e6
Don't let global --output consume diff file paths
For real supabase db diff --from … --to … --output tmp/diff.sql invocations, --output/-o is already registered as the legacy global output-format flag; apps/cli/src/shared/legacy/global-flags.ts documents that Effect hoists global flags before leaf parsing and restricts this flag to env|pretty|json|toml|yaml|table|csv. This local file-path flag therefore never reliably receives arbitrary paths, so explicit diff output either fails parsing (e.g. tmp/diff.sql) or is treated as a global format instead of writing the file that Go's db diff --output supports.
ℹ️ 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".
…grations catalog ref
Two Go-parity follow-ups from review:
- pull: the machine-mode delegated pull (initial-migra/EXPERIMENTAL) ran the Go
child with inherited stdin and reported remoteHistoryUpdated:false. Go's
PromptYesNo(...,true) updates schema_migrations on the default (non-TTY/accept),
so the report was wrong, and in a TTY the child's prompt could block a
--output-format json caller before the envelope. Run execCapture with a non-TTY
stdin ('ignore') so the prompt takes Go's default without blocking, and report
remoteHistoryUpdated:true to match the child and the native machine-mode path
(internal/db/pull/pull.go:73, internal/utils/console.go). Adds an optional
stdin to LegacyGoProxy.execCapture (default inherit).
- diff: explicit --from linked --to migrations exported the migrations catalog
without the resolved ref, so the __catalog child reloaded base config. Go
resolves refs in order and the linked LoadConfig(ref) merges [remotes.<ref>]
before the migrations catalog is built (explicit.go), so pass the ref to
exportCatalog — but only when a linked ref resolved earlier in the cascade, so
--from migrations --to linked still uses base config, matching Go's order.
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/legacy/commands/db/diff/diff.command.ts
Lines 40 to 41 in 6a5fa73
Preserve db diff --output as a file flag
In explicit diff mode this needs to be Go's file-path flag, but the legacy root already registers --output/-o as a global choice flag and documents that Effect hoists globals before the leaf parse. With this duplicate name/alias, an invocation such as supabase db diff --from local --to linked --output tmp/diff.sql is consumed or validated as the global output-format flag instead of populating flags.output, so arbitrary file paths are rejected (or the explicit file-write branch is unreachable) even though Go registers this flag locally on db diff.
ℹ️ 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".
…sion, experimental-pull repair flag Four Go-parity follow-ups from review of the db diff/pull port: - diff/pull delegated argv: re-encode each parsed --schema value as a CSV field before forwarding to the Go child, so pflag's StringSlice doesn't CSV-split a comma-containing schema (e.g. "tenant,one") a second time. New legacySchemaToCsvField mirrors Go's encoding/csv Writer. - pull.sync ListRemoteMigrations: suppress only undefined_table (42P01) like Go's pgerrcode.UndefinedTable, not every "does not exist" — a malformed history table (missing version column, 42703) now propagates instead of being treated as an initial pull. Surface the SQLSTATE on LegacyDbExecError (extracted from the driver cause chain) and match the code, with a tightened message fallback. - delegatePull: parameterize remoteHistoryUpdated. Go's experimental structured dump returns without touching schema_migrations (pull.go:49-61) so it reports false; the initial-migra path repairs history so it reports true.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc6dd28774
ℹ️ 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".
…arity, shadow password merge Four Go-parity follow-ups from review of the db diff/pull port: - diff explicit mode: seed the merged config from a changed --linked preflight. Go's root ParseDatabaseConfig runs LoadProjectRef+LoadConfig before RunExplicit (cmd/root.go:118), leaving utils.Config remote-merged, so explicit local/ migrations refs + format options use the linked [remotes.<ref>] override even when neither explicit ref is itself linked. Reuse the preflight's resolved ref to seed cfg/mergedLinkedRef. - diff explicit mode: gate on non-empty --from/--to like Go's len()>0, so (shell vars) falls through to the normal diff instead of erroring on an unknown empty target; still errors. - pull.sync: compare migration versions as BigInt over Go's full int64 range. strconv.Atoi accepts up to math.MaxInt64, so 9999999999999999 (> JS safe int, < int64 max) is a real conflict, not an initial pull; only values above int64 max are skipped (preserving the prior infinite-loop guard). Number lost precision in that range. - shadow seam: re-read the injected shadow password with the linked ref so a [remotes.<ref>].db.password override matches the password the __shadow child built the shadow with (it received --project-ref); otherwise the shadow connection fails auth.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfccaf8411
ℹ️ 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".
Go loads db diff config once in PreRun and, on the linked path, only AFTER resolving the ref (LoadProjectRef then LoadConfig), so it validates the remote-merged config: config.go merges [remotes.<ref>] before Validate. The TS handler read+validated the BASE config unconditionally up front, failing on fields a remote block overrides (db.major_version, edge_runtime.deno_version, format_options, bucket/function names, ...) before the ref was known — so db diff --linked / --use-pgadmin --linked could fail on a project Go accepts. Defer the read: the pgAdmin/pg-schema delegate paths now read no config (the forwarded Go child loads it with the ref), the native local/db-url path reads the base config (Go local/direct LoadConfig, no merge), and the linked/explicit paths read with the resolved ref (merged). Tests: a delegated --use-pgadmin --linked with an invalid base major_version succeeds (no base validation), while a native local diff with the same config still fails.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a4d9dfab6
ℹ️ 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".
…ote config The exportCatalog seam forwarded the resolved linked ref only as the SUPABASE_PROJECT_ID env var, but the hidden db schema declarative __catalog command never runs LoadProjectRef: its group PersistentPreRunE calls flags.LoadConfig directly, and LoadConfig keys the [remotes.<ref>] merge off flags.ProjectRef (populated from the env only by LoadProjectRef). So the env was silently ignored and linked catalog exports (db diff --from linked --to migrations, db schema declarative generate --linked) built from the base config. Mirror the db __shadow fix: add a --project-ref flag to __catalog, seed flags.ProjectRef from it before the group LoadConfig, and have the TS seam pass --project-ref (a flag, not env — Go-proxy channel parity) instead of SUPABASE_PROJECT_ID.
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/legacy/shared/legacy-db-config.toml-read.ts
Lines 162 to 165 in cbc32bd
Match remote blocks using Go's raw project_id
When a remote block uses an env reference like project_id = "env(PROD_REF)", this comparison expands it before deciding whether to merge the block. Go's merge loop calls v.GetString("remotes.<name>.project_id") before the LoadEnvHook used by UnmarshalExact, so the merge comparison sees the raw env(PROD_REF) string and does not apply that remote override even though validation may expand it later. In that configuration the native path can unexpectedly apply remote db/pgdelta settings and build a different shadow/diff than the Go CLI.
ℹ️ 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".
…lves Go loads db diff/pull config once, after resolving the linked ref, and validates the [remotes.<ref>]-merged result. The TS port validated the BASE config at three points before the ref was known, so a project valid only after a remote override (e.g. base db.major_version=16 overridden to 15) failed in TS where Go succeeds: - shared resolver (legacy-db-config.layer.ts): the linked branch no longer does the unconditional top-of-resolve base read; only --db-url/--local read base config (Go local/direct LoadConfig, no merge), and the linked pooler fallback reads with the ref. - diff explicit mode: the cfg base read is deferred until after the linked preflight, reading the merged config when --linked resolved a ref. - edge-runtime layer: the Deno-image config read moved from layer acquisition into run (resolved from the callers effective deno_version), so composing the diff/pull runtime no longer validates base config — even db diff --use-pgadmin --linked no longer fails at layer build. All shared-infra changes keep --local/--db-url base validation unchanged and only affect the linked path (matching Go). Full legacy suite (2416 tests) green.
3 participants
Ports
supabase db diffandsupabase db pullfrom the Go CLI to native Effect handlers in the legacy shell, replacing their Phase-0 proxy stubs. Linear: CLI-1313. Builds on the pg-delta infrastructure merged in #5586.What changed
Shared engine (promotion + new modules)
db/schema/declarative/intodb/shared/legacy-pgdelta.*, sodiff,pull, anddeclarativeimport one copy. Thegenerate/synccall sites are updated in the same change;findDropStatementsis hoisted tolegacy-sql-split.ts.legacy-diff-engine.ts(pureresolveDiffEngine/resolvePullDiffEngine/shouldUsePgDelta),legacy-migra.*(byte-exact Go templates + alegacyDiffMigrafree function over the pg-delta context with the OOM→supabase/migrabash fallback), and migration-file helpers.Live-shadow Go seam
db __shadowcommand (withPrepareShadowSource/PrepareRawShadow, refactored out ofDiffDatabase/pullDeclarativePgDelta). It provisions the shadow Postgres, prints its connection URL + container id, and leaves it running so the native TS handler runs migra/pg-delta against it and tears it down — mirroring Go'sdiffer(shadowConfig, config)for both engines (migra needs a live source, which the catalog-file__catalogseam couldn't provide).start.SetupDatabasestays unported, which is why provisioning remains in Go.db diff
--from/--tomode (always pg-delta).--use-pgadmin/--use-pg-schemarebuild the argv and delegate to the bundled Go binary (telemetry disabled on the child so the singlecli_command_executedevent comes from the TS command).db pull
--declarativepg-delta export; reconcilessupabase_migrations.schema_migrationsand updates remote history on confirmation.--use-pg-deltais hidden + emits the cobra deprecation line. The--experimentalstructured dump and initial-pullpg_dump(migra) sub-branches delegate to the Go binary (unportedpg_dump).Cross-cutting
--output-format json/stream-jsonenvelopes (text reproduces Go bytes); unit + integration + e2e tests;SIDE_EFFECTS.mdfor both; flips both commands toportedingo-cli-porting-status.md.Reviewer notes
db pullexits non-zero (No schema changes found), matching Go'serrInSync;db diffexits 0. Documented inpull/SIDE_EFFECTS.md.--file/--output/migration-name path handling reproduce pre-existing Go behavior rather than diverging from the strict 1:1 port.