fix(cli): port functions download to native TypeScript (CLI-1963) - #6082
fix(cli): port functions download to native TypeScript (CLI-1963)#6082Coly010 wants to merge 14 commits into
Conversation
Ports `supabase functions download`'s default Docker-unbundle path (`--use-docker`, default true) from wholesale Go-binary delegation to native TypeScript, in both the legacy and next shells. `--use-api` was already native; `--legacy-bundle` (hidden, deprecated pre-1.120.0 fallback requiring a host Deno-binary install with no precedent elsewhere in this codebase) is deliberately left delegating to the Go binary, per the parity-audit rationale recorded on the Linear issue. Hoists the Docker-orchestration primitives `download.ts` needs (`runChildProcess`, `isDockerRunning`, `ensureDockerNetwork`, `ensureDockerNamedVolume`, `localDockerId`, `resolveEdgeRuntimeVersion`, etc.) out of `deploy.ts` into a new `functions-docker.ts`, and deduplicates the `edge-runtime-version` pin file lookup that was copy-pasted across all four `deploy`/`download` handler files into a single `resolveEdgeRuntimeVersionPin` helper. Along the way, fixes: - CLI-1891-class validation gap: slugs sourced from the Management API's function list weren't validated before the new Docker path's temp-file write, reopening a path-traversal vector Go's own `downloadAll` already guards against. - The `next` shell's `--use-docker` flag was missing `Flag.withDefault(true)`, a real default-value divergence from both `legacy` and Go. - A brotli-decompression bug: this CLI's HTTP transport already auto-decodes `Content-Encoding: br` responses (confirmed empirically), so re-running `brotliDecompressSync` on the eszip body threw on already-decoded bytes. - Temp eszip cleanup only ran after a successful Docker run; wrapped in `Effect.ensuring` so it also runs on network/volume/spawn failures, matching Go's `defer`. - The `.suggestion` field's leading newline (needed to reproduce Go's blank separator line before the `--legacy-bundle` hint) was being trimmed away by the generic CLI error normalizer.
|
@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: a6c6cb942b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…view: CLI-1963)
Go's DockerStart only overrides the Docker network when
len(viper.GetString("network-id")) > 0 (internal/utils/docker.go:379-382).
The native functions download/deploy Docker paths used
explicitStringFlag(...) ?? localDockerId(...), which returns "" (not
undefined) for --network-id=, so an explicit empty override was invoked
verbatim instead of falling back to the generated network.
Adds explicitNonEmptyStringFlag (cobra-flag-groups.ts), which folds in Go's
len(value) > 0 gate, and switches both download.ts and deploy.ts's docker
network resolution to it.
…: CLI-1963)
Go's replaceImageTag (pkg/config/utils.go:81-84) appends the raw content of
supabase/.temp/edge-runtime-version verbatim after the image's `:`, so a pin
can legitimately already carry its own `v` prefix (both forms are exercised
elsewhere in this codebase, e.g. legacy-edge-runtime-image.unit.test.ts's
"v9.9.9" fixture vs. deploy.integration.test.ts's bare "9.9.9"). The native
download Docker path always prepended `v` to the resolved version, so a
v-prefixed pin produced `supabase/edge-runtime:vv9.9.9`, which Docker fails
to pull.
Hoists serve.ts's existing edgeRuntimeImageTag helper (which already handled
this correctly) into the shared functions-docker.ts, and applies it in
download.ts and deploy.ts, which had the same unprefixed-vs-prefixed bug in
their own inline `v${version}` construction.
…ON response (review: CLI-1963) v1GetAFunctionBody's generated contract marks its response kind: "json", so executeRaw() defaults to Accept: application/json for it (buildRequest's unconditional acceptJson for json-kind operations). Go's own downloadOne (the Docker-unbundle path this mirrors) sends no Accept header at all, unlike the server-side path's explicit multipart/form-data override, so the default JSON negotiation here could receive a negotiated JSON response instead of the raw eszip bytes and fail downstream in edge-runtime unbundle. Overrides the request's Accept header to */* (no preference) — the closest equivalent this API surface has to Go sending no header.
…er (review: CLI-1963) Go's Run calls flags.LoadConfig(fsys) unconditionally at the very top, before checking useDocker or whether Docker itself is running (download.go:135-138). The native download path only resolved/validated the project config (via resolveEdgeRuntimeImage) inside the isDockerRunning() branch, so a default `functions download` with an invalid edge_runtime.deno_version proceeded straight to the API/filesystem side-effecting server-side path whenever Docker was down or --use-api was passed, instead of failing up front like Go. Resolves resolveEdgeRuntimeImage unconditionally before branching on --use-api/--use-docker/Docker's running state.
…eview: CLI-1963) Go's DockerStart drops the named-volume bind entirely on Bitbucket (internal/utils/docker.go:400-405) rather than just skipping its explicit creation — `docker run -v <name>:...` would otherwise still implicitly create the named volume, which Bitbucket's restricted Docker environment doesn't allow. The native Docker-unbundle path's ensureDockerNamedVolume already skipped the explicit `docker volume create` under BITBUCKET_CLONE_DIR, but the manually-built `docker run -v ...` bind list still unconditionally included the named-volume bind, so the container run itself could still fail in Bitbucket's restricted environment. Applies the same BITBUCKET_CLONE_DIR carve-out deploy.ts's buildDockerBinds already uses.
…p (review: CLI-1963)
Go gates the Docker-unbundle path's temp-eszip cleanup on
viper.GetBool("DEBUG") (download.go:203), so an explicit --debug=false
resolves to false (cleanup runs). The native path used
hasGlobalLongFlag(rawArgs, "debug"), a presence-only check, so --debug=false
was treated the same as --debug and skipped cleanup — the opposite of Go.
Adds explicitBooleanLongFlag (cobra-flag-groups.ts), which reads the last
explicit occurrence's pflag-parsed boolean value instead of mere presence,
and switches this call site to it. SUPABASE_DEBUG env-var fallback remains
a separate, pre-existing gap shared by every other
hasGlobalLongFlag(rawArgs, "debug") site (e.g. deploy.ts) and the legacy
debug logger, left open rather than fixed piecemeal here.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51524c6206
ℹ️ 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".
| image: legacyGetRegistryImageUrl( | ||
| `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, | ||
| ), |
There was a problem hiding this comment.
In the native Docker download path this resolves the edge-runtime image to a single legacyGetRegistryImageUrl value. I checked Go's downloadWithDockerUnbundle -> DockerRunOnceWithConfig -> DockerStart, which calls DockerResolveImageIfNotCached and tries ECR, GHCR, then Docker Hub when no registry override is set; when ECR is unavailable/throttled and the image is not already cached, docker run public.ecr.aws/... fails here instead of falling back to a later candidate, so the newly ported default download path regresses while the previous Go-delegated path would still succeed.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap — left open rather than fixed here; documented in a comment above resolveEdgeRuntimeImage's image: field and in the PR description's "Judgement calls left open" section.
You're right that Go's DockerStart resolves config.Image through DockerResolveImageIfNotCached (internal/utils/docker.go:326-348,363-365), which tries ECR→GHCR→Docker-Hub for every container it starts — including this exact edge-runtime unbundle container — so an ECR outage/throttle the previous Go-delegated default path would have survived can now fail this native path outright.
This isn't specific to download.ts, though: deploy.ts's and serve.ts's own already-shipped native Docker paths resolve the edge-runtime image identically — a single legacyGetRegistryImageUrl value, no retry. legacyGetRegistryImageUrlCandidates's ECR/GHCR/Docker-Hub retry has, per its own doc comment, only ever been wired up for start. Fixing it only for download here would leave the identical gap in two already-shipped commands and add asymmetry between three otherwise-parallel call sites — and since docker run here shells out to the Docker CLI rather than the Engine API DockerResolveImageIfNotCached uses, a real fix needs its own docker image inspect/pull probing step (something like start's image-prepull.ts), which is meaningfully more than a one-line change. Tracking this as a shared, cross-cutting follow-up across all three functions Docker paths rather than patching it piecemeal in this PR.
| image: legacyGetRegistryImageUrl( | ||
| `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, | ||
| ), |
There was a problem hiding this comment.
In the native Docker download path this resolves the edge-runtime image to a single legacyGetRegistryImageUrl value. I checked Go's downloadWithDockerUnbundle -> DockerRunOnceWithConfig -> DockerStart, which calls DockerResolveImageIfNotCached and tries ECR, GHCR, then Docker Hub when no registry override is set; when ECR is unavailable/throttled and the image is not already cached, docker run public.ecr.aws/... fails here instead of falling back to a later candidate, so the newly ported default download path regresses while the previous Go-delegated path would still succeed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap — left open rather than fixed here; documented in a comment above resolveEdgeRuntimeImage's image: field and in the PR description's "Judgement calls left open" section.
You're right that Go's DockerStart resolves config.Image through DockerResolveImageIfNotCached (internal/utils/docker.go:326-348,363-365), which tries ECR→GHCR→Docker-Hub for every container it starts — including this exact edge-runtime unbundle container — so an ECR outage/throttle the previous Go-delegated default path would have survived can now fail this native path outright.
This isn't specific to download.ts, though: deploy.ts's and serve.ts's own already-shipped native Docker paths resolve the edge-runtime image identically — a single legacyGetRegistryImageUrl value, no retry. legacyGetRegistryImageUrlCandidates's ECR/GHCR/Docker-Hub retry has, per its own doc comment, only ever been wired up for start. Fixing it only for download here would leave the identical gap in two already-shipped commands and add asymmetry between three otherwise-parallel call sites — and since docker run here shells out to the Docker CLI rather than the Engine API DockerResolveImageIfNotCached uses, a real fix needs its own docker image inspect/pull probing step (something like start's image-prepull.ts), which is meaningfully more than a one-line change. Tracking this as a shared, cross-cutting follow-up across all three functions Docker paths rather than patching it piecemeal in this PR.
… mode (review: CLI-1963) Go's container.NetworkMode.IsUserDefined() explicitly excludes IsContainer() (docker/api/types/container/hostconfig_unix.go:23-25), so DockerNetworkCreateIfNotExists never inspects or creates a network for --network-id container:<name|id> — the mode attaches to another container's stack and is passed straight through to `docker run --network`. The shared isUserDefinedDockerNetwork predicate (used by deploy.ts, serve.ts, download.ts, and start's container lifecycle) didn't exclude this case, so the Docker download path's preflight would have run `docker network inspect`/`create container:redis` before `docker run`. Fixed once in the shared predicate so every consumer gets the same fix.
…workdir, toml-only (review: CLI-1963) Go's flags.LoadConfig only ever resolves supabase/config.toml from the already-resolved workdir, with no ancestor climb and no concept of a JSON project config (pkg/config/utils.go:43-48). resolveEdgeRuntimeImage's loadProjectConfig call omitted search: false/tomlOnly: true, so the legacy shell's Docker download path could pick up an unrelated ancestor project's config.toml, or prefer a stray supabase/config.json over config.toml — both diverging from Go. Gated on goViperCompat so the next shell keeps the package's existing (non-Go-parity) defaults, matching legacy-local-project-context.ts and start.handler.ts's established pattern for the same options. Also documents (not fixed here) a separate, pre-existing gap the same review round surfaced: resolveEdgeRuntimeImage resolves a single registry URL with no ECR/GHCR/Docker Hub retry, unlike Go's DockerResolveImageIfNotCached — shared with deploy.ts/serve.ts's own already-shipped native Docker paths, so it's a cross-cutting follow-up rather than a download-only fix.
|
Note for whoever picks this up next: this PR now has a merge conflict with |
…3-port-functions-download-to-native-typescript-both-shells # Conflicts: # apps/cli/docs/go-cli-porting-status.md
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
cli/apps/cli/src/shared/functions/download.ts
Lines 1192 to 1194 in 143addc
Validate config before legacy-bundle pre-list
For --legacy-bundle with TS machine output and no function name, this branch lists remote functions before it delegates to the Go child, but Go's download.Run calls flags.LoadConfig before choosing RunLegacy or making the list request. Fresh evidence is this legacy-bundle machine branch still pre-lists here, so an invalid supabase/config.toml can now perform or mask an API list before the config error that the previous Go-delegated invocation reported first.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const loadedConfig = yield* loadProjectConfig(dependencies.projectRoot, { | ||
| projectRef, | ||
| goViperCompat: dependencies.goViperCompat, | ||
| // `search: false`/`tomlOnly: true` only under `goViperCompat` (the legacy caller, whose | ||
| // `dependencies.projectRoot` is `cliConfig.workdir` — already Go's fully-resolved chdir |
There was a problem hiding this comment.
For the legacy Docker download path in a project that has no supabase/config.toml, this package loader returns null before loading Go's default config/environment layer, whereas Go's flags.LoadConfig still loads defaults and nested env files before any API or Docker work. In practice, SUPABASE_EDGE_RUNTIME_DENO_VERSION=1 (from the environment or supabase/.env) is honored by the previous Go-delegated default path but is ignored here, so the new native path can run the v2 edge-runtime image against a project that Go would unbundle with the deno-v1 image.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap — left open rather than fixed here, same as the registry-fallback thread above.
Verified in Go source: flags.LoadConfig → Config.Load → loadFromFile (pkg/config/config.go:579-611) merges the template defaults and enables viper.AutomaticEnv()/SetEnvPrefix("SUPABASE") before attempting the file read, and mergeFileConfig (config.go:701-716) simply returns nil on os.ErrNotExist — so Go still resolves SUPABASE_EDGE_RUNTIME_DENO_VERSION/supabase/.env even with no config.toml on disk. loadProjectConfig here returns null outright in that case, so denoVersion falls through to undefined and always resolves the v2 default.
But this isn't specific to the "config.toml absent" case, and it isn't specific to download.ts: @supabase/config's loadProjectConfig has no equivalent of Go's generic ExperimentalBindStruct+AutomaticEnv struct-field binding at all — it only expands literal env(...) references already written inside the TOML. deploy.ts's already-shipped native Docker path calls the exact same resolveEdgeRuntimeVersion(deployConfig?.edge_runtime.deno_version, ...) and has the identical gap whether or not config.toml exists.
A fix belongs in the shared config-loading layer every native caller goes through (gen types, next start, functions dev/serve/deploy, and now download), not duplicated per call site in this PR. Documented inline above resolveEdgeRuntimeImage's denoVersion read and in the PR description's "Judgement calls left open" section.
| explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id") ?? | ||
| localDockerId("network", projectId); |
There was a problem hiding this comment.
When legacy users set SUPABASE_NETWORK_ID instead of passing --network-id, Go's root viper binding resolves that environment variable and DockerStart overrides the network whenever viper.GetString("network-id") is non-empty. This raw-argv-only lookup ignores that supported env path, so CI or local setups that provide the custom network through SUPABASE_NETWORK_ID now run the edge-runtime unbundle on the generated supabase_network_<project> network instead of the network the previous Go-delegated default path joined.
AGENTS.md reference: apps/cli/AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap — left open rather than fixed here, same as the registry-fallback thread above.
Verified in Go source: root init() enables viper.AutomaticEnv() with SetEnvPrefix("SUPABASE") and a -→_ replacer, then binds every persistent flag — including --network-id — via viper.BindPFlags (cmd/root.go:316-334). With that binding, viper.GetString("network-id") resolves SUPABASE_NETWORK_ID whenever --network-id itself isn't passed on the command line, ahead of the flag's empty default. This raw-argv-only explicitNonEmptyStringFlag lookup has no equivalent env-var fallback.
But this isn't specific to download.ts either: no native command resolves SUPABASE_NETWORK_ID today. start.handler.ts's LegacyNetworkIdFlag (an Effect CLI flag with no env binding) and deploy.ts's/serve.ts's own network-id resolution have the identical gap — including start, the most heavily-audited native port in this codebase.
A fix belongs in one shared place for the global --network-id resolution (mirroring how Go resolves it once, at the root, for every command), not duplicated per Docker-path call site in this PR. Documented inline above the networkMode resolution and in the PR description's "Judgement calls left open" section.
… gaps (review: CLI-1963) Codex flagged that resolveEdgeRuntimeImage falls back to the v2 default when config.toml is absent (ignoring SUPABASE_EDGE_RUNTIME_DENO_VERSION), and that networkMode resolution never checks SUPABASE_NETWORK_ID the way Go's viper AutomaticEnv does for the --network-id persistent flag. Both are confirmed real gaps, but pre-existing and cross-cutting rather than introduced here: deploy.ts has the identical deno_version fallback today (config.toml present or not, since @supabase/config has no generic env-var struct binding at all), and start.handler.ts/deploy.ts/serve.ts's own network-id resolution don't check SUPABASE_NETWORK_ID either. Fixing either belongs in one shared place, not duplicated per Docker-path call site in download.ts alone -- left open, matching this PR's existing precedent for the registry-fallback gap. Documented inline and in the PR description's "Judgement calls left open" section instead of silently resolving the review threads.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4530b1fcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // per call site here — left open (review round on CLI-1963's `functions | ||
| // download` port). | ||
| const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; | ||
| const projectId = loadedConfig?.config?.project_id ?? projectRef; |
There was a problem hiding this comment.
When a legacy project has a Go-invalid config such as project_id = "", loadProjectConfig still returns the empty string and this nullish fallback does not use projectRef, so the new default Docker download path proceeds with supabase_network_/supabase_edge_runtime_ instead of failing flags.LoadConfig with Missing required field in config: project_id before any Docker/API work. This is separate from the earlier load-order fix: even after resolveEdgeRuntimeImage runs, the decoded config is not passed through Go's Config.Validate parity checks, including configured function slugs; route legacy callers through the shared validator before using the config.
AGENTS.md reference: apps/cli/AGENTS.md:L241-L243
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed as a real gap — left open rather than fixed here, same treatment as the registry-fallback/config-defaults/network-id-env threads above.
Verified in Go source: Config.Validate (pkg/config/config.go:990-991) rejects project_id = "" with "Missing required field in config: project_id", run unconditionally inside flags.LoadConfig before Run ever checks useDocker/Docker's running state — before any Docker/API work. This native path's loadedConfig?.config?.project_id ?? projectRef only substitutes on null/undefined, so an explicit empty project_id sails through to supabase_network_/supabase_edge_runtime_ naming instead.
This is pre-existing and cross-cutting, not specific to this PR's new code: deploy.ts's identical deployConfig?.project_id ?? projectRef fallback (deploy.ts:2201) has the exact same gap, and no native functions Docker path (deploy, serve, download) routes its loaded config through Config.Validate parity checks at all today — that port has exactly one home, legacy-config-validate.ts's legacyValidateResolvedConfig, wired up only for the db/migration loader and the status/stop resolver. Wiring Config.Validate into every native config-consuming command belongs in the shared config-loading layer, not duplicated per Docker-path call site.
Documented in a comment above resolveEdgeRuntimeImage's projectId field (7834238) and added to the PR description's "Judgement calls left open" section.
… flag (review: CLI-1963) pflag/viper string flags are shared-variable, last-Set()-wins (confirmed empirically with a scratch pflag.FlagSet.Parse probe: --network-id old --network-id ci-net resolves to ci-net; a trailing --network-id= clears an earlier non-empty value). explicitStringFlag returned on the first argv match instead of scanning for the last, unlike this file's own explicitBooleanLongFlag and the legacy shell's legacyPflagStringValue, which already implement last-wins. Fixed to keep scanning, plus regression tests covering the repeated-override and repeated-then-cleared cases.
…opping them (review: CLI-1963) Go's FunctionResponse.Slug (apps/cli-go/pkg/api/types.gen.go:6465) is a required, non-pointer string: a list entry with a missing or null "slug" decodes to the zero value "" rather than erroring, and that empty slug then fails ValidateFunctionSlug loudly in downloadAll (download.go:182-188) instead of vanishing from the list. listRemoteFunctionSlugs's flatMap filtered such entries out entirely, defeating part of the CLI-1891 validation this PR added for exactly this "compromised/malformed API response" threat model. Preserve the entry (coerced to "") so the existing validateRemoteSlug/validateSlug check catches it, matching Go instead of reporting "No functions found." or a silent partial download.
…ad configs (review: CLI-1963) Go's Config.Validate (pkg/config/config.go:990-991) rejects a config.toml with project_id = "" up front, inside flags.LoadConfig, before any Docker/API work. resolveEdgeRuntimeImage's `?? projectRef` fallback only substitutes on null/undefined, so an explicit empty project_id sails through instead. Pre-existing and cross-cutting, not specific to this PR: deploy.ts's identical deployConfig?.project_id ?? projectRef fallback (deploy.ts:2201) has the same gap, and no native functions Docker path (deploy/serve/download) routes its config through Config.Validate parity checks at all -- that port has one home today (legacy-config-validate.ts's legacyValidateResolvedConfig), wired up only for the db/migration loader and status/stop resolver. Left open, same treatment as the registry-fallback/config-defaults/network-id-env gaps already documented above -- belongs in the shared config-loading layer every native caller goes through, not duplicated per call site.
|
@codex review |
1 participant
What
Ports
supabase functions download's default Docker-unbundle path (--use-docker, defaulttrue) from wholesale Go-binary delegation to native TypeScript, in both thelegacyandnextshells.--use-apiwas already native before this PR; this closes the remaining default-path gap.--legacy-bundle(hidden, deprecated pre-1.120.0 fallback) is deliberately left delegating to the Go binary — see "Scope decision" below.Ground truth:
apps/cli-go/internal/functions/download/download.go(downloadWithDockerUnbundle,downloadOne,extractOne,getErrorLogger). Verified against it via an independent go-parity-auditor pass; see inline comments indownload.tsfor file:line citations.Linear: https://linear.app/supabase/issue/CLI-1963/port-functions-download-to-native-typescript-both-shells
Scope decision:
--legacy-bundlestays delegatedThis hidden flag requires installing/upgrading a real Deno binary on the host (
InstallOrUpgradeDeno: downloads a release zip fromdenoland/denoor a third-party ARM64 fork, extracts, chmods, installs to~/.supabase/deno) and shelling out to an embedded Deno script that itself pullsdeno.landmodules at runtime. This is unique in the Go CLI — no other command, and no already-ported TS command, manages a downloaded third-party binary on the host. Porting it would give the TS CLI a first-of-its-kind capability (unverified binary download + host install + runtime network fetches) purely to support functions deployed by a 3+-year-old CLI release. Full rationale, including the go-parity-auditor's findings on this seam, is recorded as a comment on the Linear issue.docs/go-cli-porting-status.mdreflects the partial (not fully-native) status accordingly.Bugs found and fixed along the way
downloadAllalready guards against. Fixed with the same per-slug validation Go uses, before any per-slug network/filesystem work.nextshell's--use-dockerflag was missingFlag.withDefault(true)— a real default-value divergence fromlegacy(which already had it) and from Go. Note: this changesnext's barefunctions downloadinvocation to attempt Docker unbundling by default (degrading gracefully to the server-side path with a warning if Docker isn't running), matching Go and thelegacyshell — flagging explicitly since it's the one behavior change tonextin this diff.FetchHttpClient, backed by the platformfetch) already transparently auto-decodesContent-Encoding: brresponses while still reporting the header — confirmed empirically with a local brotli-serving test server. Go's manualbrotli.NewReaderstep doesn't need porting; doing so anyway would throw on already-decoded bytes. Removed the manual decode entirely.defer-equivalent: it only ran after a successful Docker run, so a network/volume/spawn failure leftsupabase/.temp/output_<slug>.eszipon disk forever. Wrapped inEffect.ensuringso it runs on every path, matching Go'sdefer fsys.Remove(eszipPath)..suggestion's leading newline was trimmed by the generic CLI error normalizer, losing Go's blank separator line before the--legacy-bundlehint (Fprintln(os.Stderr, CmdSuggestion)). Now read raw instead of trimmed.strings.EqualFold(line, "invalid eszip v2")) — a container log line like "error: invalid eszip v2 header" would have wrongly triggered the deno-v2 upgrade suggestion. Fixed to match Go exactly.suggestLegacyBundlewas only attached on a non-zero container exit — Go attaches it to anyextractOnefailure (network/volume creation, container create/start, log streaming). Widened to cover the same scope.resolveEdgeRuntimeImage'sloadProjectConfigcall omittedsearch: false/tomlOnly: true, so a--workdirnested under an unrelated ancestor project could pick up the ancestor'sconfig.toml, and a straysupabase/config.jsoncould win overconfig.toml— both diverge from Go'sflags.LoadConfig, which only ever readssupabase/config.tomlfrom the exact resolved workdir. Now gated ongoViperCompat(legacy only;nextkeeps package defaults).--network-id container:<name|id>was treated as a user-created network: the sharedisUserDefinedDockerNetworkpredicate (used bydeploy.ts/serve.ts/download.ts/start's container lifecycle) didn't exclude Docker'scontainer:network mode, so this preflight randocker network inspect/create container:redisbeforedocker runinstead of passing the mode straight through — Go'sNetworkMode.IsUserDefined()explicitly excludesIsContainer(). Fixed in the shared predicate, sodeploy/serve/startget the same fix.--network-idflag honored the first occurrence, not the last:explicitStringFlag(shared/cli/cobra-flag-groups.ts) returned as soon as it found a match, but pflag/viper string flags are shared-variable, last-Set()-wins — confirmed empirically with a scratchpflag.FlagSet.Parseprobe on--network-id old --network-id ci-net. A caller overriding a default network (or explicitly clearing one with a trailing--network-id=) would have kept the earlier value. Fixed to scan the whole argv and keep the last match, matching the last-wins pattern this file's ownexplicitBooleanLongFlagalready used.listRemoteFunctionSlugsfiltered out any list entry with a missing/non-stringslugrather than surfacing it — defeating part of the CLI-1891 validation above for exactly the "compromised/malformed API response" case that validation exists for. Go'sFunctionResponse.Slugis a required, non-pointer field, so a missing/nullslug decodes to""and failsValidateFunctionSlugloudly rather than vanishing from the list. Fixed to preserve the entry (coerced to"") so the existing per-slug validation catches it, matching Go instead of reporting "No functions found" or a silent partial download.Refactoring
download.tsneeds (runChildProcess,isDockerRunning,ensureDockerNetwork,ensureDockerNamedVolume,localDockerId,resolveEdgeRuntimeVersion, etc.) out ofdeploy.tsinto a newshared/functions/functions-docker.ts, per this workspace's "Hoist Before You Duplicate" policy —deploy.ts,serve.ts, andlegacy/commands/start/lib/container-lifecycle.ts(existing consumers of the moved symbols) now import from the new module.edge-runtime-versionpin-file lookup, previously copy-pasted verbatim across all fourdeploy/downloadhandler files (both shells), into oneresolveEdgeRuntimeVersionPinhelper infunctions.shared.ts.Judgement calls left open (not blocking, noted for visibility)
deploy.ts'sbundleFunctionWithDockeranddownload.ts's newdownloadWithDockerUnbundle. A shared "run an edge-runtime container once" helper is a reasonable follow-up, but extracting it now means touchingdeploy.ts's working, tested Docker-bundling path for marginal DRY benefit this late in an already-large diff — left as-is.serve.tshas its own, separately-implemented edge-runtime-version-pin lookup (different default constant, different "v"-prefix handling) that wasn't folded into the new shared helper — pre-existing, not introduced by this PR, and unifying it means touching another already-tested file outside this issue's scope.supabase/edge-runtime:v${pin}double-prefixes "v" when the pin file already contains a "v"-prefixed tag) exists in bothdeploy.tsand nowdownload.ts(inherited, not introduced) —legacy/shared/legacy-edge-runtime-image.tsalready has the correct fix for a different call site; worth a follow-up to route both through it.apps/cli/docs/go-cli-porting-status.mdhas a separate, older "Functions" section (around line 147) that's stale in a different way (predates this PR, marks the wholefunctionsfamily asmissing) — out of scope for this PR, noted for a future doc-cleanup pass.resolveEdgeRuntimeImageresolves a singlelegacyGetRegistryImageUrlvalue, so an ECR outage/throttle now fails this native Docker path where the previous Go-delegated default path (viaDockerResolveImageIfNotCached) would have retried GHCR/Docker Hub. Pre-existing and shared withdeploy.ts/serve.ts's own already-shipped native Docker paths —legacyGetRegistryImageUrlCandidates's retry has only ever been wired up forstart. Extending it to all threefunctionsDocker paths is a cross-cutting follow-up, not something to fix piecemeal fordownloadalone (review round on this PR).supabase/config.tomlis absent:loadProjectConfigreturnsnullfor a project with no config file, soresolveEdgeRuntimeImage'sdenoVersionfalls through toundefinedand always resolves the v2 default, whereas Go'sflags.LoadConfigstill merges template defaults and appliesviper.AutomaticEnv()(SUPABASE_EDGE_RUNTIME_DENO_VERSION,supabase/.env) even with no file on disk. Pre-existing and cross-cutting:@supabase/confighas no equivalent of Go's generic env-var struct binding at all, config.toml present or not, sodeploy.ts's identicalresolveEdgeRuntimeVersion(deployConfig?.edge_runtime.deno_version, ...)call already has this gap. A fix belongs in the shared config-loading layer every native caller goes through, not duplicated per call site — left open (review round on this PR).SUPABASE_NETWORK_IDisn't honored for Docker network selection: Go's rootviper.AutomaticEnv()+BindPFlags(cmd/root.go:316-334) letsSUPABASE_NETWORK_IDoverride--network-idwhen the flag itself is never passed; the raw-argvexplicitNonEmptyStringFlaglookup here has no env-var fallback. Pre-existing and cross-cutting:start.handler.ts'sLegacyNetworkIdFlaganddeploy.ts's/serve.ts's own network-id resolution don't check it either — no native command does today. Belongs in one shared place for the global--network-idresolution, not duplicated per Docker-path call site — left open (review round on this PR).Config.Validateparity wiring for nativefunctionsDocker paths:resolveEdgeRuntimeImage'sloadedConfig?.config?.project_id ?? projectRefonly substitutes onnull/undefined, so a config.toml with an explicitproject_id = ""resolves to the empty string here instead of failing up front the way Go'sConfig.Validatedoes ("Missing required field in config: project_id",pkg/config/config.go:990-991, run unconditionally insideflags.LoadConfigbefore any Docker/API work). Pre-existing and cross-cutting, not introduced by this PR:deploy.ts's identicaldeployConfig?.project_id ?? projectReffallback (deploy.ts:2201) has the same gap, and no nativefunctionsDocker path (deploy,serve,download) routes its loaded config throughConfig.Validateparity checks at all — that port has exactly one home today (legacy-config-validate.ts'slegacyValidateResolvedConfig), wired up only for the db/migration loader and the status/stop resolver. Belongs in the shared config-loading layer every native caller goes through, not duplicated per Docker-path call site — left open (review round on this PR).Verification
bun run test(full unit + integration suite) — all passing.bun run --parallel "*:check"(types, lint, fmt, knip) — all passing.apps/cli-go/internal/functions/downloadreferences — none found; all remaining comments cite Go source that still exists and is still exercised by the--legacy-bundlepath.