From 4d88e6f6474d8144874335c2a2b20054b3d7ac03 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 6 Jul 2026 18:25:26 +0100 Subject: [PATCH 1/2] fix(cli): guard functions deploy --jobs on --use-api alone (Go parity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's only --jobs guard (cmd/functions.go:79-82) is `if useApi { ... } else if maxJobs > 1 { error }` — keyed on the resolved --use-api value, independent of Docker/legacy-bundle. The TS port instead guarded on `!explicitUseApi && (useDocker || legacyBundle)`, so --use-docker=false --jobs 2 silently passed in TS while Go rejected it, and the error text diverged ("cannot be used with local bundling" vs Go's "--jobs must be used together with --use-api"). Fixes CLI-1861. --- .../functions/deploy/deploy.e2e.test.ts | 32 ++++- .../deploy/deploy.integration.test.ts | 120 ++++++++++++++++++ apps/cli/src/shared/functions/deploy.ts | 7 +- 3 files changed, 153 insertions(+), 6 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts index 5f8df92de9..f70bf77319 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts @@ -56,12 +56,36 @@ describe("supabase functions deploy (legacy) — argument validation", () => { }, ); expect(exitCode).not.toBe(0); - // The Go CLI phrases this as either "must be used together with --use-api" - // or "cannot be used with local bundling" depending on version — both mean - // --jobs is rejected without server-side (--use-api) bundling. - expect(stderr).toMatch(/--jobs\b.*(--use-api|local bundling)/i); + expect(stderr).toContain("--jobs must be used together with --use-api"); }); + test( + "rejects --jobs without --use-api even with --use-docker=false (Go parity gap)", + { timeout: E2E_TIMEOUT_MS }, + async () => { + using home = makeTempHome(); + const { exitCode, stderr } = await runSupabase( + [ + "functions", + "deploy", + SLUG, + "--project-ref", + FAKE_REF, + "--use-docker=false", + "--jobs", + "2", + ], + { + entrypoint: "legacy", + home: home.dir, + env: { HOME: home.dir, SUPABASE_ACCESS_TOKEN: FAKE_TOKEN }, + }, + ); + expect(exitCode).not.toBe(0); + expect(stderr).toContain("--jobs must be used together with --use-api"); + }, + ); + test("fails without a linked project or --project-ref", { timeout: E2E_TIMEOUT_MS }, async () => { using home = makeTempHome(); const workdir = mkdtempSync(join(tmpdir(), "fn-deploy-nolink-")); diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts index 7984803387..522e96e84a 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts @@ -482,4 +482,124 @@ describe("legacy functions deploy", () => { ), ); }); + + describe("--jobs validation (Go parity: cmd/functions.go:79-82)", () => { + function setupJobsTest(rawArgs: ReadonlyArray) { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + handler: (request) => Effect.succeed(legacyJsonResponse(request, 200, [])), + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + runtimeInfo: mockRuntimeInfo({ cwd: tempRoot.current }), + }), + Layer.succeed(LegacyYesFlag, false), + Stdio.layerTest({ args: Effect.succeed(rawArgs) }), + ); + return { out, api, layer }; + } + + it.live("rejects --jobs > 1 without --use-api, even with default --use-docker", () => { + const { layer } = setupJobsTest(["functions", "deploy", "hello-world", "--jobs", "2"]); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDeploy({ + ...baseFlags, + useApi: false, + useDocker: true, + jobs: Option.some(2), + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("--jobs must be used together with --use-api"); + }); + }); + + it.live("rejects --jobs > 1 with --use-docker=false and no --use-api (Go parity gap)", () => { + // Divergence this test guards: previously the guard only fired when local + // bundling (Docker/legacy-bundle) was active, so `--use-docker=false --jobs 2` + // (no --use-api) silently passed in TS while Go rejected it. + const { layer } = setupJobsTest([ + "functions", + "deploy", + "hello-world", + "--use-docker=false", + "--jobs", + "2", + ]); + + return Effect.gen(function* () { + const error = yield* legacyFunctionsDeploy({ + ...baseFlags, + useApi: false, + useDocker: false, + jobs: Option.some(2), + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("--jobs must be used together with --use-api"); + }); + }); + + it.live("allows --jobs > 1 together with --use-api", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + handler: (request) => { + if (request.method === "GET") { + return Effect.succeed(legacyJsonResponse(request, 200, [])); + } + return Effect.succeed( + legacyJsonResponse(request, 201, { + id: "function-id", + slug: "hello-world", + name: "hello-world", + status: "ACTIVE", + version: 2, + created_at: 1_687_423_025_152, + updated_at: 1_687_423_025_152, + verify_jwt: true, + import_map: true, + entrypoint_path: "functions/hello-world/index.ts", + import_map_path: "functions/hello-world/deno.json", + }), + ); + }, + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + runtimeInfo: mockRuntimeInfo({ cwd: tempRoot.current }), + }), + Layer.succeed(LegacyYesFlag, false), + Stdio.layerTest({ + args: Effect.succeed(["functions", "deploy", "hello-world", "--use-api", "--jobs", "2"]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempRoot.current)); + yield* Effect.tryPromise(() => writeLocalFunction(tempRoot.current, "hello-world")); + + yield* legacyFunctionsDeploy({ + ...baseFlags, + useApi: true, + jobs: Option.some(2), + }); + + expect(out.stdoutText).toContain( + "Deployed Functions on project abcdefghijklmnopqrst: hello-world\n", + ); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.tryPromise(() => rm(tempRoot.current, { recursive: true, force: true })), + ), + ); + }); + }); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 46bab06023..b50cddd19c 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -2128,8 +2128,11 @@ export function deployFunctions( const useLocalBundler = !explicitUseApi && (flags.useDocker || flags.legacyBundle); const configuredJobs = Option.getOrElse(flags.jobs, () => 1); const jobs = configuredJobs === 0 ? 1 : configuredJobs; - if (useLocalBundler && jobs > 1) { - return yield* Effect.fail(new Error("--jobs cannot be used with local bundling")); + // Go parity (`cmd/functions.go:79-82`): the guard is `if useApi { ... } else if + // maxJobs > 1 { error }` — keyed on the resolved `--use-api` value alone, not on + // whether local bundling (Docker/legacy-bundle) is in play. + if (!flags.useApi && jobs > 1) { + return yield* Effect.fail(new Error("--jobs must be used together with --use-api")); } const preResolvedProjectRef = From 6f04c0873d105cfbdba6c4fb1cf17d1c85d5dc47 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Mon, 6 Jul 2026 18:43:03 +0100 Subject: [PATCH 2/2] fix(cli): key functions deploy's bundler routing on resolved --use-api too Review of the --jobs guard fix surfaced the same presence-vs-resolved-value bug one line above it: useLocalBundler keyed off explicitUseApi (true for --use-api=false, since the flag was still typed), silently forcing the API bundler even when the user explicitly disabled it. Go's `if useApi { useDocker = false }` only forces that when the resolved value is true (cmd/functions.go:79-80), so --use-api=false alone should fall through to whatever --use-docker/--legacy-bundle already resolved to. Also closes two test-coverage gaps flagged in review: the next/ shell shares deployFunctions() but had no --jobs guard coverage, and --jobs 0's normalize-to-1 branch had zero hits in either shell. --- .../functions/deploy/deploy.e2e.test.ts | 19 ++- .../deploy/deploy.integration.test.ts | 130 ++++++++++++++++++ .../deploy/deploy.integration.test.ts | 44 ++++++ apps/cli/src/shared/functions/deploy.ts | 10 +- 4 files changed, 192 insertions(+), 11 deletions(-) diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts index f70bf77319..03e82b5432 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.e2e.test.ts @@ -4,21 +4,20 @@ import { join } from "node:path"; import { describe, expect, test } from "vitest"; import { makeTempHome, runSupabase } from "../../../../../tests/helpers/cli.ts"; -// Argument-validation negatives for `functions deploy`. 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. Asserting the SPECIFIC error text also -// avoids a false pass from an unrelated non-zero exit (e.g. a missing Go binary). +// Argument-validation negatives for `functions deploy`. This validation is native TS +// (`shared/functions/deploy.ts`'s mutual-exclusivity and `--jobs` guards) ported from +// Go's cobra flag-group validation — a black-box subprocess test keeps these +// assertions valid across the shell boundary. Asserting the SPECIFIC error text also +// avoids a false pass from an unrelated non-zero exit. // -// All cases fail before any network call (cobra flag parsing / pre-resolution), -// so no auth or linked project is required. +// All cases fail before any network call (the guards run before project-ref/config +// resolution), so no auth or linked project is required. const E2E_TIMEOUT_MS = 30_000; const SLUG = "deploy-e2e-basic"; // Valid-format token + ref to clear the auth and project-ref gates (both checked -// before the Go bundler-flag validation under test). These cases all fail before -// any network call (cobra flag-group validation / the jobs check at the top of -// RunE), so neither value is ever used against a real API. +// before the bundler-flag validation under test). These cases all fail before any +// network call, so neither value is ever used against a real API. const FAKE_TOKEN = `sbp_${"0".repeat(40)}`; const FAKE_REF = "a".repeat(20); diff --git a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts index 522e96e84a..b8f23ba1dc 100644 --- a/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/legacy/commands/functions/deploy/deploy.integration.test.ts @@ -14,6 +14,7 @@ import { useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; import { mockOutput, mockRuntimeInfo } from "../../../../../tests/helpers/mocks.ts"; +import { mockChildProcessSpawner } from "../../../../../../../packages/process-compose/tests/helpers/mocks.ts"; import { legacyFunctionsDeploy } from "./deploy.handler.ts"; import type { LegacyFunctionsDeployFlags } from "./deploy.command.ts"; @@ -601,5 +602,134 @@ describe("legacy functions deploy", () => { ), ); }); + + it.live("treats --jobs 0 as 1 and does not require --use-api", () => { + const out = mockOutput({ format: "text" }); + const api = mockLegacyPlatformApi({ + handler: (request) => { + if (request.method === "GET") { + return Effect.succeed(legacyJsonResponse(request, 200, [])); + } + return Effect.succeed( + legacyJsonResponse(request, 201, { + id: "function-id", + slug: "hello-world", + name: "hello-world", + status: "ACTIVE", + version: 2, + created_at: 1_687_423_025_152, + updated_at: 1_687_423_025_152, + verify_jwt: true, + import_map: true, + entrypoint_path: "functions/hello-world/index.ts", + import_map_path: "functions/hello-world/deno.json", + }), + ); + }, + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + runtimeInfo: mockRuntimeInfo({ cwd: tempRoot.current }), + }), + Layer.succeed(LegacyYesFlag, false), + Stdio.layerTest({ + args: Effect.succeed(["functions", "deploy", "hello-world", "--jobs", "0"]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempRoot.current)); + yield* Effect.tryPromise(() => writeLocalFunction(tempRoot.current, "hello-world")); + + yield* legacyFunctionsDeploy({ + ...baseFlags, + useApi: false, + jobs: Option.some(0), + }); + + expect(out.stdoutText).toContain( + "Deployed Functions on project abcdefghijklmnopqrst: hello-world\n", + ); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.tryPromise(() => rm(tempRoot.current, { recursive: true, force: true })), + ), + ); + }); + }); + + describe("bundler routing with --use-api=false (Go parity: cmd/functions.go:79-80)", () => { + it.live("falls through to Docker bundling, not the API path, when --use-api=false", () => { + // Divergence this test guards: Go's `if useApi { useDocker = false }` only forces + // the API path when the RESOLVED value is true. `--use-api=false` alone must leave + // `useDocker`'s own value (default true) in effect, routing to Docker — previously + // `useLocalBundler` keyed off flag *presence* (`explicitUseApi`), so typing + // `--use-api=false` silently forced the API path instead. + const out = mockOutput({ format: "text" }); + const child = mockChildProcessSpawner({ exitCode: 1 }); + const api = mockLegacyPlatformApi({ + handler: (request) => { + if (request.method === "GET") { + return Effect.succeed(legacyJsonResponse(request, 200, [])); + } + return Effect.succeed( + legacyJsonResponse(request, 201, { + id: "function-id", + slug: "hello-world", + name: "hello-world", + status: "ACTIVE", + version: 2, + created_at: 1_687_423_025_152, + updated_at: 1_687_423_025_152, + verify_jwt: true, + import_map: true, + entrypoint_path: "functions/hello-world/index.ts", + import_map_path: "functions/hello-world/deno.json", + }), + ); + }, + }); + const layer = Layer.mergeAll( + buildLegacyTestRuntime({ + out, + api, + cliConfig: mockLegacyCliConfig({ workdir: tempRoot.current }), + runtimeInfo: mockRuntimeInfo({ cwd: tempRoot.current }), + }), + Layer.succeed(LegacyYesFlag, false), + child.layer, + Stdio.layerTest({ + args: Effect.succeed(["functions", "deploy", "hello-world", "--use-api=false"]), + }), + ); + + return Effect.gen(function* () { + yield* Effect.tryPromise(() => writeProjectConfig(tempRoot.current)); + yield* Effect.tryPromise(() => writeLocalFunction(tempRoot.current, "hello-world")); + + yield* legacyFunctionsDeploy({ + ...baseFlags, + useApi: false, + useDocker: true, + }); + + // Docker was actually attempted (proves useLocalBundler resolved to true); + // it wasn't running, so the command fell back to the API and still succeeded. + expect(child.spawned).toEqual([{ command: "docker", args: ["info"] }]); + expect(out.stderrText).toContain("WARNING: Docker is not running\n"); + expect(out.stdoutText).toContain( + "Deployed Functions on project abcdefghijklmnopqrst: hello-world\n", + ); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.tryPromise(() => rm(tempRoot.current, { recursive: true, force: true })), + ), + ); + }); }); }); diff --git a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts index 3710bdeacb..cb88e6763b 100644 --- a/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/next/commands/functions/deploy/deploy.integration.test.ts @@ -2175,4 +2175,48 @@ describe("functions deploy", () => { expect(error.message).toContain("--use-docker"); }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); }); + + describe("--jobs validation (Go parity: cmd/functions.go:79-82)", () => { + it.live("rejects --jobs > 1 without --use-api", () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.promise(() => writeProjectConfig(tempDir)); + const { layer } = setup(tempDir, { + rawArgs: ["functions", "deploy", "--jobs", "2"], + }); + + const error = yield* functionsDeploy({ + ...BASE_FLAGS, + useApi: false, + jobs: Option.some(2), + }).pipe(Effect.provide(layer), Effect.flip); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("--jobs must be used together with --use-api"); + }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); + }); + + it.live("allows --jobs > 1 together with --use-api", () => { + const tempDir = makeTempDir(); + + return Effect.gen(function* () { + yield* Effect.promise(() => writeProjectConfig(tempDir)); + yield* Effect.promise(() => writeLocalFunction(tempDir, "hello-world")); + + const { out, layer } = setup(tempDir, { + rawArgs: ["functions", "deploy", "hello-world", "--use-api", "--jobs", "2"], + }); + + yield* functionsDeploy({ + ...BASE_FLAGS, + functionNames: ["hello-world"], + useApi: true, + jobs: Option.some(2), + }).pipe(Effect.provide(layer)); + + expect(out.stdoutText).toContain(`Deployed Functions on project ${PROJECT_REF}`); + }).pipe(Effect.ensuring(cleanupTempDir(tempDir))); + }); + }); }); diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index b50cddd19c..956683430d 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -2103,6 +2103,10 @@ export function deployFunctions( return Effect.gen(function* () { const output = yield* Output; const commandPath = ["functions", "deploy"] as const; + // Presence-based (true for `--use-api=false`, not just bare `--use-api`) — mirrors + // cobra's `Changed()`-driven `MarkFlagsMutuallyExclusive`, so it's only used for the + // mutual-exclusivity check below. Behavior branches (bundler routing, --jobs guard) + // key off the resolved `flags.useApi` value instead, matching Go's own `if useApi`. const explicitUseApi = hasExplicitLongFlag(dependencies.rawArgs, commandPath, "use-api"); const explicitUseDocker = hasExplicitLongFlag(dependencies.rawArgs, commandPath, "use-docker"); const explicitLegacyBundle = hasExplicitLongFlag( @@ -2125,7 +2129,11 @@ export function deployFunctions( ); } - const useLocalBundler = !explicitUseApi && (flags.useDocker || flags.legacyBundle); + // Go parity (`cmd/functions.go:79-80`): `if useApi { useDocker = false }` mutates the + // resolved boolean, not a presence flag — `--use-api=false` alone must NOT force the + // API path, it should fall through to whatever `--use-docker`/`--legacy-bundle` + // already resolved to. + const useLocalBundler = !flags.useApi && (flags.useDocker || flags.legacyBundle); const configuredJobs = Option.getOrElse(flags.jobs, () => 1); const jobs = configuredJobs === 0 ? 1 : configuredJobs; // Go parity (`cmd/functions.go:79-82`): the guard is `if useApi { ... } else if