fix(cli): port db reset local recreate to native TS (CLI-1955) - #6026
Merged
Coly010 merged 52 commits intoAug 7, 2026
Merged
Conversation
`supabase db start` delegated its container-bootstrap step to the bundled Go binary via a hidden `db __db-bootstrap --mode start` seam. Ports this to native TS, including the `--from-backup` restore path (a distinct entrypoint variant, backup bind mount, health-check swallow, and full setup skip) that had zero Go test coverage to check against — verified empirically by executing the real Go binary and diffing its container-create payload byte-for-byte against the TS output. Rather than duplicating `supabase start`'s existing container-bootstrap sequence a second time, extracts a shared `legacyStartDatabase` (mirroring Go's own single `StartDatabase` function, which both `db start` and `supabase start` call) into `legacy/shared/db-bootstrap/` — along with the rest of the container-lifecycle/health-check/db-setup/postgres-spec machinery that command family already had, hoisted per this repo's "Hoist Before You Duplicate" rule now that a second command family needs it. Also: hoists the already-native `isDbRunning` probe out of the Go-proxy-named seam (zero Go involvement, a plain `docker container inspect`) so `db start` composes no Go delegation at all anymore, and removes the now-unreachable `case "start"` dispatch arm from the Go-side hidden seam (the real, customer-facing `db start` Go command and `StartDatabase` itself are untouched and remain the parity oracle). Fixes CLI-1954
…ootstrap (review: PRRT_kwDOErm0O86VhJWm) Go's apply.MigrateAndSeed (internal/migration/apply/apply.go:16-26) applies db.migrations.schema_paths instead of migration files when --experimental is set, version is empty, and pg-delta is disabled. legacyMigrateAndSeed never ported that branch because its only prior caller (migration down) always passes a concrete version, making it provably unreachable there. CLI-1954's db-setup.ts is a new caller with version: "", making the branch reachable for both db start and (since the two share this helper) supabase start. Threads experimental through db-setup.ts -> start-database.ts -> both handlers, and ports Go's Glob.SQLFiles (directory expansion, sort, dedup) via a new legacyResolveSchemaPathFiles, reusing the fs.Glob port [db.seed] sql_paths already has (hoisted to legacy-glob.ts).
…t (review: PRRT_kwDOErm0O86VhJWp) ["db", "start"] stayed in run.ts's selfManagedSignalCommands from when it delegated to the hidden `db __db-bootstrap --mode start` Go seam, which held SIGINT/SIGTERM itself. CLI-1954 removes that delegation, but the native legacyDbStart/legacyStartDatabase installs no signal handling of its own — leaving the exemption in place meant Ctrl-C mid-bring-up hard-killed the process, skipping legacyRollbackStart entirely. Same fix top-level `start` already got when it went native: rely on the global signal-interrupt wrapper's Fiber.interrupt, which drives the same Effect.onError(() => legacyRollbackStart(...)) wrapper both callers of legacyStartDatabase already use.
…nd mounts (review: PRRT_kwDOErm0O86VhJWs) secretFiles stages a secret to a HOST temp file and bind-mounts it into the container (avoiding a docker-create-argv exposure problem, CWE-214/522) - already the mechanism supabase start's PG15+ path, kong.service.ts, and supavisor.service.ts all share since before CLI-1954. Docker resolves a bind mount's source against the daemon host, not the client, so a remote DOCKER_HOST/context (which legacyGetHostname elsewhere in this codebase explicitly supports) would see a missing path, unlike Go's own heredoc/Cmd- embed delivery (no host path at all). Fixing this for real means changing how every secretFiles caller creates its container (e.g. docker cp into a created-but-not-started container instead of a bind mount) - a cross-service redesign out of scope for db start's own bootstrap port. Documenting the trade-off explicitly here so it is a tracked, deliberate limitation rather than a silent one.
…IDE_EFFECTS.md Follow-up to the legacyMigrateAndSeed fix (review: PRRT_kwDOErm0O86VhJWm): both db start's and supabase start's SIDE_EFFECTS.md were missing the new observable behavior (schema_paths files read/applied instead of migrations, and the SUPABASE_EXPERIMENTAL/--experimental env dependency) per this repo's side-effect documentation requirement.
…atch Go parity (review: PRRT_kwDOErm0O86Vh_lq, PRRT_kwDOErm0O86Vh_ly, PRRT_kwDOErm0O86Vh_lu)
Wire `schema_paths` through `legacyCheckDbToml`/`legacy-db-config.toml-read.ts`
the same way `db.seed.sql_paths` already is, instead of reading the raw,
unresolved `ProjectConfig` value in db-setup.ts:
- Honor `SUPABASE_DB_MIGRATIONS_SCHEMA_PATHS` (Go's viper AutomaticEnv,
config.go:494-498) and the matched `[remotes.*]` override tier, matching
every sibling `db.migrations`/`db.seed` field.
- Resolve each relative pattern with Go's `path.Join(builder.SupabaseDirPath,
pattern)` semantics (config.go:976-978), which cleans `.`/`..` segments —
`legacyResolveSchemaPathFiles` no longer does its own naive
`supabase/${pattern}` string-prefixing, so `./schemas/a.sql` and
`schemas/a.sql` now collapse to the same glob pattern instead of aliasing
as two different ones and applying the file twice.
- Propagate a declarative-directory read/walk failure as a `problems` entry
(Go's `walkMatchedDir`'s "failed to walk matched directory: %w") instead of
silently treating an unreadable matched directory as empty — a fresh
`db start` could previously report success while skipping an intended
schema directory entirely.
…larative apply failure (review: PRRT_kwDOErm0O86Vh_lz) Go's `applySchemaFiles` sets `utils.CmdSuggestion = "See schema file: <fp>"` immediately after a failing `ExecBatch` (apply.go:57), which root.go prints verbatim on stderr and which suppresses the generic "--debug" fallback suggestion. The native declarative-schema-files branch only carried the raw database error message, dropping this hint. `LegacyMigrationApplyError` now carries an optional `suggestion`, populated by `legacyApplySchemaFiles` for this one call site; the existing generic `normalizeCliError` fallback already surfaces any error's `suggestion` field, so no output-layer changes are needed.
…eview: PRRT_kwDOErm0O86Vii6t) Go's walkMatchedDir (pkg/config/config.go:194-207) never follows a symlinked DirEntry from fs.WalkDir: entry.Type().IsRegular() is false for a symlink regardless of target, and WalkDir never descends into a symlinked subdirectory either. The port's recursive readDirectory + follow-symlinks fs.stat replicated neither half, so a symlinked .sql file (or an entire symlinked subdirectory's contents) could be applied on the --experimental declarative schema-files bootstrap path. The FileSystem service has no non-following lstat, so legacyWalkSqlFiles manually walks each directory and probes every entry via fs.readLink (succeeding = symlink) before deciding whether to recurse or include it, mirroring WalkDir's behavior with only the primitives the service already exposes. The top-level match's own fs.stat is unchanged, since Go's top-level fs.Stat on a Glob match also follows symlinks - only the walk inside a matched directory needed the fix.
… bootstrap (review: PRRT_kwDOErm0O86Vii6v)
Go's Config.Load (flags.LoadConfig) decodes every time.Duration config
field and runs (s *sms) validate() unconditionally, for every command
that loads config - including db start, even though db start never
starts GoTrue itself. Before this PR removed the Go container-bootstrap
delegation, that validation happened for free (the subprocess loaded
config the same way any Go command does); the native path dropped it,
so a malformed auth.email.max_frequency (for example) would no longer
fail db start before Docker work, unlike Go.
Added the same eager validation commands/start/start.handler.ts
already performs for this exact reason: auth.email.max_frequency,
auth.sms.max_frequency (+ the SMS-disabled warning),
auth.sessions.{timebox,inactivity_timeout}, and
auth.mfa.phone.max_frequency, reusing the already-hoisted
legacyResolveAuthEmail/legacyResolveAuthSms/legacyResolveAuthMfa.
Hoisted resolveGotrueSessions (previously private to
commands/start/start.handler.ts) into legacy-local-config-values.ts as
legacyResolveGotrueSessions since it now has a second caller, per
apps/cli/CLAUDE.md's "Hoist Before You Duplicate".
…ed paths (review: PRRT_kwDOErm0O86Vii6w) Go's Glob.files calls fs.Glob(fsys, filepath.ToSlash(pattern)) (config.go:143-145) before any meta-detection or directory-splitting - a no-op on POSIX but on Windows it turns every backslash into a forward slash first. The port had no equivalent, so a Windows entry with backslashes (an absolute path is preserved verbatim by legacyResolveSeedSqlPath, but a relative one can carry them too) hit legacyHasGlobMeta's backslash branch and then found no "/" to split on, leaving dirPattern empty and the whole path as filePattern - silently resolving to nothing instead of the configured file. Added the same OS-gated normalization at the top of legacyGlobPattern, keyed on path.sep (mirrors Go's runtime.GOOS gate) rather than introducing a new dependency. This is shared by every legacyGlobPattern caller (db.seed.sql_paths too), not just schema_paths.
…_EFFECTS.md Pre-existing oxfmt drift (table divider rows narrower than their header/ cell widths) surfaced by fmt:check while working this workspace; no content changed.
… db start (review: PRRT_kwDOErm0O86VjUtj) Go's start.Run calls flags.LoadConfig (full config load + validation, including the eager auth.*.max_frequency/timebox/inactivity_timeout duration parsing) before AssertSupabaseDbIsRunning (internal/db/start/start.go:45-47). The native db start port had this backwards: the duration-field validation added in fea3be9 ran after the already-running return, so a malformed auth.email.max_frequency (for example) exited 0 with "already running" instead of failing, whenever Postgres happened to already be up. Moved legacyLoadLocalProjectContext + the duration-field validation block above the running check, leaving the rest of db start's own prelude (experimental gate, legacyResolveLocalConfigValues, legacyResolveDbBootstrapConfig) after it, since those correspond to Go's StartDatabase bring-up (only reached on the not-running branch), not to LoadConfig itself. Added an integration test mirroring the existing "undecryptable secret even when already running" case for this exact scenario.
…ma/seed pattern (review: PRRT_kwDOErm0O86VjUtk)
legacyGlobPattern split a glob pattern's directory component by
slicing before the last "/", collapsing a root-anchored absolute
pattern like "/*.sql" to an empty dirPattern indistinguishable from
the truly-relative no-slash case — so it globbed the workdir instead
of the filesystem root, and any match would lose its leading "/".
Verified against the real Go CLI's own io/fs.Glob (via a throwaway
probe importing apps/cli-go/pkg/config directly, per
go-removal-sweep/parity-verification.md): Glob{"/*"}.Files(fsys)
against the real, unrooted afero.NewOsFs() the CLI actually uses lists
the real filesystem root's entries, each still "/"-prefixed, not the
process's cwd. Go's identical path.Split/cleanGlobPath split also
reduces a Windows drive-root pattern (post filepath.ToSlash) to a bare
"C:" directory, which legacyResolveUnderWorkdir's path.isAbsolute check
alone doesn't recognize as "don't join under workdir" (Node's win32
isAbsolute requires the trailing separator) — gave that the same
verbatim-passthrough treatment.
Added apps/cli/src/legacy/shared/legacy-glob.unit.test.ts (previously
untested) covering both the POSIX root case and the Windows
drive-root case (via BunPath.layerWin32, deterministic regardless of
host OS), plus the pre-existing relative-pattern behavior for
regression coverage.
…rVersion gate (review: PRRT_kwDOErm0O86VkCcD) legacyStartDatabase created the Docker network before the pre-create volume-existence probe and the --from-backup-on-an-existing-volume guard. Go's StartDatabase runs VolumeInspect and that guard strictly BEFORE DockerStart, which is the ONLY place Go ever creates the network (apps/cli-go/internal/utils/docker.go:363-386) - so an invalid/uncreatable --network-id could mask the "backup volume already exists" error and leave a stray network behind on a request Go would have rejected outright. Moved the network-ensure call to run after the volume probe/guard, right before the image is used to build the container spec. Also gates the lazy setup.jwks resolve on setup.majorVersion >= 15, not just realtimeEnabledForSetup: Go's initSchema (start.go:243-254) only ever reaches initSchema15's ResolveJWKS call on PG15+; the PG13/14 branch (InitSchema14) never touches JWKS at all, so a PG13/14 database with realtime enabled must not pay for (or fail on) an external JWKS fetch it will never use (review: PRRT_kwDOErm0O86VkCcE). Also stops batch-resolving the three PG15+ setup-job images upfront via legacyEnsureImagesCached and instead threads the raw, pin-rewritten image references straight through - db-setup.ts's own legacyRunStartMigrateJob now resolves each one individually, right before it runs (review: PRRT_kwDOErm0O86VkCcF).
…upfront (review: PRRT_kwDOErm0O86VkCcF) legacyRunStartMigrateJob now resolves its own image individually, via legacyEnsureImagesCached, immediately before that specific job runs - matching Go's DockerRunJob -> DockerStart -> DockerResolveImageIfNotCached (docker.go:363-365), which resolves each one-shot migrate job's image sequentially, exactly where it's used. Previously start-database.ts batch-resolved all three (realtime/storage/auth) images upfront, so one unreachable image (e.g. Storage's) failed the whole fresh-volume setup before an earlier job (e.g. Realtime's) ever got to run, even though Go would already have run it to completion by the time it reaches Storage's own resolve. Threading projectEnvValues through this per-job resolve also preserves the existing project-dotenv-only registry-override behavior (legacyDockerRun.runCapture's own ambient resolver never sees it) - see "resolves an excluded service's migrate-job image through a project-dotenv-only registry override" in start.integration.test.ts. Also updates this module's header comment to accurately describe the still-unported pgcache.TryCacheMigrationsCatalog warm-up (start.go:371-379) as a real, tracked gap rather than a no-op divergence: the already-ported legacyTryCacheMigrationsCatalog would close it, but it needs LegacyEdgeRuntimeScript/LegacyPgDeltaSslProbe in its effect environment, which would widen legacyStartDatabase's (and both db start's and supabase start's) environment requirements across their entire call graph and test suites - deliberately deferred to a follow-up rather than folded into this hoist (review: PRRT_kwDOErm0O86VkCcB).
`supabase db reset`'s local path delegated its container-recreate work to the bundled Go binary via a hidden `db __db-bootstrap --mode recreate` / `--mode await-storage` seam. Ports this to native TS and deletes the seam entirely (both files). The issue's premise that reset "reuses the same create/health/ SetupLocalDatabase chain the native start port already implements" was wrong — Go's `resetDatabase15` never calls `StartDatabase`; it's a distinctly different composition (no volume probe, no `--from-backup` concept, unconditional setup with the *resolved* migration version instead of `""`, no rollback, no `_current_branch` write). This port builds a reset-specific `legacyRecreateLocalDatabase` directly over the same primitives `db start` uses, rather than wrapping `legacyStartDatabase`. Also native now: the PG14 recreate branch (template1 `DROP`/`CREATE DATABASE`, disconnect-clients with Go's swallow/surface semantics, replication-slot drain, `InitSchema14`/`ApplyApiPrivileges`), the concurrent satellite-container restart + Kong `nginx reload` (added same-day upstream to fix issue #6016 — this reload fails the whole command on error, unlike the existing best-effort one in `functions serve`), and the storage-container health gate. An empirical probe (real Postgres 14/15, the exact pinned pgconn/pgx versions) settled an open question about Go's `DROP`/`CREATE DATABASE` batching before this landed: it works via subtle protocol semantics the TS port doesn't need to replicate — four sequential, unwrapped statement execs reproduce the same real-world behavior more simply. Also, since this is the third `legacy/shared/db-bootstrap/` consumer: split the directory into `legacy/shared/containers/` (generic, cross- service Docker primitives) and a narrower `db-bootstrap/` (Postgres- specific), hoisted the container-CLI boilerplate duplicated across the new remove/restart primitives, and extracted the local container-input prelude `db start` and `db reset` were duplicating verbatim into a shared `legacyBuildLocalDbContainerInputs`. Known, deliberate scope boundary: `db schema declarative`'s smart-target and `db schema sync` still spawn `db reset --local` through the Go binary's own `reset.Run` via a wholly unrelated seam (`LegacyDeclarativeSeam.execInherit`) — so Go isn't fully removed from every `db reset --local` code path yet. Fixing that needs `legacyDbReset` made in-process-callable, a materially larger refactor out of scope here. Fixes CLI-1955
Contributor
Author
|
@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
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c07a87d60
ℹ️ 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".
…strap (review: PRRT_kwDOErm0O86VkkNY) Go's godotenv.Load installs a project .env's DOCKER_HOST/DOCKER_CONTEXT/etc into the process environment (pkg/config/config.go:1261) before any Docker work, so a daemon target configured only in supabase/.env still governs start/stop/status/db start. legacyLoadLocalProjectContext never applied those keys to process.env, so legacyGetHostname() and every Docker subprocess this PR's native db start bootstrap spawns silently fell back to the shell's own environment instead.
…_kwDOErm0O86VkkNb) Go's initSchema15 passes utils.GetDebugLogger() (os.Stderr under --debug, else io.Discard) as each PG15+ realtime/storage/auth one-shot migrate job's stderr writer (start.go:349-353), so a failed fresh-volume migration job's own diagnostics are visible under --debug, not just its exit code. legacyRunStartMigrateJob called runCapture with no teeStderr option at all, so db start/supabase start --debug surfaced only "error running container: exit N" regardless of the flag. Thread --debug through LegacyStartDatabaseSetupInput/LegacyStartSetupLocalDatabaseInput into runCapture's existing teeStderr option.
…eeing (review: PRRT_kwDOErm0O86VkkNY, PRRT_kwDOErm0O86VkkNb) Record both start/SIDE_EFFECTS.md fixes: DOCKER_HOST/DOCKER_CONTEXT/etc are now also read from a project .env, and --debug tees the fresh-volume one-shot migrate jobs' stderr.
…ers (review: PRRT_kwDOErm0O86VkikD) legacyIsContainerNotFoundMessage matched Docker's "No such container"/"No such object" case-sensitively, missing Podman's lowercase variants and its "no container with name or ID" wording that start.handler.ts's own Podman-aware parser already tolerates. db reset --local's new satellite restart/Kong reload tolerance (restart-services.ts) relied on this predicate, so a database-only db start or excluded storage/auth/realtime/pooler/Kong services would report a hard restart/reload failure on Podman instead of tolerating the absent container, unlike the Go implementation's errdefs.IsNotFound (which is text/case agnostic).
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… PRRT_kwDOErm0O86Vk-ex) Go's docker/cli reads DOCKER_CONFIG (`cli/config/config.go`'s EnvOverrideConfigDir) to locate config.json/the context store, and legacyGetHostname()'s dockerConfigDir() reads the same env var — but legacyIsDockerClientEnvKey's whitelist omitted it, so a project dotenv that set only DOCKER_CONFIG never reached process.env, silently falling back to the ambient ~/.docker config for both hostname resolution and every docker/podman subprocess.
…ed in db start (review: PRRT_kwDOErm0O86Vk-e0, PRRT_kwDOErm0O86Vk-e2) Go's Config.Load decodes auth.rate_limit.* (plain uints) unconditionally in the same UnmarshalExact pass as the duration fields db start already eagerly re-validates, regardless of auth.enabled or whether db start ever reads the field — a malformed SUPABASE_AUTH_RATE_LIMIT_* override must fail the command the same way. Hoisted resolveGotrueRateLimit out of commands/start/start.handler.ts into legacy-local-config-values.ts (now a second caller, per apps/cli/CLAUDE.md's "Hoist Before You Duplicate") and call it from db start's own eager-validation block. Separately, Go's (s *sms) validate() — the source of the "no SMS provider is enabled" warning — only runs inside `if c.Auth.Enabled` (config.go:1087,1145). db start's port printed it unconditionally; gate it on the same SUPABASE_AUTH_ENABLED-overridden value Go's Validate reads, so a disabled-auth project with sms.enable_signup=true and no provider no longer prints a warning Go never emits.
…e Docker (review: PRRT_kwDOErm0O86VlOHQ) Go's Config.Load decodes the ENTIRE config struct in one unconditional v.UnmarshalExact pass, for every command that loads config (including db start), regardless of whether that command's own downstream logic ever reads the field. db start's eager-validation battery previously stopped after auth.rate_limit; it now also validates auth.web3, auth.oauth_server, auth.passkey, auth.external, api.enabled, api.tls.enabled, api.max_rows, storage.vector/s3_protocol/analytics fields, local_smtp ports, analytics ports, db.pooler fields, and edge_runtime.policy/inspector_port (the field Codex's review flagged), mirroring commands/start/start.handler.ts's own identical battery. Hoisted the three GoTrue resolvers (legacyResolveGotrueWeb3, legacyResolveGotrueOAuthServer, legacyResolveGotruePasskeyWebauthn) out of start.handler.ts (where they were module-private) into the shared legacy-local-config-values.ts, since db start is now a second caller.
…tch Go Two new Codex threads on this PR both argued db start diverges from Go, but neither survives a close read against apps/cli-go/: - PRRT_kwDOErm0O86VlqIJ: claimed excluding SUPABASE_SERVICES_HOSTNAME from the project-dotenv-to-process.env install loop drops a dotenv-only override. Go's GetHostname() has exactly one call site — the utils.Config package-level var initializer — which runs before main(), before cobra parses argv, before any command's Config.Load (and its dotenv pass) ever executes. A project-dotenv-only value can never reach it; only a shell-exported one can. Verified with a scratch Go probe reproducing the exact ordering. Extending the loop to this key would be a new divergence, not a fix. - PRRT_kwDOErm0O86VlqIK: claimed the eager legacyResolveGotruePasskeyWebauthn call reimplements Config.Validate's passkey/webauthn rule. The actual "Missing required config section" rule already lives exclusively in legacyValidateResolvedConfig, invoked via legacyCheckDbToml as the very first line of this handler — before the eager-decode battery runs. The later call reuses the same shared resolver purely to surface its internal decode-hook errors eagerly (Go's unconditional Config.Load field decode), discarding the result, identical to the auth.web3/auth.oauth_server calls beside it. Both threads get a reply on the PR with this reasoning; these are doc-only comments recording it at the flagged call sites so a future reviewer doesn't re-raise the same false positive.
Go registers `network-id` as a persistent flag bound to viper under
SetEnvPrefix("SUPABASE") + AutomaticEnv() (cmd/root.go:318-334) — the same
mechanism already ported for SUPABASE_YES/SUPABASE_EXPERIMENTAL — and
DockerStart reads viper.GetString("network-id") fresh at its own call site,
well after Config.Load's dotenv pass (docker.go:379-383). Both db start and
start computed only `--network-id` flag -> generated network name, silently
dropping the shell/project-dotenv env fallback and attaching containers to
the wrong network when only the env var was set.
Adds legacyViperEnvStringWithProjectFallback (legacy-viper-env.ts) alongside
the existing bool helper, and legacyResolveNetworkId (legacy-docker-ids.ts)
composing flag -> env -> generated-name, matching Go's precedence. The exact
same duplicated snippet existed in both db/start/start.handler.ts and
start/start.handler.ts (both touched by this PR) — fixed via the one shared
helper per the "hoist before you duplicate" rule instead of patching db start
alone and leaving start with the same gap.
review: PRRT_kwDOErm0O86VlqIL
…p enabled before db start's already-running shortcut (review: PR #6022) Go's Config.Load decodes edge_runtime.enabled, db.network_restrictions.enabled, studio.enabled, and local_smtp.enabled unconditionally in the same viper AutomaticEnv() pass as every other field this eager battery already covers (pkg/config/config.go:749-756). None of their containers are ever built by db start, but a malformed SUPABASE_*_ENABLED override must still fail before AssertSupabaseDbIsRunning, matching Go — these four were previously only validated via legacyResolveLocalConfigValues in the not-running branch, so a bad override was silently accepted whenever Postgres was already running.
…tery (review: PR #6022) Go's Config.Load decodes db.pooler.enabled unconditionally, same as its port/pool_mode/default_pool_size/max_client_conn siblings, but db start's eager-validation battery only force-decoded the latter four. A malformed SUPABASE_DB_POOLER_ENABLED override was silently accepted and Postgres would still start, unlike Go.
…review: PR #6022) Go's Config.Validate rejects studio.port === 0 only when studio.enabled, before AssertSupabaseDbIsRunning. legacy-config-validate.ts marks this check L-only (D's LegacyConfigValidationInput has no studio section), so legacyCheckDbToml never catches it for db start. A malformed or zero SUPABASE_STUDIO_PORT was silently accepted whenever Postgres was already running.
… PR #6022) Go's utils.WriteFile writes supabase/.branches/_current_branch through afero.WriteFile(fsys, path, contents, 0644). Effect's writeFileString without an explicit mode falls back to Node's default (0666 before the umask), so under a permissive/group-writable umask this file could end up 0666/0664 instead of 0644, making project branch metadata writable by additional local users.
…iss (review: PR #6022) Go's DockerResolveImageIfNotCached proceeds to the pull loop only when docker image inspect fails with a confirmed errdefs.IsNotFound; every other inspect error (an auth-plugin denial, an invalid reference, an API error, ...) returns immediately instead. This port's hasLocalImage had the polarity backwards: it only fast-failed on a daemon-unreachable message and treated every other inspect failure as a cache miss, sending it through the multi-registry pull loop instead - performing unauthorized network operations, delaying the failure by each retry backoff, and replacing the real inspect error with a pull aggregate. Flips the default to fail, and only treats a confirmed "no such image" (verified empirically against a real docker image inspect) as a cache miss. Updates the existing mocks that relied on the old "anything but daemon-unreachable is a cache miss" default to return a genuine not-found response, and adds a regression test for the new fail-fast branch.
…: PR #6022) Go's GetPendingSeeds resolves db.seed.sql_paths through locals.SQLFiles(fsys) - the same Glob.SQLFiles method db.migrations.schema_paths resolves through - which expands a matched directory to its sorted, regular .sql files recursively. This port's resolveSeedFiles instead used the plainer Glob.Files-equivalent (legacyGlobPattern) with no directory expansion, so a directory seed entry (e.g. sql_paths = ["./seeds"]) resolved to the directory itself and then failed being read as a seed file. Hoists the directory-walk helper (previously private to legacy-migrate-and-seed.ts) into legacy-glob.ts so both callers share one Glob.SQLFiles port, and reuses it in legacy-seed.ts's resolveSeedFiles - preserving its existing warn-only (never hard-fail) error handling, which differs from the schema-paths caller's fail-when-empty behavior.
…4-port-db-start-container-bootstrap-natively-and-remove-the # Conflicts: # apps/cli/docs/go-cli-porting-status.md # apps/cli/src/legacy/shared/db-bootstrap/rollback.ts
Merging develop added a debug parameter to legacyRollbackStart (shared/ db-bootstrap/rollback.ts) for supabase start's own call sites; db start's call site (added on this branch) needed the same update to keep types:check green. Also re-runs oxfmt on go-cli-porting-status.md after merge conflict resolution.
…5-port-db-reset-local-recreate-natively-and-remove-the-__db # Conflicts: # apps/cli/docs/go-cli-porting-status.md # apps/cli/src/legacy/shared/db-bootstrap/rollback.ts # apps/cli/src/legacy/shared/legacy-container-cli.ts
origin/develop (#6037) added a debug parameter to legacyRollbackStart and renamed legacyEnsureStartVolume/LegacyStartVolumeCreateError to legacyEnsureVolume/LegacyVolumeCreateError independently of this branch's own container-lifecycle.ts consolidation. Update db start's call site and its stale test names/references to match post-merge.
…t-container-bootstrap-natively-and-remove-the' into columferry/cli-1955-port-db-reset-local-recreate-natively-and-remove-the-__db # Conflicts: # apps/cli/docs/go-cli-porting-status.md # apps/cli/src/legacy/commands/db/start/SIDE_EFFECTS.md # apps/cli/src/legacy/commands/db/start/start.handler.ts # apps/cli/src/legacy/commands/start/SIDE_EFFECTS.md # apps/cli/src/legacy/shared/db-bootstrap/db-setup.ts # apps/cli/src/legacy/shared/db-bootstrap/db-setup.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/rollback.ts # apps/cli/src/legacy/shared/db-bootstrap/start-database.ts
Base automatically changed from
columferry/cli-1954-port-db-start-container-bootstrap-natively-and-remove-the
to
develop
August 6, 2026 11:55
…5-port-db-reset-local-recreate-natively-and-remove-the-__db # Conflicts: # apps/cli-e2e/src/tests/live/db-reset-start.live.e2e.test.ts # apps/cli-go/cmd/db.go # apps/cli/docs/go-cli-porting-status.md # apps/cli/src/legacy/commands/db/reset/reset.handler.ts # apps/cli/src/legacy/commands/db/reset/reset.integration.test.ts # apps/cli/src/legacy/commands/db/shared/legacy-db-bootstrap.errors.ts # apps/cli/src/legacy/commands/db/shared/legacy-db-bootstrap.seam.layer.ts # apps/cli/src/legacy/commands/db/shared/legacy-db-bootstrap.seam.service.ts # apps/cli/src/legacy/commands/db/start/SIDE_EFFECTS.md # apps/cli/src/legacy/commands/db/start/start.handler.ts # apps/cli/src/legacy/commands/db/start/start.integration.test.ts # apps/cli/src/legacy/commands/db/start/start.layers.ts # apps/cli/src/legacy/commands/start/SIDE_EFFECTS.md # apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts # apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts # apps/cli/src/legacy/commands/start/lib/docker-create-args.ts # apps/cli/src/legacy/commands/start/lib/docker-create-args.unit.test.ts # apps/cli/src/legacy/commands/start/lib/health-check.ts # apps/cli/src/legacy/commands/start/lib/health-check.unit.test.ts # apps/cli/src/legacy/commands/start/lib/image-prepull.ts # apps/cli/src/legacy/commands/start/lib/image-prepull.unit.test.ts # apps/cli/src/legacy/commands/start/services/edge-runtime.service.ts # apps/cli/src/legacy/commands/start/services/gotrue.service.ts # apps/cli/src/legacy/commands/start/services/imgproxy.service.ts # apps/cli/src/legacy/commands/start/services/kong.service.ts # apps/cli/src/legacy/commands/start/services/logflare.service.ts # apps/cli/src/legacy/commands/start/services/mailpit.service.ts # apps/cli/src/legacy/commands/start/services/pg-meta.service.ts # apps/cli/src/legacy/commands/start/services/postgrest.service.ts # apps/cli/src/legacy/commands/start/services/realtime.service.ts # apps/cli/src/legacy/commands/start/services/storage.service.ts # apps/cli/src/legacy/commands/start/services/studio.service.ts # apps/cli/src/legacy/commands/start/services/supavisor.service.ts # apps/cli/src/legacy/commands/start/services/vector.service.ts # apps/cli/src/legacy/commands/start/start.gates.ts # apps/cli/src/legacy/commands/start/start.handler.ts # apps/cli/src/legacy/commands/start/start.integration.test.ts # apps/cli/src/legacy/commands/stop/SIDE_EFFECTS.md # apps/cli/src/legacy/shared/containers/container-lifecycle.ts # apps/cli/src/legacy/shared/containers/container-lifecycle.unit.test.ts # apps/cli/src/legacy/shared/containers/docker-create-args.ts # apps/cli/src/legacy/shared/containers/docker-create-args.unit.test.ts # apps/cli/src/legacy/shared/containers/health-check.ts # apps/cli/src/legacy/shared/containers/health-check.unit.test.ts # apps/cli/src/legacy/shared/containers/image-prepull.ts # apps/cli/src/legacy/shared/containers/image-prepull.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/container-lifecycle.ts # apps/cli/src/legacy/shared/db-bootstrap/container-lifecycle.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/db-setup.ts # apps/cli/src/legacy/shared/db-bootstrap/db-setup.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/docker-create-args.ts # apps/cli/src/legacy/shared/db-bootstrap/docker-create-args.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/health-check.ts # apps/cli/src/legacy/shared/db-bootstrap/health-check.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/image-prepull.ts # apps/cli/src/legacy/shared/db-bootstrap/image-prepull.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/local-db-running.ts # apps/cli/src/legacy/shared/db-bootstrap/postgres.service.ts # apps/cli/src/legacy/shared/db-bootstrap/rollback.ts # apps/cli/src/legacy/shared/db-bootstrap/rollback.unit.test.ts # apps/cli/src/legacy/shared/db-bootstrap/start-database.ts # apps/cli/src/legacy/shared/legacy-bitbucket-pipeline.ts # apps/cli/src/legacy/shared/legacy-docker-bind-classify.ts # apps/cli/src/legacy/shared/legacy-docker-image-resolve.ts # apps/cli/src/legacy/shared/legacy-docker-image-resolve.unit.test.ts # apps/cli/src/legacy/shared/legacy-glob.ts # apps/cli/src/legacy/shared/legacy-glob.unit.test.ts # apps/cli/src/legacy/shared/legacy-kong-auth.ts # apps/cli/src/legacy/shared/legacy-local-config-values.ts # apps/cli/src/legacy/shared/legacy-local-project-context.ts # apps/cli/src/legacy/shared/legacy-local-project-context.unit.test.ts # apps/cli/src/legacy/shared/legacy-start-secrets-cleanup.ts # apps/cli/src/shared/cli/run.ts # apps/cli/src/shared/cli/run.unit.test.ts
Coly010
marked this pull request as ready for review
August 6, 2026 13:55
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@9df1d7b2f09475d3a5d8b1251b327e80375ab0f9Preview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a1c55ee70
ℹ️ 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".
Hoists db reset's local-recreate composition into a shared legacyResetLocalDatabase (legacy/shared/db-bootstrap/reset-local-database.ts) and rewires db schema declarative's smart-target local-reset prompt and db schema sync's failed-apply recovery reset to call it in-process, instead of shelling out to a second supabase-go child via LegacyDeclarativeSeam .execInherit (now removed). Go's own db_schema_declarative.go calls reset.Run in-process too, sharing the outer command's PersistentPostRun — the removed subprocess design instead fired a second, independent telemetry/linked-project-cache cycle from the child process's own Execute(), which this closes. reset.handler.ts's own cfg.isLocal branch becomes a thin wrapper around the extracted function, keeping only version/seed-flags resolution and the JSON envelope. await-storage-ready.ts moves alongside it into db-bootstrap/ since it now has a second caller.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e899e290c3
ℹ️ 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".
avallete
reviewed
Aug 7, 2026
avallete
left a comment
avallete
left a comment
Member
There was a problem hiding this comment.
Two findings that we might want to address before merge:
1. HIGH — Kong reload drops --nginx-conf /home/kong/custom_nginx.template
restart-services.ts:207 execs ["kong", "reload"] bare. Go passes the template flag (reset.go:269, pinned by Go's own test), and the TS functions serve reload already passes it (shared/functions/serve.ts:1351). Without it, Kong regenerates nginx config from its default template and loses the custom email_templates listener — so every db reset --local with Kong running re-introduces issue #6059 (broken custom auth-email templates until a Kong restart). That upstream fix (#6065) landed only a week ago; this PR silently reverts it on the reset path. The unit test only asserts exec <kongId> happened, not the argv, which is why nothing caught it.
2. MODERATE — PG14 declarative reset uses raw, unnormalized schema_paths
recreate-local-database.ts:463 passes setup.config.db.migrations.schema_paths (raw config value) into MigrateAndSeed, while the PG15 path correctly uses the normalized toml.schemaPaths (db-setup.ts:913) — which is loaded four lines above and unused. Raw patterns aren't supabase/-prefixed and ignore SUPABASE_DB_MIGRATIONS_SCHEMA_PATHS. Narrow path (PG≤14 + --experimental + declarative schemas), but it globs the wrong files or hard-fails after the database has already been dropped and recreated.
…paths in db reset Bare `kong reload` in the local-db reset path regenerated nginx.conf from Kong's default template, dropping the custom email_templates listener and reintroducing #6059. Go's reloadKong (reset.go:269) always passes --nginx-conf /home/kong/custom_nginx.template, same as the functions serve reload path already does. The PG14 declarative reset also passed the raw, unresolved db.migrations.schema_paths into MigrateAndSeed instead of the normalized toml.schemaPaths the PG15 path already uses, so schema-path patterns weren't supabase/-prefix-resolved or SUPABASE_DB_MIGRATIONS_SCHEMA_PATHS-overridden. Addresses #6026 (review)
Collapses the toml fixture string in the schema_paths regression test onto one line per oxfmt's line-width rule.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9df1d7b2f0
ℹ️ 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".
avallete
approved these changes
Aug 7, 2026
Coly010
deleted the
columferry/cli-1955-port-db-reset-local-recreate-natively-and-remove-the-__db
branch
August 7, 2026 16:26
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.
Stacks on #6022
This PR is based on
columferry/cli-1954-port-db-start-container-bootstrap-natively-and-remove-the(#6022), notdevelop— it needs to edit that PR's newlegacy/shared/db-bootstrap/code before #6022 has merged. GitHub will show #6022's diff here too until that PR merges; once it does, this PR's diff will narrow to just what's described below.What changed
supabase db reset's local path delegated its container-recreate work to the bundled Go binary via a hiddendb __db-bootstrap --mode recreate/--mode await-storageseam. Ports this to native TS and deletes the seam entirely (both files).The issue's premise — that reset "reuses the same create/health/SetupLocalDatabase chain the native start port already implements" — was wrong. Go's
resetDatabase15(internal/db/reset/reset.go) never callsStartDatabase; it's a distinctly different composition (no volume-existence probe, no--from-backupconcept, unconditional setup with the resolved migration version instead of"", no rollback-on-failure, no_current_branchwrite). This port builds a reset-specificlegacyRecreateLocalDatabasedirectly over the same underlying primitivesdb startuses, rather than wrappinglegacyStartDatabase.Also native now:
DROP/CREATE DATABASE, disconnect-clients with Go's exact swallow/surface semantics (a genuine server error surfaces; a node-level socket error or the "database doesn't exist yet" case is swallowed), replication-slot drain with backoff,InitSchema14/ApplyApiPrivileges(deliberately narrower than the PG15+SetupLocalDatabase— no globals.sql/vault/roles.sql).nginx reload(the Kong-reload behavior was added same-day upstream to fix issue supabase db reset can leave Kong routing to a stale container IP, causing 502 on /auth/v1/* #6016 — this reload fails the whole command on error, unlike the existing best-effort Kong reload infunctions serve, matching Go's own two different policies for the two call sites).AwaitStorageReady) — any inspect error maps to "absent" (not just not-found), and an unhealthy-but-present container triggers a hardcoded 30s wait that fails the whole reset on timeout, not just "skip bucket seeding."An empirical probe (real Postgres 14 and 15, using the exact pinned pgconn/pgx versions from
apps/cli-go/go.mod) settled an open question before implementation: whether Go's single-batchDROP/CREATE DATABASEsequence is safe against Postgres's "cannot run inside a transaction block" restriction. It is — pgconn's batching semantics never trigger that guard — and the TS port doesn't need to replicate any of that protocol-level behavior: four sequential, unwrapped statement execs reproduce the identical real-world result more simply.Since this is the third consumer of
legacy/shared/db-bootstrap/, also did the directory split that milestone review had been deferring: split it intolegacy/shared/containers/(generic, cross-service Docker primitives used well beyond Postgres bootstrap) and a narrowerdb-bootstrap/(genuinely Postgres-specific), hoisted the container-CLI boilerplate that had been duplicated across the new remove/restart primitives into the existinglegacy-container-cli.ts, and extracted the local container-input preludedb startanddb resetwere duplicating verbatim (~130 lines) into a sharedlegacyBuildLocalDbContainerInputs.Follow-up: closing the local-reset scope boundary (CLI-2062)
The PR originally left one boundary open:
db schema declarative's smart-target local-reset prompt anddb schema sync's failed-apply recovery reset still shelled out to a secondsupabase-gochild (LegacyDeclarativeSeam.execInherit) to rundb reset --local, rather than calling the now-nativelegacyDbResetin-process. That subprocess design was itself a parity divergence: because it's a genuinely separate OS process, its ownExecute()/PersistentPostRunfired an independent secondcli_command_executedtelemetry event and linked-project-cache write on top of the outerdb schema declarative/synccommand's own — something real single-process Go never does (Go'sdb_schema_declarative.gocallsreset.Runas a plain in-process function call, sharing the one outerPersistentPostRun).This is now fixed:
cfg.isLocalbranch oflegacyDbResetinto a new sharedlegacyResetLocalDatabase(legacy/shared/db-bootstrap/reset-local-database.ts) — self-contained, resolving its own services (LegacyDebugFlag,LegacyNetworkIdFlag,RuntimeInfo,ChildProcessSpawner,FileSystem,Path,LegacyCliConfig, project-env) rather than takingLegacyDbResetFlags/CliArgs, so it's callable from any Effect context.reset.handler.ts's owncfg.isLocalbranch is now a thin wrapper around it, keeping only the version/seed-flags plumbing and the JSON envelope (both specific to the top-leveldb resetcommand).db schema declarative's smart-target anddb schema sync's recovery-reset call sites to calllegacyResetLocalDatabasedirectly, dropping the--network-idargv-forwarding (the function now resolvesLegacyNetworkIdFlagitself from the shared context — a closer match to Go's single-process model). The synthesized`database reset failed (exit ${code})`error message is replaced with a message built from the real typed failure (`database reset failed: ${error.message}`), since there's no longer a literal subprocess exit code.execInheritentirely — from theLegacyDeclarativeSeaminterface, its real implementation, and every test mock that stubbed it.await-storage-ready.tsintolegacy/shared/db-bootstrap/alongsidelegacyResetLocalDatabase, since it now has a second caller.generate.layers.ts/sync.layers.tsnow exposelegacyDockerRunLayerdirectly (previously only nested inside their ownedgeRuntimecomposition) — needed forlegacyResetLocalDatabase's PG15+ one-shot migrate jobs, the same waydb start/db reset's own layers do.generate/sync's local-reset integration tests to exercise the real native reset (mockedChildProcessSpawner+ Docker CLI route, hoisted into a new sharedtests/helpers/legacy-local-reset.ts) instead of asserting trackedexecInheritcall args, and added explicit assertions that the outer command's telemetry-flush/linked-project-cache-write finalizer fires exactly once even though its body now calls an in-process helper that could, if wrongly implemented, double it.apps/cli-go) that no Go code becomes dead from removing this TS call site:internal/db/reset/reset.goremains fully reachable both via the Go binary's own top-leveldb resetcommand and via the remaining--experimentalremote-delegation path inreset.handler.ts.Why
Part of the M9 "Final Cleanup — Go Removal" milestone.
Fixes CLI-1955
Fixes CLI-2062