Uh oh!
There was an error while loading. Please reload this page.
fix(cli): match Go's functions serve docker-down error, kong reload args, and secrets filtering - #5976
Conversation
…rgs, and secret name casing (CLI-1984)
Coly010
commented
Jul 28, 2026
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:bfdda3a730
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… (review: codex) - toPlainEdgeRuntimeConfig uppercases [edge_runtime.secrets] names again: Go's config loader rewrites every key with strings.ToUpper (pkg/config/config.go:766-771) before set.ListSecrets reads it, so container env names are always UPPERCASE regardless of authored casing. The empty/unresolved-value filtering (SHA256>0 gate) stays. - assertLocalDbRunning preserves the spawn-failure cause and legacyIsDockerDaemonUnreachable also classifies the runtime-not-found message, so a machine with neither docker nor podman on PATH still gets 'failed to inspect service: ...' plus the Docker Desktop install hint -- matching Go, where the missing daemon socket is a connection failure (internal/utils/misc.go:155-166) and never a bare inspect error.
Coly010
commented
Jul 29, 2026
@codex review |
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:817e60b4f0
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@a3920b048325fb546cd069485c5680bffa919432Preview package for commit |
…nt (review: codex) Go attaches the Docker Desktop suggestion whenever client.IsErrConnectionFailed fires (internal/utils/misc.go:160-162), and the pinned SDK (docker/docker v28.5.2) wraps every failed Windows npipe open as errConnectionFailed with the "error during connect" prefix (client/request.go:175-185) — a word order none of the Unix-socket phrases matched. Add that uniform transport-failure wrap to the stderr classifier so both Windows variants (elevated and non-elevated) get the install hint, and deliberately skip the localized inner OS text.
…assertion (review: codex) Go only fetches third-party JWKS inside ServeFunctions (internal/functions/serve/serve.go:141, ResolveJWKS, pkg/config/config.go:1727-1786), strictly after restartEdgeRuntime's AssertSupabaseDbIsRunning (serve.go:110-113) — the TS wrapper fetched it before the assertion, so a down Docker daemon could wait on up to ~20s of OIDC/JWKS requests first. Split resolveAuthArtifacts into a config-load-time half (signing keys, jwt_secret length check, key generation — Go's LoadConfig work, kept before the assertion so invalid auth config still beats docker-down) and finalizeAuthArtifacts (the remote fetch + key assembly, run after the assertion in Go's ServeFunctions position). Integration tests lock both orderings.
Coly010
commented
Jul 29, 2026
@codex review |
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
kanadgupta
left a comment
There was a problem hiding this comment.
Claude flagged a couple small things but otherwise LGTM
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…message (review: kanadgupta) Corrects `edge-runtime.service.ts`'s doc comment, which described the pre-817e60b4 verbatim-keys behavior instead of the shipped uppercase transform. Exports `LEGACY_CONTAINER_RUNTIME_NOT_FOUND_MESSAGE` from legacy-container-cli.ts so legacy-docker-suggest.ts's daemon-unreachable matcher (and its test) reference the producer's literal directly instead of a hardcoded copy that could silently drift.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:dcc0d46c27
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…4-functions-serve-parity-docker-down-error-kong-reload-args
Uh oh!
There was an error while loading. Please reload this page.
…0-edgecosmetic-parity-sweep-from-the-2026-07-24-audit Resolves the semantic conflict with #5976: the new prune stdout helper now uses the renamed legacyContainerRuntimeNotFoundMessage constant (ci: Check code quality, Run unit and integration tests)
…LI-1990) (supabase#5978) Batch sweep of the small confirmed edge/cosmetic Go-parity divergences from the 2026-07-24 audit. Every item was re-verified against `apps/cli-go` at current develop before changing anything (audit line numbers were stale); several items turned out to be already fixed or in-flight in other PRs and are listed as such. Fixes CLI-1990 — https://linear.app/supabase/issue/CLI-1990/edgecosmetic-parity-sweep-from-the-2026-07-24-audit-batchable-tail ## Item-by-item | # | Item | Status | Notes | |---|------|--------|-------| | 1 | `functions delete` line colour | **Fixed** | Aqua slug + ref, stdout-gated (`delete.go:20`) | | 2 | `functions deploy` success ref Aqua, `Bundling Function:` bold, `No Functions specified…` bold | **Fixed** | `deploy.go:70,35`, `bundle.go:30`; stdout-gated where stdout-bound | | 3 | Prune bullets ` • <bold>` | **Already fixed** | by supabase#5947 (CLI-1974), commit `c4b45874` | | 4 | serve `supabase start is not running.` Aqua | **Deferred** | open PR supabase#5976 modifies the same `assertLocalDbRunning` hunk in `shared/functions/serve.ts`; one-liner to do after it merges | | 5 | `encryption update-root-key` Finished line + bogus comment | **Fixed** | Aqua'd; comment claimed a nonexistent "render Aqua as plain" convention | | 6 | start rollback missing `Stopping containers...` | **Fixed** | stderr, matching Go's `DockerRemoveAll` writer on the start-failure path (`start.go:77`) | | 7 | `--debug` `Pruned …` reports | **Fixed** | `Pruned containers:/volumes:/network:` (singular network) `[a b c]` on stderr; prune stdout now collected (also removes a latent unread-pipe hazard); `LegacyDebugFlag` threaded through stop/start/rollback | | 8 | Per-retry `Retrying after Ns: <image>` banner | **Fixed** | `4s`/`8s` per Go's `2<<(i+1)` schedule (`docker.go:314`); the failed attempt's error line is played by the already-teed `docker pull` stderr | | 9 | `inspect db blocking` backtick `blocking_statement` | **Fixed** | col 2 only; col 5 (`blocked_statement`) stays bare per Go's format string (`blocking.go:56`) | | 10 | `seed buckets` mutex bracket `[local linked]` | **Fixed** | cobra keeps registration order for the group list and sorts only the "were all set" list; corrected the misleading comment in `legacy-db-target-flags.ts` (storage's `[linked local]` stays correct) | | 11 | `snippets download` 4 UUID forms + lowercase URL | **Fixed** | faithful `uuid.Parse` port incl. the braced-form `s[1:]` quirk; canonical lowercase interpolated into the URL; Go's three error branches verbatim | | 12 | `storage cp --jobs` negative rejection | **Fixed** | pflag's exact uint error (`invalid argument "-1" for "-j, --jobs" flag: strconv.ParseUint: …`), before mutex validation and without telemetry; the documented `0 → 1` clamp stays (Go's 0 deadlocks) | | 13 | `postgres-config` value coercion + `%+v` floats | **Fixed** | exact `ParseBool` case set; int64-overflow → verbatim string; pretty table renders numbers with Go's float64 `%g` (`1000000` → `1e+06`) via hoisted `legacyGoFormatFloat` (also reused by `db query`) | | 14 | init template file modes | **Fixed** | 0644/0755 pinned; the gitignore *append*-branch write is deliberately left unpinned — mode only applies at creation, and supabase#5977 rewrites that exact line | | 15 | login fallback dir 0700→0755 | **Fixed** | Go pins the dir 0755 (`access_token.go:91`); the token file stays 0600, so no secret exposure | | 16 | `test new` 0644 | **Fixed** | + dir 0755, like Go's `utils.WriteFile` | | 17 | `inspect report` 0755/0644 | **Fixed** | | | 18 | bootstrap invalid-stored-token fast-fail | **Deferred** | not small: `resolveLegacyAccessToken` deliberately collapses invalid→`None` for many callers (sso, snippets, postgres-config, …); distinguishing invalid-vs-missing needs a shared-semantics change | | 19 | `migration new` Created line on stdin-copy failure | **Fixed** | mirrors Go's deferred `Println` (`new.go:24-28`); also stdout-gates the Bold path (CLI-1546 class) | | 20 | telemetry state-file recovery | **Fixed** | all-or-nothing decode like Go's `decodeState`; a corrupt file resets `enabled` to true and rotates identity, exactly like Go | | 21 | unlink/services ref-read error | **Fixed (services)** | unlink already matched Go; services now warns `failed to load project ref: …` on a read error and continues unlinked (TOCTOU NotFound stays silent, like Go's `ErrNotLinked` branch) | | 22 | `domains` CNAME dump byte codes | **Kept documented** | premise inaccurate: the non-reproducible part of Go's `%+v` is a runtime *pointer address* for `ValidationErrors`, not byte codes; TS's deterministic rendering is already documented in `domains.format.ts` | | 23 | sso `--domains=` explicit-empty | **Deferred** | `sso update` already matches Go (len-check drops it); the `add` fix touches `sso/add/add.handler.ts`, in-flight in open PR supabase#5974 | | 24 | `db dump --file ""` | **Fixed** | keys off `len > 0` like Go: empty means stdout, no file open, no `Dumped schema to …` line | | 25 | network-restrictions CIDR-before-ref ordering | **Deferred, kept documented** | direction is inverted vs the issue text: TS validates CIDR *before* ref, Go resolves ref first in `PersistentPreRunE`; aligning overlaps open PR supabase#5975 (incl. its integration test file). The `SIDE_EFFECTS.md` note frames this as intended based on an incomplete Go reading — worth revisiting after supabase#5975 | ## Known residuals (deliberate, documented in code) - `postgres-config` digits in `(2^53, 2^63)` still lose precision on the way in (`JSON.stringify` cannot emit exact int64 tokens); Go sends exact integers there. Values beyond int64 now match Go (string fallback). - Colour TTY gating: stderr-bound colour gates on stderr's TTY (per `legacy-colors.ts`/CLI-1546 convention), whereas Go's lipgloss gates everything on stdout. Deliberate, pre-existing convention; only observable when exactly one of stdout/stderr is a TTY. - Bun's `util.styleText` currently ignores `validateStream`/`NO_COLOR` (verified on Bun 1.3.x), so under Bun piped output still carries ANSI for *all* legacy colour sites — a pre-existing runtime gap that predates this PR and deserves its own issue. - The services warning's error suffix is Effect's error text, not Go's `*PathError` bytes — the `failed to load project ref: ` prefix is the parity-bearing part. - `--jobs abc`/`3.5` still surface Effect CLI's parser error rather than pflag's; this PR scopes to negatives (the only case `Flag.integer` accepts that Go rejects). ## Review notes Four-perspective review (architect / engineer / security / DX) run pre-PR; all approve. Engineer fuzz-verified `legacyGoFormatFloat` (23k values) and `legacyParseSnippetUuid` (~8k inputs) byte-identical to Go/google-uuid. Security signed off on the 0700→0755 fallback-dir change (token file unchanged at 0600, matches Go exactly). Remaining findings were the documented residuals above.
## TL;DR fixes custom auth email templates silently reverting to GoTrue's built-in defaults after `db reset`, `functions serve` (every hot reload) and `branch switch` `kong reload` re-renders nginx.conf from Kong's default template, dropping the `:8088` email-templates server that only bring-up's `--nginx-conf` defines. Every reload now hands that template back.... Third pass on this argument: - PS: first complete one: supabase#5906 added it to `functions serve`'s reload supabase#5976 reverted it for Go parity ("reload reuses the prefix bring-up prepared" it doesn't) and - supabase#6017's new `db reset` reload (fixing issue supabase#6016's 502s) shipped bare on the same assumption. Fixing Go's call sites too removes the parity argument, so this can't be reverted on parity grounds a fourth time. supabase#6017's DNS re-resolution is unchanged. verified 502→200 on a rotated container IP with the new argv. Broken stacks self-heal on the next reset. A template-less container (only one never created by `supabase start`, written unconditionally since the start) now fails the reload loudly, fatal in `db reset`, stderr warning in `functions serve`/`branch switch`...... <details><summary>repro / before vs after</summary> Real stack, kong's `StartedAt` never changes (container never restarted): 0. after supabase start listeners=[8000 8001 8088 8443 8444] template=custom ✓ 1. after BASELINE db reset listeners=[8000 8001 8443 8444] template=REFUSED → built-in default 2. after FIXED db reset listeners=[8000 8001 8088 8443 8444] template=custom ✓ - supabase#6016 non-regression: warm 200 → rotate auth IP → 502 502 502 → new reload → 200, :8088 intact. </details> ## ref: - closessupabase#6059
What changed
Three Go-parity fixes for
functions serveinapps/cli/src/shared/functions/serve.ts, plus one minor watcher parity item and review follow-ups.1. Docker-down error and ordering — removed the TS-only upfront docker precheck (
failed to run docker. Docker Desktop is a prerequisite …). Go'srestartEdgeRuntimehas no such precheck (internal/functions/serve/serve.go:107-113): it loads config first, then asserts the local DB container, so a down Docker daemon surfaces asfailed to inspect service: <connection error>with the Docker Desktop install hint on its own stderr line (internal/utils/misc.go:155-166CmdSuggestion, rendered bycmd/root.go:300-303).assertLocalDbRunningnow attaches that hint as an errorsuggestionwhen the daemon is unreachable, and the daemon-unreachable matcher (legacy-docker-suggest.ts) additionally recognizes socket permission-denied errors, which the pinned Docker SDK (v28.5.2,client/request.go:144-152) also classifies as connection failures. As of 817e60b (review follow-up),assertLocalDbRunningalso preserves the spawn-failure cause when neitherdockernorpodmanis on PATH, and the matcher classifies that runtime-not-found message too — the shell-out equivalent of Go's missing daemon socket, so a no-runtime machine still gets the install hint instead of a barefailed to inspect service. Ordering verified against Go: everything Go validates during config load before its DB assert (signing keys,jwt_secret,deno_version—pkg/config/config.go:1110-1172) already runs before the TS DB assert too.2. Kong reload args —
reloadKongnow runs barekong reload, matching Go (serve.go:129). The--nginx-conf /home/kong/custom_nginx.templateargument belongs tostart's Kong bring-up entrypoint (start.go:589-592), which persists the prefix configurationkong reloadreuses — it was never part of Go's serve path, TLS-enabled stacks included.3.
[edge_runtime.secrets]filtering (uppercasing retained) —toPlainEdgeRuntimeConfignow injects only entries with a resolved non-empty value, mirroring Go'sset.ListSecretsSHA256 > 0gate (internal/secrets/set/set.go:48-52,pkg/config/secret.go:94-107): empty values and unresolvedenv(VAR)literals are skipped. Secret names keep the pre-existing.toUpperCase(): an earlier revision of this PR injected names verbatim based on a reading ofset.ListSecretsalone, but Go's config loader rewrites every key withstrings.ToUpper(pkg/config/config.go:766-771, the viper #1014 workaround) beforeListSecretsruns, so container env names are always UPPERCASE regardless of authored casing — corrected in review follow-up 817e60b.Both shells / shared consumers
shared/functions/serve.tsis shared infrastructure. Fix 3's helper is also used by legacystart's edge-runtime bring-up — the uppercased/filtered semantics are Go-correct there too (Gostartreaches the identicalListSecretspath viaServeFunctions,start.go:1104), and astartintegration assertion now pins it. Fixes 1–2 arefunctions serve-only code paths; other commands' docker-down messages (legacy-docker-run.layer.tsetc.) are untouched. The widened daemon-unreachable matcher benefits all its consumers (serve, db bootstrap seam, image resolve/prepull) in the Go-correct direction. Thenextshell does not consume these paths (functions devruns via@supabase/stack).Minor watcher items (from the issue)
File change detected: <path> (WRITE|CREATE|REMOVE)— instead of always(update); the nodefs.watchlayer classifiesrenameevents via an existence check.RENAME/CHMOD/OR-combined ops are unreachable under node's watcher (documented in-code).SUPABASE_FUNCTIONS_WATCH_LIMITcap/hint (watcher.go:114-142) — TS uses a single recursive watch, not per-directory fsnotify registration, so the cap doesn't fall out naturally; and the debounce is still trailing-500 ms vs Go's fire-first (watcher.go:102-104) — a semantics change judged too risky to bundle here.Known residuals (documented, deliberately not fixed here)
encrypted:secret values (pre-existingpackages/configgap;startdecrypts explicitly, Go decrypts during config load).supabase start is not running.lacks Go's Aqua color on "supabase start" (pre-existing).fs.watchrename→create/delete classification has no direct test (real-watcher tests are flaky); the printed-token mapping is covered via the mocked watcher.Fixes CLI-1984
https://linear.app/supabase/issue/CLI-1984/functions-serve-parity-docker-down-error-kong-reload-args-edge