feat(cli): port supabase start command to native TypeScript#5847
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 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. |
…agerly
Go's Config.Load decodes storage.analytics.{max_namespaces,max_tables,
max_catalogs} and storage.vector.{max_buckets,max_indexes} as plain
uints in the same unconditional Viper/mapstructure pass as the
storage.vector.enabled/storage.s3_protocol.enabled fields already
validated eagerly here (pkg/config/storage.go:28-39), before start
touches Docker. The native port never validated these five env
overrides at all, so a malformed value was silently ignored instead
of hard-failing before any Docker work, regardless of --exclude storage.
review: PRRT_kwDOErm0O86S5NR3
Go folds a SUPABASE_AUTH_EMAIL_TEMPLATE_<NAME>_CONTENT (or notification _CONTENT) env override into Content *string via the same generic Viper AutomaticEnv bind as every other field (config.go:749), before Config.Validate runs. (e *email) validate (config.go:1293-1301) then rejects the template if Content is set without ContentPath. legacyResolveAuthEmail only applied _SUBJECT/_CONTENT_PATH overrides, and readAuthEmailTemplateContent's presence check only looked at the raw TOML content key, so a _CONTENT override was invisible on both sides: native start could accept and launch Docker on a config Go's CLI would refuse. Fold the env override into content_present, computed once in legacyResolveAuthEmail and consumed by the validator instead of re-derived from the raw document. review: PRRT_kwDOErm0O86S5NR7
…3-port-supabase-start
… pull retry Go's DockerResolveImageIfNotCached (apps/cli-go/internal/utils/docker.go: 326-334) inspects via the typed Engine API client and distinguishes errdefs.IsNotFound from any other inspect error, returning the latter immediately without ever entering the pull-retry loop. hasLocalImage shelled out to `docker image inspect` with stdout/stderr both ignored, so a down daemon and a genuine cache miss looked identical: both non-zero exits. A down daemon fell through the full 3-registry x 3-attempt pull-retry loop (~36s of backoff) before a generic "failed to pull from all registries" message instead of failing fast with a clear daemon error. Capture stderr (stdout stays ignored to avoid the cache-hit pipe-buffer-deadlock hazard) and check it against legacyIsDockerDaemonUnreachable, failing immediately on a match. A genuine spawn failure (no docker/podman binary) now also propagates instead of being silently swallowed to false, matching how the pull loop already treats that case. review: PRRC 3639121719 (#5847 (comment))
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db815ffcac
ℹ️ 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".
…pt test
"rolls back on a SIGINT-style interruption mid-bring-up" only waited
for a `create` call before interrupting, but
mockStartContainerCliSpawner resolves every Docker call synchronously
with no fake delay. Effect.forkChild({ startImmediately: true }) runs
a forked fiber synchronously up to its first genuine suspension point,
so a whole bring-up (10+ containers, no real waits) could occasionally
run to full completion before the test's own polling loop was ever
scheduled, making Fiber.interrupt a no-op on an already-succeeded
fiber. Failed intermittently in CI (run 30027017791) though never
locally.
Marks the db container never-healthy, mirroring the neighboring
post-bring-up test, so bringUp's own Postgres health-check wait
genuinely retries on its real 1s Schedule.spaced backoff before the
fiber is interrupted -- a deterministic suspension point instead of a
race against a zero-delay mock. Ran the isolated test 5x locally
before this change to confirm the fix holds, plus the full file (142
tests) after.
ci: Run unit and integration tests
writeFile's mode option is only a creation-time hint the kernel ANDs with the process umask, not a chmod. Under umask 077/027, the staged secret files (bind-mounted read-only into a non-root in-container reader -- Kong uid 100, Postgres's dropped-root entrypoint) landed at 0600 instead of the intended world-readable 0644, causing EACCES in the container. Go has no equivalent exposure here -- it heredocs secret content directly into the container's entrypoint, never touching the host filesystem -- so this is judged on the TS implementation's own merits, not parity. Force the final mode with an explicit chmod after writeFile. review: PRRT_kwDOErm0O86TVHX1
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f7c84d4e9
ℹ️ 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".
…table Go's mergeDefaultValues (apps/cli-go/pkg/config/config.go:690-698) merges an ejected default config.toml into Viper before the user's own file. That default template unconditionally includes uncommented [auth.sms.twilio] (templates/config.toml:288-293) and [auth.external.apple] (templates/config.toml:322-335) tables -- the only SMS provider and the only external OAuth provider given a real default section, every sibling is just named in a comment. Since Viper merges never delete keys, those two providers' fields are always registered with Viper, so their env overrides apply even when the user's config.toml never declares the table -- confirmed for twilio by running the real Go binary directly. legacyResolveAuthSms and legacyResolveAuthExternalProviders gated every provider on the raw document's table being present, so an env-only Twilio or Apple setup (SUPABASE_AUTH_SMS_TWILIO_ENABLED=true or SUPABASE_AUTH_EXTERNAL_APPLE_ENABLED=true with no corresponding TOML table) was silently dropped instead of taking effect. The other 4 SMS providers and every other external OAuth provider have no default template entry, so their presence gate is unchanged and still correct parity. review: PRRT_kwDOErm0O86TVHX3, PRRT_kwDOErm0O86TVHX7
legacyResolveLocalConfigValues's own SMS validation only runs `if (authEnabled)`, so with auth.enabled = false that path is skipped entirely. start.handler.ts separately calls legacyResolveAuthSms directly and unconditionally to validate auth.sms.max_frequency, but the call wasn't wrapped -- a malformed override (e.g. SUPABASE_AUTH_SMS_ENABLE_SIGNUP=bad) threw a plain Error that escaped as an unhandled Effect defect instead of the normal LegacyStartInvalidConfigError config-load failure. Wrap the call in Effect.try, mirroring the existing resolvedEmail fix a few lines above for the identical bug class. review: PRRT_kwDOErm0O86TVhiT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 789d7c7077
ℹ️ 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".
Go's time.ParseDuration rejects any duration whose accumulated nanosecond count exceeds math.MaxInt64 (~292.47 years, real max 2562047h47m16.854775807s), and truncates a sub-nanosecond fractional remainder toward zero rather than rounding (uint64(float64(f) * ...) is a truncating conversion). legacyParseGoDuration had neither check: an extreme value like auth.sms.max_frequency = "2562048h" silently produced an imprecise large number instead of throwing, and "0.5ns" rounded up to "1ns" instead of truncating to "0s". Since native start relies on this parser for eager GoTrue/db duration validation before Docker work, a malformed or extreme user-configured value could be accepted or mis-normalized instead of failing like Go. Switch Math.round to Math.trunc for the fraction and add an overflow check against math.MaxInt64, reusing the existing "time: invalid duration ..." error convention. review: PRRT_kwDOErm0O86TWwAD
Every field routed through legacyEnvOverrideUint (auth.jwt_expiry, db.major_version, db.pooler.default_pool_size, realtime.max_header_length, api.max_rows, storage max-count fields, ...) is a plain Go uint, 64 bits wide on every platform this CLI ships for. Go decodes these via mapstructure's decodeUint under Viper's WeaklyTypedInput, which runs strconv.ParseUint(str, 0, 64) -- genuinely failing for any value exceeding 2^64-1. The TS helper only checked that the override was all digits and returned Number(value), silently accepting (and losing precision on) an overflowing value like SUPABASE_AUTH_JWT_EXPIRY=18446744073709551616 instead of rejecting it like Go does. Add a BigInt-based bound check (not a Number comparison, which can't exactly represent values this close to 2^64 and would incorrectly reject legitimate large uint64 values Go itself accepts). review: PRRT_kwDOErm0O86TWwAF
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0f5433fb0
ℹ️ 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".
The previous overflow fix compared total against Number(9223372036854775807n), which rounds UP to 9223372036854776000 -- a value 1ns past Go's true math.MaxInt64 ceiling (9223372036854775808ns, i.e. 2^63) could still pass the check. The precision problem went deeper than the bound: the whole accumulator (total, and each term's n/frac/post) was a plain number, and durations near Go's real max (~9.223e18ns) are already ~1000x past Number.MAX_SAFE_INTEGER, so the accumulator itself silently rounded near the boundary -- independent of the bound's own precision. This could both accept a genuinely overflowing value and reject Go's actual valid maximum, depending on which way a given input happened to round. Rewrite the parse loop to accumulate in BigInt throughout, converting to Number only at the very end for the returned value (still lossy that close to the ceiling, an accepted limit given real durations are nowhere near it -- what matters, rejecting out-of-range overrides like Go does, no longer depends on number precision at all). review: PRRT_kwDOErm0O86TYJTp
Go decodes every uint-typed config field via strconv.ParseUint(str, 0, 64), where base 0 auto-detects the numeric base from a prefix: 0b/0B -> binary, 0o/0O -> octal, 0x/0X -> hex, and a bare leading 0 followed by more digits is ALSO octal -- so "010" parses as decimal 8, not 10. Underscores are permitted between digits in base-0 mode. ParseUint never accepts a leading sign. Verified against a real Go binary across prefix casing, underscore placement, invalid-octal-digit rejection, and sign rejection. legacyEnvOverrideUint only accepted /^\d+$/ and parsed decimally, so SUPABASE_DB_POOLER_MAX_CLIENT_CONN=0100 (or similar zero-padded values on any of this helper's callers -- api.max_rows, auth.jwt_expiry, db.major_version, etc.) silently resolved to a DIFFERENT number than Go's own CLI would use, not just a rejection mismatch -- a zero-padding user could get 64 in Go and 100 in native start. Add parseGoBaseZeroUint, a pure helper matching this grammar exactly, and route legacyEnvOverrideUint through it in place of the decimal- only check, preserving the existing LEGACY_UINT_MAX overflow bound. review: PRRT_kwDOErm0O86TYJTw
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57fab71dc5
ℹ️ 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".
… overrides Two related config-load gaps, both fixed together since they're adjacent changes in the same resolver file: legacyEnvOverridePort only accepted /^\d+$/ and parsed decimally, the same bug already fixed for the uncapped legacyEnvOverrideUint helper. Go decodes every SUPABASE_*_PORT override as uint16 via strconv.ParseUint(str, 0, 16) -- base 0 auto-detects from a prefix, so SUPABASE_LOCAL_SMTP_PORT=010 parses as octal 8 in Go, not decimal 10, and 0x1F90 is accepted as 8080 rather than rejected. Route legacyEnvOverridePort through the existing parseGoBaseZeroUint helper and apply its own uint16 bound afterward, same split strconv.ParseUint itself uses between parsing the literal and checking the bit width. legacyResolveLocalConfigValues read config.experimental.webhooks?.enabled/config.experimental.pgdelta?.format_options straight off the decoded config with no env-override applied at all, unlike every sibling analytics.* field a few lines above. Go binds SUPABASE_EXPERIMENTAL_WEBHOOKS_ENABLED/ SUPABASE_EXPERIMENTAL_PGDELTA_FORMAT_OPTIONS through ExperimentalBindStruct/AutomaticEnv before experimental.validate() runs -- the same mechanism the db/migration loader already applies correctly for the pgdelta override. Apply the same legacyEnvOverrideBool/legacyEnvOverride calls here. review: PRRT_kwDOErm0O86TZRqh, PRRT_kwDOErm0O86TZRql, PRRT_kwDOErm0O86TZRqx
Two config-load gaps in start.handler.ts, fixed together since both
land in the same eager-validation region:
auth.passkey.enabled/auth.webauthn.* and per-provider
auth.external.<name>.{enabled,skip_nonce_check,email_optional} decode
in Go's same unconditional Config.Load pass as the neighboring
auth.rate_limit/auth.web3/auth.oauth_server fields already fixed
eagerly here. The TS resolvers for these (resolveGotruePasskeyWebauthn,
legacyResolveAuthExternalProviders) were only reached from
resolveGotrueEnvInput's gotrue-only branch, itself gated on
auth.enabled and --exclude gotrue -- so a malformed
[auth.passkey] enabled = "bad" (or a custom provider's enabled flag)
with auth disabled or gotrue excluded was silently accepted instead of
failing like Go's Config.Load does. Hoist eager, result-discarding
calls to both resolvers next to the existing rate_limit/web3/
oauth_server checks.
Go's DecryptSecretHookFunc decrypts every dotenvx encrypted: value in
edge_runtime.secrets unconditionally during Config.Load, so the real
Edge Runtime container always receives plaintext. The native path only
did env() interpolation/redaction via toPlainEdgeRuntimeConfig, never
decryption -- ciphertext would reach the container's env file
verbatim. Decrypt each secret value with the existing
legacyIsEncryptedSecret/legacyCollectDotenvPrivateKeys/
legacyDecryptSecret helpers (already used for db.vault) right before
building the Edge Runtime bring-up input.
review: PRRT_kwDOErm0O86TZRqo, PRRT_kwDOErm0O86TZRqt
There was a problem hiding this comment.
💡 Codex Review
Validate ssl_enforcement env bools
When [db.ssl_enforcement] is present and SUPABASE_DB_SSL_ENFORCEMENT_ENABLED=bad overrides its enabled value, Go decodes that bool during Config.Load and rejects the config before any Docker work. Here the DB validation input only carries port and majorVersion, and neither this resolver nor legacyCheckDbToml reads the SSL-enforcement override, so native legacy start can run with an env override the Go CLI fails; resolve this bool override during config loading as well.
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".
…s flag Go's Config.Load decodes analytics.vector_port and storage.analytics.enabled unconditionally in the same pass as their already-validated sibling fields (analytics.port; storage.analytics.max_namespaces/max_tables/max_catalogs, storage.vector.enabled, storage.s3_protocol.enabled). Native start left both fields to be read lazily deep in the container-spec builders, so a malformed env override was silently accepted instead of hard-failing like Go. Resolves review threads PRRT_kwDOErm0O86TZRq1, PRRT_kwDOErm0O86Tf79s.
[db.network_restrictions] ships uncommented in Go's embedded default config.toml and NetworkRestrictions is a non-pointer db struct field, so Config.Load always registers a default and decodes this override unconditionally, same bucket as db.port/db.major_version (already validated here). Native start never validated it at all. Resolves review thread PRRT_kwDOErm0O86Tf79o.
legacyCheckDbToml (shared by start/db push/db pull/db reset/db start) only extracted bucket names from [storage.buckets.*]; the per-bucket file_size_limit was parsed lazily inside legacySeedBucketsRun, reached only on a fresh volume with Storage actually enabled. A malformed value on a reused-volume restart, or when the stack is already running, went completely unvalidated, unlike Go's Config.Load, which decodes every bucket's file_size_limit via the sizeInBytes hook unconditionally. Resolves review thread PRRT_kwDOErm0O86Tf79w.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41b84ede22
ℹ️ 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".
| @@ -379,7 +727,10 @@ function resolveSignedKey( | |||
| const LegacyJwkSchema = Schema.Struct({ | |||
| kty: Schema.String, | |||
| kid: Schema.optionalKey(Schema.String), | |||
| use: Schema.optionalKey(Schema.String), | |||
| key_ops: Schema.optionalKey(Schema.Array(Schema.String)), | |||
| alg: Schema.optionalKey(Schema.String), | |||
There was a problem hiding this comment.
When auth.signing_keys_path contains multiple keys and a later key has alg: "HS256" or any value other than ES256/RS256, Go's JSON decode into the Algorithm type rejects the file during Config.Load. This schema accepts arbitrary strings, and only the first key is later checked while generating the implicit anon/service-role keys, so native start can proceed and pass invalid GOTRUE_JWT_KEYS/JWKS metadata to services; narrow alg here to the same two values when decoding the signing-keys file.
Useful? React with 👍 / 👎.
| const ordered: Record<string, unknown> = { kty: key.kty }; | ||
| if (key.kid !== undefined && key.kid.length > 0) ordered["kid"] = key.kid; | ||
| if (key.use !== undefined && key.use.length > 0) ordered["use"] = key.use; | ||
| if (key.key_ops !== undefined) ordered["key_ops"] = key.key_ops; |
There was a problem hiding this comment.
When a configured signing key explicitly contains "key_ops": [], Go's json:",omitempty" on the []string field omits key_ops from the GOTRUE_JWT_KEYS payload, but this serializer emits an empty array. The JWKS path already normalizes empty operations away, so GoTrue receives a different private-key document than the stable CLI and key-ops-aware parsing can treat it as a key with no permitted operations; skip key_ops when the array is empty.
Useful? React with 👍 / 👎.
| Effect.map((content) => content.trim()), | ||
| Effect.orElseSucceed(() => ""), | ||
| ); | ||
| if (version.length > 0) { |
There was a problem hiding this comment.
When a linked supabase/.temp/*-version file exists but contains only whitespace, Go tests the raw file length before trimming in replaceImageTag, so the pin is still applied as an empty image tag and start fails when it tries to pull that invalid reference. This helper trims before deciding whether the pin exists, so a corrupt/interrupted linked-version file is silently ignored and native start/services fall back to defaults instead of matching the stable CLI's visible bad pin; keep presence detection separate from the trimmed tag.
AGENTS.md reference: apps/cli/AGENTS.md:L246-L256
Useful? React with 👍 / 👎.
2 participants
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.