Skip to content

fix(cli): port shadow database provisioning to native TS (CLI-1956) - #6027

Merged
Coly010 merged 137 commits into
developfrom
columferry/cli-1956-port-shadow-database-provisioning-natively-and-remove-the-db
Aug 10, 2026
Merged

fix(cli): port shadow database provisioning to native TS (CLI-1956)#6027
Coly010 merged 137 commits into
developfrom
columferry/cli-1956-port-shadow-database-provisioning-natively-and-remove-the-db

Conversation

@Coly010

@Coly010 Coly010 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What changed

Ports the shadow-database provisioning used by db diff/db pull (create → health-wait → connect → setup/migrate → remove) from the hidden Go db __shadow seam to native TypeScript, and removes that seam from apps/cli-go/cmd/db.go. This was the last local-container orchestration db diff/db pull's native engines still delegated to Go for.

New shared primitives live in legacy/shared/db-bootstrap/shadow-database.ts (create/connect/setup/migrate/remove — kept as separate composable pieces rather than one monolithic function, since the two known future callers need different subsets: migration squash (CLI-1969) needs create → health-wait → connect → setup only, while db diff --use-pgadmin (CLI-1968) needs create → health-wait → migrate). legacy/commands/db/shared/legacy-shadow-source.ts composes these for db diff/db pull's --target-local declarative branch, which also needs pg-delta. legacy-pgdelta.apply.ts is a from-scratch port of Go's pgdelta.ApplyDeclarative.

Hoisted a shared legacyResolveDbSetupPrelude (db-setup.ts) so fresh-db setup and shadow setup stop duplicating the same JWKS/image-pull resolution, per this repo's "Hoist Before You Duplicate" rule.

Why

