Skip to content

fix(cli): port db diff --use-pgadmin to a native differ-container invocation (CLI-1968) - #6150

Merged
Coly010 merged 5 commits into
developfrom
columferry/cli-1968-port-db-diff-use-pgadmin-to-a-native-differ-container
Aug 12, 2026
Merged

fix(cli): port db diff --use-pgadmin to a native differ-container invocation (CLI-1968)#6150
Coly010 merged 5 commits into
developfrom
columferry/cli-1968-port-db-diff-use-pgadmin-to-a-native-differ-container

Conversation

@Coly010

@Coly010 Coly010 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What changed

Ports the db diff --use-pgadmin engine from delegating the whole invocation to the bundled Go binary to a native TypeScript implementation:

  • legacy-pgadmin-diff.ts (new): the differ-container loop (one supabase/pgadmin-schema-diff:cli-0.0.5 run, or one per --schema, with Go's exact argv/env/network/labels/extraHosts) plus pure ports of Go's ProcessDiffOutput (JSON diff filtering: type whitelist, extension-dependency skip, InternalSchemas exclusion, pgAdmin header assembly) and ProcessDiffProgress (stderr %-regex → status lines, including Go's greedy-submatch and .-matches-\r semantics via the s flag).
  • diff.handler.ts: the pgadmin branch now runs natively — AssertSupabaseDbIsRunning parity check → native shadow (CLI-1956 primitives, acquireUseRelease) → differ run → the shared SaveDiff tail (No schema changes found at len < 2, --file migration write + warning, stdout print). rebuildDelegateArgs became the pg-schema-only rebuildPgSchemaDelegateArgs: --use-pg-schema is now the sole remaining Go delegation on db diff (CLI-1960).
  • Two new error classes with error-actionability metadata; SIDE_EFFECTS.md rewrite; porting-status + binary-distribution doc flips.

Why

Part of M9 (Go removal). Unlike --use-pg-schema (in-process stripe/pg-schema-diff Go library — documented keep-in-Go exception), the pgAdmin engine was never Go-bound: it is a plain container invocation plus pure output post-processing. Its only real blocker was native shadow provisioning, which landed in CLI-1956 (#6027).

Reviewer-relevant context

  • Deliberate divergence — do not "restore parity" here. Go's DiffStream uses value receivers (internal/utils/container_output.go:79,87), so Collect() always reads an empty buffer: the Go CLI's --use-pgadmin has always printed No schema changes found (exit 0), never written a migration file, and can never hit a JSON parse error (verified empirically against Go 1.26). This port implements the intended algorithm instead; the ruling is recorded in SIDE_EFFECTS.md ("Deliberate divergence") and as a comment on CLI-1968.
  • Network static ruling: on the default project bridge network, neither of Go's 127.0.0.1 URLs is reachable from the differ container — identically in both implementations (same argv/network → same outcome). --network-id host alone doesn't rescue it either (it also rewires the shadow, discarding its port publication); both databases are reachable only with host networking plus a contrived shadow_port = 5432 override — identically on both binaries — which is exactly where the divergence above becomes user-visible. A gated live scenario was added to diff.live.test.ts (not executed in CI).
  • Reviewed by a Go-parity audit (container argv/URLs/filter chain/error text verified against Go source and empirical Go runs, including encoding/json acceptance rules and strings.TrimSpace Unicode semantics) and an engineer review; all findings applied — notably: /s regex flag, type predicates instead of an as cast, a 3-way docker error-reason mapping (image_inspect), machine-mode status lines redirected to stderr instead of dropped, restored child-telemetry assertions on the surviving pg-schema delegation, and legacyTrimGoSpace hoisted to commands/db/shared/legacy-go-string.ts.
  • Tests: 53 unit + a full pgadmin integration block (exact argv/image/labels/network, all filter branches, text/json/stream-json, delegation-removal regression guards). Branch coverage on the new code is 100% except two provably unreachable defensive fallbacks (documented).
  • Merge-order note: fix(cli): port migration squash to native TS (CLI-1969) #6151 (CLI-1969) moves legacy-pg-dump.env.ts (this PR imports LEGACY_INTERNAL_SCHEMAS from its current path) — whichever merges second needs a one-line import-path fix.

Fixes CLI-1968

@Coly010 Coly010 self-assigned this Aug 11, 2026
@Coly010
Coly010 marked this pull request as ready for review August 11, 2026 10:19
@Coly010
Coly010 requested a review from a team as a code owner August 11, 2026 10:19
Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@b37dc5eeca04bfbde1af643060dce6ece445030e

Preview package for commit b37dc5e.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b42eaed583

ℹ️ 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".

Comment thread apps/cli/src/legacy/commands/db/diff/diff.live.test.ts
Comment thread apps/cli/src/legacy/commands/db/diff/diff.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts
Comment thread apps/cli/src/legacy/commands/db/diff/diff.errors.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1111ab283

ℹ️ 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".

Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts Outdated
Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts
Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09f0ec0363

ℹ️ 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".

Comment thread apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fbe84e1d2b

ℹ️ 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".

Comment thread apps/cli/src/legacy/commands/db/diff/legacy-pgadmin-diff.ts
Comment thread apps/cli/src/legacy/commands/db/diff/diff.live.test.ts Outdated
@Coly010
Coly010 added this pull request to the merge queue Aug 12, 2026
Merged via the queue into develop with commit d0d566d Aug 12, 2026
35 checks passed
@Coly010
Coly010 deleted the columferry/cli-1968-port-db-diff-use-pgadmin-to-a-native-differ-container branch August 12, 2026 09:24
pull Bot pushed a commit to oogalieboogalie/cli that referenced this pull request Aug 12, 2026
## What changed

Rewrites `migration squash` from a Go-proxy wrapper into native
TypeScript — the last `LegacyGoProxy` use in the migration family:

- **`squash.handler.ts`** (full rewrite): version validation (bare
`invalid version number`, Go's non-repair-wrapped form) → shadow DB
(create → health-wait → connect → `SetupDatabase`, deliberately
*without* the `contrib_regression` template, matching Go) → before/after
`auth`+`storage` schema dumps → apply migrations → full-schema dump
rewritten into the last migration file (single `O_TRUNC` open, Go
parity) + separator + line-by-line diff → merged-file deletion
(non-fatal per file) → local suggestion / remote prompt + history
baseline (`DELETE version <= $1` + `INSERT` in one transaction, matching
`migration repair`'s established shape).
- **`squash.diff.ts`** (new): byte-exact port of Go's `lineByLineDiff` +
`bufio.ScanLines`, unit-tested against Go's own `testdata/` fixtures.
**`squash.dump.ts`**, **`squash.errors.ts`** (error-actionability
taxonomy) round it out.
- **Promotions** per "Hoist Before You Duplicate":
`legacy-pg-dump.{env,run,scripts}.ts` moved `commands/db/shared/` →
`legacy/shared/` (squash is the first cross-family consumer; the
Go-script byte-equality test moved with them), and
`legacyShadowRunInputFromLocalContainerInputs` hoisted into
`shared/db-bootstrap/shadow-database.ts` (re-export shim removed;
diff/pull/pgdelta-cache call sites repointed).
- TS-only additive surface: `--output-format json`/`stream-json` payload
`{ squashedInto, removed, removeFailures, alreadyEarliest, isLocal,
baselinedVersion }` (Go owns no machine output here; `removed` =
successful deletions, `removeFailures` = the rest).

## Why

Part of M9 (Go removal). `migration squash` was the last wrapped
migration command; every building block became available once CLI-1956
(supabase#6027) landed. Going native also dissolves the go-parity-audit §3.4
caveat — the proxy couldn't distinguish "`--local` unset" from
"`--local=false`"; the native command models `--local` with
`Flag.withDefault(true)` exactly like `migration up`.

## Reviewer-relevant context

- **Go-parity audit verdict: parity OK.** Highlights it verified: the
decline-prompt-is-**success** path (exit 0 — opposite of
repair/fetch/down), `Baselining migration history to <v>` printing
*before* `Connecting to remote database...`, resolve-before-validate
ordering, the single-migration no-op with zero Docker work,
leak-on-create-failure parity, the exact three-container pg_dump env
matrix (`EXTRA_FLAGS=--schema=auth|storage` vs `EXCLUDED_SCHEMAS`,
`EXTRA_SED=/^--/d`), and `lineByLineDiff` byte-exact against Go's
goldens plus throwaway `bufio` runs. The explicit baseline transaction
was adjudicated equivalent to Go's `pgx.Batch` (single implicit
transaction either way).
- Engineer review found no production bugs; all its items are applied —
including an ordered `BEGIN/DELETE/INSERT/COMMIT` assertion (an
INSERT-before-DELETE mutation previously shipped green),
workdir-relativized error paths via `legacyRelativizeErrorMessage`, and
the `dbConnection` classification matching `repair`'s identical failure.
- Tests: 16 unit (Go testdata read directly from `apps/cli-go`) + 48
integration + 2 Docker-free e2e; 100% branch coverage on the squash
files; key assertions mutation-tested.
- `go-cli-porting-status.md`: both squash rows flipped to `ported`, and
pre-existing aggregate drift corrected (Database family was recorded
5/19 ported, actually 43/43; Project/Stack Lifecycle 2→4; grand summary
now 51/118 fully ported, 59/118 partial).
- Merge-order note: this PR moves `legacy-pg-dump.env.ts`, which supabase#6150
(CLI-1968) imports from the old path — whichever merges second needs a
one-line import-path fix (both also touch
`tests/helpers/legacy-mocks.ts`, trivially).

Fixes CLI-1969
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.

2 participants