Skip to content

feat(cli): port supabase start command to native TypeScript#5847

Open
Coly010 wants to merge 79 commits into
developfrom
columferry/cli-1323-port-supabase-start
Open

feat(cli): port supabase start command to native TypeScript#5847
Coly010 wants to merge 79 commits into
developfrom
columferry/cli-1323-port-supabase-start

Conversation

@Coly010

@Coly010 Coly010 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What changed

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.

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.

Why

Continues the legacy-shell migration from Go-binary-proxy commands to native TypeScript, closing CLI-1323.

Reviewer-relevant context

  • 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.

Note

This branch is currently 9 commits behind develop (unrelated deps/docker bump commits) — opening as draft to get CI/review visibility; happy to rebase before this comes out of draft.

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
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

👋 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 open-for-contribution label, then open a pull request that links the issue with a closing keyword (e.g. Closes #123).
See CONTRIBUTING.md for the full workflow. Once a maintainer adds the open-for-contribution label to a linked open issue, reopen or open a new pull request and it will be accepted.

@github-actions github-actions Bot closed this Jul 9, 2026
@Coly010 Coly010 reopened this Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

👋 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 open-for-contribution label, then open a pull request that links the issue with a closing keyword (e.g. Closes #123).
See CONTRIBUTING.md for the full workflow. Once a maintainer adds the open-for-contribution label to a linked open issue, reopen or open a new pull request and it will be accepted.

@github-actions github-actions Bot closed this Jul 9, 2026
@Coly010 Coly010 self-assigned this Jul 9, 2026
@Coly010 Coly010 reopened this Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

👋 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 open-for-contribution label, then open a pull request that links the issue with a closing keyword (e.g. Closes #123).
See CONTRIBUTING.md for the full workflow. Once a maintainer adds the open-for-contribution label to a linked open issue, reopen or open a new pull request and it will be accepted.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

👋 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 open-for-contribution label, then open a pull request that links the issue with a closing keyword (e.g. Closes #123).
See CONTRIBUTING.md for the full workflow. Once a maintainer adds the open-for-contribution label to a linked open issue, reopen or open a new pull request and it will be accepted.

@github-actions github-actions Bot closed this Jul 9, 2026
@avallete avallete reopened this Jul 9, 2026
pull Bot pushed a commit to chizee/cli that referenced this pull request Jul 9, 2026
)

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>
@Coly010

Coly010 commented Jul 10, 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: 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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.gates.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/services/postgres.service.ts
@Coly010

Coly010 commented Jul 10, 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: 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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/lib/db-setup.ts
Comment thread apps/cli/src/legacy/commands/start/start.gates.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/services/gotrue.service.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts
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.
@Coly010

Coly010 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

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.
@Coly010

Coly010 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

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.
@Coly010

Coly010 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Coly010 added 2 commits July 21, 2026 15:40
…(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).

@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: 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".

Comment thread apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts Outdated
…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.

@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: 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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
…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`.

@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


P2 Badge Decode auth subtrees before the auth gate

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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/shared/functions/serve.ts Outdated
Coly010 added 3 commits July 21, 2026 18:21
… (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.

@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: 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".

Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
…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.

@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: 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".

Comment thread apps/cli/src/shared/functions/serve.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/services/edge-runtime.service.ts
Coly010 added 2 commits July 21, 2026 19:47
…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.

@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: 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".

Comment thread apps/cli/src/shared/cli/run.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts Outdated
Coly010 added 2 commits July 21, 2026 20:38
…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".

@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: 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".

Comment thread apps/cli/src/legacy/commands/start/services/edge-runtime.service.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts
Coly010 added 2 commits July 21, 2026 21:32
…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.

@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: 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".

Comment thread apps/cli/src/legacy/commands/start/services/vector.service.ts
Comment thread apps/cli/src/legacy/commands/start/lib/db-setup.ts
…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.

@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


P2 Badge Validate SMTP overrides even when auth is off

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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts
Coly010 added 2 commits July 21, 2026 23:16
…: #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

@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

const webhooksEnabled = config.experimental.webhooks?.enabled === true;
const pgdeltaFormatOptions = config.experimental.pgdelta?.format_options ?? "";

P2 Badge Apply env overrides to experimental validation

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".

Comment thread apps/cli/src/legacy/commands/start/start.handler.ts Outdated
Comment thread apps/cli/src/legacy/commands/start/start.handler.ts
Comment thread apps/cli/src/legacy/shared/legacy-local-config-values.ts
Coly010 added 3 commits July 22, 2026 12:45
Go's run() (start.go:264-1288) keeps the bulk health-check wait, the
ignore-health-check storage recheck-and-seed, and bucket seeding in the
same function body as bring-up, and Run() rolls back on any non-nil
error from that single call, including a canceled context from
SIGINT/SIGTERM (start.go:73-81). The native port only wrapped bringUp
itself in Effect.onError(legacyRollbackStart), so an interrupt landing
during the post-bring-up tail left the newly-created stack/volumes
running instead of rolling back.

Wrap the whole tail in one Effect.onError, matching bringUp's own
pipe, and drop the now-redundant per-step manual rollback calls.

review: PRRT_kwDOErm0O86S5NRx
…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
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