feat(cli): port supabase start command to native TypeScript#5847
feat(cli): port supabase start command to native TypeScript#5847Coly010 wants to merge 76 commits into
Conversation
Ports `supabase start` from a Go-binary proxy to native TypeScript in the legacy CLI shell (CLI-1323), continuing the local-dev-stack shell migration alongside the already-ported `stop`/`status`/`db push`/`db reset`/`db start`. ## What changed Talks directly to Docker/Podman via subprocess, mirroring Go's sequential per-container `DockerStart` — no Docker Compose, and no `@supabase/stack/effect` orchestration (that runtime targets a deliberately different local-dev product and would silently manage the wrong set of containers). Brings up all 14 containers in Go's real start order, with per-service config-boolean + `--exclude` gating, Go-byte-exact env/image resolution, a bulk health-check phase (Docker healthcheck for most services, an HTTP-HEAD-through-Kong bypass for PostgREST/Edge Runtime), and full rollback on any bring-up failure. Also natively implements the fresh-volume DB schema/migration/seed pipeline, Edge Runtime container bring-up, and fresh-volume storage bucket seeding — previously tracked as out-of-scope follow-ups for this port, now closed. Only the linked-project version-check suggestion (a best-effort "update available" hint with zero Management API dependency otherwise) remains out of scope by design. ## Notable review-driven fixes - `legacyParseGoDuration` (the `config.toml` duration-string parser feeding Go-parity env vars like `GOTRUE_SESSIONS_TIMEBOX`) silently accepted a malformed duration with no digits (a bare unit like `"s"`, or a lone `"."` with no digits on either side) and returned `0` instead of erroring like Go's real `time.ParseDuration` — both cases now throw Go's exact `time: invalid duration "..."` message. - `start.services.ts`'s descriptive `enabledGate` metadata for Mailpit referenced the deprecated `inbucket` config section instead of its `local_smtp` rename. Fixed, and added a mechanical cross-check test that evaluates every service's `enabledGate` string against `start.gates.ts`'s real computed gate across a battery of synthetic configs, so a future drift between the two fails loudly instead of silently. - Confirmed and closed the PostgREST health-check's TLS/CA trust gap for `[api.tls] enabled = true` local stacks (self-signed Kong cert) — the local-Kong-CA-trust mechanism already used by `seed buckets`/`storage`/ `db reset` is now wired into `start`'s health-check HTTP client too. Closes CLI-1323
|
👋 Thanks for the contribution! This pull request isn't linked to a tracked issue, so it's being closed automatically. Please open an issue first, wait for a maintainer to add the |
…3-port-supabase-start
|
👋 Thanks for the contribution! This pull request isn't linked to a tracked issue, so it's being closed automatically. Please open an issue first, wait for a maintainer to add the |
|
👋 Thanks for the contribution! This pull request isn't linked to a tracked issue, so it's being closed automatically. Please open an issue first, wait for a maintainer to add the |
|
👋 Thanks for the contribution! This pull request isn't linked to a tracked issue, so it's being closed automatically. Please open an issue first, wait for a maintainer to add the |
) The contribution gate identified internal maintainers solely from the PR's `author_association`, which GitHub only reports as `MEMBER` when a user's organization membership is public. A private org member (e.g. a `supabase/cli` team member who keeps membership private) is reported as `CONTRIBUTOR`/`NONE`, so the gate wrongly closed their PRs as "no-linked-issue" (see supabase#5847). Resolve maintainer status from the author's effective repository permission (`admin`/`write`), which reflects team/org-granted access that `author_association` does not surface, falling back to it only when the cheap signals (bot, public internal association) are inconclusive. The permission endpoint needs just `Metadata: read`, already covered by the workflow's `contents: read`. Claude-Session: https://claude.ai/code/session_01D41gYiFBSU7adE4ppnUUKg --------- Co-authored-by: Claude <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 623dd8b18e
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 623dd8b18e
ℹ️ 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".
supabase start's TS port had five spots where Go's real behaviour was
silently dropped:
- supabase/.temp/storage-migration (the linked project's Storage
migration pin) was never read, so DB_MIGRATIONS_FREEZE_AT was always
empty on a fresh DB setup and the Storage container itself.
- supabase/.temp/{gotrue,rest,storage,realtime,studio,pgmeta,logflare,
pooler}-version pins were never applied to the images start
pulls/creates, unlike Go's Config.Load rewrite. Hoisted the existing
services command's reader into shared/legacy-service-version-overrides.ts
and reused services.shared.ts's tag-rewrite helpers instead of a third
copy.
- --network-id was read by several other native ports but never by
start itself, so the override never reached any container or the
Docker network start creates.
- SUPABASE_API_PORT's env override was computed for status URLs but
never exposed for Kong/Edge Runtime's own container specs, so they
kept publishing/binding the un-overridden config.api.port.
- Studio's function bind mounts were hardcoded to [], unlike Go's
unconditional (Edge-Runtime-independent) PopulatePerFunctionConfigs
call — extracted the existing per-function bind computation in
shared/functions/serve.ts into a reusable resolveFunctionBindMounts
so both callers share one calculation.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…l in start Three more Go-parity gaps in supabase start found by review: - db.root_key (including encrypted: values) was never resolved or passed to the Postgres container, so every project booted with the hard-coded default pgsodium key instead of a customized one, breaking decryption of existing encrypted data. Resolved in legacyResolveLocalConfigValues off the raw config document (unmodeled in @supabase/config's schema), reusing the existing decrypt helpers. - DockerStart's Linux-only host.docker.internal:host-gateway mapping was already correctly ported for the one-shot migrate jobs and Edge Runtime bring-up, but never made it into the common legacyStartContainer path the other 13 services go through. - auth.external_url (also unmodeled in the schema, with its own Go regression test) was never read, so GoTrue's API_EXTERNAL_URL/JWT issuer default/mailer verify URL/OAuth redirect fallbacks always derived from apiUrl even when a project intentionally exposed auth at a different host.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…es tag ordering Three more Go-parity gaps found by review: - [auth.email.smtp] present without an explicit enabled key should default to enabled (Go sets this at load time), but the schema always decodes enabled: false when the key is absent, so GoTrue silently fell back to Mailpit. Reuses the existing correct resolution already implemented for config validation, exposed as legacyResolveAuthEmailSmtp. - auth.external providers outside the schema's fixed ~19-provider set (e.g. a custom [auth.external.my_oidc] block) never reached GoTrue's env, even though Go's Auth.External is a genuine map iterated unconditionally and this port's own config validation already accepts arbitrary provider names. Reuses the same raw-document iteration validateAuthExternalProviders already established. - A registry override with a port (SUPABASE_INTERNAL_IMAGE_REGISTRY= localhost:5000) broke the Postgres version-tag comparison, since Go compares the pre-registry-rewrite image while this port was comparing the already-rewritten one. Threaded the original image through as a separate configImage field used only for that comparison.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…base (review: #PRRT_kwDOErm0O86Sk1oL) Go's baseConfig.resolve (pkg/config/config.go:901-916) rebases auth.email.notification.*.content_path against <workdir>/supabase before mountEmailTemplates ever runs, asymmetric to templates which rebase against workdir alone. The port already applies this same asymmetric-base logic for the file-existence check in readAuthEmailTemplateContent, but discarded the resolved path afterward — buildKongEmailTemplateMounts passed the raw, still-relative notification.content_path straight to Kong's mount builder, which resolves relative paths against workdir (the template base), so a relative notification path was validated against <workdir>/supabase/... but mounted from <workdir>/.... Reused legacyResolveEmailTemplateContentPath to rebase notifications before they reach the mount builder; templates are unaffected.
…PRRT_kwDOErm0O86Sk1oE) Go's start rolls back on Ctrl-C: cmd/root.go wraps every command's context with signal.NotifyContext, and internal/start/start.go rolls back on any non-nil run() error, including the context.Canceled a SIGINT produces. The native TS start handler installs no signal handling of its own, so it depended entirely on the CLI's global signal wrapper - but start was excluded from that wrapper as a leftover from when it proxied to the Go binary (which managed its own signals). That left Ctrl-C mid-bring-up as a raw, unhandled OS signal that hard-killed the process, skipping every Effect finalizer including legacyRollbackStart. Removed start from run.ts's selfManagedSignalCommands so it participates in the global signalAwareProgram wrapper again, and swapped the rollback pipeline's Effect.tapError for Effect.onError: tapError is built on Cause.findError, which only matches Fail-tagged causes and never sees a pure fiber interrupt, while onError fires on any failure outcome (including interruption and defects) and runs its cleanup uninterruptibly, matching Go's unconditional error check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0371d4ee0d
ℹ️ 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".
…(review: #PRRT_kwDOErm0O86Smcnp) Go's Config.Load decodes db.health_timeout in the same unconditional mapstructure.StringToTimeDurationHookFunc() pass as every other duration field, before start.Run touches Docker at all - a malformed value fails before network/image/Postgres work, and Go's own rollback (only reached when run() itself errors) never even runs. The port parsed this value only inside bringUp, right before the health wait, after Postgres's container had already been created and started - hoisted the parse alongside the other eagerly-validated config-override fields (pooler port/pool_mode/etc.), and updated the existing malformed db.health_timeout test to assert no containers were ever created instead of expecting a rollback.
…ly (review: #PRRT_kwDOErm0O86SmcnZ, #PRRT_kwDOErm0O86Smcnh) Go decodes storage.file_size_limit (sizeInBytes.UnmarshalText) and every GoTrue-related time.Duration field (auth.email/sms.max_frequency, sessions.timebox/inactivity_timeout, mfa.phone.max_frequency) in the same single, unconditional Config.Load mapstructure pass, before start.Run touches Docker at all - regardless of whether Storage/GoTrue are excluded or auth is disabled. The port only parsed storage.file_size_limit inside storage.service.ts, reached solely from the per-service loop after Postgres's container already existed, and skipped entirely when Storage is excluded/disabled. The 5 GoTrue duration fields had the identical gap: parsed only inside GoTrue's own env builder, never reached at all when auth is disabled or gotrue excluded - so a malformed value was silently accepted where Go hard-fails unconditionally. Hoisted all 6 fields into eager wrapConfigOverride validations alongside the other config-override fields already resolved before any Docker work, reusing the existing per-service catchDefect as defense-in-depth rather than removing it. Added regression tests proving each field still fails even when the corresponding service is excluded/disabled, and updated the existing auth.email.max_frequency test's expectations (no rollback needed - the value is now rejected before anything is created).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6de98ff272
ℹ️ 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".
…e (review: #PRRT_kwDOErm0O86Sn1Dr) A narrower follow-up to the earlier top-level bring-up rollback fix: legacyStartContainer's own staged-secrets cleanup used Effect.tapError, which is built on Cause.findError and never sees a pure SIGINT/SIGTERM interrupt - the same gap already fixed for start.handler.ts's rollback. If interruption landed after legacyStageStartSecretFiles wrote a container's secret files but before docker create/start returned, the plaintext file was left on disk indefinitely: the outer rollback only discovers and cleans directories for containers found via docker ps, so a container interrupted before it was ever created is invisible to that path entirely. This is judged on its own correctness/security merits rather than Go parity - Go never stages secrets to a host file at all (it heredocs them directly via the Engine API), so there's no Go behavior to match here. Swapped Effect.tapError for Effect.onError: the cleanup closure already only depends on the deterministic staged directory computed before docker create is attempted, so it fires regardless of whether a container was ever created, and onError's cleanup effect is guaranteed to run uninterruptibly even on a pure interrupt. Added a regression test that hangs docker create indefinitely, interrupts the fiber mid- create, and asserts the staged file is gone - verified it fails against the old tapError wiring first.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f7b6723cf
ℹ️ 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".
…cker work (review: #PRRT_kwDOErm0O86So2uf) Go decodes storage.s3_protocol.enabled as a plain bool through its generic Viper/mapstructure Config.Load pass - the exact same mechanism as storage.vector.enabled, which was already hoisted eagerly. This sibling field was left as a bare legacyEnvOverrideBool call inline in the Storage spec builder, only reached when Storage's own container spec is built - never parsed at all when Storage is excluded or disabled, and only after network/image/Postgres/other-service Docker work had already run. Same class of gap already fixed for storage.file_size_limit, the GoTrue duration fields, and db.health_timeout. Hoisted into an eager wrapConfigOverride call alongside storageVectorEnabled, with the Storage spec builder now referencing the hoisted value instead of recomputing it. Added a regression test proving it still fails config load even with `--exclude storage`.
There was a problem hiding this comment.
💡 Codex Review
When auth.enabled is false, this gate skips the only parsing path for unmodeled auth subtrees such as present [auth.passkey]/[auth.webauthn] and [auth.hook.*] sections. A malformed value like enabled = "abc" in [auth.passkey], or a malformed override such as SUPABASE_AUTH_HOOK_SEND_EMAIL_ENABLED=abc for a configured hook, fails Go's Viper/mapstructure decode during Config.Load before validation is gated by auth.enabled, but the native path accepts it because GoTrue is never built. Parse these decode-time fields before this gate, and only gate the semantic required-field validation on authEnabled.
ℹ️ 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".
… (review: #PRRT_kwDOErm0O86Sp4Xj)
Go's Config.Load decodes auth.rate_limit.* (plain uints),
auth.web3.*.enabled, and auth.oauth_server.{enabled,allow_dynamic_registration}
(plain bools) unconditionally in a single pass, before start.Run touches
Docker, regardless of auth.enabled/--exclude gotrue. The TS port only
validated these when GoTrue's own container spec was built, so a bad
override silently passed for an excluded/disabled auth service. Hoists
eager wrapConfigOverride calls next to the existing GoTrue duration
validations in start.handler.ts.
…(review: #PRRT_kwDOErm0O86Sp4Xn) Go's Config.Load decodes edge_runtime.policy (RequestPolicy via UnmarshalText) and edge_runtime.inspector_port (a plain uint) unconditionally in the same pass, before start.Run touches Docker, regardless of --exclude edge-runtime. The TS port only validated these deep inside the Edge Runtime bring-up branch, well after Postgres (and possibly other services) had already been created. Hoists both into eager wrapConfigOverride calls next to dbHealthTimeoutSeconds, and removes the now-redundant inline Effect.try blocks (the Edge Runtime branch already re-resolves both against the env-interpolated subtree for the real container build).
…, not just docker run (review: #PRRT_kwDOErm0O86Sp4Xt)
startEdgeRuntimeContainer wrote its JWT/service-role secrets env file,
multiline-env script, and serve-main template to a persistent staging
directory, but only wired Effect.onInterrupt around the final docker
run step. A failure or interrupt during the staging-write window
itself (or between two writes) left those files on disk indefinitely,
with no cleanup ever firing.
Replaces the three per-file .cleanup() closures with a single
directory-wide rm(stagingDir, { recursive: true, force: true })
effect defined right after stagingDir is computed, widens the wrapped
scope to cover the whole staging-write-through-docker-run window, and
swaps Effect.onInterrupt for Effect.onError so it also fires on a
plain failure (Fail/Die), not just an interrupt. Removes the
now-redundant explicit cleanup call in the non-zero-exit branch.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e6982de88
ℹ️ 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".
…with legacy (review: #PRRT_kwDOErm0O86Sq_rs, #PRRT_kwDOErm0O86Sq_rv) apps/cli/AGENTS.md requires every exported token from src/legacy/** to carry the Legacy/legacy prefix, so IDE auto-complete never suggests legacy-only exports while working in next/. These three helpers were promoted from file-local to exported so start.handler.ts could reuse them, but kept their bare names — renamed to legacyEnvOverride/legacyEnvOverridePort/legacyEnvOverrideUint and updated the one real call site (start.handler.ts); no behavior change.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f2181fa83
ℹ️ 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".
…time bring-up (review: #PRRT_kwDOErm0O86Srohx) Go's restartEdgeRuntime (functions serve's own restart wrapper) prints "Setting up Edge Functions runtime..." right before calling ServeFunctions (serve.go:124-125); ServeFunctions itself — called directly by start.go:1104, bypassing restartEdgeRuntime entirely — never prints it. The shared startEdgeRuntimeContainer core wrongly printed this banner unconditionally, so native start added a user-visible stderr line on every Edge Runtime bring-up that Go's start never emits. Moved the banner out of the shared core and into the functions-serve-only startEdgeRuntime wrapper, matching where reloadKong already lives for the same reason.
…oad (review: #PRRT_kwDOErm0O86Srohr) startEdgeRuntimeContainer's own Effect.onError already cleans up staged secrets on any failure/interrupt while it's running, but that scope ends once it returns successfully. functions serve's own restart wrapper (startEdgeRuntime) then calls reloadKong afterward, and its own interrupt handler only removed the container — an interrupt mid-reload left the staged env/JWKS files on disk with no cleanup ever firing, since the returned runtime's own cleanup was never tracked here. Tracks the returned StartedRuntime and runs its cleanup alongside the existing container removal on interrupt. Widened the shared runChildProcess test mock to support a never-resolving "pending" response so a new regression test can land an interrupt precisely during the Kong reload call; verified it fails against the pre-fix code before restoring the fix.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e1e8c0136
ℹ️ 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".
…per (review: #PRRT_kwDOErm0O86SstHU) An earlier fix removed top-level `start` from run.ts's shared selfManagedSignalCommands list so native legacy start's rollback-on- interrupt would fire, matching Go's own SIGINT-triggered rollback. But the list is matched purely against argv command-path segments, with no notion of which shell registered the matching command, so it also stopped exempting next start — a completely different command tree that already races its own flows (foreground/non-interactive) against interruptOnSignal and drives its own controlled shutdown (markStopping). The global wrapper's Fiber.interrupt could now win that race and force a raw interrupt (generic exit 130) instead of the flow's own clean stop. Adds an additive, per-shell exemption (RunCliOptions.additionalSelfManagedSignalCommands) instead of baking next's start into the shared list, since run.ts itself has no way to distinguish which shell is running. next/cli/main.ts opts start back out; legacy's own call site is unchanged, so native start keeps using the global wrapper for its Go-parity rollback.
…ing Go's decode (review: #PRRT_kwDOErm0O86SstHc) legacyRawUnmodeledBool (auth.passkey.enabled, auth.external.<provider>. enabled/skip_nonce_check/email_optional — fields @supabase/config has no schema for) silently returned false for any raw TOML value that wasn't already a boolean or a parseable string, e.g. auth.passkey. enabled = 123 or a custom provider's enabled = [1, 2]. Verified against Go's actual decode: Viper's defaultDecoderConfig sets WeaklyTypedInput: true as its own hardcoded default (viper.go:976-994; config.go's UnmarshalExact call only overrides TagName/Squash/ ZeroFields/DecodeHook, never touching this flag), so mapstructure's decodeBool weakly coerces a raw number by truthiness (int/uint/float != 0) rather than erroring — Go's Config.Load happily decodes `enabled = 123` as true. Only a genuinely unconvertible type (an array or inline table, TOML's other value kinds) hits mapstructure's unconditional default: error case. Adds the missing number branch (coerce by truthiness) and changes the fallback from a silent false to a typed config error for anything else, matching mapstructure's real decodeBool behavior exactly instead of guessing at "reject everything non-boolean/non-string".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f4b59ff5
ℹ️ 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".
…en signing_keys_path is set (review: #PRRT_kwDOErm0O86Stb2K) legacyResolveLocalConfigValues gated its own anon/service_role signing key on authEnabled, so a config with auth.enabled = false and a configured auth.signing_keys_path fell back to symmetric HS256 signing — while legacyResolveLocalJwks (the published JWKS document) already correctly gates only on signingKeysPath's own emptiness, publishing just the default ES256 key with no oct fallback in that same scenario. The mismatch meant PostgREST/Edge Runtime would reject the printed anon/service_role keys against the published JWKS. Go's generateJWT (apikeys.go:77) checks len(a.SigningKeysPath) > 0 && len(a.SigningKeys) > 0, NOT auth.enabled — a.SigningKeys is unconditionally seeded with the default ES256 key at NewConfig() time and only ever replaced by the file's keys (when the file read, itself gated on auth.enabled, actually runs), so it's never empty either way. Reuses legacyResolveConfiguredSigningKeys (which already gates the real file read) instead of duplicating that gate, matching legacyResolveLocalJwks's identical pattern. Deletes the now-dead loadFirstSigningKey helper.
…RT_kwDOErm0O86Stb2N) Go's function struct (pkg/config/config.go:290-296) has no env field — Config.Load's v.UnmarshalExact (ErrorUnused: true) rejects any unknown key unconditionally, before any Docker work. Empirically confirmed against the real Go binary: a config with [functions.foo.env] fails with "'functions[foo]' has invalid keys: env". @supabase/config's own schema DOES model [functions.<slug>.env] (a legitimate next/-only feature, shared infrastructure the legacy shell can't remove), so native start silently accepted and ran a config the stable CLI would reject outright. Adds an eager legacy-only rejection alongside the other config validations, before any Docker work. Removes the now-invalid "resolves a per-function env(...) ref" test, which exercised exactly the scenario Go rejects — env(...) interpolation itself stays covered via [edge_runtime.secrets] and other existing tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b0ebecad8
ℹ️ 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".
…view: #PRRT_kwDOErm0O86SuXq6)
legacyCheckDbToml's own resolution of [db.vault], db.migrations.enabled,
the effective api.auto_expose_new_tables tri-state, and SUPABASE_DB_
SEED_ENABLED only ran inside legacyStartSetupLocalDatabase, which is
itself gated on the DB container's healthcheck passing AND a fresh
volume (Go's NoBackupVolume gate) - so a malformed field went
completely unvalidated on an ordinary restart against an existing
volume. Go's Config.Load/Validate runs this same decode+validate pass
unconditionally, before start.Run touches Docker at all, regardless of
volume state.
Adds an eager legacyCheckDbToml call alongside the other config
validations, called purely for its validation side effect and
discarded - legacyStartSetupLocalDatabase's own internal call (an
already-accepted duplicate config-load pass matching db start's own
independent resolution) still resolves the real value for its own use
when it runs.
Also fixes a real bug this eager hoist surfaced: legacyCheckDbToml's
own auth.webauthn.rp_origins resolution only accepted a literal TOML
array, rejecting a valid comma-separated string (raw or env(...)-
resolved) as "missing" - Go decodes rp_origins through the same
StringToSliceHookFunc(",") hook as every other []string field.
Matches start.handler.ts's own resolveGotruePasskeyWebauthn handling
of this identical field.
There was a problem hiding this comment.
💡 Codex Review
When auth.enabled is false but an [auth.email.smtp] table is present, a malformed dotenv/ambient override such as SUPABASE_AUTH_EMAIL_SMTP_PORT=abc is skipped here because legacyResolveAuthEmailSmtp only runs inside the auth-enabled validation block, and native start later skips the GoTrue builder as well. Go still decodes auth.email.smtp.port as a uint16 during Config.Load (apps/cli-go/pkg/config/config.go:743-756, auth.go:255-259) before any Docker work, so this accepts or starts with a config the stable CLI rejects; parse the SMTP override in the eager config-load path regardless of auth.enabled while keeping the required-field validation gated.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
ℹ️ 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".
…: #PRRT_kwDOErm0O86SvIt5) legacyResolveAuthSms passed auth.sms.enable_signup straight through to GOTRUE_EXTERNAL_PHONE_ENABLED, so a config with enable_signup = true but every SMS provider disabled would let GoTrue accept phone-based signups it can't actually deliver an OTP for. Go's (s *sms) validate() (config.go:1348-1416) takes the case s.EnableSignup: switch branch only reached when every named provider is disabled, mutates EnableSignup = false, and prints a warning before buildGotrueEnv ever reads it. Mirrors both halves: legacyResolveAuthSms now downgrades enable_signup to false whenever none of the 5 providers ends up enabled (matching Go's mutation), and start.handler.ts prints the matching stderr warning when that branch fires, at the same eager-validation point where the SMS config is already resolved once for auth.sms.max_frequency. Fixed two existing tests that had (incorrectly, pre-fix) asserted enable_signup survives with no provider enabled - added a real provider to isolate what they actually test (the override mechanism) from this downgrade.
…3-port-supabase-start # Conflicts: # apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts # apps/cli/src/shared/functions/serve.ts
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/legacy/shared/legacy-local-config-values.ts
Lines 2711 to 2712 in bd65ac5
When an [experimental.webhooks] or [experimental.pgdelta] table is present, Go’s Viper env binding applies SUPABASE_EXPERIMENTAL_WEBHOOKS_ENABLED / SUPABASE_EXPERIMENTAL_PGDELTA_FORMAT_OPTIONS before experimental.validate() runs, so ...=false should reject webhooks and an invalid JSON format override should fail config load. This code passes the raw decoded TOML values into legacyValidateResolvedConfig, so native start can proceed with an env-overridden config the Go CLI rejects; resolve these two fields through projectEnvValues before building experimentalInput.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
ℹ️ 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".
| const healthResult = yield* legacyWaitForHealthyServices(spawner, started, { | ||
| postgrest: postgrestGateway, | ||
| edgeRuntime: edgeRuntimeGateway, | ||
| }).pipe( | ||
| Effect.result, |
There was a problem hiding this comment.
Roll back interrupts after bring-up
When SIGINT/SIGTERM arrives after bringUp has returned—especially during this 30s bulk health wait—the only legacyRollbackStart finalizer has already gone out of scope. The new evidence is that Effect.onError(legacyRollbackStart...) wraps only the bringUp effect above this block, while Go keeps this health wait inside run() and rolls back any non-ignored error/context cancellation in apps/cli-go/internal/start/start.go:73-81,1270-1279; as a result, canceling native start here can leave the newly-created stack/volumes running instead of rolling them back. Wrap the post-bring-up health/bucket-seeding tail in the same rollback-on-interrupt handling.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
| const storageS3ProtocolEnabled = yield* wrapConfigOverride("storage.s3_protocol.enabled", () => | ||
| legacyEnvOverrideBool( | ||
| "SUPABASE_STORAGE_S3_PROTOCOL_ENABLED", | ||
| config.storage.s3_protocol.enabled, | ||
| "storage.s3_protocol.enabled", |
There was a problem hiding this comment.
Validate storage max env overrides before start
When SUPABASE_STORAGE_ANALYTICS_MAX_TABLES=bad (or the sibling analytics/vector max-count overrides) is set, Go decodes those uint fields during the same Config.Load pass as storage.vector.enabled and storage.s3_protocol.enabled, before any Docker work. The native path validates the enabled booleans here but never reads storage.analytics.max_{namespaces,tables,catalogs} or storage.vector.max_{buckets,indexes}, so malformed overrides can be ignored while the stack starts; add the sibling legacyEnvOverrideUint checks to this eager storage-validation block.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
| content_path: | ||
| legacyEnvOverride(`${envPrefix}_CONTENT_PATH`, tmpl.content_path, projectEnvValues) ?? | ||
| tmpl.content_path, |
There was a problem hiding this comment.
Reject email template content env overrides
When auth is enabled and a configured email template or notification sets SUPABASE_AUTH_EMAIL_TEMPLATE_<NAME>_CONTENT (or the notification _CONTENT variant), Go folds that env value into the template’s Content *string before email.validate(), which rejects inline content unless a content_path is used. This resolver only applies the _SUBJECT/_CONTENT_PATH overrides, while the later validation checks only raw TOML content presence, so native start can accept and run a config the Go CLI fails before Docker; include the _CONTENT override in the same presence validation.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
What changed
Ports
supabase startfrom a Go-binary proxy to native TypeScript in the legacy CLI shell (CLI-1323), continuing the local-dev-stack shell migration alongside the already-portedstop/status/db push/db reset/db start.Talks directly to Docker/Podman via subprocess, mirroring Go's sequential per-container
DockerStart— no Docker Compose, and no@supabase/stack/effectorchestration (that runtime targets a deliberately different local-dev product and would silently manage the wrong set of containers). Brings up all 14 containers in Go's real start order, with per-service config-boolean +--excludegating, Go-byte-exact env/image resolution, a bulk health-check phase (Docker healthcheck for most services, an HTTP-HEAD-through-Kong bypass for PostgREST/Edge Runtime), and full rollback on any bring-up failure.Also natively implements the fresh-volume DB schema/migration/seed pipeline, Edge Runtime container bring-up, and fresh-volume storage bucket seeding — previously tracked as out-of-scope follow-ups for this port, now closed. Only the linked-project version-check suggestion (a best-effort "update available" hint with zero Management API dependency otherwise) remains out of scope by design.
Why
Continues the legacy-shell migration from Go-binary-proxy commands to native TypeScript, closing CLI-1323.
Reviewer-relevant context
legacyParseGoDuration(theconfig.tomlduration-string parser feeding Go-parity env vars likeGOTRUE_SESSIONS_TIMEBOX) silently accepted a malformed duration with no digits (a bare unit like"s", or a lone"."with no digits on either side) and returned0instead of erroring like Go's realtime.ParseDuration— both cases now throw Go's exacttime: invalid duration "..."message.start.services.ts's descriptiveenabledGatemetadata for Mailpit referenced the deprecatedinbucketconfig section instead of itslocal_smtprename. Fixed, and added a mechanical cross-check test that evaluates every service'senabledGatestring againststart.gates.ts's real computed gate across a battery of synthetic configs, so a future drift between the two fails loudly instead of silently.[api.tls] enabled = truelocal stacks (self-signed Kong cert) — the local-Kong-CA-trust mechanism already used byseed buckets/storage/db resetis now wired intostart's health-check HTTP client too.Note
This branch is currently 9 commits behind
develop(unrelateddeps/dockerbump commits) — opening as draft to get CI/review visibility; happy to rebase before this comes out of draft.