Part of the M9 milestone (Go removal) — this and the three PRs below it in the stack (#6021 CLI-1953, #6022 CLI-1954, #6026 CLI-1955) progressively remove the Go delegations that anchor the bundled Go binary. This PR removes the last one blocking db diff/db pull's native engines.

Reviewer-relevant context

  • The parent stack PRs (fix(cli): route bootstrap's db push step to native TS db push (CLI-1953) #6021, fix(cli): port db start container bootstrap to native TS (CLI-1954) #6022, fix(cli): port db reset local recreate to native TS (CLI-1955) #6026) have all merged, so this diff is now standalone.
  • An earlier revision described a "randomized per-invocation staged secret dir" for the shadow container; review showed that machinery was dead — secrets are delivered straight into the container via docker cp and nothing ever creates a staged dir on disk — so it was deleted outright. legacyRemoveShadowDatabase is now just (spawner, containerId).
  • Neither db diff nor db pull wires the LegacyDeclarativeSeam layer any more — db diff --use-pgadmin/--use-pg-schema proxy the whole invocation to the bundled Go binary rather than going through the seam. The seam now serves only db schema declarative generate/sync's baseline/declarative catalog modes (the remaining CLI-1959 scope).
  • A deep-review fix batch is included on top of the port (observable db diff/db pull behavior is unchanged except where noted): shared project-id resolution at every pg-delta site (fixes supabase_edge_runtime_: volume binds under env-only project ids), Go's PGDELTA_DEBUG shadow-catalog export in db diff, config validation before the "Creating shadow database..." banner, the relative path in the declarative-dir-not-found error, Go int64 bounds in the apply-output decoder, byte-ordered (Go fs.WalkDir) SQL-file walking, remote-override gating for ~20 more config keys, DEBUG resolution through the merged project env like viper, Go's exact unhealthy-container line format, %q/TrimSpace-exact apply-failure rendering, percent-round-tripping of special-character shadow DB passwords, and a rename of the apply-side error class that shared its Data.TaggedError tag with declarative.errors.ts's.
  • New shadow/apply error classes declare the error-actionability taxonomy metadata that landed on develop meanwhile (chore(cli): define error taxonomy #6132), and db diff/db pull's SIDE_EFFECTS.md now document the in-process shadow bring-up (dotenv/TLS/roles.sql reads and the SUPABASE_* override family).
  • The shadow container honors a config.toml [db] password — a deliberate TS extension carried over from develop's --local handling (Go rejects that key at config load and always uses postgres); documented at the builder, with the strict-rejection question tracked as a follow-up.

Fixes CLI-1956

Coly010 and others added 25 commits July 31, 2026 20:26
`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
…0 in /apps/cli-go in the go-minor group across 1 directory (#6023)

Bumps the go-minor group with 1 update in the /apps/cli-go directory:
[github.com/docker/go-connections](https://github.com/docker/go-connections).

Updates `github.com/docker/go-connections` from 0.7.0 to 0.8.0
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/docker/go-connections/commit/754f9060ef9371a6e9504a82e25bd0bce0cfe406"><code>754f906</code></a>
Merge pull request <a
href="https://redirect.github.com/docker/go-connections/issues/158">#158</a>
from thaJeztah/no_umask</li>
<li><a
href="https://github.com/docker/go-connections/commit/20f47a112d2119c502530055300c3ba272fa3e17"><code>20f47a1</code></a>
sockets: read somaxconn from system instead of SOMAXCONN</li>
<li><a
href="https://github.com/docker/go-connections/commit/e195e2a4e6e63b1ac25d4e1c2511170bdb030781"><code>e195e2a</code></a>
sockets: set socket permissions without umask hack</li>
<li><a
href="https://github.com/docker/go-connections/commit/32c72ec777e66c7f4a391097f400ea62eff7e63f"><code>32c72ec</code></a>
Merge pull request <a
href="https://redirect.github.com/docker/go-connections/issues/162">#162</a>
from thaJeztah/abstract_sockets</li>
<li><a
href="https://github.com/docker/go-connections/commit/f3526e58848fc48baf375ca42aa23d0d18dafefa"><code>f3526e5</code></a>
sockets: improve abstract Unix socket handling</li>
<li><a
href="https://github.com/docker/go-connections/commit/fd93b41aeecfdee02fe3e7be3f8799c8842f8cef"><code>fd93b41</code></a>
Merge pull request <a
href="https://redirect.github.com/docker/go-connections/issues/163">#163</a>
from thaJeztah/rm_log</li>
<li><a
href="https://github.com/docker/go-connections/commit/d0c75596e3ef03a6fb9e78befc9b18d9eed28e7e"><code>d0c7559</code></a>
sockets: update more tests to use tempSocketPath utility</li>
<li><a
href="https://github.com/docker/go-connections/commit/7106f49a36e292e9d0cad10f1842505304254cc4"><code>7106f49</code></a>
Merge pull request <a
href="https://redirect.github.com/docker/go-connections/issues/161">#161</a>
from thaJeztah/todone</li>
<li><a
href="https://github.com/docker/go-connections/commit/fa1caa79d797b5a4d622aecc371dd91c677d7e32"><code>fa1caa7</code></a>
sockets: fix some remaining TODOs in Windows code</li>
<li><a
href="https://github.com/docker/go-connections/commit/31d55b210c6596022c9dd2a2a06093d62e086dac"><code>31d55b2</code></a>
Merge pull request <a
href="https://redirect.github.com/docker/go-connections/issues/160">#160</a>
from thaJeztah/inmemory_context</li>
<li>Additional commits viewable in <a
href="https://github.com/docker/go-connections/compare/v0.7.0...v0.8.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=github.com/docker/go-connections&package-manager=go_modules&previous-version=0.7.0&new-version=0.8.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…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).
… 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.
Ports the shadow-database provisioning used by `db diff`/`db pull`
(create → health-wait → connect → setup/migrate → remove) from the
hidden Go `db __shadow` seam to native TypeScript, and removes that
seam from apps/cli-go/cmd/db.go. This was the last local-container
orchestration `db diff`/`db pull`'s native engines still delegated to
Go.

New shared primitives live in legacy/shared/db-bootstrap/shadow-database.ts
(create/connect/setup/migrate/remove), composed by
legacy/commands/db/shared/legacy-shadow-source.ts for db diff/pull's
--target-local declarative branch. legacy-pgdelta.apply.ts is a
from-scratch port of Go's pgdelta.ApplyDeclarative. Hoisted a shared
legacyResolveDbSetupPrelude (db-setup.ts) so fresh-db and shadow setup
stop duplicating the same JWKS/image-pull resolution.

Fixes CLI-1956
@Coly010

Coly010 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@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: d63bb5825f

ℹ️ 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/shared/db-bootstrap/shadow-database.ts
Comment thread apps/cli/src/legacy/commands/db/shared/legacy-pgdelta.apply.ts Outdated
Comment thread apps/cli/src/legacy/commands/db/shared/legacy-shadow-source.ts Outdated
Comment thread apps/cli/src/legacy/commands/db/shared/legacy-shadow-source.ts Outdated
Comment thread apps/cli/src/legacy/commands/db/shared/legacy-shadow-source.ts Outdated
Comment thread apps/cli/src/legacy/shared/db-bootstrap/db-setup.ts Outdated
Coly010 added 3 commits August 1, 2026 06:49
…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
…iew: Codex, PR #6022)

Go's DockerStart unconditionally sets com.supabase.cli.project and
com.docker.compose.project on every container it starts, one-shot jobs
included (apps/cli-go/internal/utils/docker.go:371-376). The generic
LegacyDockerRun.runCapture path this shell's PG15+ realtime/storage/auth
migrate jobs run through never emitted --label at all, so a job left
running after a client interruption or daemon disconnect was invisible
to supabase stop/rollback's project-label filter
(legacy-docker-remove-all.ts). LegacyDockerRunOpts now carries an
optional labels map, threaded through from the setup pipeline's own
projectId.
Base automatically changed from columferry/cli-1955-port-db-reset-local-recreate-natively-and-remove-the-__db to develop August 7, 2026 16:26
Coly010 added 3 commits August 7, 2026 17:44
This branch's local CLI-1954/1955 snapshot split container-lifecycle,
docker-create-args, health-check, image-prepull, and pinned-image into a
separate shared/containers/ directory. The actual merged #6022/#6026 PRs
on develop never adopted that split -- everything stayed flat under
shared/db-bootstrap/. Realigning to develop's canonical layout before
merging develop in, so the upcoming merge does normal content-level
3-way merges instead of add/add path-divergence conflicts.
…6-port-shadow-database-provisioning-natively-and-remove-the-db

# Conflicts:
#	apps/cli-go/cmd/db.go
#	apps/cli/docs/go-cli-porting-status.md
#	apps/cli/src/legacy/commands/db/diff/SIDE_EFFECTS.md
#	apps/cli/src/legacy/commands/db/pull/SIDE_EFFECTS.md
#	apps/cli/src/legacy/commands/db/pull/pull.handler.ts
#	apps/cli/src/legacy/commands/db/pull/pull.integration.test.ts
#	apps/cli/src/legacy/commands/db/push/push.handler.ts
#	apps/cli/src/legacy/commands/db/reset/SIDE_EFFECTS.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/reset/reset.layers.ts
#	apps/cli/src/legacy/commands/db/schema/declarative/declarative.orchestrate.integration.test.ts
#	apps/cli/src/legacy/commands/db/schema/declarative/sync/sync.integration.test.ts
#	apps/cli/src/legacy/commands/db/shared/legacy-pgdelta.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/services/edge-runtime.service.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/db-bootstrap/bootstrap-config.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/local-container-inputs.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/recreate-local-database.ts
#	apps/cli/src/legacy/shared/db-bootstrap/restart-services.ts
#	apps/cli/src/legacy/shared/db-bootstrap/restart-services.unit.test.ts
#	apps/cli/src/legacy/shared/db-bootstrap/start-database.ts
#	apps/cli/src/legacy/shared/legacy-container-cli.ts
#	apps/cli/src/legacy/shared/legacy-db-config.toml-read.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-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-pgdelta.cache.ts
#	apps/cli/src/legacy/shared/legacy-start-secrets-cleanup.ts
…view

Two independent review passes (go-parity-auditor, engineer-reviewer) over
the develop merge found:

- db/start's SIDE_EFFECTS.md had lost its Exit Codes/Output sections and
  several env-var rows to a bad conflict resolution; restored from develop.
- stop's SIDE_EFFECTS.md reverted develop's docker-cp doc fix, re-claiming
  a deleted function (legacyStageStartSecretFiles) still stages secrets.
- Several doc comments (shadow-database.ts, postgres.service.ts,
  legacy-docker-ids.ts) and SIDE_EFFECTS.md rows (db/diff, db/pull,
  declarative/sync) still described the shadow's pgsodium key as
  host-bind-mounted; it's delivered via docker cp now, same as every other
  container, so it never touches disk. Corrected the claims and clarified
  that LEGACY_CLI_SECRET_DIR_LABEL's remaining purpose is orphan-container
  recognition, not secret-directory reclaim.
- diff.layers.ts/pull.layers.ts still wired and referenced
  LegacyDeclarativeSeam, which nothing in db diff's call chain uses any
  more now that shadow provisioning (including the migrations-catalog
  shadow) is fully native. Dropped the dead layer and fixed the comments.
- Stale "db __shadow seam still live" claims in diff/SIDE_EFFECTS.md,
  binary-distribution.md, and cmd/start.go's comment.
@Coly010 Coly010 self-assigned this Aug 10, 2026
@Coly010
Coly010 marked this pull request as ready for review August 10, 2026 09:14
@Coly010
Coly010 requested a review from a team as a code owner August 10, 2026 09:14
@github-actions

Copy link
Copy Markdown
Contributor

Supabase CLI preview

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

Preview package for commit ded665c.

Comment thread apps/cli/src/legacy/shared/db-bootstrap/postgres.service.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: ded665cdd6

ℹ️ 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/shared/db-bootstrap/shadow-database.ts
Comment thread apps/cli/src/legacy/commands/db/start/start.handler.ts
…6-port-shadow-database-provisioning-natively-and-remove-the-db
legacyDbStart loaded the project context once eagerly (for pre-Docker
validation), then again inside legacyBuildLocalDbContainerInputs on the
not-already-running path — doubling @supabase/config's deprecated-section
stderr warnings for a single invocation. Go's flags.LoadConfig runs exactly
once. Thread the already-loaded context through as a preloadedContext param
instead of reloading it.

@avallete avallete left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Major findings (both re-verified by me in the code)

1. A shadow container that is created but fails to start is never removed. container-lifecycle.ts:833-839 runs docker createdocker cpdocker start with no rollback if steps 2–3 fail. Since legacyCreateShadowDatabase is the acquire of Effect.acquireUseRelease (pull.handler.ts:542, diff.handler.ts:562), a failed acquire means release never runs — and --rm can't help because AutoRemove only fires for started containers. Concrete scenario: two concurrent db diff/db pull runs both bind db.shadow_port; the loser's docker start fails on the port conflict and leaves a Created container behind, every retry adding another. Nothing but an unrelated supabase stop sweeps them. Go leaks identically (shadow.go:38-41 discards the id on error), so this isn't a regression — but concurrent invocations are exactly the case the PR's own design note is about, and the fix is cheap: clean up inside legacyCreateShadowDatabase's error path.

2. Four pg-delta call sites still derive the Deno-cache volume from env-only cliConfig.projectId, unlike the native paths this PR fixed. diff.handler.ts:262, diff.handler.ts:291, sync.handler.ts:146, and generate.handler.ts:133 all use Option.getOrElse(cliConfig.projectId, () => ""), while the native diff/pull paths were changed in this same PR to legacySanitizeProjectId(legacyResolveLocalProjectId(...)). I confirmed LegacyCliConfig.projectId reads only SUPABASE_PROJECT_ID (legacy-cli-config.layer.ts:172-176). So a project that gets its project_id from config.toml (or Go's workdir-basename fallback) mounts the literal supabase_edge_runtime_ volume — one shared Deno cache across all such projects on the machine, diverging from Go's UpdateDockerIds. The migrationsCtx site is squarely in this PR's changed surface (it gained projectEnv and the cfg argument here). Given the legacy shell's strict 1:1 parity contract, this should get the same fix the native paths got.

The PR description needs correcting — its headline design note describes machinery that can never fire

The randomized shadow-<uuid> secret-directory mechanism is dead code across five production files (shadow-database.ts:205-315, container-lifecycle.ts:140-156, legacy-docker-ids.ts:129-147, plus the lifecycle/cleanup files). Since #6022, secret files are docker cp'd from an os.tmpdir() mkdtemp that's removed via Effect.ensuring — no code path ever creates start-secrets/shadow-<uuid>/ on host disk. Consequences: legacyCleanupShadowSecretDir's "older binary" justification is unreachable by construction (an older binary staged under its id, not this run's fresh UUID), the ~15-line containerGone gate guards a no-op, and LEGACY_CLI_SECRET_DIR_LABEL + the 4th docker ps format field exist solely to let stop reclaim a directory that never exists. Three unit tests hand-create the directory production can never produce. The design note is also internally contradictory: it says the name-keyed sweep "can never find" the unnamed shadow, yet the PR adds the label specifically so the sweep can. Per the repo's refactoring policy this should collapse to legacyRemoveShadowDatabase(spawner, containerId).

Two more description-drift items: it claims "db diff still needs LegacyDeclarativeSeam" but head commit ded665cd correctly removed it from diff.layers.ts too, and legacy-pgdelta.seam.service.ts:16-22 still claims start.SetupDatabase/pgdelta.ApplyDeclarative "have no native TS equivalent yet" — falsified by this same PR, and contradicted by lines 34-40 of the same file (two reviewers found this independently; the head commit was itself a doc-correction pass that missed it).

Minor findings

  • Duplicate error tag (verified): two distinct LegacyDeclarativeApplyError classes with the identical Data.TaggedError tag now coexist in one error channel — legacy-pgdelta.apply.ts:34 vs declarative.errors.ts:80. Latent (no catchTag on that tag today), but a future recovery handler would silently catch the wrong one, and typed narrowing can't distinguish them. Cheap rename now (LegacyPgDeltaDeclarativeApplyError).
  • Debug-flag token parsing gap: legacyDebugFlagExplicitlyFalse (global-flags.ts:356-366) scans raw args without nonValueConsumedTokens(argsBeforeOperandTerminator(...)) like its siblings, so db pull -- --debug=false or --password --debug=false suppresses debug where pflag wouldn't. Impact limited to debug logging.
  • Shadow honors [db] password where Go can't: Go's Db.Password is toml:"-" so the shadow always gets "postgres"; the TS shadow builder takes the toml-read password (postgres.service.ts:446). Self-consistent so nothing breaks, but the two builders now disagree about the same Go field and one of their comments is wrong.
  • SIDE_EFFECTS.md gaps: the diff/pull env-var and files-read tables weren't updated for the now-in-process shadow bring-up (misses SUPABASE_NETWORK_ID, SUPABASE_DB_HEALTH_TIMEOUT, the SUPABASE_* override family, .env*/roles.sql/TLS-cert reads — all documented for the identical machinery in db start/db reset's files). Also diff/SIDE_EFFECTS.md:38 references a seam-only mode: "diff" concept that no longer exists.
  • Dead exports: legacySetupShadowDatabase and legacyResolveDebug have zero production call sites (speculative generality for CLI-1968/1969, carrying their own tests). Repo policy prefers deferring these.
  • Output-parity nits in the apply port: status quoted with plain quotes vs Go's %q, absolute vs relative path in the "directory not found" error, .trim() stripping a BOM Go's bytes.TrimSpace keeps, several apply.go:NNN doc references off by 2–6 lines (the comments are the parity spec there, so they should be right or dropped).

…provisioning-natively-and-remove-the-db

# Conflicts:
#	apps/cli/src/legacy/shared/db-bootstrap/container-lifecycle.ts
#	apps/cli/src/legacy/shared/db-bootstrap/db-setup.ts
#	apps/cli/src/legacy/shared/legacy-docker-lifecycle.ts
…eardown

- delete the unused shadow secret-dir plumbing (secrets are delivered via
  docker cp and never staged on disk): LEGACY_CLI_SECRET_DIR_LABEL, the
  shadow-<uuid> generation, LegacyContainerOpts.secretDirId, the 4th
  docker ps TSV column, and the label branch in legacyCleanupStartSecrets
- legacyRemoveShadowDatabase is now (spawner, containerId); the
  acquire-window container leak is documented as deliberate Go parity
  (docker.go:420-436, shadow.go:38-41)
- drop legacySetupShadowConn's withTemplate param — template SQL is
  unconditional, matching Go's setupShadowConn
- add shadow connect-retry/teardown unit tests (schedule attempt counts,
  spawn-failure remove, empty-containerId no-op)
- classify LegacyShadowDbError for the error-actionability taxonomy
…key list

- export LEGACY_ENV_OVERRIDABLE_KEYS as const with a
  LegacyRemoteOverridableKey type (template patterns for the
  auth.external / auth.email.template / auth.email.notification families)
- replace the ad-hoc remote-wins closures with one legacyMakeRemoteWins
  helper shared by every consumer
- gate ~20 keys a matched [remotes.<ref>] block sets at viper's OVERRIDE
  tier that env could previously clobber (auth.sms provider fields, sms
  template/max_frequency, smtp host/user/admin_email/sender_name,
  jwt_issuer, additional_redirect_urls, email.max_frequency, mfa.phone.*,
  auth.webauthn rp_id/rp_origins, analytics gcp_*)
- fix stale config.go citations and reword wrong project_id comments
- resolve LegacyPgDeltaContext.projectId through one shared
  legacyResolvePgDeltaProjectId at all six pg-delta sites (env-only
  resolution previously produced supabase_edge_runtime_: volume binds)
- port Go diff.go:228-244's PGDELTA_DEBUG shadow-catalog export into
  db diff's pg-delta branch (export, discard on success, warn on failure)
- build the shadow catalog's LocalDbContainerInputs BEFORE printing
  "Creating shadow database..." so config errors surface first, like Go
- legacy-pgdelta.apply.ts: print the RELATIVE declarative dir in the
  not-found error (Go apply.go:304), add the Go int64 range check to
  legacyIsGoIntNumber, pass workdir to the edge-runtime run, classify
  LegacyDeclarativeApplyError for the actionability taxonomy
- resolve DEBUG via legacyResolveDebugWithProjectEnv in the pg-delta
  cache (Go reads viper.GetBool("DEBUG")); harden
  legacyDebugFlagExplicitlyFalse to ignore tokens after the "--" operand
  terminator and tokens consumed as another flag's value; delete the
  caller-less legacyResolveDebug and restore the guard tests against the
  surviving resolver
- fold Go fs.WalkDir byte-order sorting + no-follow semantics into the
  shared legacyWalkSqlFiles, delete the duplicate shadow-source walker,
  move legacyCompareUtf8Bytes into legacy-glob.ts, and byte-sort the
  migrate/seed callers' trailing sorts to match
- refresh stale Go citations in apply.ts / pull.handler.ts (dead
  ensureMigrationWritten -> live swallowInitialInSync, plus line drift)
…asswords

- health-check prints "<id> <reason>" without the ": " separator Go
  never prints
- percent-decode legacyStartInternalDbPassword and re-encode in
  legacyStartInternalDbUrl so special-character shadow passwords survive
  the container-env round trip (fixes Realtime's DB_PASSWORD)
@github-actions

Copy link
Copy Markdown
Contributor

Supabase CLI preview

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

Preview package for commit 578d783.

@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: 578d783eeb

ℹ️ 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.handler.ts
…6-port-shadow-database-provisioning-natively-and-remove-the-db
…ering

- rename legacy-pgdelta.apply.ts's error class to
  LegacyPgDeltaDeclarativeApplyError: it shared its Data.TaggedError tag
  string with the unrelated LegacyDeclarativeApplyError in
  declarative.errors.ts, so a future catchTag would silently match both
- render the apply status with legacyGoQuote — Go uses %q (apply.go:156),
  so plain quotes diverge on quote/control characters in a malformed
  payload
- trim with the Unicode White_Space set (new legacyTrimGoSpace) at every
  site mirroring Go's strings/bytes.TrimSpace — JS .trim() also strips
  U+FEFF, so a BOM-prefixed pg-delta payload parsed here where Go fails
…TS gaps

- legacy-pgdelta.seam.service.ts no longer claims start.SetupDatabase /
  pgdelta.ApplyDeclarative lack native TS equivalents (both landed in
  CLI-1956); baseline/declarative stay seam-backed only because the
  catalog export hasn't been composed on top of them yet
- the shadow builder's password doc claimed threading [db] password
  "matches Go's NewContainerConfig" — false: Go decodes with the json tag
  and json:"-" makes a literal [db] password key a fatal UnmarshalExact
  error, so Go's value is invariably "postgres"; the threading is a
  deliberate TS extension mirroring --local, now documented as such
  (postgres.service.ts, legacy-db-config.toml-read.ts, test title)
- document the in-process shadow bring-up in diff/pull SIDE_EFFECTS.md:
  dotenv family, api.tls cert/key and roles.sql reads, and the
  SUPABASE_DB_SHADOW_PORT / DB_MAJOR_VERSION / DB_HEALTH_TIMEOUT /
  DB_SETTINGS_* / PROJECT_ID / NETWORK_ID env overrides, each scoped to
  the branch that actually reads it; drop diff's stale seam-era
  mode:"diff" reference
@Coly010

Coly010 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the deep review @avallete — every finding was re-verified and is now dispositioned; all fixes pushed (2a34ea0de12729ec67).

Major 1 — created-but-unstarted shadow container leaks. Confirmed, and kept as deliberate Go parity rather than fixed: Go's DockerStart discards the container id on the same failure path (docker.go:420-436, shadow.go:38-41), so cleaning up inside legacyCreateShadowDatabase would make TS and Go visibly diverge under exactly the concurrent-invocation scenario you describe. The leak window is documented at legacyCreateShadowDatabase's doc comment with cross-references from every call site. Worth revisiting once the Go oracle stops being the contract.

Major 2 — four env-only projectId sites. Fixed in 7a999ca5f: all six pg-delta sites (including migrationsCtx) now resolve through one shared legacyResolvePgDeltaProjectId (sanitized resolveLocalProjectId(env, toml, workdir) with remote-match suppression), plus an integration test pinning the supabase_edge_runtime_demo bind for a toml-only project_id.

Description drift. All corrected: the dead shadow-<uuid> secret-dir machinery was deleted outright in 2a34ea0de (legacyRemoveShadowDatabase is now (spawner, containerId), the label + 4th docker ps column are gone), the PR body no longer describes it; the body's seam bullet now says neither db diff nor db pull wires LegacyDeclarativeSeam (pgadmin/pg-schema proxy the whole invocation to Go); and legacy-pgdelta.seam.service.ts's header no longer claims start.SetupDatabase/pgdelta.ApplyDeclarative lack native equivalents (12729ec67).

Minor findings:

  • Duplicate tag: renamed the apply-side class (tag string included) to LegacyPgDeltaDeclarativeApplyError (77a4e9f85).
  • Debug-flag scanning: legacyDebugFlagExplicitlyFalse now wraps nonValueConsumedTokens(argsBeforeOperandTerminator(args)), with -- / --password guard tests (7a999ca5f).
  • [db] password: verified empirically against the real Go loader — the divergence is real but sharper than framed: Go doesn't silently ignore the key, it rejects it at config load (json:"-" is the tag viper decodes with, so UnmarshalExact fails with 'db' has invalid keys: password), meaning Go's shadow invariably gets postgres. Honoring the toml key is a pre-existing develop extension (--local connections already do it); this PR only extends that value to the shadow for internal consistency. The false "matches Go's NewContainerConfig" comments and the test title are corrected in 12729ec67; whether to port Go's key rejection is a follow-up, since it would change pre-existing --local behavior.
  • SIDE_EFFECTS.md: diff/pull now document the in-process shadow bring-up — dotenv family, api.tls cert/key + roles.sql reads, and SUPABASE_DB_SHADOW_PORT / DB_MAJOR_VERSION / DB_HEALTH_TIMEOUT / DB_SETTINGS_* / PROJECT_ID / NETWORK_ID — each row scoped to the branch that actually reads it, and diff's stale seam-era mode: "diff" reference is gone (12729ec67).
  • Dead exports: legacyResolveDebug deleted (7a999ca5f). legacySetupShadowDatabase is kept deliberately — it's the exact subset migration squash (CLI-1969) needs per the PR's composable-pieces design note; deleting it only to re-add it next issue felt like churn, but happy to defer it if you feel strongly.
  • Output nits: apply status now rendered with the existing strconv.Quote port (legacyGoQuote), all TrimSpace-mirror sites switched to a Unicode-White_Space-only trim (JS .trim() also eats U+FEFF; Go's doesn't), the not-found error prints the relative dir, and the drifted apply.go/pull.go citations were re-audited line-by-line (77a4e9f85, 7a999ca5f).

@github-actions

Copy link
Copy Markdown
Contributor

Supabase CLI preview

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

Preview package for commit 12729ec.

@Coly010
Coly010 added this pull request to the merge queue Aug 10, 2026
Merged via the queue into develop with commit cb417e5 Aug 10, 2026
32 checks passed
@Coly010
Coly010 deleted the columferry/cli-1956-port-shadow-database-provisioning-natively-and-remove-the-db branch August 10, 2026 13:39
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