fix(cli): port db test to native TypeScript (CLI-1962) - #6063
Merged
Coly010 merged 9 commits intoAug 6, 2026
Conversation
`db test` was the last Go-proxy seam in the db/test family: it forwarded argv to the Go binary while its visible sibling `test db` had already been natively ported (PR #5522). Go itself models these as two cobra.Command registrations sharing the literal same RunE (cmd/test.go:19-20 borrows cmd/db.go's dbTestCmd.RunE), so this port makes `db test` reuse `test db`'s flag config and assembled handler verbatim instead of re-implementing pgTAP enable/disable and the pg_prove docker invocation a second time. Since the shared implementation is now used by two different top-level command families (db and test), and legacy/commands/<family>/ files may not import another family's internals (enforced by code-structure.unit.test.ts), the whole test-db implementation (handler, errors, pg-prove-args builder, config, assembled command handler, runtime layer) moves out of legacy/commands/test/db/ into legacy/shared/legacy-test-db.*, mirroring Go's own internal/db/test package consumed by two thin cmd/ registrations. The recorded cli_command_executed telemetry `command` property now reflects the actual invoked path ("db test" vs "test db"), matching Go's cmd.CommandPath() parity (cmd/root_analytics.go:33) — the runtime layer factory takes a commandPath argument so each entry point passes its own. This also resolves the proxy-only "--local defaults to true" modelling caveat: the flag now drives resolveLegacyDbTargetFlags's presence-based selection directly, the same mechanism test db already used, so Go's real default is reflected exactly with no proxy-only quirk to carry over.
Fix issues flagged by review on the db test/test db native TypeScript port: the alias telemetry test now dispatches through the real legacyDbTestCommand (via Command.runWith) instead of hand-building a runtime layer with a test-supplied commandPath, so a regression in test.command.ts's own commandPath wiring is actually caught; a vacuous hidden-flag defect assertion now checks the real Die/Fail discriminator and defect message instead of a near-always-true negative string check; two weak alias-integration assertions are replaced with assertions on actual docker invocation and exit-code state; the Short/description strings are single-sourced to prevent drift between the two Go-parity entry points; and stale Go line-number references (db.go/docker.go) and the porting-status doc's implementation location/--network-id note are corrected.
Contributor
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…2-port-db-test-to-native-typescript # Conflicts: # apps/cli/docs/go-cli-porting-status.md
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@53320795ece242d1be211a42acbed70cdc24cf53Preview 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
…2-port-db-test-to-native-typescript # Conflicts: # apps/cli/docs/go-cli-porting-status.md
…on.test.ts Dispatching through legacyDbCommand (the whole `db` subtree) pulls in every sibling subcommand's global-flag and LegacyGoProxy requirements at the type level, even though this test only runs `db test`. The develop merge bumped effect/@effect/vitest, which now enforces `it.live`'s R = Scope exactly, surfacing the gap.
![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: ef5a01557b
ℹ️ 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".
…resolution The merge-conflict resolution edited table cell text without re-running oxfmt, so the padded column widths drifted and fmt:check failed in CI.
…ag test The develop merge bumped effect to a version whose CliError._tag for this case is upstream-misspelled "UnknownSubcomand" (see normalize-error.ts and subcommand-flag-suggestions.ts, which already correct it for user-facing output). This assertion checks the raw, un-normalized tag, so it must match the upstream spelling rather than the aspirational correct one.
…2-port-db-test-to-native-typescript # Conflicts: # apps/cli/docs/go-cli-porting-status.md
Coly010
deleted the
columferry/cli-1962-port-db-test-to-native-typescript
branch
August 6, 2026 11:30
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
Ports
db test(a hidden, Phase-0 Go-proxy command) and itstest dbalias to native TypeScript. Go registers the sameRunEfunction under two cobra commands (cmd/db.go:422-429hidden,cmd/test.go:17-20visible) with byte-identical flags (--db-url,--linked,--local— defaulttrueon both). This PR mirrors that structure: hoisted the shared implementation (handler, errors, pg-prove-args, a new command-handler, and runtime layers) intoapps/cli/src/legacy/shared/legacy-test-db.*, with bothdb/test/test.command.tsandtest/db/db.command.tsreduced to thin registrations differing only in theircommandPath(needed for Go-parity telemetry, sincecmd.CommandPath()differs even thoughRunEis identical).The underlying pgTAP-enable/pg_prove-container-invocation logic itself was already natively correct from a prior PR (#5522) — this PR is primarily a rewire (removing the
db testproxy handler), not a re-port. Confirmed via go-parity-auditor: bind-mount/workingDir derivation, network-mode switching, BitbucketSecurityOpthandling, Linux-onlyExtraHosts, and exit-code propagation (error running container: exit N) all already matched Go.This also resolves the audit's previously-flagged "
--localdefault-true modelling caveat" — confirmed the resolved default istrueon both entry points, matching Go exactly.Bonus, undisclosed-until-now parity fix:
test db's help text previously read "Run pgTAP tests on the local or linked database." — Go's actualShort(shared viadbTestCmd.Short) is "Tests local database with pgTAP". Both entry points now byte-match.Why
Part of the M9 "Go removal" milestone. Removes the
LegacyGoProxydependency fordb test/test dbentirely.Review notes
Reviewed independently by go-parity-auditor, engineer-reviewer, and architect-reviewer — all approved after a follow-up commit. The engineer-reviewer caught (via mutation testing) that one new test didn't actually exercise the real command wiring and another assertion was vacuous; both are now fixed and mutation-verified. Also fixed: two stale Go line-number references in carried-over comments, a stale doc pointer in
docs/go-cli-porting-status.md, and single-sourced the Short/description strings between the two command registrations to prevent future drift.Fixes CLI-1962