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 7a70bcb517..9ed1bce3aa 100644 --- a/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/functions/download/SIDE_EFFECTS.md @@ -22,9 +22,17 @@ ## Subprocesses -| Command | When | Purpose | -| ------------------------------------ | ----------------------------------- | ----------------------------------- | -| `supabase-go functions download ...` | `--use-docker` or `--legacy-bundle` | preserve hidden compatibility modes | +| 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. ## Environment Variables @@ -50,11 +58,13 @@ Prints progress and success messages as functions are downloaded. ### `--output-format json` -Prints a structured success result with the downloaded function slugs and project ref. +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. ### `--output-format stream-json` -Prints a structured success result with the downloaded function slugs and project ref. +Same envelope as `json` above (including on the proxy path). ## Notes @@ -62,4 +72,7 @@ Prints a structured success result with the downloaded function slugs and projec - 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. diff --git a/apps/cli/src/legacy/commands/functions/download/download.command.ts b/apps/cli/src/legacy/commands/functions/download/download.command.ts index 0d3f559dd0..1024b268fe 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.command.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.command.ts @@ -19,6 +19,7 @@ const config = { ), useDocker: Flag.boolean("use-docker").pipe( Flag.withDescription("Use Docker to unbundle functions locally."), + Flag.withDefault(true), Flag.withHidden, ), legacyBundle: Flag.boolean("legacy-bundle").pipe( diff --git a/apps/cli/src/legacy/commands/functions/download/download.e2e.test.ts b/apps/cli/src/legacy/commands/functions/download/download.e2e.test.ts new file mode 100644 index 0000000000..f52a23d3be --- /dev/null +++ b/apps/cli/src/legacy/commands/functions/download/download.e2e.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "vitest"; +import { makeTempHome, runSupabase } from "../../../../../tests/helpers/cli.ts"; + +// Argument-validation negatives for `functions download`. This validation +// lives in the Go CLI today (the legacy TS command proxies to it); a +// black-box subprocess test keeps these assertions valid through the +// eventual native TS port — it guards behavior, not implementation. Mirrors +// `deploy.e2e.test.ts`'s coverage for the sibling `--use-docker` default bug +// (CLI-1862). +// +// The mutex-conflict cases below fail before any network call (flag parsing +// / mutex validation), so no auth or linked project is required. + +const E2E_TIMEOUT_MS = 30_000; +const SLUG = "download-e2e-basic"; +const FAKE_TOKEN = `sbp_${"0".repeat(40)}`; +const FAKE_REF = "a".repeat(20); + +describe("supabase functions download (legacy) — argument validation", () => { + const conflicts = [ + { name: "--use-api + --use-docker", flags: ["--use-api", "--use-docker"] }, + { name: "--use-api + --legacy-bundle", flags: ["--use-api", "--legacy-bundle"] }, + { name: "--use-docker + --legacy-bundle", flags: ["--use-docker", "--legacy-bundle"] }, + ] as const; + + for (const { name, flags } of conflicts) { + test(`rejects ${name} as mutually exclusive`, { timeout: E2E_TIMEOUT_MS }, async () => { + using home = makeTempHome(); + const { exitCode, stderr } = await runSupabase( + ["functions", "download", SLUG, "--project-ref", FAKE_REF, ...flags], + { + entrypoint: "legacy", + home: home.dir, + env: { HOME: home.dir, SUPABASE_ACCESS_TOKEN: FAKE_TOKEN }, + }, + ); + expect(exitCode).not.toBe(0); + expect(stderr).toMatch(/none of the others can be|mutually exclusive/i); + }); + } + + // CLI-1862: `--use-docker` now defaults to `true` (Go parity). Before the + // fix, that default was counted as "explicitly selected" by the mutex + // check, so passing `--use-api` alone was incorrectly rejected as + // conflicting with the (unpassed) `--use-docker` default. Covered in + // `download.integration.test.ts` ("does not treat the --use-docker default + // as conflicting with an explicit --use-api") via a mocked platform API + // instead of here: now that the mutex check is fixed, `--use-api` alone + // passes validation and proceeds to the native downloader, which calls the + // real Management API with the fake token/ref — an argument-validation + // test shouldn't depend on that network round-trip. + + // CLI-1862: the TS→Go proxy call must not forward the now-defaulted + // `--use-docker` alongside an explicit `--legacy-bundle` — the Go binary + // re-parses this argv itself and enforces the same mutual exclusivity, so + // forwarding both breaks `--legacy-bundle` outright. Covered in + // `download.integration.test.ts` ("forwards only --legacy-bundle to the Go + // proxy...") via a mocked `LegacyGoProxy` instead of here: unlike + // `--use-api`, `--legacy-bundle` routes to the Go binary's `RunLegacy` + // downloader, which calls `InstallOrUpgradeDeno` before any network call + // (`apps/cli-go/internal/functions/download/download.go`). Each e2e run + // gets a fresh `SUPABASE_HOME`, so this would trigger a real, uncached + // Deno download from GitHub on every run — a real cross-boundary + // dependency this suite shouldn't take on to prove a pure TS-side routing + // decision. +}); 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 70cb9da73c..4b666da7f6 100644 --- a/apps/cli/src/legacy/commands/functions/download/download.handler.ts +++ b/apps/cli/src/legacy/commands/functions/download/download.handler.ts @@ -1,4 +1,4 @@ -import { Effect, Option } from "effect"; +import { Effect, Option, Stdio } from "effect"; import { downloadFunctions, makeGoProxyDownloadArgs, @@ -20,11 +20,14 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu const linkedProjectCache = yield* LegacyLinkedProjectCache; const telemetryState = yield* LegacyTelemetryState; const proxy = yield* LegacyGoProxy; + const stdio = yield* Stdio.Stdio; + const rawArgs = yield* stdio.args; let resolvedProjectRef = Option.none(); yield* downloadFunctions(flags, { api, projectRoot: cliConfig.workdir, + rawArgs, resolveProjectRef: (projectRef) => resolver.resolve(projectRef).pipe( Effect.tap((ref) => @@ -33,8 +36,23 @@ export const legacyFunctionsDownload = Effect.fn("legacy.functions.download")(fu }), ), ), - proxyDownload: (proxyFlags, projectRef) => - proxy.exec(makeGoProxyDownloadArgs(proxyFlags, projectRef)), + // The delegated Go binary runs its own `Execute()` and would otherwise + // fire its own `cli_command_executed` on top of this command's own + // `withLegacyCommandInstrumentation` wrapper. Suppress it so proxied + // invocations record exactly one event, matching Go (mirrors `db pull` / + // `db diff`'s delegated-call pattern). + // + // In machine-output mode the child's stdout is captured and discarded + // instead of inherited, matching `db pull`/`db diff`'s delegated-call + // 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 env = { SUPABASE_TELEMETRY_DISABLED: "1" }; + return captureOutput + ? Effect.asVoid(proxy.execCapture(args, { env, stdin: "ignore" })) + : proxy.exec(args, { env }); + }, }).pipe( Effect.ensuring( Effect.suspend(() => 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 a6384d4e06..74948f7583 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,7 @@ import { describe, expect, it } from "@effect/vitest"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; -import { Effect, Layer, Option } from "effect"; +import { Effect, Exit, Layer, Option, Stdio } from "effect"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; import { @@ -53,14 +53,26 @@ function multipartResponse(request: Parameters> = []; + const envs: Array | undefined> = []; + const captureCalls: Array> = []; + const captureEnvs: Array | undefined> = []; return { calls, + envs, + captureCalls, + captureEnvs, layer: Layer.succeed(LegacyGoProxy, { - exec: (args) => + exec: (args, opts) => Effect.sync(() => { calls.push([...args]); + envs.push(opts?.env); + }), + execCapture: (args, opts) => + Effect.sync(() => { + captureCalls.push([...args]); + captureEnvs.push(opts?.env); + return ""; }), - execCapture: () => Effect.succeed(""), }), }; } @@ -86,6 +98,15 @@ describe("legacy functions download", () => { telemetry: telemetry.layer, }), proxy.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + "abcdefghijklmnopqrst", + ]), + }), ); return Effect.gen(function* () { @@ -108,7 +129,7 @@ describe("legacy functions download", () => { }).pipe(Effect.provide(layer)); }); - it.live("keeps hidden Docker compatibility mode behind the Go proxy", () => { + it.live("proxies to Docker by default (Go parity), with no flags passed", () => { const out = mockOutput({ format: "text" }); const api = mockLegacyPlatformApi(); const proxy = mockProxy(); @@ -119,9 +140,20 @@ describe("legacy functions download", () => { cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), }), proxy.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "hello-world", + "--project-ref", + "abcdefghijklmnopqrst", + ]), + }), ); 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 }); expect(api.requests).toEqual([]); @@ -135,6 +167,395 @@ describe("legacy functions download", () => { "--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)); + }); + + it.live( + "does not treat the --use-docker default as conflicting with an explicit --use-api", + () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + handler: (request) => + request.url.endsWith("/body") + ? Effect.succeed(multipartResponse(request)) + : Effect.succeed(legacyJsonResponse(request, 200, {})), + }); + 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", + "--project-ref", + "abcdefghijklmnopqrst", + ]), + }), + ); + + return Effect.gen(function* () { + // The CLI parser resolves `useDocker: true` here too (its default), + // even though only `--use-api` was passed explicitly. Neither the + // mutex check nor the routing decision should treat that default as + // if the user had asked for Docker. + yield* legacyFunctionsDownload({ ...baseFlags, useApi: true, useDocker: true }); + + expect(proxy.calls).toEqual([]); + expect( + yield* Effect.tryPromise(() => + readFile( + join(tempRoot.current, "supabase", "functions", "hello-world", "index.ts"), + "utf8", + ), + ), + ).toBe("console.log('legacy native')"); + }).pipe(Effect.provide(layer)); + }, + ); + + 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", + () => { + 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 proxy = mockProxy(); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.layer, + Stdio.layerTest({ + args: Effect.succeed([ + "functions", + "download", + "--project-ref", + "abcdefghijklmnopqrst", + "--output-format", + "json", + ]), + }), + ); + + return Effect.gen(function* () { + yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.none(), + 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", + }, + }), + ); + }).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(); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.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 layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + }), + proxy.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("forwards only --legacy-bundle to the Go proxy, not the --use-docker default too", () => { + 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", + "--legacy-bundle", + "--project-ref", + "abcdefghijklmnopqrst", + ]), + }), + ); + + return Effect.gen(function* () { + // `useDocker: true` mirrors the CLI parser's default (CLI-1862) even + // though only `--legacy-bundle` was passed explicitly. The Go proxy + // call must not forward both, or the Go binary's own + // MarkFlagsMutuallyExclusive rejects the combination. + yield* legacyFunctionsDownload({ ...baseFlags, useDocker: true, legacyBundle: true }); + + expect(proxy.calls).toEqual([ + [ + "functions", + "download", + "hello-world", + "--project-ref", + "abcdefghijklmnopqrst", + "--legacy-bundle", + ], + ]); + expect(proxy.envs).toEqual([{ SUPABASE_TELEMETRY_DISABLED: "1" }]); + }).pipe(Effect.provide(layer)); + }); + + it.live("rejects an invalid slug before ever reaching the Go proxy", () => { + 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", + "../../etc", + "--project-ref", + "abcdefghijklmnopqrst", + ]), + }), + ); + + return Effect.gen(function* () { + // `useDocker: true` is the real default (CLI-1862). Before this fix, + // slug validation only ran on the native path, so a malformed slug + // would sail past it and straight into the Go proxy argv. + const exit = yield* legacyFunctionsDownload({ + ...baseFlags, + functionName: Option.some("../../etc"), + useDocker: true, + }).pipe(Effect.exit); + + expect(Exit.isFailure(exit)).toBe(true); + expect(proxy.calls).toEqual([]); }).pipe(Effect.provide(layer)); }); }); 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 02b9aed1b2..15c6238e84 100644 --- a/apps/cli/src/next/commands/functions/download/download.handler.ts +++ b/apps/cli/src/next/commands/functions/download/download.handler.ts @@ -1,4 +1,4 @@ -import { Effect } from "effect"; +import { Effect, Stdio } from "effect"; import { PlatformApi } from "../../../auth/platform-api.service.ts"; import { ProjectHome } from "../../../config/project-home.service.ts"; import { @@ -13,12 +13,23 @@ export const functionsDownload = Effect.fnUntraced(function* (flags: FunctionsDo const api = yield* PlatformApi; const projectHome = yield* ProjectHome; const proxy = yield* LegacyGoProxy; + const stdio = yield* Stdio.Stdio; + const rawArgs = yield* stdio.args; yield* downloadFunctions(flags, { api, projectRoot: projectHome.projectRoot, + rawArgs, resolveProjectRef, - proxyDownload: (proxyFlags, projectRef) => - proxy.exec(makeGoProxyDownloadArgs(proxyFlags, projectRef), { cwd: projectHome.projectRoot }), + // 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 cwd = projectHome.projectRoot; + return captureOutput + ? Effect.asVoid(proxy.execCapture(args, { cwd, stdin: "ignore" })) + : proxy.exec(args, { cwd }); + }, }); }); 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 b8095be511..4aee254ab6 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 @@ -4,7 +4,7 @@ import { existsSync, mkdtempSync } from "node:fs"; import { mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { Effect, Layer, Option } from "effect"; +import { Effect, Layer, Option, Stdio } from "effect"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; @@ -276,6 +276,7 @@ function setup( format?: "text" | "json" | "stream-json"; linked?: boolean; projectRoot?: string; + rawArgs?: ReadonlyArray; } = {}, ) { const out = mockOutput({ format: opts.format ?? "text", interactive: false }); @@ -289,6 +290,7 @@ function setup( mockRuntimeInfo({ cwd }), mockProjectLinkState(opts.linked === false ? undefined : LINK_STATE), mockProjectHome(opts.projectRoot ?? cwd), + Stdio.layerTest({ args: Effect.succeed(opts.rawArgs ?? []) }), ); return { out, api, layer, proxy }; @@ -296,17 +298,25 @@ function setup( function mockLegacyGoProxy() { const calls: string[][] = []; + const captureCalls: string[][] = []; return { layer: Layer.succeed(LegacyGoProxy, { exec: (args: ReadonlyArray) => Effect.sync(() => { calls.push([...args]); }), - execCapture: () => Effect.succeed(""), + execCapture: (args: ReadonlyArray) => + Effect.sync(() => { + captureCalls.push([...args]); + return ""; + }), }), get calls() { return calls; }, + get captureCalls() { + return captureCalls; + }, }; } @@ -803,6 +813,7 @@ describe("functions download", () => { bodyBySlug: { "hello-world": multipart, }, + rawArgs: ["functions", "download", "hello-world", "--use-api"], }); yield* functionsDownload({ @@ -821,7 +832,9 @@ describe("functions download", () => { return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer, proxy } = setup(tempDir); + const { layer, proxy } = setup(tempDir, { + rawArgs: ["functions", "download", "hello-world", "--legacy-bundle"], + }); yield* functionsDownload({ ...BASE_FLAGS, @@ -841,7 +854,9 @@ describe("functions download", () => { return Effect.gen(function* () { yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); - const { layer, proxy } = setup(tempDir); + const { layer, proxy } = setup(tempDir, { + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + }); yield* functionsDownload({ ...BASE_FLAGS, @@ -856,11 +871,157 @@ describe("functions download", () => { ); }); + it.live("captures the Go proxy's output and emits a JSON envelope in machine mode", () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer, proxy } = setup(tempDir, { + format: "json", + rawArgs: ["functions", "download", "hello-world", "--use-docker"], + }); + + // 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, + 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(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( + "lists remote functions before delegating when no function name is given in machine mode", + () => { + const tempDir = makeTempDir(); + + 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"], + }); + + 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, + }, + }), + ); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live( + "reports no functions found without delegating when the project is empty in machine mode", + () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { out, layer, proxy } = setup(tempDir, { + format: "json", + list: [], + rawArgs: ["functions", "download", "--use-docker"], + }); + + // 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* functionsDownload({ + ...BASE_FLAGS, + functionName: Option.none(), + useDocker: true, + }).pipe(Effect.provide(layer)); + + 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: PROJECT_REF }, + }), + ); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }, + ); + + it.live("fails before delegating when the pre-flight function list fails in machine mode", () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempDir)); + const { layer, proxy } = setup(tempDir, { + format: "json", + listStatus: 503, + listBody: { message: "unavailable" }, + rawArgs: ["functions", "download", "--use-docker"], + }); + + // 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 error = yield* functionsDownload({ + ...BASE_FLAGS, + functionName: Option.none(), + useDocker: true, + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect(proxy.calls).toEqual([]); + expect(proxy.captureCalls).toEqual([]); + }).pipe( + Effect.ensuring(Effect.tryPromise(() => rm(tempDir, { recursive: true, force: true }))), + ); + }); + it.live("rejects mutually exclusive compatibility flags", () => { const tempDir = makeTempDir(); return Effect.gen(function* () { - const { api, layer, proxy } = setup(tempDir); + const { api, layer, proxy } = setup(tempDir, { + rawArgs: ["functions", "download", "hello-world", "--use-api", "--legacy-bundle"], + }); const error = yield* functionsDownload({ ...BASE_FLAGS, diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 46bab06023..e0f828c340 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -16,7 +16,11 @@ import { Output } from "../output/output.service.ts"; import { spawnContainerCli } from "../../legacy/shared/legacy-container-cli.ts"; import { legacyGetRegistryImageUrl } from "../../legacy/shared/legacy-docker-registry.ts"; import { findGitRootPath } from "../git/git-root.ts"; -import { invalidFunctionSlugDetail, validateFunctionSlugMessage } from "./functions.shared.ts"; +import { + hasExplicitLongFlag, + invalidFunctionSlugDetail, + validateFunctionSlugMessage, +} from "./functions.shared.ts"; import { ConflictingFunctionDeployFlagsError, FunctionDeployCancelledError, @@ -192,30 +196,6 @@ function validateDeploySlug(slug: string): Effect.Effect, - commandPath: ReadonlyArray, - flagName: string, -): boolean { - const commandIndex = rawArgs.findIndex((_, index) => - commandPath.every((segment, offset) => rawArgs[index + offset] === segment), - ); - if (commandIndex === -1) { - return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); - } - - for (let index = commandIndex + commandPath.length; index < rawArgs.length; index += 1) { - const token = rawArgs[index]; - if (token === undefined || token === "--") { - return false; - } - if (token === `--${flagName}` || token.startsWith(`--${flagName}=`)) { - return true; - } - } - return false; -} - function explicitBooleanFlag( rawArgs: ReadonlyArray, commandPath: ReadonlyArray, diff --git a/apps/cli/src/shared/functions/download.ts b/apps/cli/src/shared/functions/download.ts index 562066ee4b..1d6ea222a5 100644 --- a/apps/cli/src/shared/functions/download.ts +++ b/apps/cli/src/shared/functions/download.ts @@ -7,7 +7,11 @@ import { Effect, FileSystem, Option } from "effect"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; import type * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; import { Output } from "../output/output.service.ts"; -import { invalidFunctionSlugDetail, validateFunctionSlugMessage } from "./functions.shared.ts"; +import { + hasExplicitLongFlag, + invalidFunctionSlugDetail, + validateFunctionSlugMessage, +} from "./functions.shared.ts"; import { ConflictingFunctionDownloadFlagsError, FunctionDownloadNotFoundError, @@ -34,12 +38,21 @@ export interface DownloadFunctionsDependencies< > { readonly api: ApiClient; readonly projectRoot: string; + readonly rawArgs: ReadonlyArray; readonly resolveProjectRef: ( projectRef: Option.Option, ) => Effect.Effect; + /** + * `captureOutput` is `true` whenever `output.format !== "text"`: the Go + * 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. + */ readonly proxyDownload: ( flags: DownloadFunctionsOptions, projectRef: string, + captureOutput: boolean, ) => Effect.Effect; } @@ -57,11 +70,14 @@ export function makeGoProxyDownloadArgs( args.push(flags.functionName.value); } args.push("--project-ref", projectRef); - if (flags.useDocker) { - args.push("--use-docker"); - } + // 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"); } return args; } @@ -107,13 +123,17 @@ function validateSlug(slug: string): Effect.Effect, ): Effect.Effect { const selected = [ - flags.useApi ? "--use-api" : undefined, - flags.useDocker ? "--use-docker" : undefined, - flags.legacyBundle ? "--legacy-bundle" : undefined, + hasExplicitLongFlag(rawArgs, downloadCommandPath, "use-api") ? "--use-api" : undefined, + hasExplicitLongFlag(rawArgs, downloadCommandPath, "use-docker") ? "--use-docker" : undefined, + hasExplicitLongFlag(rawArgs, downloadCommandPath, "legacy-bundle") + ? "--legacy-bundle" + : undefined, ].filter((flag) => flag !== undefined); return selected.length <= 1 @@ -734,17 +754,61 @@ export function downloadFunctions, + commandPath: ReadonlyArray, + flagName: string, +): boolean { + const commandIndex = rawArgs.findIndex((_, index) => + commandPath.every((segment, offset) => rawArgs[index + offset] === segment), + ); + if (commandIndex === -1) { + return rawArgs.some((token) => token === `--${flagName}` || token.startsWith(`--${flagName}=`)); + } + + for (let index = commandIndex + commandPath.length; index < rawArgs.length; index += 1) { + const token = rawArgs[index]; + if (token === undefined || token === "--") { + return false; + } + if (token === `--${flagName}` || token.startsWith(`--${flagName}=`)) { + return true; + } + } + return false; +}