diff --git a/apps/cli/docs/go-cli-porting-status.md b/apps/cli/docs/go-cli-porting-status.md index 1252d0b50d..017693f2de 100644 --- a/apps/cli/docs/go-cli-porting-status.md +++ b/apps/cli/docs/go-cli-porting-status.md @@ -158,7 +158,7 @@ leaf-flag parity unaudited," not as a confirmed parity gap. | -------------------- | --------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `functions delete` | `partial` | [`../src/next/commands/functions/delete/`](../src/next/commands/functions/delete/delete.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Natively ported in the legacy shell. | | `functions deploy` | `partial` | [`../src/next/commands/functions/deploy/`](../src/next/commands/functions/deploy/deploy.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Natively ported in the legacy shell. | -| `functions download` | `partial` | [`../src/next/commands/functions/download/`](../src/next/commands/functions/download/download.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Hybrid in the legacy shell: native for `--use-api`, delegates wholesale to Go for the default (`--use-docker`) and `--legacy-bundle` paths — see [Legacy Shell Command Status](#legacy-shell-command-status) below. | +| `functions download` | `partial` | [`../src/next/commands/functions/download/`](../src/next/commands/functions/download/download.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Native for `--use-api` and the default Docker-unbundle path (`--use-docker`, CLI-1963) in both shells; hidden `--legacy-bundle` still delegates to Go — see [Legacy Shell Command Status](#legacy-shell-command-status) below. | | `functions list` | `partial` | [`../src/next/commands/functions/list/`](../src/next/commands/functions/list/list.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Natively ported in the legacy shell. | | `functions new` | `partial` | [`../src/next/commands/functions/new/`](../src/next/commands/functions/new/new.command.ts) | Command surface exists in `next/`; flag-parity against Go not yet audited (see section note above). Natively ported in the legacy shell. | | `functions serve` | `partial` | [`../src/next/commands/functions/dev/`](../src/next/commands/functions/dev/dev.command.ts) | `next/`'s `functions dev` is a TS-native local Functions workflow (`--stack`, `--env-file`, `--no-verify-jwt`) rather than a flag-parity port of Go's `serve` — kept `partial` here pending a decision on whether it counts as this row's counterpart or belongs in [TS-only Commands](#ts-only-commands) instead. Natively ported in the legacy shell. | @@ -308,7 +308,7 @@ Legend: | `gen keys` | `wrapped` | [`../src/legacy/commands/gen/keys/keys.command.ts`](../src/legacy/commands/gen/keys/keys.command.ts) | | `functions list` | `ported` | [`../src/legacy/commands/functions/list/list.command.ts`](../src/legacy/commands/functions/list/list.command.ts) | | `functions delete` | `ported` | [`../src/legacy/commands/functions/delete/delete.command.ts`](../src/legacy/commands/functions/delete/delete.command.ts) | -| `functions download` | `ported` | [`../src/legacy/commands/functions/download/download.command.ts`](../src/legacy/commands/functions/download/download.command.ts) — native for `--use-api` (lists, downloads, and extracts via the Management API directly); default (`--use-docker`) and `--legacy-bundle` delegate wholesale to Go | +| `functions download` | `ported` | [`../src/legacy/commands/functions/download/download.command.ts`](../src/legacy/commands/functions/download/download.command.ts) — native for `--use-api` (lists, downloads, and extracts via the Management API directly) and the default Docker-unbundle path (`--use-docker`, CLI-1963); hidden `--legacy-bundle` still delegates to the Go binary (pre-1.120.0 fallback requiring a host Deno-binary install with no precedent elsewhere in this codebase — tracked separately, see CLI-1963) | | `functions deploy` | `ported` | [`../src/legacy/commands/functions/deploy/deploy.command.ts`](../src/legacy/commands/functions/deploy/deploy.command.ts) | | `functions new` | `ported` | [`../src/legacy/commands/functions/new/new.command.ts`](../src/legacy/commands/functions/new/new.command.ts) | | `functions serve` | `ported` | [`../src/legacy/commands/functions/serve/serve.command.ts`](../src/legacy/commands/functions/serve/serve.command.ts) | diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts index 00bb20b7ba..59bec4fd78 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.handler.ts @@ -1,8 +1,7 @@ -import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; -import { readFile } from "node:fs/promises"; import { join } from "node:path"; import { Effect, Option, Stdio } from "effect"; import { deployFunctions } from "../../../../shared/functions/deploy.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { legacyAqua, legacyBold } from "../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; @@ -29,12 +28,8 @@ export const legacyFunctionsDeploy = Effect.fn("legacy.functions.deploy")(functi const runtimeInfo = yield* RuntimeInfo; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; - const edgeRuntimeVersion = yield* Effect.tryPromise(() => - readFile(join(cliConfig.workdir, "supabase", ".temp", "edge-runtime-version"), "utf8"), - ).pipe( - Effect.map((version) => version.trim()), - Effect.catch(() => Effect.succeed("")), - Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin( + join(cliConfig.workdir, "supabase"), ); let resolvedProjectRef = Option.none(); diff --git a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md index 9c9c6a1845..11f217cb53 100644 --- a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md @@ -2,66 +2,77 @@ ## Files Read -| Path | Format | When | -| ----------------------------------------------- | ---------- | ------------------------------------------------------------- | -| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | -| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | -| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | -| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | -| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | +| Path | Format | When | +| ----------------------------------------------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/access-token` | plain text | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| `/profile` | plain text | when `--profile` and `SUPABASE_PROFILE` are both unset | +| `.yaml` | YAML | when `SUPABASE_PROFILE` or `--profile` points to a file | +| `/supabase/.temp/project-ref` | plain text | when `--project-ref` and `SUPABASE_PROJECT_ID` are both unset | +| `/supabase/.temp/edge-runtime-version` | plain text | Read unconditionally by `resolveEdgeRuntimeVersionPin()` in the handler, before the shared downloader chooses `--use-api` vs Docker — only affects the resolved edge-runtime image tag on the Docker-unbundle path | +| `/supabase/config.toml` | TOML | Read unconditionally after resolving the project ref, before checking `--use-api`/`--use-docker` or whether Docker is running — resolves `edge_runtime.deno_version` and `project_id` (`loadProjectConfig`) for the Docker-unbundle path. `goViperCompat`'s `tomlOnly: true` means `config.json` is never read here, unlike other `loadProjectConfig` callers. Matches Go's `flags.LoadConfig` running unconditionally at the top of `Run` (`download.go:131-138`): a malformed config now fails here even on the `--use-api` invocation. | +| `/telemetry.json` | JSON | when present, before post-run telemetry state is refreshed | ## Files Written -| Path | Format | When | -| --------------------------------------------------- | ------ | ----------------------------------------------------------------------- | -| `/supabase/functions//` | bytes | for each source file returned by the API | -| `/supabase/.temp/linked-project.json` | JSON | after resolving a project ref, cached on both success and failure paths | -| `/telemetry.json` | JSON | after command completion, flushed on both success and failure paths | +| Path | Format | When | +| --------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/functions//` | bytes | for each source file returned by the API (`--use-api`, or the Docker-unbundle fallback when Docker isn't running) | +| `/supabase/.temp/output_.eszip` | bytes | Docker-unbundle path (default): downloaded eszip, extracted into `supabase/functions//...` by the edge-runtime container; removed after the attempt unless `--debug` is set | +| `/supabase/.temp/linked-project.json` | JSON | after resolving a project ref, cached on both success and failure paths | +| `/telemetry.json` | JSON | after command completion, flushed on both success and failure paths | ## API Routes -| Method | Path | Auth | Request body | Response (used fields) | -| ------ | ------------------------------------------ | ------------ | ------------ | ----------------------------------------------------- | -| `GET` | `/v1/projects/{ref}/functions` | Bearer token | none | function slugs, when downloading all | -| `GET` | `/v1/projects/{ref}/functions/{slug}` | Bearer token | none | entrypoint path, when absent from metadata | -| `GET` | `/v1/projects/{ref}/functions/{slug}/body` | Bearer token | none | multipart function source | -| `GET` | `/v1/projects` | Bearer token | none | project picker options when no ref is supplied in TTY | -| `GET` | `/v1/projects/{ref}` | Bearer token | none | linked project metadata used by the post-run cache | +| Method | Path | Auth | Request body | Response (used fields) | +| ------ | ------------------------------------------ | ------------ | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `GET` | `/v1/projects/{ref}/functions` | Bearer token | none | function slugs, when downloading all | +| `GET` | `/v1/projects/{ref}/functions/{slug}` | Bearer token | none | entrypoint path, when absent from multipart metadata (`--use-api` path only) | +| `GET` | `/v1/projects/{ref}/functions/{slug}/body` | Bearer token | none | `--use-api`: multipart function source (`Accept: multipart/form-data`). Docker-unbundle: raw eszip bytes; a `Content-Encoding: br` response is decoded transparently by the HTTP transport, not by this command | +| `GET` | `/v1/projects` | Bearer token | none | project picker options when no ref is supplied in TTY | +| `GET` | `/v1/projects/{ref}` | Bearer token | none | linked project metadata used by the post-run cache | ## Subprocesses -| Command | When | Purpose | -| ------------------------------------ | ----------------------------------------------------------------- | ----------------------------------- | -| `supabase-go functions download ...` | `--use-docker` (default) or `--legacy-bundle`, unless `--use-api` | preserve hidden compatibility modes | - -The delegated call runs with `SUPABASE_TELEMETRY_DISABLED=1` so the Go child's -own `cli_command_executed` doesn't double-count on top of this command's own -telemetry (mirrors `db pull`/`db diff`'s delegated-call pattern). In -`--output-format json|stream-json`, the child's stdout is captured and -discarded instead of inherited (`LegacyGoProxy.execCapture`) — the raw text -never reaches the terminal, and this command emits the `Output` envelope -itself once the child exits successfully. +| Command | When | Purpose | +| ---------------------------------------------------------------------------- | -------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| `docker info` | `--use-docker` (default), unless `--use-api` | check whether Docker is running before choosing the Docker-unbundle downloader | +| `docker network inspect` / `network create` / `volume create` | Docker-unbundle path, when Docker is running | ensure the shared per-project network/named volume exist (same primitives as `functions deploy`'s Docker bundler) | +| `docker run --rm ... unbundle --eszip ... --output ...` | Docker-unbundle path, when Docker is running | extract the downloaded eszip into `supabase/functions//...` | +| `supabase-go functions download ... --legacy-bundle` | `--legacy-bundle` only | preserve the hidden, deprecated pre-1.120.0 bundling fallback (native TS port tracked separately, CLI-1963) | + +The `--legacy-bundle` delegated call runs with `SUPABASE_TELEMETRY_DISABLED=1` +so the Go child's own `cli_command_executed` doesn't double-count on top of +this command's own telemetry (mirrors `db pull`/`db diff`'s delegated-call +pattern). In `--output-format json|stream-json`, the child's stdout is +captured and discarded instead of inherited (`LegacyGoProxy.execCapture`) — +the raw text never reaches the terminal, and this command emits the `Output` +envelope itself once the child exits successfully. The Docker-unbundle path's +own container stdout is routed the same way: to the real stdout in text mode, +to stderr in machine-output modes (CLI-1546). ## Environment Variables -| Variable | Purpose | Required? | -| ----------------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------- | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | -| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | -| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | -| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | -| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | -| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | +| Variable | Purpose | Required? | +| ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `/access-token`) | +| `SUPABASE_HOME` | overrides where `telemetry.json` and `profile` are read/written | no (defaults to `~/.supabase`) | +| `SUPABASE_NO_KEYRING` | disables the OS keyring, forcing the access-token file fallback | no | +| `SUPABASE_PROFILE` | select a built-in profile or YAML profile file with `api_url:` | no (falls back to `~/.supabase/profile` -> `supabase`) | +| `SUPABASE_PROJECT_ID` | provides the project ref when `--project-ref` is unset | no (falls back to `/supabase/.temp/project-ref`) | +| `SUPABASE_WORKDIR` | sets `` for local Supabase temp files | no (falls back to `--workdir` -> nearest ancestor with `supabase/config.toml` -> cwd) | +| `BITBUCKET_CLONE_DIR` | Docker-unbundle path: when set, skips creating the named Deno-cache volume and omits its bind mount from the `docker run` command (Bitbucket's restricted Docker environment rejects both) | no | +| `SUPABASE_INTERNAL_IMAGE_REGISTRY` | selects the registry the edge-runtime unbundle image is pulled from (`legacyGetRegistryImageUrl`); read unconditionally while resolving the image, before the `--use-api`/Docker choice is finalized — also consumed on the `--use-api` invocation even though it never pulls an image | no (defaults to `public.ecr.aws`) | ## Exit Codes -| Code | Condition | -| ---- | -------------------------------------- | -| `0` | success | -| `1` | API error (non-2xx response) | -| `1` | authentication error (no token found) | -| `1` | network / connection failure | -| `1` | invalid function slug or flag conflict | +| Code | Condition | +| ---- | ---------------------------------------------------------------------- | +| `0` | success | +| `1` | API error (non-2xx response) | +| `1` | authentication error (no token found) | +| `1` | network / connection failure | +| `1` | invalid function slug or flag conflict | +| `1` | Docker-unbundle container exited non-zero (suggests `--legacy-bundle`) | ## Telemetry Events Fired @@ -73,25 +84,50 @@ itself once the child exits successfully. ### `--output-format text` (Go CLI compatible) -Prints progress and success messages as functions are downloaded. +Prints progress and success messages as functions are downloaded. The Docker-unbundle path prints +`Downloading function: ` (lowercase "function", unlike the `--use-api` path's "Downloading +Function:") and does **not** print a final "Downloaded Function ... from project ..." line — that +line only appears on the `--use-api` and `--legacy-bundle` paths (Go parity, `download.go`). ### `--output-format json` Prints a structured success result with the downloaded function slugs and project ref. On the -Docker/legacy-bundle proxy path, the Go child's stdout is captured/discarded (never inherited) so -it can't corrupt the envelope; the slug list is resolved independently for the payload. +`--legacy-bundle` proxy path, the Go child's stdout is captured/discarded (never inherited) so it +can't corrupt the envelope; the slug list is resolved independently for the payload. On the +Docker-unbundle path, the `unbundle` container's own stdout is routed to stderr instead of stdout +for the same reason. ### `--output-format stream-json` -Same envelope as `json` above (including on the proxy path). +Same envelope as `json` above (including on the proxy and Docker-unbundle paths). ## Notes - If no function name is provided, downloads all functions. - Requires a linked project (`--project-ref` or linked project config). -- Native downloads reject path traversal and symlink escapes before writing source files. -- `--use-docker` and `--legacy-bundle` are hidden flags forwarded to the Go binary for backward compatibility; they are mutually exclusive with `--use-api`. -- `--use-docker` defaults to `true` (Go parity), so a bare `supabase functions download` proxies to the Go binary's Docker-based unbundler unless `--use-api` resolves to `true`, which forces the native server-side download path instead (`apps/cli-go/cmd/functions.go:51-53`: `if useApi { useDocker = false }` reads the resolved flag value, not presence — `--use-api=false` still proxies). -- If Docker is not running, the Go binary itself prints `WARNING: Docker is not running` to stderr and falls back to its own server-side unbundler — the command still exits `0` without Docker installed or running. -- The mutual-exclusivity check only counts flags the user explicitly passed on the command line, not `--use-docker`'s default value — so `--use-api` alone never trips the "mutually exclusive" error. The Go proxy call itself also only ever forwards one of `--use-docker`/`--legacy-bundle`, never both, even though `--use-docker` defaults to `true`. -- Refreshes the linked-project telemetry cache and flushes telemetry state after resolving a project ref. +- The `--use-api` path rejects path traversal and symlink escapes before writing source files + (`resolveDownloadDestination`/`ensureContainedPath`) — the Docker-unbundle path has no equivalent + check of its own; it delegates the actual file writes to the `unbundle` subcommand running inside + the edge-runtime container, through the `supabase/functions` bind mount, matching Go's own + `extractOne` (which has no path-containment check either — this is a pre-existing, not + CLI-1963-introduced, gap shared with the Go CLI). Slugs sourced from the Management API's function + list (downloading-all) are validated against the same pattern as user-supplied slugs, on both + paths, before any per-slug download runs (CLI-1891 parity). +- `--legacy-bundle` is a hidden flag forwarded to the Go binary for backward compatibility — it + requires installing a real Deno binary on the host (`InstallOrUpgradeDeno`) and is a pre-1.120.0 + compatibility fallback; native TS port tracked separately (CLI-1963). `--use-docker` is a hidden + flag but now runs natively. +- `--use-docker`, `--use-api`, and `--legacy-bundle` are mutually exclusive. +- `--use-docker` defaults to `true` (Go parity), so a bare `supabase functions download` runs the + native Docker-unbundle downloader unless `--use-api` resolves to `true`, which forces the native + server-side download path instead (`apps/cli-go/cmd/functions.go:51-53`: `if useApi { useDocker = +false }` reads the resolved flag value, not presence — `--use-api=false` still runs Docker-unbundle). +- If Docker is not running, this command itself prints `WARNING: Docker is not running` to stderr + and falls back to the native server-side unbundler — the command still exits `0` without Docker + installed or running. +- The mutual-exclusivity check only counts flags the user explicitly passed on the command line, + not `--use-docker`'s default value — so `--use-api` alone never trips the "mutually exclusive" + error. The `--legacy-bundle` Go proxy call itself only ever forwards `--legacy-bundle`, never + `--use-docker` alongside it, even though `--use-docker` defaults to `true`. +- Refreshes the linked-project telemetry cache and flushes telemetry state after resolving a + project ref. diff --git a/apps/cli/src/legacy/commands/functions/download/download.handler.ts b/apps/cli/src/legacy/commands/functions/download/download.handler.ts index 4b666da7f6..f53170995b 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.handler.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.handler.ts @@ -1,9 +1,12 @@ +import { join } from "node:path"; import { Effect, Option, Stdio } from "effect"; import { downloadFunctions, - makeGoProxyDownloadArgs, + makeGoProxyLegacyBundleArgs, } from "../../../../shared/functions/download.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; +import { legacyAqua, legacyBold } from "../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../auth/legacy-platform-api.service.ts"; import { LegacyCliConfig } from "../../../config/legacy-cli-config.service.ts"; import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; @@ -22,12 +25,24 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu const proxy = yield* LegacyGoProxy; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin( + join(cliConfig.workdir, "supabase"), + ); let resolvedProjectRef = Option.none(); yield* downloadFunctions(flags, { api, projectRoot: cliConfig.workdir, rawArgs, + goViperCompat: true, + edgeRuntimeVersion, + // Go: `utils.Bold` on the `Downloading function:` slug (`downloadOne`, + // `download.go:219`, stderr) — matches `legacyBold`'s default TTY gate. + styleEmphasis: (text) => legacyBold(text), + // Go: `utils.Aqua` on the suggested `--legacy-bundle` command + // (`suggestLegacyBundle`, `download.go:315`, stderr) — matches + // `legacyAqua`'s default TTY gate. + styleAqua: (text) => legacyAqua(text), resolveProjectRef: (projectRef) => resolver.resolve(projectRef).pipe( Effect.tap((ref) => @@ -47,7 +62,7 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu // pattern for the CLI-1546 "stdout is payload-only in machine mode" // invariant — `downloadFunctions` emits the `Output` envelope itself. proxyDownload: (proxyFlags, projectRef, captureOutput) => { - const args = makeGoProxyDownloadArgs(proxyFlags, projectRef); + const args = makeGoProxyLegacyBundleArgs(proxyFlags.functionName, projectRef); const env = { SUPABASE_TELEMETRY_DISABLED: "1" }; return captureOutput ? Effect.asVoid(proxy.execCapture(args, { env, stdin: "ignore" })) diff --git a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts index af37969828..a5c10c6f7a 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.integration.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from "@effect/vitest"; -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; -import { Effect, Exit, Layer, Option, Stdio } from "effect"; +import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; +import { existsSync } from "node:fs"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { Deferred, Effect, Exit, Layer, Option, PlatformError, Sink, Stdio, Stream } from "effect"; +import { ChildProcessSpawner } from "effect/unstable/process"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; import { commandRuntimeLayer } from "../../../../shared/runtime/command-runtime.layer.ts"; @@ -17,12 +20,107 @@ import { useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; import { mockOutput } from "../../../../../tests/helpers/mocks.ts"; +import { mockChildProcessSpawner } from "../../../../../../../packages/process-compose/tests/helpers/mocks.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; +import { legacyContainerRuntimeNotFoundMessage } from "../../../shared/legacy-container-cli.ts"; import { ConflictingFunctionDownloadFlagsError } from "../../../../shared/functions/download.errors.ts"; import { legacyFunctionsDownloadHandler } from "./download.command.ts"; import type { LegacyFunctionsDownloadFlags } from "./download.command.ts"; import { legacyFunctionsDownload } from "./download.handler.ts"; +const PROJECT_ID = "abcdefghijklmnopqrst"; + +/** + * Mutates the shared spawner options object from inside `onSpawn`, scoped to + * the `docker run ... unbundle` invocation specifically — every earlier + * Docker call (`info`, `network inspect`, `volume create`) in the same test + * already resolved by the time this fires, since `download.ts` awaits each + * child process sequentially, so this only ever affects the unbundle step's + * own exit code/stdio. + */ +function mockDockerUnbundle( + opts: { + readonly runExitCode?: number; + readonly runStdout?: ReadonlyArray; + readonly runStderr?: ReadonlyArray; + } = {}, +) { + const spawnerOpts: { + exitCode?: number; + stdout?: string[]; + stderr?: string[]; + onSpawn?: (record: { command: string; args: ReadonlyArray }) => void; + } = { exitCode: 0 }; + spawnerOpts.onSpawn = (record) => { + if (record.command === "docker" && record.args[0] === "run") { + spawnerOpts.exitCode = opts.runExitCode ?? 0; + spawnerOpts.stdout = opts.runStdout === undefined ? [] : [...opts.runStdout]; + spawnerOpts.stderr = opts.runStderr === undefined ? [] : [...opts.runStderr]; + } + }; + return mockChildProcessSpawner(spawnerOpts); +} + +/** + * A real ENOENT-style spawn failure for the `docker run ... unbundle` step + * specifically — distinct from `mockDockerUnbundle`'s non-zero exit code, + * which models the container starting but the `unbundle` binary itself + * failing. This models `child_process.spawn` (or the container runtime + * binary) never starting at all, which `runChildProcess` surfaces as an + * `unknown` cause rather than an `{ exitCode, stdout, stderr }` result. + * Mirrors `legacy-container-cli.unit.test.ts`'s `mockSpawner({ bothMissing: + * true })`: failing both the `docker` and `podman` fallback attempts for the + * `run` step is what makes `spawnContainerCli` surface + * `legacyContainerRuntimeNotFoundMessage` instead of retrying indefinitely. + * Every other Docker call (`info`, `network inspect`, `volume create`) + * succeeds with exit code 0, so only the unbundle step itself fails. + */ +function mockDockerRunSpawnFailure() { + const spawned: Array<{ command: string; args: ReadonlyArray }> = []; + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + const cmd = command._tag === "StandardCommand" ? command.command : ""; + const args = command._tag === "StandardCommand" ? command.args : []; + spawned.push({ command: cmd, args }); + + if (args[0] === "run") { + return yield* Effect.fail( + PlatformError.systemError({ + _tag: "NotFound", + module: "ChildProcess", + method: "spawn", + description: `${cmd} not found`, + }), + ); + } + + const exitDeferred = yield* Deferred.make(); + yield* Deferred.succeed(exitDeferred, ChildProcessSpawner.ExitCode(0)); + + return ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(1000 + spawned.length), + stdout: Stream.empty, + stderr: Stream.empty, + all: Stream.empty, + exitCode: Deferred.await(exitDeferred), + isRunning: Effect.succeed(false), + stdin: Sink.drain, + kill: () => Effect.void, + unref: Effect.succeed(Effect.void), + getInputFd: () => Sink.drain, + getOutputFd: () => Stream.empty, + }); + }), + ); + + return { + get spawned() { + return spawned; + }, + layer: Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner), + }; +} + const tempRoot = useLegacyTempWorkdir("supabase-functions-download-legacy-"); // `withLegacyCommandInstrumentation` threads `flags`/`command`/etc. through @@ -157,49 +255,70 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); - it.live("proxies to Docker by default (Go parity), with no flags passed", () => { - const out = mockOutput({ format: "text" }); - const api = mockLegacyPlatformApi(); - const proxy = mockProxy(); - const layer = Layer.mergeAll( - buildLegacyTestRuntime({ - out, - api, - cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), - }), - proxy.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - ]), - }), - ); + it.live( + "runs the native Docker unbundle path by default (Go parity), with no flags passed", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + // Non-empty stdout/stderr on the `docker run` step exercises both the + // text-mode stdout routing branch and the always-to-stderr container + // stderr branch in `downloadWithDockerUnbundle`. + const child = mockDockerUnbundle({ + runStdout: ["unbundle: wrote index.ts"], + runStderr: ["unbundle: warning about deno.json"], + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + PROJECT_ID, + ]), + }), + ); - return Effect.gen(function* () { - // `useDocker: true` mirrors what the CLI parser now resolves to by - // default (CLI-1862) — no `--use-docker` flag appears in argv above. - yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + return Effect.gen(function* () { + // `useDocker: true` mirrors what the CLI parser now resolves to by + // default (CLI-1862) — no `--use-docker` flag appears in argv above. + // CLI-1963: this now runs the native Docker-unbundle path instead of + // delegating to the Go proxy. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); - expect(api.requests).toEqual([]); - expect(proxy.calls).toEqual([ - [ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--use-docker", - ], - ]); - // The delegated Go binary must not also fire its own - // `cli_command_executed` on top of this command's own instrumentation. - expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - }).pipe(Effect.provide(layer)); - }); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect(api.requests.some((request) => request.url.endsWith("/hello-world/body"))).toBe( + true, + ); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + expect(out.stderrText).toContain("Downloading function: hello-world\n"); + expect(out.stdoutText).toContain("unbundle: wrote index.ts\n"); + expect(out.stderrText).toContain("unbundle: warning about deno.json\n"); + // Go parity finding (CLI-1963 audit): unlike the server-side and + // `--legacy-bundle` paths, `downloadWithDockerUnbundle` never prints + // a "Downloaded Function ... from project ..." success line — + // guarded here against a future accidental regression. + expect(out.stderrText).not.toContain("Downloaded Function"); + // No `--debug` — the temp eszip file is removed after the run. + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }, + ); it.live( "does not treat the --use-docker default as conflicting with an explicit --use-api", @@ -251,119 +370,13 @@ describe("legacy functions download", () => { }, ); - it.live("still proxies to Docker when --use-api=false is passed explicitly", () => { - const out = mockOutput({ format: "text" }); - const api = mockLegacyPlatformApi(); - const proxy = mockProxy(); - const layer = Layer.mergeAll( - buildLegacyTestRuntime({ - out, - api, - cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), - }), - proxy.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "hello-world", - "--use-api=false", - "--project-ref", - "abcdefghijklmnopqrst", - ]), - }), - ); - - return Effect.gen(function* () { - // Go's override is value-based (`if useApi { useDocker = false }`, - // apps/cli-go/cmd/functions.go:51-53), not presence-based. An explicit - // `--use-api=false` must not be treated like `--use-api` — it should - // leave the `--use-docker` default (true) in effect and still proxy. - yield* legacyFunctionsDownload({ ...baseFlags, useApi: false, useDocker: true }); - - expect(api.requests).toEqual([]); - expect(proxy.calls).toEqual([ - [ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--use-docker", - ], - ]); - expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - }).pipe(Effect.provide(layer)); - }); - - it.live("emits a JSON success envelope when proxying to Docker in machine-output mode", () => { - const out = mockOutput({ format: "json" }); - const api = mockLegacyPlatformApi(); - const proxy = mockProxy(); - const layer = Layer.mergeAll( - buildLegacyTestRuntime({ - out, - api, - cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), - }), - proxy.layer, - Stdio.layerTest({ - args: Effect.succeed([ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--output-format", - "json", - ]), - }), - ); - - return Effect.gen(function* () { - // CLI-1546: stdout is payload-only in machine mode, so the Go child's - // raw output must be captured/discarded (not inherited) and this - // command must emit the `Output` envelope itself, matching the native - // path's shape. - yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); - - expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - [ - "functions", - "download", - "hello-world", - "--project-ref", - "abcdefghijklmnopqrst", - "--use-docker", - ], - ]); - expect(proxy.captureEnvs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - data: { function_slugs: ["hello-world"], project_ref: "abcdefghijklmnopqrst" }, - }), - ); - }).pipe(Effect.provide(layer)); - }); - it.live( - "lists remote functions before delegating when no function name is given in machine mode", + "still runs the native Docker unbundle path when --use-api=false is passed explicitly", () => { - const out = mockOutput({ format: "json" }); - const api = mockLegacyPlatformApi({ - handler: (request) => - request.url.endsWith("/functions") - ? Effect.succeed( - legacyJsonResponse(request, 200, [ - { slug: "hello-world" }, - { slug: "goodbye-world" }, - ]), - ) - : Effect.succeed(legacyJsonResponse(request, 200, {})), - }); + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -371,53 +384,48 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", + "hello-world", + "--use-api=false", "--project-ref", - "abcdefghijklmnopqrst", - "--output-format", - "json", + PROJECT_ID, ]), }), ); return Effect.gen(function* () { - yield* legacyFunctionsDownload({ - ...baseFlags, - functionName: Option.none(), - useDocker: true, - }); + // Go's override is value-based (`if useApi { useDocker = false }`, + // apps/cli-go/cmd/functions.go:51-53), not presence-based. An + // explicit `--use-api=false` must not be treated like `--use-api` — + // it should leave the `--use-docker` default (true) in effect and + // still run the native Docker path (CLI-1963). + yield* legacyFunctionsDownload({ ...baseFlags, useApi: false, useDocker: true }); expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "--project-ref", "abcdefghijklmnopqrst", "--use-docker"], - ]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - data: { - function_slugs: ["hello-world", "goodbye-world"], - project_ref: "abcdefghijklmnopqrst", - }, - }), - ); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); }).pipe(Effect.provide(layer)); }, ); it.live( - "reports no functions found without delegating when the project is empty in machine mode", + "emits a JSON success envelope when running the native Docker path in machine-output mode", () => { const out = mockOutput({ format: "json" }); - const api = mockLegacyPlatformApi({ - handler: (request) => - request.url.endsWith("/functions") - ? Effect.succeed(legacyJsonResponse(request, 200, [])) - : Effect.succeed(legacyJsonResponse(request, 200, {})), - }); + const api = mockLegacyPlatformApi(); const proxy = mockProxy(); + // Non-empty container stdout exercises the machine-mode branch that + // routes it to stderr instead of stdout (CLI-1546: stdout stays + // payload-only in json/stream-json modes). + const child = mockDockerUnbundle({ runStdout: ["unbundle: wrote index.ts"] }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -425,12 +433,14 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", + "hello-world", "--project-ref", - "abcdefghijklmnopqrst", + PROJECT_ID, "--output-format", "json", ]), @@ -438,38 +448,46 @@ describe("legacy functions download", () => { ); return Effect.gen(function* () { - // An empty project has nothing to delegate — this must match the - // native path's "No functions found." short-circuit instead of - // still invoking the Go/Docker child and reporting a misleading - // "Downloaded Edge Function source." success with an empty list. - yield* legacyFunctionsDownload({ - ...baseFlags, - functionName: Option.none(), - useDocker: true, - }); + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path; + // this asserts the JSON envelope this command emits itself still + // shows up correctly, with no delegated Go child's stdout to worry + // about capturing. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); expect(proxy.calls).toEqual([]); expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + expect(out.stdoutText).toBe(""); + expect(out.stderrText).toContain("unbundle: wrote index.ts\n"); expect(out.messages).toContainEqual( expect.objectContaining({ type: "success", - message: "No functions found.", - data: { function_slugs: [], project_ref: "abcdefghijklmnopqrst" }, + data: { function_slugs: ["hello-world"], project_ref: PROJECT_ID }, }), ); }).pipe(Effect.provide(layer)); }, ); - it.live("fails before delegating when the pre-flight function list fails in machine mode", () => { + it.live("lists remote functions and downloads each natively via Docker in machine mode", () => { const out = mockOutput({ format: "json" }); const api = mockLegacyPlatformApi({ handler: (request) => request.url.endsWith("/functions") - ? Effect.succeed(legacyJsonResponse(request, 500, { message: "unavailable" })) + ? Effect.succeed( + legacyJsonResponse(request, 200, [ + { slug: "hello-world" }, + { slug: "goodbye-world" }, + ]), + ) : Effect.succeed(legacyJsonResponse(request, 200, {})), }); const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); const layer = Layer.mergeAll( buildLegacyTestRuntime({ out, @@ -477,12 +495,13 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + child.layer, Stdio.layerTest({ args: Effect.succeed([ "functions", "download", "--project-ref", - "abcdefghijklmnopqrst", + PROJECT_ID, "--output-format", "json", ]), @@ -490,19 +509,1144 @@ describe("legacy functions download", () => { ); return Effect.gen(function* () { - // The pre-flight list failure must be reported before any download - // side effect — the delegated proxy must never be invoked (CLI-1862 - // review: a listing failure after a successful delegated download - // must not mask that success). - const exit = yield* legacyFunctionsDownload({ + yield* legacyFunctionsDownload({ ...baseFlags, functionName: Option.none(), useDocker: true, - }).pipe(Effect.exit); + }); - expect(Exit.isFailure(exit)).toBe(true); expect(proxy.calls).toEqual([]); expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.filter( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toHaveLength(2); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + data: { + function_slugs: ["hello-world", "goodbye-world"], + project_ref: PROJECT_ID, + }, + }), + ); + }).pipe(Effect.provide(layer)); + }); + + it.live("runs docker with the expected binds, network, and unbundle command", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + // Go: `extractOne` (`download.go:260-266`) — bind order and network + // reuse the same primitives `deploy.ts`'s own Docker-bundling path + // already uses. + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + expect(child.spawned.find((spawned) => spawned.args[0] === "volume")).toEqual({ + command: "docker", + args: [ + "volume", + "create", + "--label", + `com.supabase.cli.project=${PROJECT_ID}`, + "--label", + `com.docker.compose.project=${PROJECT_ID}`, + `supabase_edge_runtime_${PROJECT_ID}`, + ], + }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + const hostEszipPath = resolve( + tempRoot.current, + "supabase", + ".temp", + "output_hello-world.eszip", + ); + const functionsDir = resolve(tempRoot.current, "supabase", "functions"); + expect(runCommand?.args).toContain( + `supabase_edge_runtime_${PROJECT_ID}:/root/.cache/deno:rw`, + ); + expect(runCommand?.args).toContain( + `${hostEszipPath}:/root/eszips/output_hello-world.eszip:ro`, + ); + expect(runCommand?.args).toContain(`${functionsDir}:/home/deno:rw`); + expect(runCommand?.args).toContain("--network"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + // The unbundle tail is always the LAST 6 args regardless of whether + // `--add-host` (Linux-only) was inserted before it. + expect(runCommand?.args.slice(-6)).toEqual([ + `public.ecr.aws/supabase/edge-runtime:v${DEFAULT_VERSIONS["edge-runtime"]}`, + "unbundle", + "--eszip", + "/root/eszips/output_hello-world.eszip", + "--output", + "/home/deno/hello-world", + ]); + }).pipe(Effect.provide(layer)); + }); + + it.live("omits the named Deno cache volume bind on Bitbucket", () => { + // 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 :...` would otherwise still + // implicitly create the named volume, which Bitbucket's restricted Docker + // environment doesn't allow (review round on CLI-1963's `functions + // download` port; `deploy.ts`'s `buildDockerBinds` already applies this + // same carve-out). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + const previousBitbucketCloneDir = process.env["BITBUCKET_CLONE_DIR"]; + process.env["BITBUCKET_CLONE_DIR"] = "/opt/atlassian/pipelines/agent/build"; + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).not.toContain( + `supabase_edge_runtime_${PROJECT_ID}:/root/.cache/deno:rw`, + ); + const hostEszipPath = resolve( + tempRoot.current, + "supabase", + ".temp", + "output_hello-world.eszip", + ); + expect(runCommand?.args).toContain( + `${hostEszipPath}:/root/eszips/output_hello-world.eszip:ro`, + ); + }) + .pipe(Effect.provide(layer)) + .pipe( + Effect.ensuring( + Effect.sync(() => { + if (previousBitbucketCloneDir === undefined) { + delete process.env["BITBUCKET_CLONE_DIR"]; + } else { + process.env["BITBUCKET_CLONE_DIR"] = previousBitbucketCloneDir; + } + }), + ), + ); + }); + + it.live("requests the raw eszip body instead of a negotiated JSON response", () => { + // `v1GetAFunctionBody`'s generated contract marks its response + // `kind: "json"`, so `executeRaw` would otherwise default to + // `Accept: application/json` (`buildRequest`'s unconditional `acceptJson` + // for json-kind operations) and risk a negotiated JSON response instead + // of the raw eszip body Go's un-overridden request receives (review round + // on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const bodyRequest = api.requests.find((request) => request.url.endsWith("/hello-world/body")); + expect(bodyRequest?.headers["accept"]).toBe("*/*"); + }).pipe(Effect.provide(layer)); + }); + + it.live("uses an explicit --network-id override instead of the derived network name", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "custom-network", + ]), + }), + ); + + return Effect.gen(function* () { + // `--network-id` is a persistent root flag (`cmd/root.go:328`), not + // registered on `functions download` itself — + // `explicitNonEmptyStringFlag` scans the whole argv unscoped. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", "custom-network"], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("custom-network"); + expect(runCommand?.args).not.toContain(`supabase_network_${PROJECT_ID}`); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "falls back to the generated network name when --network-id is passed with an empty value", + () => { + // Go only overrides the network when `len(viper.GetString("network-id")) > 0` + // (`internal/utils/docker.go:379-382`) — an explicit-but-empty + // `--network-id=` must fall through to the generated network name just + // like an omitted flag (review round on CLI-1963's `functions download` + // port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id=", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live("honors the final occurrence of a repeated --network-id flag", () => { + // pflag/viper string flags are shared-variable, last-`Set()`-wins + // (confirmed empirically: `pflag.FlagSet.Parse` on + // `--network-id old --network-id custom-network` resolves to + // `custom-network`) — `explicitStringFlag` must keep scanning past the + // first match instead of returning early (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "old-network", + "--network-id", + "custom-network", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("custom-network"); + expect(runCommand?.args).not.toContain("old-network"); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "falls back to the generated network name when the final --network-id occurrence is empty", + () => { + // Same last-wins rule as above, applied to Go's `len(networkId) > 0` + // gate: a non-empty default followed by an explicit-but-empty override + // must fall through to the generated network name, not the earlier + // non-empty value. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "custom-network", + "--network-id=", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + expect(runCommand?.args).not.toContain("custom-network"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "does not climb to an ancestor project's config.toml for the Docker download path", + () => { + // Go's `flags.LoadConfig` only ever resolves `supabase/config.toml` from + // the already-resolved workdir, with no ancestor climb + // (`NewPathBuilder`, `pkg/config/utils.go:43-48`) — mirrored here by + // `resolveEdgeRuntimeImage`'s `search: false` (review round on + // CLI-1963's `functions download` port). A nested workdir with no + // `supabase/config.toml` of its own must fall back to `--project-ref` + // for network/volume naming, not an ancestor project's configured + // `project_id`, even though `cliConfig.workdir` sits right inside one. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const nestedWorkdir = join(tempRoot.current, "nested"); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: nestedWorkdir }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => mkdir(nestedWorkdir, { recursive: true })); + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ['project_id = "ancestor-project"', ""].join("\n"), + ), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toEqual({ + command: "docker", + args: ["network", "inspect", `supabase_network_${PROJECT_ID}`], + }); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain(`supabase_network_${PROJECT_ID}`); + expect(runCommand?.args).not.toContain("supabase_network_ancestor-project"); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live("prefers config.toml over a stray config.json for the Docker download path", () => { + // Go's `NewPathBuilder`/`Config.Load` (`pkg/config/utils.go:43-48`) has + // no concept of a JSON project config file — it always resolves + // `supabase/config.toml`, mirrored here by `resolveEdgeRuntimeImage`'s + // `tomlOnly: true` (review round on CLI-1963's `functions download` + // port). A workdir with both files must resolve `project_id` from + // `config.toml`, not prefer the JSON file as the package loader + // otherwise would. + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ['project_id = "toml-project"', ""].join("\n"), + ), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.json"), + JSON.stringify({ project_id: "json-project" }), + ), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("supabase_network_toml-project"); + expect(runCommand?.args).not.toContain("supabase_network_json-project"); + }).pipe(Effect.provide(layer)); + }); + + it.live("skips network creation for a container: network mode", () => { + // Go's `container.NetworkMode.IsUserDefined()` + // (`docker/api/types/container/hostconfig_unix.go:23-25`) explicitly + // excludes `IsContainer()` — `--network-id container:redis` attaches to + // another container's network stack, so `DockerNetworkCreateIfNotExists` + // never inspects or creates a network for it, and the mode is passed + // straight through to `docker run --network` (review round on + // CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--network-id", + "container:redis", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect(child.spawned.find((spawned) => spawned.args[0] === "network")).toBeUndefined(); + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args).toContain("container:redis"); + }).pipe(Effect.provide(layer)); + }); + + it.live("does not double-prefix an already v-prefixed edge-runtime-version pin", () => { + // Go's `replaceImageTag` (`pkg/config/utils.go:81-84`) appends the pin + // file's raw content verbatim after the image's `:`, so a pin already + // carrying its own `v` prefix (a legitimate form — see + // `legacy-edge-runtime-image.unit.test.ts`'s own `"v9.9.9"` fixture) must + // not be prepended with a second `v` (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase", ".temp"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile(join(tempRoot.current, "supabase", ".temp", "edge-runtime-version"), "v9.9.9\n"), + ); + + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + const runCommand = child.spawned.find((spawned) => spawned.args[0] === "run"); + expect(runCommand?.args.slice(-6)[0]).toBe("public.ecr.aws/supabase/edge-runtime:v9.9.9"); + }).pipe(Effect.provide(layer)); + }); + + it.live("keeps the temporary eszip file when --debug is passed", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--debug", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(true); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "removes the temporary eszip file when --debug=false overrides the flag's own presence", + () => { + // Go gates this on `viper.GetBool("DEBUG")`, so an explicit + // `--debug=false` resolves to `false` (cleanup runs) — a presence-only + // check would get this backwards and treat `--debug=false` like + // `--debug` (review round on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + "--debug=false", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }); + + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "fails on an invalid project config before falling back when Docker is not running", + () => { + // Go's `Run` calls `flags.LoadConfig(fsys)` unconditionally at the very + // top, before checking whether Docker is running (`download.go:135-138`) + // — an invalid `supabase/config.toml` must fail up front instead of + // silently falling through to the server-side path's API/filesystem + // side effects (review round on CLI-1963's `functions download` port). + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + // Every docker command (including the `docker info` probe) fails, + // modeling Docker not running. + const child = mockChildProcessSpawner({ exitCode: 1 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 3", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + "Failed reading config: Invalid edge_runtime.deno_version: 3.", + ); + expect(api.requests).toEqual([]); + }).pipe(Effect.provide(layer)); + }, + ); + + describe("docker unbundle container failures", () => { + it.live("fails with the legacy-bundle suggestion when the container exits non-zero", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["boom"] }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("error running container: exit 1"); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "prepends the deno v2 suggestion when deno_version is 1 and the container reports an invalid eszip", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ + runExitCode: 1, + // Go's scanner requires a full-line, case-insensitive match + // (`strings.EqualFold(line, "invalid eszip v2")`, `download.go:295`) + // — a line merely containing the phrase as a substring (e.g. + // "error: invalid eszip v2 header") does not fire the suggestion. + runStderr: ["invalid eszip v2"], + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 1", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("error running container: exit 1"); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "Please use deno v2 in supabase/config.toml to download this Function:\n\n[edge_runtime]\ndeno_version = 2\n" + + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "does not prepend the deno v2 suggestion when deno_version is 1 but the container's error is unrelated", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerUnbundle({ runExitCode: 1, runStderr: ["permission denied"] }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => + mkdir(join(tempRoot.current, "supabase"), { recursive: true }), + ); + yield* Effect.tryPromise(() => + writeFile( + join(tempRoot.current, "supabase", "config.toml"), + ["[edge_runtime]", "deno_version = 1", ""].join("\n"), + ), + ); + + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + }).pipe(Effect.provide(layer)); + }, + ); + }); + + it.live("fails when ensureDockerNetwork can't create a missing network", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const spawnerOpts: { + exitCode?: number; + stderr?: string[]; + onSpawn?: (record: { command: string; args: ReadonlyArray }) => void; + } = { exitCode: 0 }; + spawnerOpts.onSpawn = (record) => { + spawnerOpts.exitCode = record.command === "docker" && record.args[0] === "network" ? 1 : 0; + spawnerOpts.stderr = ["permission denied"]; + }; + const child = mockChildProcessSpawner(spawnerOpts); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to create docker network: supabase_network_${PROJECT_ID}`, + ); + expect(child.spawned.some((spawned) => spawned.args[0] === "volume")).toBe(false); + expect(child.spawned.some((spawned) => spawned.args[0] === "run")).toBe(false); + // Go parity fix (CLI-1963 review): `Effect.ensuring` wraps the whole + // Docker-extraction sequence, so the temp eszip written just before it + // is still cleaned up even though the failure happened before Docker + // ever ran — not only after a successful `runChildProcess` call. + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }); + + it.live( + "fails with the docker-step prefix when the unbundle container itself cannot be spawned", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi(); + const proxy = mockProxy(); + const child = mockDockerRunSpawnFailure(); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--use-docker", + "--project-ref", + PROJECT_ID, + ]), + }), + ); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true }).pipe( + Effect.flip, + ); + + // Distinct from `ensureDockerNetwork`/`ensureDockerNamedVolume` + // failures (asserted above), which already self-describe and must + // NOT gain this prefix — a bare spawn/runtime-not-found failure from + // `runChildProcess` itself carries no context of its own about which + // command was running, so `withDockerStepFailure` adds one. + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to run the edge-runtime unbundle container: ${legacyContainerRuntimeNotFoundMessage}`, + ); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + "\nIf your function is deployed using CLI < 1.120.0, trying running supabase functions download --legacy-bundle hello-world instead.", + ); + expect(child.spawned.some((spawned) => spawned.args[0] === "run")).toBe(true); + expect( + existsSync(join(tempRoot.current, "supabase", ".temp", "output_hello-world.eszip")), + ).toBe(false); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live( + "reports no functions found without delegating when the project is empty in machine mode", + () => { + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed(legacyJsonResponse(request, 200, [])) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); + const proxy = mockProxy(); + // Deterministic stand-in for `emptyEnv()`'s real `ChildProcessSpawner` + // (via `BunServices`, pulled in by `buildLegacyTestRuntime`) — `useDocker: + // true` still probes `docker info` even though this project has no + // functions to download, so this must not spawn a real `docker` process. + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + // An empty project has nothing to delegate — this must match the + // native path's "No functions found." short-circuit instead of + // still invoking the Go/Docker child and reporting a misleading + // "Downloaded Edge Function source." success with an empty list. + yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, + }); + + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + message: "No functions found.", + data: { function_slugs: [], project_ref: "abcdefghijklmnopqrst" }, + }), + ); + }).pipe(Effect.provide(layer)); + }, + ); + + it.live("fails before delegating when the pre-flight function list fails in machine mode", () => { + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed(legacyJsonResponse(request, 500, { message: "unavailable" })) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + // The pre-flight list failure must be reported before any download + // side effect — the delegated proxy must never be invoked (CLI-1862 + // review: a listing failure after a successful delegated download + // must not mask that success). + const exit = yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, + }).pipe(Effect.exit); + + expect(Exit.isFailure(exit)).toBe(true); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + }).pipe(Effect.provide(layer)); + }); + + it.live("fails loudly instead of silently dropping a malformed function-list entry", () => { + // Go: `FunctionResponse.Slug` (`pkg/api/types.gen.go:6465`) is a + // required, non-pointer `string` — a list entry with no "slug" key + // decodes to the zero value "" and then fails `ValidateFunctionSlug` + // in `downloadAll` (`download.go:182-188`), rather than being dropped + // from the list. A malicious/compromised API response returning + // `[{}]` must surface an error here too, never "No functions found." + // nor a silent partial download (review round on CLI-1963's + // `functions download` port). + const out = mockOutput({ format: "json" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/functions") + ? Effect.succeed(legacyJsonResponse(request, 200, [{}])) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); + const proxy = mockProxy(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + child.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + const exit = yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + useDocker: true, + }).pipe(Effect.exit); + + expect(Exit.isFailure(exit)).toBe(true); + expect(proxy.calls).toEqual([]); + expect(out.messages).not.toContainEqual( + expect.objectContaining({ type: "success", message: "No functions found." }), + ); }).pipe(Effect.provide(layer)); }); diff --git a/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts b/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts index 8391ab53d7..a0c9dcd02e 100644 --- a/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/serve/serve.integration.test.ts @@ -14,7 +14,7 @@ import { mockLegacyTelemetryStateTracked, useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; -import { toDockerPath } from "../../../../shared/functions/deploy.ts"; +import { toDockerPath } from "../../../../shared/functions/functions-docker.ts"; import { mockOutput, mockProcessControl, @@ -72,10 +72,10 @@ const deployMockState = vi.hoisted(() => ({ }, })); -vi.mock("../../../../shared/functions/deploy.ts", async () => { - const actual = await vi.importActual( - "../../../../shared/functions/deploy.ts", - ); +vi.mock("../../../../shared/functions/functions-docker.ts", async () => { + const actual = await vi.importActual< + typeof import("../../../../shared/functions/functions-docker.ts") + >("../../../../shared/functions/functions-docker.ts"); const { Effect } = await import("effect"); return { diff --git a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts index 96d39ce92b..c5cb7ee66d 100644 --- a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts +++ b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.ts @@ -30,7 +30,7 @@ import { LEGACY_CLI_PROJECT_LABEL, LEGACY_CLI_WORKDIR_LABEL, } from "../../../shared/legacy-docker-ids.ts"; -import { isUserDefinedDockerNetwork } from "../../../../shared/functions/deploy.ts"; +import { isUserDefinedDockerNetwork } from "../../../../shared/functions/functions-docker.ts"; import { legacyBuildStartContainerCreateArgs, legacyApplyBitbucketStartContainerFilter, @@ -54,9 +54,10 @@ type Spawner = ChildProcessSpawner["Service"]; * otherwise silently stop recognizing the local stack's containers. * * A same-value private constant already exists at - * `shared/functions/deploy.ts` (`dockerComposeProjectLabel`, for the unrelated - * `functions deploy` Docker Desktop extension gateway) but is neither exported - * nor in the same Docker-usage domain as `start` — not hoisted from there. + * `shared/functions/functions-docker.ts` (`dockerComposeProjectLabel`, for the + * unrelated `functions deploy`/`functions serve` Docker Desktop extension + * gateway) but is neither exported nor in the same Docker-usage domain as + * `start` — not hoisted from there. */ export const LEGACY_COMPOSE_PROJECT_LABEL = "com.docker.compose.project"; @@ -229,8 +230,9 @@ function legacyPortConflictSuggestion(hostPort: string, serviceLabel: string): s * created (`docker network create host` errors with "operation is not * permitted on predefined host network"), so this returns immediately without * spawning `docker network create` at all for those names, reusing the same - * `isUserDefinedDockerNetwork` check `shared/functions/deploy.ts` already - * applies for the unrelated `functions deploy` extension-gateway network. + * `isUserDefinedDockerNetwork` check `shared/functions/functions-docker.ts` + * already applies for the unrelated `functions deploy`/`functions serve` + * extension-gateway network. */ export function legacyEnsureStartNetwork( spawner: Spawner, diff --git a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts index cd77a460b4..66cb6084ed 100644 --- a/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts +++ b/apps/cli/src/legacy/commands/start/lib/container-lifecycle.unit.test.ts @@ -747,6 +747,22 @@ describe("legacyEnsureStartNetwork", () => { ); }, ); + + it.live("skips docker network create for a container: network mode", () => { + // Go's `container.NetworkMode.IsUserDefined()` + // (`docker/api/types/container/hostconfig_unix.go:23-25`) explicitly + // excludes `IsContainer()` — `--network-id container:redis` attaches to + // another container's network stack, not a name `docker network create` + // could ever act on (review round on CLI-1963's `functions download` + // port, which surfaced the same gap in the shared + // `isUserDefinedDockerNetwork` predicate this helper reuses). + const mock = mockSpawner(() => ({ exitCode: 1, stderr: "some failure" })); + return legacyEnsureStartNetwork(mock.spawner, "container:redis", {}).pipe( + Effect.map(() => { + expect(mock.spawned).toEqual([]); + }), + ); + }); }); describe("legacyEnsureStartVolume", () => { diff --git a/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts b/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts index acd17b7a1b..8d31bfaaf6 100644 --- a/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts +++ b/apps/cli/src/next/commands/functions/deploy/deploy.handler.ts @@ -1,12 +1,10 @@ -import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; import { Effect, Stdio } from "effect"; import { CliConfig } from "../../../config/cli-config.service.ts"; import { PlatformApi } from "../../../auth/platform-api.service.ts"; import { ProjectHome } from "../../../config/project-home.service.ts"; import { RuntimeInfo } from "../../../../shared/runtime/runtime-info.service.ts"; import { deployFunctions } from "../../../../shared/functions/deploy.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { resolveProjectRef } from "../functions.shared.ts"; import type { FunctionsDeployFlags } from "./deploy.command.ts"; @@ -19,13 +17,7 @@ export const functionsDeploy = Effect.fn("functions.deploy")(function* ( const runtimeInfo = yield* RuntimeInfo; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; - const edgeRuntimeVersion = yield* Effect.tryPromise(() => - readFile(join(projectHome.supabaseDir, ".temp", "edge-runtime-version"), "utf8"), - ).pipe( - Effect.map((version) => version.trim()), - Effect.catch(() => Effect.succeed("")), - Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), - ); + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin(projectHome.supabaseDir); yield* deployFunctions(flags, { api, diff --git a/apps/cli/src/next/commands/functions/download/download.command.ts b/apps/cli/src/next/commands/functions/download/download.command.ts index 4432db7c0b..aecd8adc61 100644 --- a/apps/cli/src/next/commands/functions/download/download.command.ts +++ b/apps/cli/src/next/commands/functions/download/download.command.ts @@ -25,6 +25,7 @@ const config = { ), useDocker: Flag.boolean("use-docker").pipe( Flag.withDescription("Use Docker to unbundle functions client-side."), + Flag.withDefault(true), Flag.withHidden, ), legacyBundle: Flag.boolean("legacy-bundle").pipe( diff --git a/apps/cli/src/next/commands/functions/download/download.handler.ts b/apps/cli/src/next/commands/functions/download/download.handler.ts index 15c6238e84..bb63eaf913 100644 --- a/apps/cli/src/next/commands/functions/download/download.handler.ts +++ b/apps/cli/src/next/commands/functions/download/download.handler.ts @@ -3,8 +3,9 @@ import { PlatformApi } from "../../../auth/platform-api.service.ts"; import { ProjectHome } from "../../../config/project-home.service.ts"; import { downloadFunctions, - makeGoProxyDownloadArgs, + makeGoProxyLegacyBundleArgs, } from "../../../../shared/functions/download.ts"; +import { resolveEdgeRuntimeVersionPin } from "../../../../shared/functions/functions.shared.ts"; import { LegacyGoProxy } from "../../../../shared/legacy/go-proxy.service.ts"; import { resolveProjectRef } from "../functions.shared.ts"; import type { FunctionsDownloadFlags } from "./download.command.ts"; @@ -15,17 +16,20 @@ export const functionsDownload = Effect.fnUntraced(function* (flags: FunctionsDo const proxy = yield* LegacyGoProxy; const stdio = yield* Stdio.Stdio; const rawArgs = yield* stdio.args; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersionPin(projectHome.supabaseDir); yield* downloadFunctions(flags, { api, projectRoot: projectHome.projectRoot, rawArgs, + goViperCompat: false, + edgeRuntimeVersion, resolveProjectRef, // In machine-output mode the child's stdout is captured and discarded // instead of inherited (CLI-1546: stdout is payload-only in machine // mode) — `downloadFunctions` emits the `Output` envelope itself. proxyDownload: (proxyFlags, projectRef, captureOutput) => { - const args = makeGoProxyDownloadArgs(proxyFlags, projectRef); + const args = makeGoProxyLegacyBundleArgs(proxyFlags.functionName, projectRef); const cwd = projectHome.projectRoot; return captureOutput ? Effect.asVoid(proxy.execCapture(args, { cwd, stdin: "ignore" })) diff --git a/apps/cli/src/next/commands/functions/download/download.integration.test.ts b/apps/cli/src/next/commands/functions/download/download.integration.test.ts index 9c1fa1a75d..cfe88deb0d 100644 --- a/apps/cli/src/next/commands/functions/download/download.integration.test.ts +++ b/apps/cli/src/next/commands/functions/download/download.integration.test.ts @@ -23,6 +23,7 @@ import { mockProjectLinkState, mockRuntimeInfo, } from "../../../../../tests/helpers/mocks.ts"; +import { mockChildProcessSpawner } from "../../../../../../../packages/process-compose/tests/helpers/mocks.ts"; import type { FunctionsDownloadFlags } from "./download.command.ts"; import { ConflictingFunctionDownloadFlagsError, @@ -30,6 +31,7 @@ import { InvalidFunctionSlugError, UnsafeFunctionDownloadPathError, } from "../../../../shared/functions/download.errors.ts"; +import { invalidFunctionSlugDetail } from "../../../../shared/functions/functions.shared.ts"; import { functionsDownload } from "./download.handler.ts"; const PROJECT_REF = "abcdefghijklmnopqrst"; @@ -74,6 +76,7 @@ function textResponse( status: number, body: ResponseBody = "", contentType = "text/plain", + extraHeaders: Readonly> = {}, ): HttpClientResponse.HttpClientResponse { return HttpClientResponse.fromWeb( request, @@ -81,6 +84,7 @@ function textResponse( status, headers: { "content-type": contentType, + ...extraHeaders, }, }), ); @@ -182,7 +186,15 @@ function mockDownloadApi(opts: { functionStatusBySlug?: Readonly>; functionBodyBySlug?: Readonly>; bodyBySlug?: Readonly< - Record + Record< + string, + { + status?: number; + body: ResponseBody; + contentType: string; + headers?: Readonly>; + } + > >; bodyErrorBySlug?: Readonly>; }) { @@ -233,6 +245,7 @@ function mockDownloadApi(opts: { response?.status ?? 200, response?.body ?? "", response?.contentType ?? "multipart/form-data; boundary=missing", + response?.headers ?? {}, ), ); } @@ -277,6 +290,7 @@ function setup( linked?: boolean; projectRoot?: string; rawArgs?: ReadonlyArray; + childLayer?: ReturnType["layer"]; } = {}, ) { const out = mockOutput({ format: opts.format ?? "text", interactive: false }); @@ -293,6 +307,10 @@ function setup( Stdio.layerTest({ args: Effect.succeed(opts.rawArgs ?? ["functions", "download"]), }), + // Overrides `emptyEnv()`'s real `ChildProcessSpawner` (via `BunServices`) + // so `--use-docker`'s now-default-true native path never spawns a real + // `docker` process — CLI-1963. + opts.childLayer ?? mockChildProcessSpawner({ exitCode: 0 }).layer, ); return { out, api, layer, proxy }; @@ -709,48 +727,88 @@ describe("functions download", () => { ); }); - it.live("downloads remote slugs from download-all without local slug validation", () => { + it.live("rejects a malicious remote slug from download-all before any per-slug work", () => { const tempDir = makeTempDir(); - const multipart = multipartBody([ - { - headers: { - "Content-Disposition": 'form-data; name="metadata"', - "Content-Type": "application/json", - }, - body: JSON.stringify({ deno2_entrypoint_path: "source/index.ts" }), - }, - { - headers: { - "Content-Disposition": 'form-data; name="file"; filename="source/index.ts"', - }, - body: "console.log('remote')", - }, - ]); + // Mirrors Go's own `TestDownloadAllRejectsMaliciousSlug` regression test + // (`apps/cli-go/internal/functions/download/download_test.go`) — a + // path-traversal-shaped slug returned by the (untrusted) list endpoint. + const maliciousSlug = "../../../../../poc-escaped-outside-project"; return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer } = setup(tempDir, { - list: [makeFunction({ slug: "1remote" })], - bodyBySlug: { - "1remote": multipart, - }, + const { api, layer } = setup(tempDir, { + list: [makeFunction({ slug: maliciousSlug })], }); - yield* functionsDownload({ + // CLI-1891 (Go parity): every slug sourced from the Management API's + // function list must be validated before any per-slug network or + // filesystem work — not just user-supplied CLI arguments. + const error = yield* functionsDownload({ ...BASE_FLAGS, functionName: Option.none(), - }).pipe(Effect.provide(layer)); + }).pipe(Effect.provide(layer), Effect.flip); - expect( - yield* Effect.tryPromise(() => - readFile(join(tempDir, "supabase", "functions", "1remote", "index.ts"), "utf8"), - ), - ).toBe("console.log('remote')"); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `failed to download function ${maliciousSlug}: ${invalidFunctionSlugDetail}`, + ); + expect((error as Error & { suggestion?: string }).suggestion).toBe( + `The Supabase API returned an unexpected function slug (${maliciousSlug}). Retry the command, and if this keeps happening, verify your network connection is not being intercepted before contacting Supabase support.`, + ); + // Only the list call happened — no GET to the malicious slug's own + // body/metadata endpoints, and nothing was written to disk. + expect(api.requests).toEqual([ + `https://api.supabase.com/v1/projects/${PROJECT_REF}/functions`, + ]); + expect(existsSync(join(tempDir, "supabase", "functions"))).toBe(false); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); }); + it.live( + "fails the whole list before downloading anything when a slug is typed as a non-string", + () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + // Go's generated client unmarshals the whole `[]FunctionResponse` + // array in one `json.Unmarshal` call + // (`apps/cli-go/pkg/api/client.gen.go:22186-22208`); a type mismatch + // on any single element's `slug` (a required `string` field) fails + // that call outright, so `downloadAll` fails with "failed to list + // functions: ..." before downloading anything — including the + // earlier, well-formed "ok" entry. Confirmed empirically: + // `json.Unmarshal([]byte(`[{"slug":"ok"},{"slug":123}]`), &dest)` + // returns a `*json.UnmarshalTypeError`, and the generated parser + // returns before ever assigning `response.JSON200`. + const { api, layer } = setup(tempDir, { + listBody: [{ slug: "ok" }, { slug: 123 }], + }); + + const error = yield* functionsDownload({ + ...BASE_FLAGS, + functionName: Option.none(), + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(InvalidFunctionDownloadResponseError); + expect((error as Error).message).toBe( + "failed to read functions list: expected function slug to be a string, got number", + ); + // Only the list call happened — "ok" was never downloaded, matching + // Go's atomic list-decode failure instead of downloading it before + // hitting the later entry's error. + expect(api.requests).toEqual([ + `https://api.supabase.com/v1/projects/${PROJECT_REF}/functions`, + ]); + expect(existsSync(join(tempDir, "supabase", "functions"))).toBe(false); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + it.live("prints the download-all success line when the project has one function", () => { const tempDir = makeTempDir(); const multipart = multipartBody([ @@ -851,56 +909,125 @@ describe("functions download", () => { ); }); - it.live("delegates --use-docker with the linked project ref to the Go proxy", () => { + it.live( + "runs the native Docker unbundle path for --use-docker with the linked project ref", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer, proxy } = setup(tempDir, { + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path + // instead of delegating to the Go proxy. + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + const runCommand = child.spawned.find( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ); + expect(runCommand?.args).toContain("unbundle"); + expect(out.stderrText).toContain("Downloading function: hello-world\n"); + // No `--debug` — the temp eszip file is removed after the run. + expect(existsSync(join(tempDir, "supabase", ".temp", "output_hello-world.eszip"))).toBe( + false, + ); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live("runs the native Docker path and emits a JSON envelope in machine mode", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer, proxy } = setup(tempDir, { + const { out, layer, proxy } = setup(tempDir, { + format: "json", + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, }); + // CLI-1963: `--use-docker` now runs the native Docker-unbundle path; + // this asserts the JSON envelope this command emits itself still + // shows up correctly once the native path is exercised in machine mode. yield* functionsDownload({ ...BASE_FLAGS, useDocker: true, }).pipe(Effect.provide(layer)); - expect(proxy.calls).toEqual([ - ["functions", "download", "hello-world", "--project-ref", PROJECT_REF, "--use-docker"], - ]); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.some((spawned) => spawned.command === "docker" && spawned.args[0] === "run"), + ).toBe(true); + expect(out.messages).toContainEqual( + expect.objectContaining({ + type: "success", + message: "Downloaded Edge Function source.", + data: { + function_slugs: ["hello-world"], + project_ref: PROJECT_REF, + }, + }), + ); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); }); - it.live("captures the Go proxy's output and emits a JSON envelope in machine mode", () => { + it.live("lists remote functions and downloads each natively via Docker in machine mode", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); const { out, layer, proxy } = setup(tempDir, { format: "json", - rawArgs: ["functions", "download", "hello-world", "--use-docker"], + list: [makeFunction({ slug: "hello-world" }), makeFunction({ slug: "goodbye-world" })], + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + "goodbye-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + rawArgs: ["functions", "download", "--use-docker"], + childLayer: child.layer, }); - // CLI-1546: stdout is payload-only in machine mode, so the delegated - // Go child's raw output must be captured/discarded (not inherited), - // and this command must emit the `Output` envelope itself. yield* functionsDownload({ ...BASE_FLAGS, + functionName: Option.none(), useDocker: true, }).pipe(Effect.provide(layer)); expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "hello-world", "--project-ref", PROJECT_REF, "--use-docker"], - ]); + expect(proxy.captureCalls).toEqual([]); + expect( + child.spawned.filter( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toHaveLength(2); expect(out.messages).toContainEqual( expect.objectContaining({ type: "success", message: "Downloaded Edge Function source.", data: { - function_slugs: ["hello-world"], + function_slugs: ["hello-world", "goodbye-world"], project_ref: PROJECT_REF, }, }), @@ -911,38 +1038,130 @@ describe("functions download", () => { }); it.live( - "lists remote functions before delegating when no function name is given in machine mode", + "defaults --use-docker to true so a bare invocation still runs the native Docker path", () => { const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { out, layer, proxy } = setup(tempDir, { - format: "json", - list: [makeFunction({ slug: "hello-world" }), makeFunction({ slug: "goodbye-world" })], - rawArgs: ["functions", "download", "--use-docker"], + const { layer, proxy } = setup(tempDir, { + bodyBySlug: { + "hello-world": { body: "fake-eszip-bytes", contentType: "application/octet-stream" }, + }, + // No `--use-docker` at all — mirrors a bare `supabase functions + // download hello-world` invocation relying on the flag's default. + rawArgs: ["functions", "download", "hello-world"], + childLayer: child.layer, }); + // `useDocker: true` is what `download.command.ts`'s + // `Flag.withDefault(true)` resolves to when the flag is omitted + // (CLI-1963 parity fix — `next` was previously missing this default, + // unlike the legacy shell's equivalent command). yield* functionsDownload({ ...BASE_FLAGS, - functionName: Option.none(), useDocker: true, }).pipe(Effect.provide(layer)); expect(proxy.calls).toEqual([]); - expect(proxy.captureCalls).toEqual([ - ["functions", "download", "--project-ref", PROJECT_REF, "--use-docker"], - ]); - expect(out.messages).toContainEqual( - expect.objectContaining({ - type: "success", - message: "Downloaded Edge Function source.", - data: { - function_slugs: ["hello-world", "goodbye-world"], - project_ref: PROJECT_REF, + expect( + child.spawned.some( + (spawned) => spawned.command === "docker" && spawned.args[0] === "run", + ), + ).toBe(true); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live( + "falls back to the native server-side path with a warning when Docker is not running", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 1 }); + const multipart = multipartBody([ + { + headers: { + "Content-Disposition": 'form-data; name="metadata"', + "Content-Type": "application/json", + }, + body: JSON.stringify({ deno2_entrypoint_path: "source/index.ts" }), + }, + { + headers: { + "Content-Disposition": 'form-data; name="file"; filename="source/index.ts"', + }, + body: "console.log('fallback')", + }, + ]); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer } = setup(tempDir, { + bodyBySlug: { "hello-world": multipart }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + expect(child.spawned).toEqual([{ command: "docker", args: ["info"] }]); + expect(out.stderrText).toContain("WARNING: Docker is not running\n"); + expect( + yield* Effect.tryPromise(() => + readFile(join(tempDir, "supabase", "functions", "hello-world", "index.ts"), "utf8"), + ), + ).toBe("console.log('fallback')"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live( + "writes the eszip response body to disk exactly as received, regardless of Content-Encoding", + () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + // Arbitrary binary bytes, not valid brotli — this mocked `Response` (a + // hand-built `new Response(body, {...})`, unlike a real `fetch()`) + // never applies transport-level content-decoding, so a + // `Content-Encoding: br` header here must have zero effect on what + // `downloadEszipBody` does with it. If production code ever tried to + // brotli-decompress this body again, decompression itself would throw + // on these bytes, failing this test. + const rawEszipBytes = new Uint8Array([0, 1, 2, 253, 254, 255, 10, 13, 0, 128, 200]); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyBySlug: { + "hello-world": { + body: new Blob([rawEszipBytes]), + contentType: "application/octet-stream", + headers: { "content-encoding": "br" }, }, - }), + }, + // `--debug` keeps the temp eszip file on disk after a successful + // run so this test can inspect the exact bytes that were written. + rawArgs: ["functions", "download", "hello-world", "--use-docker", "--debug"], + childLayer: child.layer, + }); + + yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer)); + + const written = yield* Effect.tryPromise(() => + readFile(join(tempDir, "supabase", ".temp", "output_hello-world.eszip")), ); + expect(new Uint8Array(written)).toEqual(rawEszipBytes); }).pipe( Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), ); @@ -1346,6 +1565,65 @@ describe("functions download", () => { ); }); + it.live("maps eszip body transport errors with Go-style wording (Docker path)", () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyErrorBySlug: { + "hello-world": new Error("network error"), + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + // `downloadEszipBody` (the Docker path's own GET) uses a distinct + // error prefix ("failed to get function body") from the server-side + // `downloadBody`'s ("failed to download function") — Go parity. + const error = yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe("failed to get function body: network error"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }); + + it.live("maps unexpected eszip body statuses with Go-style wording (Docker path)", () => { + const tempDir = makeTempDir(); + const child = mockChildProcessSpawner({ exitCode: 0 }); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + bodyBySlug: { + "hello-world": { + status: 503, + body: "unavailable", + contentType: "text/plain", + }, + }, + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + childLayer: child.layer, + }); + + const error = yield* functionsDownload({ + ...BASE_FLAGS, + useDocker: true, + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toBe("Error status 503: unavailable"); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }); + it.live("maps metadata fallback transport errors with Go-style wording", () => { const tempDir = makeTempDir(); const multipart = multipartBody([ diff --git a/apps/cli/src/shared/cli/cobra-flag-groups.ts b/apps/cli/src/shared/cli/cobra-flag-groups.ts index 8b2d6c0727..88d80c350c 100644 --- a/apps/cli/src/shared/cli/cobra-flag-groups.ts +++ b/apps/cli/src/shared/cli/cobra-flag-groups.ts @@ -28,6 +28,90 @@ export function hasExplicitLongFlag( return false; } +/** + * Raw value of `--`/`--=value` anywhere in argv + * (unscoped — no command-path anchoring), or `undefined` if absent. + * pflag string flags are shared-variable, last-`Set()`-wins (same rule + * {@link explicitBooleanLongFlag} and `legacyPflagStringValue` already + * follow) — a repeated `-- old -- new` must resolve to + * `new`, so this keeps scanning after a match instead of returning early + * (review round on CLI-1963's `functions download` port). Not exported — + * every current call site needs Go's `len(value) > 0` gate too (see + * {@link explicitNonEmptyStringFlag}); re-export this directly if a future + * caller genuinely needs presence-only semantics. + */ +function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { + let result: string | undefined; + for (let index = 0; index < rawArgs.length; index += 1) { + const token = rawArgs[index]; + if (token === `--${flagName}`) { + result = rawArgs[index + 1]; + } else if (token?.startsWith(`--${flagName}=`)) { + result = token.slice(flagName.length + 3); + } + } + return result; +} + +/** + * Same as {@link explicitStringFlag}, but treats an explicit empty value + * (`--=`) as unset — matching Go call sites that gate on + * `len(viper.GetString(flagName)) > 0` rather than mere presence (e.g. + * `--network-id`, `apps/cli-go/internal/utils/docker.go:379-382`). pflag + * still marks the flag `Changed` for `--network-id=`, but Go's own + * `if networkId := viper.GetString("network-id"); len(networkId) > 0` + * falls through to the generated network name for that value just like an + * omitted flag would (review round on CLI-1963's `functions download` port). + */ +export function explicitNonEmptyStringFlag(rawArgs: ReadonlyArray, flagName: string) { + const value = explicitStringFlag(rawArgs, flagName); + return value !== undefined && value.length > 0 ? value : undefined; +} + +/** + * Whether `--` (or `--=`) appears anywhere in argv, + * unscoped. + */ +export function hasGlobalLongFlag(rawArgs: ReadonlyArray, flagName: string) { + return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); +} + +const PFLAG_BOOLEAN_FALSE_VALUES: ReadonlySet = new Set([ + "0", + "f", + "F", + "false", + "FALSE", + "False", +]); + +/** + * Last explicit `--`/`--=` boolean occurrence in + * argv, or `undefined` when the flag never appears — matching pflag/viper's + * shared-variable last-`Set()`-wins semantics (mirrors + * `legacyExperimentalFlagFromArgs`, `shared/legacy/global-flags.ts`). A bare + * `--` records pflag's bool `NoOptDefVal` (`true`); an inline value + * is parsed through pflag's `strconv.ParseBool` false set — anything else + * (including garbage) is truthy, same as `cast.ToBool`'s permissive default. + * Unlike {@link hasGlobalLongFlag}, this distinguishes `--=false` + * from presence alone, which matters for Go call sites gated on + * `viper.GetBool` rather than "was the flag passed at all". + */ +export function explicitBooleanLongFlag( + rawArgs: ReadonlyArray, + flagName: string, +): boolean | undefined { + let result: boolean | undefined; + for (const token of rawArgs) { + if (token === `--${flagName}`) { + result = true; + } else if (token.startsWith(`--${flagName}=`)) { + result = !PFLAG_BOOLEAN_FALSE_VALUES.has(token.slice(flagName.length + 3)); + } + } + return result; +} + /** * Value-taking long flags registered persistently on the Go root command * (`apps/cli-go/cmd/root.go:324-333`: `--workdir`, `--network-id`, diff --git a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts index 7c9a9bbe07..1cab6b610b 100644 --- a/apps/cli/src/shared/cli/hidden-flag.unit.test.ts +++ b/apps/cli/src/shared/cli/hidden-flag.unit.test.ts @@ -137,13 +137,30 @@ describe("native hidden flags", () => { "--backup=false", ]).pipe(Effect.exit); expect(JSON.stringify(stopExit)).not.toContain("UnrecognizedFlag"); - yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test" })([ + // `functions download --use-docker` now runs the native Docker-unbundle + // path (CLI-1963) instead of forwarding to `LegacyGoProxy` — it can fail + // for Docker-related reasons in this proxy-only test layer, same as + // `start`/`stop` above, so this only proves the hidden flag still parses. + // `--legacy-bundle` is the one remaining case that still forwards to the + // proxy, asserted below. + const downloadUseDockerExit = yield* Command.runWith(legacyTestRoot, { + version: "0.0.0-test", + })([ "functions", "download", "hello", "--project-ref", "abcdefghijklmnopqrst", "--use-docker", + ]).pipe(Effect.exit); + expect(JSON.stringify(downloadUseDockerExit)).not.toContain("UnrecognizedFlag"); + yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test" })([ + "functions", + "download", + "hello", + "--project-ref", + "abcdefghijklmnopqrst", + "--legacy-bundle", ]); const useDockerExit = yield* Command.runWith(legacyTestRoot, { version: "0.0.0-test", @@ -171,7 +188,14 @@ describe("native hidden flags", () => { ); expect(proxy.calls).toEqual([ - ["functions", "download", "hello", "--project-ref", "abcdefghijklmnopqrst", "--use-docker"], + [ + "functions", + "download", + "hello", + "--project-ref", + "abcdefghijklmnopqrst", + "--legacy-bundle", + ], ]); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 1043a3a149..9ad0320e8e 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -8,19 +8,19 @@ import { loadProjectConfig, type ResolvedFunctionConfig as ManifestFunctionConfig, } from "@supabase/config"; -import { Duration, Effect, Option, Schema, Stream } from "effect"; -import { ChildProcessSpawner } from "effect/unstable/process"; +import { Duration, Effect, Option, Schema } from "effect"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; import { legacyPromptYesNo } from "../legacy/legacy-prompt-yes-no.ts"; import { CONTEXT_CANCELED_MESSAGE } from "../output/errors.ts"; import { Output } from "../output/output.service.ts"; -import { spawnContainerCli } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyBold } from "../../legacy/shared/legacy-colors.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; import { findGitRootPath } from "../git/git-root.ts"; import { cobraMutuallyExclusiveErrorMessage, + explicitNonEmptyStringFlag, hasExplicitLongFlag, + hasGlobalLongFlag, } from "../cli/cobra-flag-groups.ts"; import { FUNCTIONS_BUNDLER_MUTEX_GROUP, @@ -33,14 +33,22 @@ import { InvalidFunctionDeploySlugError, NoFunctionsToDeployError, } from "./deploy.errors.ts"; +import { + edgeRuntimeImageTag, + ensureDockerNamedVolume, + ensureDockerNetwork, + isDockerRunning, + localDockerId, + resolveEdgeRuntimeVersion, + runChildProcess, + toDockerPath, + toSlash, +} from "./functions-docker.ts"; const COMPRESSED_ESZIP_MAGIC = "EZBR"; -const DENO1_EDGE_RUNTIME_VERSION = "1.68.4"; const DEPLOY_RATE_LIMIT_MAX_RETRIES = 8; const SUPABASE_FUNCTIONS_DIR = "supabase/functions"; const IMPORT_MAP_GUIDE_URL = "https://supabase.com/docs/guides/functions/import-maps"; -const INVALID_PROJECT_ID = /[^a-zA-Z0-9_.-]+/g; -const MAX_PROJECT_ID_LENGTH = 40; const WINDOWS_ABSOLUTE_PATH = /^[A-Za-z]:\//; const importPathPattern = /(?:import|export)\s+(?:type\s+)?(?:{[^{}]+}|.*?)\s*(?:from)?\s*['"](.*?)['"]|import\(\s*['"](.*?)['"]\)/gi; @@ -213,6 +221,18 @@ function validateDeploySlug(slug: string): Effect.Effect` was passed + * explicitly after `commandPath`, matching cobra's `Changed()`; + * `Option.none()` otherwise. Used only by `deployFunctions`'s + * `--no-verify-jwt` override below — kept private per this file's own + * "used by one command only -> keep it in the command's own directory" rule. + */ function explicitBooleanFlag( rawArgs: ReadonlyArray, commandPath: ReadonlyArray, @@ -222,45 +242,6 @@ function explicitBooleanFlag( return hasExplicitLongFlag(rawArgs, commandPath, flagName) ? Option.some(value) : Option.none(); } -function explicitStringFlag(rawArgs: ReadonlyArray, flagName: string) { - for (let index = 0; index < rawArgs.length; index += 1) { - const token = rawArgs[index]; - if (token === `--${flagName}`) { - return rawArgs[index + 1]; - } - if (token?.startsWith(`--${flagName}=`)) { - return token.slice(flagName.length + 3); - } - } - return undefined; -} - -function hasGlobalLongFlag(rawArgs: ReadonlyArray, flagName: string) { - return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); -} - -function isDenoConfigFile(pathname: string) { - const name = basename(pathname).toLowerCase(); - return name === "deno.json" || name === "deno.jsonc"; -} - -function toSlash(pathname: string) { - return pathname.replaceAll("\\", "/"); -} - -export function normalizeProjectId(source: string) { - const sanitized = source.replaceAll(INVALID_PROJECT_ID, "_").replace(/^[_.-]+/, ""); - return sanitized.length > MAX_PROJECT_ID_LENGTH - ? sanitized.slice(0, MAX_PROJECT_ID_LENGTH) - : sanitized; -} - -export function localDockerId(name: string, projectId: string) { - return `supabase_${name}_${normalizeProjectId(projectId)}`; -} - -const dockerCliProjectLabel = "com.supabase.cli.project"; -const dockerComposeProjectLabel = "com.docker.compose.project"; /** * Must stay in sync with `LEGACY_CLI_WORKDIR_LABEL` * (`legacy/shared/legacy-docker-ids.ts:95`) — same string literal, kept as a @@ -280,18 +261,6 @@ export const dockerWorkdirLabel = "com.supabase.cli.workdir"; */ const dockerNpmEnvNames = ["NPM_CONFIG_REGISTRY"] as const; -export function dockerProjectLabels(projectId: string) { - return { - [dockerCliProjectLabel]: projectId, - [dockerComposeProjectLabel]: projectId, - }; -} - -export function toDockerPath(hostPath: string) { - const normalized = toSlash(resolve(hostPath)); - return normalized.replace(/^[A-Za-z]:/, ""); -} - function toBundledFileUrl(hostPath: string) { const url = new URL("file:///"); url.pathname = toDockerPath(hostPath).replaceAll("%", "%25"); @@ -1107,15 +1076,6 @@ function createBundledMetadata( }; } -function collectByteStream(stream: Stream.Stream) { - const decoder = new TextDecoder(); - return Stream.runFold( - stream, - () => "", - (text, chunk) => text + decoder.decode(chunk, { stream: true }), - ).pipe(Effect.map((text) => text + decoder.decode())); -} - function sanitizeDockerBinds( binds: ReadonlyArray, functionsDir: string, @@ -1261,84 +1221,6 @@ function shouldUseDenoJsonDiscovery(entrypoint: string, importMap: string) { return isDenoConfigFile(importMap) && dirname(importMap) === dirname(entrypoint); } -export function isUserDefinedDockerNetwork(networkMode: string) { - return ( - networkMode.length > 0 && - networkMode !== "default" && - networkMode !== "bridge" && - networkMode !== "host" && - networkMode !== "none" - ); -} - -export const ensureDockerNetwork = Effect.fnUntraced(function* ( - networkMode: string, - projectId: string, -) { - if (!isUserDefinedDockerNetwork(networkMode)) { - return; - } - - const inspect = yield* runChildProcess("docker", ["network", "inspect", networkMode], { - stdout: "ignore", - stderr: "ignore", - }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); - if (inspect.exitCode === 0) { - return; - } - - const labels = dockerProjectLabels(projectId); - const create = yield* runChildProcess( - "docker", - [ - "network", - "create", - "--label", - `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, - "--label", - `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, - networkMode, - ], - { - stdout: "ignore", - stderr: "pipe", - }, - ); - if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { - return yield* Effect.fail(new Error(`failed to create docker network: ${networkMode}`)); - } -}); - -export const ensureDockerNamedVolume = Effect.fnUntraced(function* ( - volumeName: string, - projectId: string, -) { - if (process.env["BITBUCKET_CLONE_DIR"] !== undefined) { - return; - } - - const labels = dockerProjectLabels(projectId); - const create = yield* runChildProcess( - "docker", - [ - "volume", - "create", - "--label", - `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, - "--label", - `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, - volumeName, - ], - { - stdout: "ignore", - stderr: "pipe", - }, - ); - if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { - return yield* Effect.fail(new Error(`failed to create docker volume: ${volumeName}`)); - } -}); - async function shouldUsePackageJsonDiscovery(entrypoint: string, importMap: string) { if (importMap.length > 0) { return false; @@ -1351,48 +1233,6 @@ async function shouldUsePackageJsonDiscovery(entrypoint: string, importMap: stri } } -// Runs a container CLI command and collects its output. Every caller runs -// `docker`, so the spawn goes through `spawnContainerCli` to fall back to -// `podman` on Docker-less hosts. `command` is retained for the extendEnv -// default and the `functions serve` dependency-injection seam. -export const runChildProcess = Effect.fnUntraced(function* ( - command: string, - args: ReadonlyArray, - opts: { - readonly stdout?: "pipe" | "ignore"; - readonly stderr?: "pipe" | "ignore"; - readonly env?: Readonly>; - readonly extendEnv?: boolean; - } = {}, -) { - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const child = yield* spawnContainerCli(spawner, [...args], { - stdin: "ignore", - stdout: opts.stdout ?? "pipe", - stderr: opts.stderr ?? "pipe", - env: opts.env, - extendEnv: opts.extendEnv ?? command === "docker", - }); - - const [stdout, stderr, exitCode] = yield* Effect.all( - [ - opts.stdout === "ignore" ? Effect.succeed("") : collectByteStream(child.stdout), - opts.stderr === "ignore" ? Effect.succeed("") : collectByteStream(child.stderr), - child.exitCode.pipe(Effect.map(Number)), - ], - { concurrency: "unbounded" }, - ); - return { exitCode, stdout, stderr }; -}); - -const isDockerRunning = Effect.fnUntraced(function* () { - const result = yield* runChildProcess("docker", ["info"], { - stdout: "ignore", - stderr: "ignore", - }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); - return result.exitCode === 0; -}); - const bundleFunctionWithDocker = Effect.fnUntraced(function* ( projectId: string, edgeRuntimeVersion: string, @@ -1440,7 +1280,11 @@ const bundleFunctionWithDocker = Effect.fnUntraced(function* ( } command.push( - legacyGetRegistryImageUrl(`supabase/edge-runtime:v${edgeRuntimeVersion}`), + // `edgeRuntimeImageTag`, not a bare `v${edgeRuntimeVersion}` prepend — + // `edgeRuntimeVersion` can come from a `.temp/edge-runtime-version` pin + // that's already `v`-prefixed (see the helper's doc in + // `functions-docker.ts`); blindly prepending `v` double-prefixes it. + legacyGetRegistryImageUrl(`supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`), "bundle", "--entrypoint", toDockerPath(config.entrypoint), @@ -2161,21 +2005,6 @@ const deployViaDocker = Effect.fnUntraced(function* ( } }); -export function resolveEdgeRuntimeVersion( - denoVersion: number | undefined, - defaultVersion: string, -): Effect.Effect { - if (denoVersion === undefined || denoVersion === 2) { - return Effect.succeed(defaultVersion); - } - if (denoVersion === 1) { - return Effect.succeed(DENO1_EDGE_RUNTIME_VERSION); - } - return Effect.fail( - new Error(`Failed reading config: Invalid edge_runtime.deno_version: ${denoVersion}.`), - ); -} - const pruneFunctions = Effect.fnUntraced(function* ( projectRef: string, configs: ReadonlyArray, @@ -2377,7 +2206,13 @@ export function deployFunctions( join(dependencies.projectRoot, SUPABASE_FUNCTIONS_DIR), configs, dependencies.api, - explicitStringFlag(dependencies.rawArgs, "network-id"), + // Go only treats `--network-id` as an override when + // `len(viper.GetString("network-id")) > 0` + // (`internal/utils/docker.go:379-382`) — an explicit-but-empty + // `--network-id=` must fall through to the generated network + // name (`dockerNetworkId?: string` → `undefined`) just like an + // omitted flag. + explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id"), debugEnabled, styleEmphasis, ); diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 7b9882f6dd..2d849fd4bd 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -1,6 +1,7 @@ import { operationDefinitions, type ApiClient } from "@supabase/api/effect"; +import { loadProjectConfig } from "@supabase/config"; import { randomUUID } from "node:crypto"; -import { open, rename, rm } from "node:fs/promises"; +import { mkdir, open, rename, rm, writeFile } from "node:fs/promises"; import { dirname, isAbsolute, join, posix, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { Effect, FileSystem, Option } from "effect"; @@ -9,8 +10,21 @@ import type * as HttpClientResponse from "effect/unstable/http/HttpClientRespons import { Output } from "../output/output.service.ts"; import { cobraMutuallyExclusiveErrorMessage, + explicitBooleanLongFlag, + explicitNonEmptyStringFlag, hasExplicitLongFlag, } from "../cli/cobra-flag-groups.ts"; +import { legacyDescribeContainerCliFailure } from "../../legacy/shared/legacy-container-cli.ts"; +import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; +import { + edgeRuntimeImageTag, + ensureDockerNamedVolume, + ensureDockerNetwork, + isDockerRunning, + localDockerId, + resolveEdgeRuntimeVersion, + runChildProcess, +} from "./functions-docker.ts"; import { FUNCTIONS_BUNDLER_MUTEX_GROUP, invalidFunctionSlugDetail, @@ -25,6 +39,11 @@ import { } from "./download.errors.ts"; const legacyEntrypointPath = "file:///src/index.ts"; +// Go: `utils.DockerDenoDir`/`utils.DockerEszipDir` (`internal/utils/deno.go:34-35`) +// — fixed container-side paths for the docker-unbundle path, unrelated to +// deploy's `toDockerPath` host-mirroring scheme. +const DOCKER_DENO_DIR = "/home/deno"; +const DOCKER_ESZIP_DIR = "/root/eszips"; export interface DownloadFunctionsOptions { readonly functionName: Option.Option; @@ -34,15 +53,62 @@ export interface DownloadFunctionsOptions { readonly legacyBundle: boolean; } +interface DownloadRuntimeDependencies { + readonly api: ApiClient; + readonly projectRoot: string; +} + +/** Adds what the Docker-unbundle path needs beyond the server-side path. */ +interface DownloadDockerRuntimeDependencies extends DownloadRuntimeDependencies { + readonly rawArgs: ReadonlyArray; + /** + * Optional shell-specific styling hook for the `Downloading function:` + * progress line — mirrors `deploy.ts`'s `DeployFunctionsDependencies.styleEmphasis`. + * Defaults to identity (plain text); the legacy shell injects Go's bold + * styling here so the next shell stays isolated from `legacy/`-specific + * rendering. Go: `utils.Bold(slug)` (`downloadOne`, `download.go:219`). + */ + readonly styleEmphasis?: (text: string) => string; + /** + * Optional shell-specific styling hook for the `--legacy-bundle` command + * suggested inside {@link suggestLegacyBundle} — same isolation rationale + * as {@link styleEmphasis}, just a different Go colour. Go: + * `utils.Aqua("supabase functions download --legacy-bundle "+slug)` + * (`suggestLegacyBundle`, `download.go:315`). + */ + readonly styleAqua?: (text: string) => string; +} + +/** + * What {@link resolveEdgeRuntimeImage} needs to resolve the Docker + * edge-runtime image tag — split out so it's declared once instead of + * duplicated across `DownloadFunctionsDependencies`'s fields. + */ +interface EdgeRuntimeImageDependencies { + readonly projectRoot: string; + /** + * `true` in the legacy shell, `false` in `next` — forwarded verbatim to + * `loadProjectConfig`'s `goViperCompat` option (matches every other + * `functions`-family command, e.g. `deploy.ts`'s own `DeployFunctionsDependencies`). + */ + readonly goViperCompat: boolean; + /** + * Fallback edge-runtime image tag used when the project config doesn't pin + * `edge_runtime.deno_version` to `1` (which forces the older + * `DENO1_EDGE_RUNTIME_VERSION`) — mirrors `deploy.ts`'s own + * `edgeRuntimeVersion` dependency, read via + * `resolveEdgeRuntimeVersionPin` by the shell-specific handler. + */ + readonly edgeRuntimeVersion: string; +} + export interface DownloadFunctionsDependencies< ResolveError, ResolveRequirements, ProxyError, ProxyRequirements, -> { - readonly api: ApiClient; - readonly projectRoot: string; - readonly rawArgs: ReadonlyArray; +> + extends DownloadDockerRuntimeDependencies, EdgeRuntimeImageDependencies { readonly resolveProjectRef: ( projectRef: Option.Option, ) => Effect.Effect; @@ -51,7 +117,8 @@ export interface DownloadFunctionsDependencies< * child's raw stdout must not reach the terminal (it would corrupt the * JSON/NDJSON envelope, CLI-1546's "stdout is payload-only in machine * mode" invariant), so the dependency must capture/discard it (e.g. via - * `LegacyGoProxy.execCapture`) instead of inheriting stdio. + * `LegacyGoProxy.execCapture`) instead of inheriting stdio. Only invoked + * for `--legacy-bundle` today — `--use-docker` now runs natively (CLI-1963). */ readonly proxyDownload: ( flags: DownloadFunctionsOptions, @@ -60,29 +127,18 @@ export interface DownloadFunctionsDependencies< ) => Effect.Effect; } -interface DownloadRuntimeDependencies { - readonly api: ApiClient; - readonly projectRoot: string; -} - -export function makeGoProxyDownloadArgs( - flags: DownloadFunctionsOptions, +// `--legacy-bundle` is the only case `downloadFunctions()` still delegates to +// the Go binary for (CLI-1963) — `functionName` is the one remaining piece of +// user input the delegating branch needs to forward. +export function makeGoProxyLegacyBundleArgs( + functionName: Option.Option, projectRef: string, ): ReadonlyArray { const args: string[] = ["functions", "download"]; - if (Option.isSome(flags.functionName)) { - args.push(flags.functionName.value); - } - args.push("--project-ref", projectRef); - // At most one of these may reach the Go binary — it re-parses this argv - // fresh and enforces the same mutual exclusivity itself. `legacyBundle` - // takes priority since `useDocker` now defaults to `true` (CLI-1862) and - // would otherwise ride along on every `--legacy-bundle` invocation. - if (flags.legacyBundle) { - args.push("--legacy-bundle"); - } else if (flags.useDocker) { - args.push("--use-docker"); + if (Option.isSome(functionName)) { + args.push(functionName.value); } + args.push("--project-ref", projectRef, "--legacy-bundle"); return args; } @@ -127,6 +183,31 @@ function validateSlug(slug: string): Effect.Effect` argument), which fails with a plain `InvalidFunctionSlugError` and no + * "failed to download function" prefix or suggestion. + */ +function validateRemoteSlug(slug: string): Effect.Effect { + if (validateFunctionSlugMessage(slug) === undefined) { + return Effect.void; + } + + return Effect.fail( + Object.assign(new Error(`failed to download function ${slug}: ${invalidFunctionSlugDetail}`), { + suggestion: `The Supabase API returned an unexpected function slug (${slug}). Retry the command, and if this keeps happening, verify your network connection is not being intercepted before contacting Supabase support.`, + }), + ); +} + const downloadCommandPath = ["functions", "download"] as const; function validateDownloadFlags( @@ -599,9 +680,42 @@ const listRemoteFunctionSlugs = Effect.fnUntraced(function* (api: ApiClient, pro if (!Array.isArray(parsed)) { throw new Error("expected functions list response to be an array"); } - return parsed.flatMap((value) => { + // Go: `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 + // (`encoding/json`'s documented null-into-non-pointer no-op), and that + // empty slug then fails loudly downstream (`validateRemoteSlug`, + // matching Go's own per-item `ValidateFunctionSlug` in `downloadAll`, + // `download.go:182-188`) instead of silently vanishing from the list. + // Coercing here (rather than filtering the entry out, as before) + // preserves that "always surface an unexpected API response, never + // silently download fewer functions than requested" invariant — the + // exact CLI-1891 threat model `validateRemoteSlug` exists for (review + // round on CLI-1963's `functions download` port). + // + // A "slug" present but typed as something other than string/null is a + // different case: Go's generated client decodes the *entire* array in + // one `json.Unmarshal` call (`ParseV1ListAllFunctionsResponse`, + // `apps/cli-go/pkg/api/client.gen.go:22186-22208`), and a type mismatch + // on any single element fails that whole call — confirmed empirically + // (`json.Unmarshal([]byte(`+"`"+`[{"slug":"ok"},{"slug":123}]`+"`"+`), &dest)` + // returns a `*json.UnmarshalTypeError`; `dest` is partially populated in + // memory, but `ParseV1ListAllFunctionsResponse` returns before ever + // assigning `response.JSON200`, discarding it), so `V1ListAllFunctionsWithResponse` + // returns an error and `downloadAll` fails with "failed to list + // functions: ..." before downloading anything — not after downloading + // the earlier, well-formed entries. Throwing here (rather than + // coercing to `""` like the missing/null case above) preserves that + // same fail-before-any-download ordering. + return parsed.map((value) => { const slug = getObjectProperty(value, "slug"); - return typeof slug === "string" ? [slug] : []; + if (slug === null || slug === undefined) { + return ""; + } + if (typeof slug !== "string") { + throw new Error(`expected function slug to be a string, got ${typeof slug}`); + } + return slug; }); }, catch: (cause) => @@ -678,6 +792,451 @@ const downloadBody = Effect.fnUntraced(function* ( return yield* Effect.fail(new Error(`Error status ${response.status}: ${body}`)); }); +// Go: `downloadOne` (`apps/cli-go/internal/functions/download/download.go:218-245`) +// sends this request with no `Accept` header set at all (contrast +// `downloadBody` above, which requests `multipart/form-data` for the +// server-side path). This operation's generated contract marks its response +// `kind: "json"` (`packages/api/src/generated/contracts.ts`), so +// `executeRaw` would otherwise default to `Accept: application/json` here +// (`buildRequest`'s unconditional `acceptJson` for json-kind operations, +// `packages/api/src/internal/client.ts`) and risk a negotiated JSON response +// instead of the raw eszip body — overriding to `*/*` (no preference) is the +// closest equivalent this API surface has to Go sending no header at all. +// Go explicitly decodes a brotli `Content-Encoding` itself because Go's +// `http.Transport` only auto-decodes `gzip`; this TS CLI's transport +// (`effect/unstable/http`'s `FetchHttpClient`, backed by the platform +// `fetch`) already transparently decodes `br` per the Fetch spec — while +// still reporting `Content-Encoding: br` on the exposed `Response.headers` +// (confirmed empirically: a `fetch()` against a real `Content-Encoding: br` +// response returns already-decompressed bytes from `arrayBuffer()`). +// Re-running `brotliDecompressSync` here would therefore throw on +// already-decoded bytes, so this reads the body as-is and does not +// re-implement Go's manual decode step. Error prefix ("failed to get +// function body") is deliberately distinct from `downloadBody`'s ("failed to +// download function") — the two Go call sites use different wording. +const downloadEszipBody = Effect.fnUntraced(function* ( + api: ApiClient, + projectRef: string, + slug: string, +) { + const response = yield* api + .executeRaw( + operationDefinitions.v1GetAFunctionBody, + { + ref: projectRef, + function_slug: slug, + }, + { Accept: "*/*" }, + ) + .pipe(Effect.mapError((error) => mapTransportError("failed to get function body", error))); + + if (response.status !== 200) { + const body = yield* response.text.pipe(Effect.orElseSucceed(() => "")); + return yield* Effect.fail(new Error(`Error status ${response.status}: ${body}`)); + } + + return new Uint8Array( + yield* response.arrayBuffer.pipe( + Effect.mapError( + (cause) => + new Error( + `failed to download file: ${cause instanceof Error ? cause.message : String(cause)}`, + ), + ), + ), + ); +}); + +function suggestLegacyBundle( + slug: string, + styleAqua: (text: string) => string = (text) => text, +): string { + // Go: `suggestLegacyBundle` (`download.go:314-316`) — verbatim, including + // the source's own "trying running" wording and its leading newline. Go + // wraps only the suggested command itself in `utils.Aqua`, not the whole + // sentence — `styleAqua` mirrors that scope exactly. + return `\nIf your function is deployed using CLI < 1.120.0, trying running ${styleAqua(`supabase functions download --legacy-bundle ${slug}`)} instead.`; +} + +function suggestDenoV2(): string { + // Go: `suggestDenoV2` (`download.go:306-312`), verbatim including its + // trailing newline. + return "Please use deno v2 in supabase/config.toml to download this Function:\n\n[edge_runtime]\ndeno_version = 2\n"; +} + +/** + * Attaches Go's `suggestLegacyBundle` hint to any Docker-extraction failure — + * matches `downloadWithDockerUnbundle`'s `CmdSuggestion += + * suggestLegacyBundle(slug)` (`download.go:211-214`), which runs whenever + * `extractOne` fails for *any* reason (network/volume creation, container + * create/start, log streaming, container inspect), not just a non-zero exit + * code. `ensureDockerNetwork`/`ensureDockerNamedVolume` already prefix their + * own "failed to create docker network/volume: ..." context on the failures + * they raise themselves (`functions-docker.ts`), so this only normalizes + * (never re-prefixes) whatever `legacyDescribeContainerCliFailure` reports. + */ +function withLegacyBundleSuggestion(slug: string, styleAqua?: (text: string) => string) { + return (cause: unknown): Error => + Object.assign(new Error(legacyDescribeContainerCliFailure(cause)), { + suggestion: suggestLegacyBundle(slug, styleAqua), + }); +} + +/** + * Same as {@link withLegacyBundleSuggestion}, plus a `step` prefix — for + * `runChildProcess` itself, whose own failure (a spawn error, or the + * `PlatformError` `functions-docker.ts`'s hoisted `collectByteStream` erases + * to `unknown`) carries no context of its own about which command was + * running, unlike `ensureDockerNetwork`/`ensureDockerNamedVolume`'s + * self-describing errors. + */ +function withDockerStepFailure(step: string, slug: string, styleAqua?: (text: string) => string) { + return (cause: unknown): Error => + Object.assign(new Error(`${step}: ${legacyDescribeContainerCliFailure(cause)}`), { + suggestion: suggestLegacyBundle(slug, styleAqua), + }); +} + +// Go: `Config.EdgeRuntime.Image` (`extractOne`, `download.go:271`) resolves +// from `edge_runtime.deno_version` — `1` pins the older +// `DENO1_EDGE_RUNTIME_VERSION`, anything else (including unset) uses the +// project's configured/default tag (`resolveEdgeRuntimeVersion`, shared with +// `deploy.ts`). `project_id` mirrors `deploy.ts`'s own +// `deployConfig?.project_id ?? projectRef` fallback for Docker network/volume +// naming (`GetId`, `internal/utils/config.go:57-58`). Resolved once per +// invocation by the caller (`downloadFunctions`), not once per slug — Go's +// `Config` is likewise loaded once, before any per-function work. +const resolveEdgeRuntimeImage = Effect.fnUntraced(function* ( + dependencies: EdgeRuntimeImageDependencies, + projectRef: string, +) { + 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 + // target, same reasoning as `legacy-local-project-context.ts`/`start.handler.ts`). Go's + // `flags.LoadConfig` (`pkg/config/utils.go:43-48`) only ever resolves `supabase/config.toml` + // from that exact workdir, with no ancestor climb and no concept of a JSON project config — + // leaving these unset here would let an unrelated ancestor project's config win, or a stray + // `supabase/config.json` be preferred over `config.toml`, for the legacy shell's Docker + // download path specifically. The `next` shell keeps the package defaults (ancestor search, + // JSON preferred), matching its other non-Go-parity `loadProjectConfig` callers. + search: dependencies.goViperCompat ? false : undefined, + tomlOnly: dependencies.goViperCompat, + }); + // A project with no `supabase/config.toml`/`config.json` makes + // `loadProjectConfig` return `null` outright, so `denoVersion` below falls + // through to `undefined` and this always resolves the v2 default. Go's + // `flags.LoadConfig` never short-circuits like that: `Config.Load` → + // `loadFromFile` (`pkg/config/config.go:579-611`) merges the template + // defaults and enables `viper.AutomaticEnv()` with `SetEnvPrefix("SUPABASE")` + // *before* attempting to read the file — `mergeFileConfig` (`config.go:701-716`) + // simply no-ops on `os.ErrNotExist` — so `SUPABASE_EDGE_RUNTIME_DENO_VERSION=1` + // (or the same key in `supabase/.env`, via `loadNestedEnv`) still pins the + // deno-v1 image even with no config.toml on disk. Pre-existing, not + // introduced by this PR: `@supabase/config`'s `loadProjectConfig` has no + // equivalent of Go's generic `ExperimentalBindStruct`+`AutomaticEnv` field + // binding at all (it only expands literal `env(...)` references already + // written inside the TOML), so `deploy.ts`'s identical + // `resolveEdgeRuntimeVersion(deployConfig?.edge_runtime.deno_version, ...)` + // call has the same 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`, …), not duplicated + // per call site here — left open (review round on CLI-1963's `functions + // download` port). + const denoVersion = loadedConfig?.config?.edge_runtime.deno_version; + // `?? projectRef` only substitutes on `null`/`undefined`, so a config.toml + // with an explicit `project_id = ""` still resolves to the empty string + // here (`supabase_network_`/`supabase_edge_runtime_`) instead of failing + // up front. Go's `Config.Validate` rejects that same config with "Missing + // required field in config: project_id" (`pkg/config/config.go:990-991`) + // before `flags.LoadConfig` ever returns to `Run` — before any Docker/API + // work. Pre-existing and cross-cutting, not introduced by 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 a loaded 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 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 here — left open, same as the config-defaults gap + // above (review round on CLI-1963's `functions download` port). + const projectId = loadedConfig?.config?.project_id ?? projectRef; + const edgeRuntimeVersion = yield* resolveEdgeRuntimeVersion( + denoVersion, + dependencies.edgeRuntimeVersion, + ); + return { + projectId, + denoVersion, + // `edgeRuntimeImageTag` (not a bare `v${edgeRuntimeVersion}` prepend) — + // `dependencies.edgeRuntimeVersion` comes from a `.temp/edge-runtime-version` + // pin that may already carry its own `v` prefix (see the helper's doc). + // + // Single `legacyGetRegistryImageUrl` value, not the ECR→GHCR→Docker-Hub + // retry `legacyGetRegistryImageUrlCandidates` gives `start` (review round + // on CLI-1963's `functions download` port). Go's `DockerStart` resolves + // `config.Image` through `DockerResolveImageIfNotCached` + // (`internal/utils/docker.go:326-348,363-365`), which tries every + // registry candidate — including for this exact edge-runtime unbundle + // container — so an ECR outage/throttle that the previous Go-delegated + // default path would have survived can now fail this native path outright. + // Pre-existing, not introduced by this PR: `deploy.ts`'s and `serve.ts`'s + // own already-shipped native Docker paths resolve this identically + // (single-URL, no retry) — `legacyGetRegistryImageUrlCandidates` has only + // ever been wired up for `start` (see its own doc comment). Extending the + // retry to all three `functions` Docker paths is a shared, cross-cutting + // follow-up, not something to fix piecemeal for `download` alone. + // + // No `projectEnvValues` argument either (review round on CLI-1963's + // `functions download` port): Go's `flags.LoadConfig` → `loadNestedEnv` + // (`pkg/config/config.go:1220-1258`) calls `godotenv.Load` on every + // project dotenv file, which `os.Setenv`s each key into the process env + // — ambient-wins, but a `SUPABASE_INTERNAL_IMAGE_REGISTRY` set only in + // `supabase/.env` (not the ambient shell) is visible to `GetRegistry()`'s + // later `viper.GetString("INTERNAL_IMAGE_REGISTRY")` read + // (`internal/utils/docker.go:221-227`) regardless. `loadProjectConfig` + // above only uses its own dotenv read internally, for `env(...)` + // interpolation — it doesn't return the values, so this call falls back + // to `legacyGetRegistryOverride`'s ambient-only `process.env` read and + // misses a project-local registry mirror configured only via dotenv. + // Pre-existing and cross-cutting, not introduced by this PR: `deploy.ts` + // (`deploy.ts:1287`) and `serve.ts` (`serve.ts:1756`) call the same + // `legacyGetRegistryImageUrl` with no `projectEnvValues` either — the + // only caller that resolves and threads it today is `start`, via + // `legacyLoadLocalProjectContext`/`legacyGetRegistryImageUrlCandidates`. + // Loading project dotenv for every native `functions` Docker path belongs + // in the shared config-loading layer, not duplicated per call site here + // — left open, same treatment as the config-defaults/network-id-env/ + // Config.Validate gaps above. + image: legacyGetRegistryImageUrl( + `supabase/edge-runtime:${edgeRuntimeImageTag(edgeRuntimeVersion)}`, + ), + }; +}); + +interface EdgeRuntimeImage { + readonly projectId: string; + readonly denoVersion: number | undefined; + readonly image: string; +} + +// Go: `downloadWithDockerUnbundle`/`extractOne` +// (`download.go:198-282`) — downloads the function body as an eszip, writes +// it to a temp file, then runs the edge-runtime image's `unbundle` +// subcommand against it, mounting the *shared* `supabase/functions` +// directory (not the slug's own subdirectory — `download_test.go:267-271` +// asserts this explicitly). +const downloadWithDockerUnbundle = Effect.fnUntraced(function* ( + dependencies: DownloadDockerRuntimeDependencies, + edgeRuntimeImage: EdgeRuntimeImage, + projectRef: string, + slug: string, +) { + const output = yield* Output; + const styleEmphasis = dependencies.styleEmphasis ?? ((text: string) => text); + const styleAqua = dependencies.styleAqua ?? ((text: string) => text); + + // Go: `downloadOne` (`download.go:219`) — lowercase "function", distinct + // from the server-side path's "Downloading Function:" (capital F, + // `downloadWithServerSideUnbundle`, `download.go:329`). Both Go call sites + // bold the slug (`utils.Bold`); this path is new in CLI-1963, so it picks + // up the styling hook now. `downloadSingle`'s server-side path below has + // the identical gap, but predates this PR (#5527) — left as-is here. + yield* output.raw(`Downloading function: ${styleEmphasis(slug)}\n`, "stderr"); + + const eszip = yield* downloadEszipBody(dependencies.api, projectRef, slug); + + const tempDir = join(dependencies.projectRoot, "supabase", ".temp"); + yield* Effect.tryPromise({ + try: () => mkdir(tempDir, { recursive: true }), + catch: (cause) => + new Error(`failed to mkdir: ${cause instanceof Error ? cause.message : String(cause)}`), + }); + const eszipFileName = `output_${slug}.eszip`; + const eszipPath = join(tempDir, eszipFileName); + yield* Effect.tryPromise({ + try: () => writeFile(eszipPath, eszip), + catch: (cause) => + new Error( + `failed to download file: ${cause instanceof Error ? cause.message : String(cause)}`, + ), + }); + + // Go: the `defer fsys.Remove(eszipPath)` cleanup is registered right after + // the write and covers the whole of `extractOne`, including the container + // run — it fires on every return path, success or failure + // (`download.go:203-209`). `Effect.ensuring` below is the equivalent: it + // wraps every step from here on so a failure resolving the network/volume, + // spawning Docker, or a non-zero container exit all still clean up the + // temp eszip, matching Go instead of only doing so on the happy path. + // + // Go gates this on `viper.GetBool("DEBUG")` (`download.go:203`), which + // resolves an explicit `--debug=false` to `false` (cleanup runs) — a plain + // presence check would get that backwards, so this reads the last explicit + // occurrence's boolean value instead (`explicitBooleanLongFlag`), falling + // back to `false` (cleanup runs) when `--debug` never appears. `SUPABASE_DEBUG` + // env-var fallback is a separate, pre-existing gap shared with every other + // `hasGlobalLongFlag(rawArgs, "debug")` call site in this file family + // (e.g. `deploy.ts`) and the legacy debug logger itself, none of which + // currently honor it either — left open rather than fixed piecemeal here. + const debugEnabled = explicitBooleanLongFlag(dependencies.rawArgs, "debug") ?? false; + const cleanupEszip = debugEnabled + ? Effect.void + : Effect.tryPromise({ + try: () => rm(eszipPath, { force: true }), + catch: (cause) => (cause instanceof Error ? cause.message : String(cause)), + }).pipe(Effect.catch((message) => output.raw(`${message}\n`, "stderr"))); + + const { projectId, denoVersion, image } = edgeRuntimeImage; + const functionsDir = resolve(dependencies.projectRoot, "supabase", "functions"); + const hostEszipPath = resolve(eszipPath); + const dockerEszipPath = posix.join(DOCKER_ESZIP_DIR, eszipFileName); + const dockerOutputPath = posix.join(DOCKER_DENO_DIR, slug); + + // Go: `viper.GetString("network-id")` else `NetId` (`docker.go:379-383`) — + // `--network-id` is a persistent root flag (`cmd/root.go:328`), not + // registered on `functions download` itself. Go only treats the override as + // set when `len(networkId) > 0`, so an explicit-but-empty `--network-id=` + // must fall through to the generated network name too, not just an omitted + // flag — `explicitNonEmptyStringFlag` (unlike the unexported + // `explicitStringFlag`) treats that case as unset for exactly this reason. + // + // Go's root `init()` also binds every persistent flag (including + // `network-id`) through `viper.BindPFlags` after enabling + // `viper.AutomaticEnv()` with `SetEnvPrefix("SUPABASE")` and a `-`→`_` + // replacer (`cmd/root.go:316-334`), so `SUPABASE_NETWORK_ID` overrides the + // flag's empty default whenever `--network-id` itself is never passed — + // this raw-argv-only lookup has no equivalent env-var fallback. Pre-existing, + // not introduced by this PR: `start.handler.ts`'s `LegacyNetworkIdFlag` and + // `deploy.ts`'s/`serve.ts`'s own network-id resolution don't check + // `SUPABASE_NETWORK_ID` either — no native command does today. A fix + // belongs in one shared place for the global `--network-id` resolution, + // not duplicated per Docker-path call site here — left open (review round + // on CLI-1963's `functions download` port). + const networkMode = + explicitNonEmptyStringFlag(dependencies.rawArgs, "network-id") ?? + localDockerId("network", projectId); + + const extract = Effect.gen(function* () { + yield* ensureDockerNetwork(networkMode, projectId).pipe( + Effect.mapError(withLegacyBundleSuggestion(slug, styleAqua)), + ); + yield* ensureDockerNamedVolume(localDockerId("edge_runtime", projectId), projectId).pipe( + Effect.mapError(withLegacyBundleSuggestion(slug, styleAqua)), + ); + + // Bind order matches `extractOne` (`download.go:260-266`) exactly. 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 :...` would otherwise still + // implicitly create the named volume, which Bitbucket's restricted Docker + // environment doesn't allow, same carve-out as `deploy.ts`'s + // `buildDockerBinds`. + const binds = [ + ...(process.env["BITBUCKET_CLONE_DIR"] === undefined + ? [`${localDockerId("edge_runtime", projectId)}:/root/.cache/deno:rw`] + : []), + `${hostEszipPath}:${dockerEszipPath}:ro`, + `${functionsDir}:${DOCKER_DENO_DIR}:rw`, + ]; + // No `com.supabase.cli.project`/`com.docker.compose.project` labels on + // this container itself — Go's `DockerStart` (`internal/utils/docker.go:372-376`) + // sets both unconditionally on `config.Labels` for every container it + // starts via the Engine API, including this exact unbundle container + // (`DockerRunOnceWithConfig` → `DockerStart`, `download.go:268`), so + // label-based cleanup/inspection can't associate an orphaned one-shot + // container with the project if the CLI is interrupted mid-run. Pre-existing + // and cross-cutting, not introduced by this PR: `deploy.ts`'s own + // `bundleFunctionWithDocker` builds an equally raw `docker run` command + // (its own `command.push(image, "bundle", ...)`) with the identical gap — + // only `ensureDockerNetwork`/`ensureDockerNamedVolume` (`functions-docker.ts`) + // thread `dockerProjectLabels` today, for the network/volume they create, + // not for the one-shot containers either Docker path runs. Adding `--label` + // to every `functions` Docker `run` invocation belongs in a shared + // container-build helper both call sites use, not duplicated per call site + // here — left open (review round on CLI-1963's `functions download` port). + const command = [ + "run", + "--rm", + ...binds.flatMap((bind) => ["-v", bind]), + "--network", + networkMode, + ]; + if (process.platform === "linux") { + command.push("--add-host", "host.docker.internal:host-gateway"); + } + command.push(image, "unbundle", "--eszip", dockerEszipPath, "--output", dockerOutputPath); + + // Go pipes the container's stdout/stderr straight to `os.Stdout`/`getErrorLogger()` + // while the container runs (`DockerRunOnceWithConfig`, copied live via the + // log stream) — this awaits `runChildProcess`, which buffers the whole + // run via `collectByteStream`'s `Stream.runFold` and only writes below + // once the process exits, so live progress/error output is hidden and + // stdout/stderr ordering can't be preserved relative to each other while + // the container is still running. Pre-existing and cross-cutting, not + // introduced by this PR: `deploy.ts`'s `bundleFunctionWithDocker` (added + // in #5561, before `functions-docker.ts` existed as its own file) calls + // the exact same `runChildProcess` helper the exact same way for its own + // bundler container. A real fix needs a streaming variant of + // `runChildProcess` used by both `functions` Docker paths, not a + // one-off change here — left open (review round on CLI-1963's `functions + // download` port). + const result = yield* runChildProcess("docker", command, { + stdout: "pipe", + stderr: "pipe", + }).pipe( + Effect.mapError( + withDockerStepFailure("failed to run the edge-runtime unbundle container", slug, styleAqua), + ), + ); + + // Go pipes the container's stdout straight to `os.Stdout` (`download.go:279`); + // machine-output modes must keep stdout payload-only (CLI-1546), so this + // mirrors `deploy.ts`'s own `bundleFunctionWithDocker` routing. + if (result.stdout.length > 0) { + yield* output.raw(result.stdout, output.format === "text" ? "stdout" : "stderr"); + } + if (result.stderr.length > 0) { + yield* output.raw(result.stderr, "stderr"); + } + + if (result.exitCode !== 0) { + // Go's `getErrorLogger` (deno-v1 only) sets `CmdSuggestion = + // suggestDenoV2()` (assignment) as soon as a full stderr line reads + // "invalid eszip v2" (case-insensitive), then `downloadWithDockerUnbundle` + // appends `suggestLegacyBundle` (`+=`) once extraction has failed + // (`download.go:213,284-304`). Go's own implementation races these two + // goroutines (the pipe writer is never closed) — this resolves that + // race deterministically to the common (non-race) ordering instead of + // reproducing the nondeterminism. The line match is exact (not a + // substring) to match Go's `strings.EqualFold(line, "invalid eszip v2")`. + const invalidEszipV2 = + denoVersion === 1 && + result.stderr + .split(/\r?\n/) + .some((line) => line.trim().toLowerCase() === "invalid eszip v2"); + const suggestion = + (invalidEszipV2 ? suggestDenoV2() : "") + suggestLegacyBundle(slug, styleAqua); + return yield* Effect.fail( + Object.assign(new Error(`error running container: exit ${result.exitCode}`), { + suggestion, + }), + ); + } + + // Go: `downloadWithDockerUnbundle` has no final "Downloaded Function ..." + // print, unlike `RunLegacy`/`downloadWithServerSideUnbundle` — its only + // stdout/stderr text is "Downloading function: ..." above plus whatever + // the `unbundle` container itself wrote. + return slug; + }); + + return yield* extract.pipe(Effect.ensuring(cleanupEszip)); +}); + const downloadSingle = Effect.fnUntraced(function* ( dependencies: DownloadRuntimeDependencies, projectRef: string, @@ -764,11 +1323,17 @@ export function downloadFunctions MAX_PROJECT_ID_LENGTH + ? sanitized.slice(0, MAX_PROJECT_ID_LENGTH) + : sanitized; +} + +export function localDockerId(name: string, projectId: string) { + return `supabase_${name}_${normalizeProjectId(projectId)}`; +} + +const dockerCliProjectLabel = "com.supabase.cli.project"; +const dockerComposeProjectLabel = "com.docker.compose.project"; + +export function dockerProjectLabels(projectId: string) { + return { + [dockerCliProjectLabel]: projectId, + [dockerComposeProjectLabel]: projectId, + }; +} + +export function toDockerPath(hostPath: string) { + const normalized = toSlash(resolve(hostPath)); + return normalized.replace(/^[A-Za-z]:/, ""); +} + +function collectByteStream(stream: Stream.Stream) { + const decoder = new TextDecoder(); + return Stream.runFold( + stream, + () => "", + (text, chunk) => text + decoder.decode(chunk, { stream: true }), + ).pipe(Effect.map((text) => text + decoder.decode())); +} + +// Runs a container CLI command and collects its output. Every caller runs +// `docker`, so the spawn goes through `spawnContainerCli` to fall back to +// `podman` on Docker-less hosts. `command` is retained for the extendEnv +// default and the `functions serve` dependency-injection seam. +export const runChildProcess = Effect.fnUntraced(function* ( + command: string, + args: ReadonlyArray, + opts: { + readonly stdout?: "pipe" | "ignore"; + readonly stderr?: "pipe" | "ignore"; + readonly env?: Readonly>; + readonly extendEnv?: boolean; + } = {}, +) { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const child = yield* spawnContainerCli(spawner, [...args], { + stdin: "ignore", + stdout: opts.stdout ?? "pipe", + stderr: opts.stderr ?? "pipe", + env: opts.env, + extendEnv: opts.extendEnv ?? command === "docker", + }); + + const [stdout, stderr, exitCode] = yield* Effect.all( + [ + opts.stdout === "ignore" ? Effect.succeed("") : collectByteStream(child.stdout), + opts.stderr === "ignore" ? Effect.succeed("") : collectByteStream(child.stderr), + child.exitCode.pipe(Effect.map(Number)), + ], + { concurrency: "unbounded" }, + ); + return { exitCode, stdout, stderr }; +}); + +// Go: `container.NetworkMode.IsContainer()` (`docker/api/types/container/hostconfig.go:152-155`, +// via the unexported `containerID` helper, same file:493-499) — `--network container:` +// (Docker's syntax for attaching to another container's network stack) is recognized by a bare +// `"container:"` prefix before the first `:`, regardless of what (if anything) follows it. +function isContainerDockerNetworkMode(networkMode: string) { + const separatorIndex = networkMode.indexOf(":"); + return separatorIndex !== -1 && networkMode.slice(0, separatorIndex) === "container"; +} + +// Go: `container.NetworkMode.IsUserDefined()` (`docker/api/types/container/hostconfig_unix.go:23-25`) +// — `!IsDefault() && !IsBridge() && !IsHost() && !IsNone() && !IsContainer()`. Omitting the +// `IsContainer()` exclusion would make `DockerNetworkCreateIfNotExists` +// (`internal/utils/docker.go:63`) run `docker network inspect`/`create` against a +// `container:` mode, which isn't a network name at all — Go passes that mode straight +// through to the container's `NetworkMode` without ever touching the network subsystem. +export function isUserDefinedDockerNetwork(networkMode: string) { + return ( + networkMode.length > 0 && + networkMode !== "default" && + networkMode !== "bridge" && + networkMode !== "host" && + networkMode !== "none" && + !isContainerDockerNetworkMode(networkMode) + ); +} + +export const ensureDockerNetwork = Effect.fnUntraced(function* ( + networkMode: string, + projectId: string, +) { + if (!isUserDefinedDockerNetwork(networkMode)) { + return; + } + + const inspect = yield* runChildProcess("docker", ["network", "inspect", networkMode], { + stdout: "ignore", + stderr: "ignore", + }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); + if (inspect.exitCode === 0) { + return; + } + + const labels = dockerProjectLabels(projectId); + const create = yield* runChildProcess( + "docker", + [ + "network", + "create", + "--label", + `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, + "--label", + `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, + networkMode, + ], + { + stdout: "ignore", + stderr: "pipe", + }, + ); + if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { + return yield* Effect.fail(new Error(`failed to create docker network: ${networkMode}`)); + } +}); + +export const ensureDockerNamedVolume = Effect.fnUntraced(function* ( + volumeName: string, + projectId: string, +) { + if (process.env["BITBUCKET_CLONE_DIR"] !== undefined) { + return; + } + + const labels = dockerProjectLabels(projectId); + const create = yield* runChildProcess( + "docker", + [ + "volume", + "create", + "--label", + `${dockerCliProjectLabel}=${labels[dockerCliProjectLabel]}`, + "--label", + `${dockerComposeProjectLabel}=${labels[dockerComposeProjectLabel]}`, + volumeName, + ], + { + stdout: "ignore", + stderr: "pipe", + }, + ); + if (create.exitCode !== 0 && !create.stderr.includes("already exists")) { + return yield* Effect.fail(new Error(`failed to create docker volume: ${volumeName}`)); + } +}); + +export const isDockerRunning = Effect.fnUntraced(function* () { + const result = yield* runChildProcess("docker", ["info"], { + stdout: "ignore", + stderr: "ignore", + }).pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: "", stderr: "" }))); + return result.exitCode === 0; +}); + +export function resolveEdgeRuntimeVersion( + denoVersion: number | undefined, + defaultVersion: string, +): Effect.Effect { + if (denoVersion === undefined || denoVersion === 2) { + return Effect.succeed(defaultVersion); + } + if (denoVersion === 1) { + return Effect.succeed(DENO1_EDGE_RUNTIME_VERSION); + } + return Effect.fail( + new Error(`Failed reading config: Invalid edge_runtime.deno_version: ${denoVersion}.`), + ); +} + +/** + * Formats a resolved edge-runtime version as a Docker tag, tolerating a + * pin that's already `v`-prefixed. `resolveEdgeRuntimeVersion`'s own + * defaults are bare (`"1.74.2"`, `DENO1_EDGE_RUNTIME_VERSION`), but a value + * sourced from `supabase/.temp/edge-runtime-version` can legitimately be + * either form — Go's `replaceImageTag` (`pkg/config/utils.go:81-84`) appends + * the pin file's raw content verbatim after the image's `:`, and both forms + * are exercised elsewhere in this codebase (`legacy-edge-runtime-image.ts`'s + * own `replaceImageTag` port, and its and `services.integration.test.ts`'s + * `"v9.9.9"` fixtures alongside `deploy.integration.test.ts`'s bare + * `"9.9.9"`). Blindly prepending `v` — as every caller below did before this + * helper existed — double-prefixes an already-`v`-prefixed pin + * (`supabase/edge-runtime:vv9.9.9`), which docker then simply fails to pull. + */ +export function edgeRuntimeImageTag(version: string): string { + return version.startsWith("v") ? version : `v${version}`; +} diff --git a/apps/cli/src/shared/functions/functions.shared.ts b/apps/cli/src/shared/functions/functions.shared.ts index 8c961e867d..731785e7d6 100644 --- a/apps/cli/src/shared/functions/functions.shared.ts +++ b/apps/cli/src/shared/functions/functions.shared.ts @@ -1,3 +1,8 @@ +import { DEFAULT_VERSIONS } from "@supabase/stack/effect"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; +import { Effect } from "effect"; + const functionSlugPattern = /^[A-Za-z][A-Za-z0-9_-]*$/; export const invalidFunctionSlugDetail = @@ -16,3 +21,20 @@ export const FUNCTIONS_PROJECT_REF_SAFE_FLAGS = ["project-ref"] as const; // `MarkFlagsMutuallyExclusive("use-api", "use-docker", "legacy-bundle")` // (`cmd/functions.go:158,182`). export const FUNCTIONS_BUNDLER_MUTEX_GROUP = ["use-api", "use-docker", "legacy-bundle"] as const; + +/** + * Go: `Config.EdgeRuntime.Image` reflects `supabase/.temp/edge-runtime-version` + * when present (`pkg/config/config.go:847-849`) — shared by every `functions` + * command that resolves a Docker edge-runtime image (`deploy`, `download`) in + * both shells, so this is the single home for the file-read rather than four + * copies of the same `readFile` -> `trim` -> fallback pipeline. + */ +export const resolveEdgeRuntimeVersionPin = Effect.fnUntraced(function* (supabaseDir: string) { + return yield* Effect.tryPromise(() => + readFile(join(supabaseDir, ".temp", "edge-runtime-version"), "utf8"), + ).pipe( + Effect.map((version) => version.trim()), + Effect.catch(() => Effect.succeed("")), + Effect.map((version) => version || DEFAULT_VERSIONS["edge-runtime"]), + ); +}); diff --git a/apps/cli/src/shared/functions/serve.ts b/apps/cli/src/shared/functions/serve.ts index a4aab1f19f..725e27c70f 100644 --- a/apps/cli/src/shared/functions/serve.ts +++ b/apps/cli/src/shared/functions/serve.ts @@ -51,19 +51,22 @@ import { discoverFunctionSlugs, dockerBindContainerPath, dockerBindHostPath, - dockerProjectLabels, dockerWorkdirLabel, + rawFunctionConfigRecord, + resolveFunctionConfigs, + type ResolvedDeployFunctionConfig, +} from "./deploy.ts"; +import { + dockerProjectLabels, + edgeRuntimeImageTag, ensureDockerNamedVolume, ensureDockerNetwork, localDockerId, normalizeProjectId, - rawFunctionConfigRecord, resolveEdgeRuntimeVersion, - resolveFunctionConfigs, runChildProcess, toDockerPath, - type ResolvedDeployFunctionConfig, -} from "./deploy.ts"; +} from "./functions-docker.ts"; const decodeProjectConfig = Schema.decodeUnknownSync(ProjectConfigSchema); const defaultProjectConfig = decodeProjectConfig({}); @@ -1380,10 +1383,6 @@ async function writeServeMainTemplateFile(template: string, dir: string) { return { bind: `${pathname}:${serveMainContainerPath}:ro,Z` } as const; } -function edgeRuntimeImageTag(version: string) { - return version.startsWith("v") ? version : `v${version}`; -} - const resolveServeFunctionConfigs = Effect.fnUntraced(function* ( projectRoot: string, supabaseDir: string, diff --git a/apps/cli/src/shared/output/normalize-error.ts b/apps/cli/src/shared/output/normalize-error.ts index 7d0976dd05..c40f72a34e 100644 --- a/apps/cli/src/shared/output/normalize-error.ts +++ b/apps/cli/src/shared/output/normalize-error.ts @@ -194,12 +194,18 @@ export function normalizeCliError( const code = readString(error, "_tag") ?? "UnknownError"; const message = readString(error, "message") ?? readString(error, "detail") ?? code; const detail = readString(error, "detail"); - const suggestion = readString(error, "suggestion"); + // Raw read: some producers' suggestion text is meaningful leading/trailing + // whitespace, not incidental — e.g. `suggestLegacyBundle`'s Go-parity + // string (`shared/functions/download.ts`) starts with `\n` to reproduce + // Go's blank separator line before the hint (`cmd/root.go:301-302`, + // `Fprintln(os.Stderr, CmdSuggestion)`). `readString` would trim exactly + // that away. + const suggestion = readRawString(error, "suggestion"); return { code, message, ...(detail && detail !== message ? { detail } : {}), - ...(suggestion ? { suggestion } : {}), + ...(suggestion !== undefined && suggestion.length > 0 ? { suggestion } : {}), }; }