From d4e37d5ac83233379039c7669694bc73e3b376b0 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 28 Aug 2026 23:31:40 -0300 Subject: [PATCH 01/15] feat(cli): prompt for worker name if not provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the `name` argument to `supabase workers new` optional and prompts for it when it is omitted, so a bare `supabase workers new` walks through name, runtime and size rather than failing the parse. The name is the one input this command cannot default — it is the directory, the `[workers.]` key and the hostname all at once. So where the runtime and size prompts fall back to a default when there is nowhere to ask, the name prompt has nothing to fall back to: with `-o json|yaml|toml|env` or no interactive terminal, the command fails with a new `MissingWorkerNameError` pointing at `supabase workers new api`. The prompt validates against everything the command would otherwise refuse a moment later — a non-DNS-label name, and a name `config.toml` already records — so a typo is corrected in place instead of ending the run. That also means the project has to be loaded before the first prompt, and the machine-output check moves up with it: `-o` leaves `output.format` as `text`, and Clack writes its terminal UI to stdout, so a name prompt would land in front of the payload for the same reason the runtime prompt would. The handler's inline name validation is replaced by the shared `legacyValidateWorkerName`, which the rest of the command family already uses, so an explicitly-passed name and a prompted one are refused on identical terms. `mockOutput` now records `promptTextCalls` so tests can assert on the prompt's message and exercise its `validate` callback. --- .../commands/workers/new/SIDE_EFFECTS.md | 20 ++- .../commands/workers/new/new.command.ts | 9 +- .../commands/workers/new/new.handler.ts | 85 +++++++++---- .../workers/new/new.integration.test.ts | 116 ++++++++++++++---- apps/cli/src/shared/workers/workers.errors.ts | 16 +++ apps/cli/tests/helpers/mocks.ts | 8 +- 6 files changed, 204 insertions(+), 50 deletions(-) diff --git a/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md index 41c30b0376..89e38122e3 100644 --- a/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md @@ -1,4 +1,4 @@ -# `supabase workers new ` +# `supabase workers new [name]` > **Local-disk only.** Nothing is deployed and no Management API route is > called; `workers push` is what talks to the platform. @@ -35,6 +35,13 @@ same resolver `start`/`stop`/`status` use) and never climbs to an ancestor. A therefore records the worker in that directory's own `config.toml` — created if absent — rather than in the ancestor project's. +The name is prompted for when the command line does not carry one, and the +prompt refuses a name that is not a DNS label or that `config.toml` already +records — so nothing is asked, and nothing written, for a name the command was +going to refuse. With `-o json|yaml|toml|env` or no interactive terminal there is +nowhere to ask, and the command fails instead of defaulting: unlike the runtime +and size, the name has no default to fall back on. + Writes to `config.toml` are append-only. A worker already recorded under `[workers.]` is refused outright — before the runtime and size prompts, and before anything reaches disk — because editing an entry the user owns is @@ -61,6 +68,7 @@ root. | ---- | ----------------------------------------------------------------------------------- | | `0` | success | | `1` | invalid worker name — the name must be a DNS label | +| `1` | no name given, and nowhere to ask for one — not a terminal, or `-o` is in force | | `1` | bad `--source`: outside the project, or a path the CLI owns | | `1` | destination exists and is not empty | | `1` | the worker is already recorded in `config.toml`, in any form | @@ -83,7 +91,9 @@ root. No custom events — only the `cli_command_executed` that the instrumentation wrapper emits for every command. -Nothing is emitted for a failure the parser catches, such as a missing worker -name or a `--runtime`/`--size` value outside the choice list. The wrapper is -installed by `Command.withHandler`, so a command that never reaches its handler -never reaches the instrumentation either — and `telemetry.json` is not written. +Nothing is emitted for a failure the parser catches, such as a +`--runtime`/`--size` value outside the choice list. The wrapper is installed by +`Command.withHandler`, so a command that never reaches its handler never reaches +the instrumentation either — and `telemetry.json` is not written. A missing name +is _not_ one of those: the argument is optional, so a bare `workers new` reaches +the handler, which asks for the name or fails for want of anywhere to ask. diff --git a/apps/cli/src/legacy/commands/workers/new/new.command.ts b/apps/cli/src/legacy/commands/workers/new/new.command.ts index 19d4b7be9a..1ce376961f 100644 --- a/apps/cli/src/legacy/commands/workers/new/new.command.ts +++ b/apps/cli/src/legacy/commands/workers/new/new.command.ts @@ -12,7 +12,10 @@ import { legacyWorkersNew } from "./new.handler.ts"; const config = { name: Argument.string("name").pipe( - Argument.withDescription("Worker name. Doubles as its directory, and its hostname."), + Argument.withDescription( + "Worker name. Doubles as its directory, and its hostname. Prompted when omitted.", + ), + Argument.optional, ), runtime: Flag.choice("runtime", WORKER_RUNTIMES).pipe( Flag.withDescription( @@ -51,6 +54,10 @@ export const legacyWorkersNewCommand = Command.make("new", config).pipe( ), Command.withShortDescription("Scaffold a worker locally"), Command.withExamples([ + { + command: "supabase workers new", + description: "Prompt for the name, then for runtime and size", + }, { command: "supabase workers new api", description: "Scaffold supabase/workers/api, prompting for runtime and size", diff --git a/apps/cli/src/legacy/commands/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/workers/new/new.handler.ts index 9b5e73774c..cc777f7e4e 100644 --- a/apps/cli/src/legacy/commands/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/workers/new/new.handler.ts @@ -33,18 +33,22 @@ import { } from "../../../../shared/workers/worker-runtimes.ts"; import { WORKER_STACKS } from "../../../../shared/workers/worker-stacks.ts"; import { - InvalidWorkerNameError, + MissingWorkerNameError, WorkerDirectoryExistsError, } from "../../../../shared/workers/workers.errors.ts"; -import { legacyLoadWorkersProjectForEntryWrite } from "../workers.shared.ts"; +import { + legacyLoadWorkersProjectForEntryWrite, + legacyValidateWorkerName, + type LegacyWorkersProject, +} from "../workers.shared.ts"; import type { LegacyWorkersNewFlags } from "./new.command.ts"; /** - * `supabase workers new ` — scaffold `supabase/workers//` from the + * `supabase workers new [name]` — scaffold `supabase/workers//` from the * chosen runtime's starter files and record the choice in `config.toml`. * Nothing is deployed; this is entirely local-disk work. * - * The runtime and size are resolved *before* anything is written, so a + * The name, runtime and size are all resolved *before* anything is written, so a * cancelled prompt leaves nothing behind for this worker at all. */ @@ -53,6 +57,49 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { return [defaultValue, ...values.filter((value) => value !== defaultValue)]; } +/** + * The worker name, asked for when the command line did not carry one. + * + * The name is the one input here that cannot be defaulted — it is the + * directory, the `config.toml` key and the hostname — so a bare + * `supabase workers new` asks rather than failing the parse. The prompt + * validates against everything the command would otherwise refuse a moment + * later, so a mistyped or already-recorded name is corrected in place instead + * of ending the run. + */ +const resolveName = Effect.fnUntraced(function* (options: { + readonly explicit: Option.Option; + /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ + readonly machineOutput: boolean; + readonly project: LegacyWorkersProject; +}) { + if (Option.isSome(options.explicit)) { + return options.explicit.value; + } + + const output = yield* Output; + if (output.format === "text" && output.interactive && !options.machineOutput) { + return yield* output.promptText("What should this worker be called?", { + validate: (value) => { + const invalid = validateWorkerNameMessage(value); + if (invalid !== undefined) { + return invalid; + } + return options.project.section.workers[value] === undefined + ? undefined + : `"${value}" is already configured in ${options.project.configPath}.`; + }, + }); + } + + return yield* Effect.fail( + new MissingWorkerNameError({ + detail: "Worker name is required in non-interactive mode.", + suggestion: "Pass a worker name, for example `supabase workers new api`.", + }), + ); +}); + const resolveRuntime = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ @@ -134,21 +181,21 @@ export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( yield* Effect.gen(function* () { const project = yield* legacyLoadWorkersProjectForEntryWrite(); - const name = flags.name; - const invalid = validateWorkerNameMessage(name); - if (invalid !== undefined) { - return yield* Effect.fail( - new InvalidWorkerNameError({ - detail: `"${name}" is not a valid worker name. ${invalid}`, - suggestion: "Worker names become hostnames, so they must be DNS labels.", - }), - ); - } + // `-o` leaves `output.format` as `text`, and the prompts go through Clack, + // which writes its terminal UI to stdout with no stream override — so a + // prompt would land in front of the payload just as the notices did. Read + // before the first prompt rather than beside the last, since the name is + // now asked for too. + const machineOutput = yield* legacyWorkersMachineOutputRequested(); + + const name = yield* resolveName({ explicit: flags.name, machineOutput, project }); + yield* legacyValidateWorkerName(name); // Refused before anything is asked or written. `new` creates a worker; // changing one that already exists is a `config.toml` edit, and the file is // the user's. Checking here rather than only in `planWorkerEntry` means the - // prompts never run for a name that was going to be refused anyway. + // runtime and size prompts never run for a name that was going to be + // refused anyway; the name prompt rejects it up front for the same reason. if (project.section.workers[name] !== undefined) { return yield* Effect.fail( new WorkerAlreadyConfiguredError({ @@ -159,12 +206,8 @@ export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( } // Resolved before anything is written, so cancelling either prompt leaves - // nothing behind — the name included. - // `-o` leaves `output.format` as `text`, and `promptSelect` goes through - // Clack, which writes its terminal UI to stdout with no stream override — so - // a prompt would land in front of the payload just as the notices did. With a - // machine format requested there is nowhere to ask, so the defaults stand. - const machineOutput = yield* legacyWorkersMachineOutputRequested(); + // nothing behind — the name included. With a machine format requested there + // is nowhere to ask, so the defaults stand. const runtime = yield* resolveRuntime({ explicit: flags.runtime, machineOutput }); const size = yield* resolveSize({ explicit: flags.size, machineOutput }); diff --git a/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts index e180f0620e..d6d1e7279f 100644 --- a/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts @@ -13,6 +13,7 @@ import { import { InvalidWorkerNameError, InvalidWorkerSourceError, + MissingWorkerNameError, WorkerDirectoryExistsError, } from "../../../../shared/workers/workers.errors.ts"; import { legacyWorkersNew } from "./new.handler.ts"; @@ -27,7 +28,7 @@ verify_jwt = false function flags(overrides: Partial = {}): LegacyWorkersNewFlags { return { - name: "api", + name: Option.some("api"), runtime: Option.none(), size: Option.none(), source: Option.none(), @@ -54,7 +55,7 @@ describe("legacy workers new", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); const workerDir = join(repo.dir, "supabase", "workers", "api"); expect(existsSync(join(workerDir, "index.mjs"))).toBe(true); @@ -69,6 +70,75 @@ describe("legacy workers new", () => { expect(out.stdoutText).toContain("supabase workers push api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + it.live("asks for the name when the command line carries none", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["orders"], + promptSelectResponses: ["node", "2gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.none() })); + + expect(out.promptTextCalls.map((call) => call.message)).toEqual([ + "What should this worker be called?", + ]); + expect(existsSync(join(repo.dir, "supabase", "workers", "orders", "index.mjs"))).toBe(true); + expect(repo.config()).toContain("[workers.orders]"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The prompt is the last place a mistyped or taken name can be corrected + // without ending the run, so it refuses both there rather than after asking. + it.live("refuses a bad or already-recorded name at the name prompt", () => { + const repo = project({ + "supabase/config.toml": `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + }); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["orders"], + promptSelectResponses: ["node", "2gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.none() })); + + const validate = out.promptTextCalls[0]?.opts?.validate; + expect(validate).toBeDefined(); + expect(validate?.("My_Worker")).toContain("lowercase letters"); + expect(validate?.("api")).toContain("already configured"); + expect(validate?.("orders")).toBeUndefined(); + expect(existsSync(join(repo.dir, "supabase", "workers", "orders", "index.mjs"))).toBe(true); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Nowhere to ask means nothing to scaffold under: the name is the directory, + // the config key and the hostname, and none of those has a default. + it.live.each([ + { label: "not interactive", setup: { interactive: false } }, + // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. + { label: "-o json", setup: { goOutput: "json" as const } }, + ])("refuses a bare new when there is nowhere to ask ($label)", ({ setup }) => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + // An answer is waiting, so a prompt would succeed rather than fail some + // other way. + promptTextResponses: ["orders"], + ...setup, + }); + + return Effect.gen(function* () { + const error = yield* legacyWorkersNew(flags({ name: Option.none() })).pipe(Effect.flip); + + expect(error).toBeInstanceOf(MissingWorkerNameError); + expect(out.promptTextCalls).toEqual([]); + expect(existsSync(join(repo.dir, "supabase", "workers"))).toBe(false); + expect(repo.config()).toBe(CONFIG_WITH_COMMENTS); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("prompts for runtime and size when neither is given", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ @@ -77,7 +147,7 @@ describe("legacy workers new", () => { }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); expect(out.promptSelectCalls.map((call) => call.message)).toEqual([ "Which runtime should this worker use?", @@ -94,7 +164,7 @@ describe("legacy workers new", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, format: "json" }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); expect(out.promptSelectCalls).toHaveLength(0); expect(repo.config()).toContain('runtime = "deno"'); @@ -110,12 +180,12 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("deno"), size: Option.some("4gb") }), + flags({ name: Option.some("api"), runtime: Option.some("deno"), size: Option.some("4gb") }), ); const recorded = repo.config(); const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -136,7 +206,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -154,7 +224,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some("packages/api"), }), @@ -174,7 +244,7 @@ describe("legacy workers new", () => { for (const source of [".", "..", "supabase", "supabase/functions"]) { const error = yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some(source), }), @@ -194,7 +264,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: created.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); expect(existsSync(join(created.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); expect(readFileSync(join(created.dir, "supabase", "config.toml"), "utf8")).toBe( @@ -212,7 +282,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -228,7 +298,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); expect(existsSync(join(repo.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); @@ -240,7 +310,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -257,7 +327,9 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - const error = yield* legacyWorkersNew(flags({ name: "My_Worker" })).pipe(Effect.flip); + const error = yield* legacyWorkersNew(flags({ name: Option.some("My_Worker") })).pipe( + Effect.flip, + ); expect(error).toBeInstanceOf(InvalidWorkerNameError); expect(existsSync(join(repo.dir, "supabase", "workers"))).toBe(false); @@ -286,7 +358,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("deno") }), + flags({ name: Option.some("api"), runtime: Option.some("deno") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -307,7 +379,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); const jsonPath = join(repo.dir, "supabase", "config.json"); expect(readFileSync(jsonPath, "utf8")).toBe(configJson); @@ -332,7 +404,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); // The ancestor project is untouched. expect(repo.config()).toBe(CONFIG_WITH_COMMENTS); @@ -357,7 +429,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerConfigWriteUnsafeError); @@ -374,7 +446,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -396,7 +468,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some("generated"), }), @@ -423,7 +495,7 @@ describe("legacy workers new", () => { }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); const payload: unknown = JSON.parse(out.stdoutText); // The defaults stand, because there was nowhere to ask. @@ -439,7 +511,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some(join("supabase", "config.toml")), }), diff --git a/apps/cli/src/shared/workers/workers.errors.ts b/apps/cli/src/shared/workers/workers.errors.ts index 44826c9315..eda518866c 100644 --- a/apps/cli/src/shared/workers/workers.errors.ts +++ b/apps/cli/src/shared/workers/workers.errors.ts @@ -21,6 +21,22 @@ export class InvalidWorkerNameError extends Data.TaggedError("InvalidWorkerNameE } } +/** + * A bare `new` had no name to scaffold under, and nowhere to ask for one. + * + * The name is the one input this command cannot default — it is the directory, + * the `config.toml` key and the hostname all at once — so with `-o` in force or + * no interactive terminal there is nothing to do but say so. + */ +export class MissingWorkerNameError extends Data.TaggedError("MissingWorkerNameError")<{ + readonly detail: string; + readonly suggestion: string; +}> { + get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { + return actionability.provideFlags; + } +} + /** * A symlink in the worker source points outside the build context. * diff --git a/apps/cli/tests/helpers/mocks.ts b/apps/cli/tests/helpers/mocks.ts index 6e766d28c2..02bec33028 100644 --- a/apps/cli/tests/helpers/mocks.ts +++ b/apps/cli/tests/helpers/mocks.ts @@ -283,6 +283,10 @@ export function mockOutput( } | undefined; }> = []; + const promptTextCalls: Array<{ + message: string; + opts?: { defaultValue?: string; validate?: (v: string) => string | undefined }; + }> = []; const promptTextResponses = [...(opts.promptTextResponses ?? [])]; const promptSelectResponses = [...(opts.promptSelectResponses ?? [])]; const promptPasswordResponses = [...(opts.promptPasswordResponses ?? [])]; @@ -387,10 +391,11 @@ export function mockOutput( promptText: (() => { let callCount = 0; return ( - _msg: string, + message: string, options?: { defaultValue?: string; validate?: (v: string) => string | undefined }, ) => { callCount++; + promptTextCalls.push({ message, opts: options }); // Exercise the validate callback to cover both branches (line 140) if (options?.validate) { options.validate(""); // truthy branch: returns error message @@ -451,6 +456,7 @@ export function mockOutput( events, promptConfirmCalls, promptSelectCalls, + promptTextCalls, rawChunks, get stdoutText() { return rawChunks From edc71990a255a0fd188063aa7f1ead65bcb9e998 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 28 Aug 2026 23:41:09 -0300 Subject: [PATCH 02/15] fix(cli): gate the workers new prompts on stdin too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `output.interactive` is derived solely from `tty.stdoutIsTty`, so with stdin piped or redirected and stdout still on a terminal it stayed true. A bare `printf 'api\n' | supabase workers new` therefore opened Clack's name prompt and read the worker name off the pipe instead of taking the documented `MissingWorkerNameError` path — and the runtime and size prompts consumed whatever followed rather than falling back to their defaults. The three resolvers now share one `canPromptFor` decision, made once before the first prompt, which pairs `output.interactive` with `tty.stdinIsTty` the way `workers delete` already guards its confirmation. A prompt is only answerable from a keyboard, so both streams have to be a terminal. --- .../commands/workers/new/SIDE_EFFECTS.md | 26 ++++---- .../commands/workers/new/new.handler.ts | 63 ++++++++++++------- .../workers/new/new.integration.test.ts | 23 +++++++ 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md index 89e38122e3..cfba4b1535 100644 --- a/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/workers/new/SIDE_EFFECTS.md @@ -38,9 +38,11 @@ absent — rather than in the ancestor project's. The name is prompted for when the command line does not carry one, and the prompt refuses a name that is not a DNS label or that `config.toml` already records — so nothing is asked, and nothing written, for a name the command was -going to refuse. With `-o json|yaml|toml|env` or no interactive terminal there is -nowhere to ask, and the command fails instead of defaulting: unlike the runtime -and size, the name has no default to fall back on. +going to refuse. With `-o json|yaml|toml|env`, a redirected stdout, or a stdin +that is not a terminal, there is nowhere to ask, and the command fails instead +of defaulting: unlike the runtime and size, the name has no default to fall back +on. Every prompt is gated on both streams, so `printf 'api\n' | supabase workers +new` takes that failure path rather than reading the worker name off the pipe. Writes to `config.toml` are append-only. A worker already recorded under `[workers.]` is refused outright — before the runtime and size prompts, @@ -64,15 +66,15 @@ root. ## Exit Codes -| Code | Condition | -| ---- | ----------------------------------------------------------------------------------- | -| `0` | success | -| `1` | invalid worker name — the name must be a DNS label | -| `1` | no name given, and nowhere to ask for one — not a terminal, or `-o` is in force | -| `1` | bad `--source`: outside the project, or a path the CLI owns | -| `1` | destination exists and is not empty | -| `1` | the worker is already recorded in `config.toml`, in any form | -| `1` | the rendered `config.toml` would not parse, or `[workers]` is a sealed inline table | +| Code | Condition | +| ---- | -------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | invalid worker name — the name must be a DNS label | +| `1` | no name given, and nowhere to ask for one — stdin or stdout is not a terminal, or `-o` is in force | +| `1` | bad `--source`: outside the project, or a path the CLI owns | +| `1` | destination exists and is not empty | +| `1` | the worker is already recorded in `config.toml`, in any form | +| `1` | the rendered `config.toml` would not parse, or `[workers]` is a sealed inline table | ## Environment Variables diff --git a/apps/cli/src/legacy/commands/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/workers/new/new.handler.ts index cc777f7e4e..654b25a3b7 100644 --- a/apps/cli/src/legacy/commands/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/workers/new/new.handler.ts @@ -8,6 +8,7 @@ import { } from "../workers.output.ts"; import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { RuntimeInfo } from "../../../../shared/runtime/runtime-info.service.ts"; +import { Tty } from "../../../../shared/runtime/tty.service.ts"; import { commitWorkerEntry, planWorkerEntry, @@ -57,6 +58,26 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { return [defaultValue, ...values.filter((value) => value !== defaultValue)]; } +/** + * Whether this run has a terminal to ask on. + * + * `-o json|yaml|toml|env` leaves `output.format` as `text`, and the prompts go + * through Clack, which writes its terminal UI to stdout with no stream + * override — so a machine format is as non-interactive as a redirected stdout, + * whichever flag asked for it. + * + * `output.interactive` only tracks *stdout*, so on its own it still let + * `printf 'api\n' | supabase workers new` feed the pipe straight into the name + * prompt instead of taking the documented non-interactive path. A prompt is + * only answerable from a keyboard, so stdin has to be a terminal too — the same + * pair `workers delete` guards its confirmation with. + */ +const canPromptFor = Effect.fnUntraced(function* (machineOutput: boolean) { + const output = yield* Output; + const tty = yield* Tty; + return output.format === "text" && output.interactive && !machineOutput && tty.stdinIsTty; +}); + /** * The worker name, asked for when the command line did not carry one. * @@ -69,16 +90,16 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { */ const resolveName = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; readonly project: LegacyWorkersProject; }) { if (Option.isSome(options.explicit)) { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; return yield* output.promptText("What should this worker be called?", { validate: (value) => { const invalid = validateWorkerNameMessage(value); @@ -102,8 +123,8 @@ const resolveName = Effect.fnUntraced(function* (options: { const resolveRuntime = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; }) { // `--runtime` is a choice flag, so the parser has already rejected anything // outside the catalog by the time it gets here. @@ -111,8 +132,8 @@ const resolveRuntime = Effect.fnUntraced(function* (options: { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; const selected = yield* output.promptSelect( "Which runtime should this worker use?", defaultFirst([...WORKER_RUNTIMES], DEFAULT_WORKER_RUNTIME).map((runtime) => ({ @@ -129,15 +150,15 @@ const resolveRuntime = Effect.fnUntraced(function* (options: { const resolveSize = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; }) { if (Option.isSome(options.explicit)) { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; const selected = yield* output.promptSelect( "Which instance size should this worker use?", defaultFirst([...WORKER_SIZES], DEFAULT_WORKER_SIZE).map((size) => ({ @@ -181,14 +202,12 @@ export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( yield* Effect.gen(function* () { const project = yield* legacyLoadWorkersProjectForEntryWrite(); - // `-o` leaves `output.format` as `text`, and the prompts go through Clack, - // which writes its terminal UI to stdout with no stream override — so a - // prompt would land in front of the payload just as the notices did. Read - // before the first prompt rather than beside the last, since the name is - // now asked for too. + // Decided once, before the first prompt rather than beside the last, since + // the name is now asked for too — every prompt below shares the answer. const machineOutput = yield* legacyWorkersMachineOutputRequested(); + const canPrompt = yield* canPromptFor(machineOutput); - const name = yield* resolveName({ explicit: flags.name, machineOutput, project }); + const name = yield* resolveName({ explicit: flags.name, canPrompt, project }); yield* legacyValidateWorkerName(name); // Refused before anything is asked or written. `new` creates a worker; @@ -206,10 +225,10 @@ export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( } // Resolved before anything is written, so cancelling either prompt leaves - // nothing behind — the name included. With a machine format requested there - // is nowhere to ask, so the defaults stand. - const runtime = yield* resolveRuntime({ explicit: flags.runtime, machineOutput }); - const size = yield* resolveSize({ explicit: flags.size, machineOutput }); + // nothing behind — the name included. With nowhere to ask, the defaults + // stand; only the name has nothing to fall back to. + const runtime = yield* resolveRuntime({ explicit: flags.runtime, canPrompt }); + const size = yield* resolveSize({ explicit: flags.size, canPrompt }); // Validated before anything is written: this is the directory the starter // files land in, so a value naming the project root, `supabase/`, or diff --git a/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts index d6d1e7279f..38f6e12619 100644 --- a/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/workers/new/new.integration.test.ts @@ -119,6 +119,10 @@ describe("legacy workers new", () => { { label: "not interactive", setup: { interactive: false } }, // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. { label: "-o json", setup: { goOutput: "json" as const } }, + // `printf 'orders\n' | supabase workers new`: stdout is still a terminal, so + // `output.interactive` on its own would have fed the pipe straight into the + // name prompt instead of taking this documented path. + { label: "piped stdin", setup: { stdinIsTty: false } }, ])("refuses a bare new when there is nowhere to ask ($label)", ({ setup }) => { const repo = project(); const { layer, out } = setupLegacyWorkers({ @@ -159,6 +163,25 @@ describe("legacy workers new", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The runtime and size prompts do have defaults to fall back on, so a piped + // stdin must leave them unasked rather than consuming the pipe. + it.live("takes the defaults without prompting when stdin is piped", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + stdinIsTty: false, + promptSelectResponses: ["node", "4gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.some("api") })); + + expect(out.promptSelectCalls).toEqual([]); + expect(repo.config()).toContain('runtime = "deno"'); + expect(repo.config()).toContain('size = "2gb"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("falls back to the defaults without prompting when not interactive", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, format: "json" }); From 93c7aa6c5d19a67f175cfab00b6b852e813dac97 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 28 Aug 2026 23:31:40 -0300 Subject: [PATCH 03/15] feat(cli): prompt for worker name if not provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the `name` argument to `supabase workers new` optional and prompts for it when it is omitted, so a bare `supabase workers new` walks through name, runtime and size rather than failing the parse. The name is the one input this command cannot default — it is the directory, the `[workers.]` key and the hostname all at once. So where the runtime and size prompts fall back to a default when there is nowhere to ask, the name prompt has nothing to fall back to: with `-o json|yaml|toml|env` or no interactive terminal, the command fails with a new `MissingWorkerNameError` pointing at `supabase workers new api`. The prompt validates against everything the command would otherwise refuse a moment later — a non-DNS-label name, and a name `config.toml` already records — so a typo is corrected in place instead of ending the run. That also means the project has to be loaded before the first prompt, and the machine-output check moves up with it: `-o` leaves `output.format` as `text`, and Clack writes its terminal UI to stdout, so a name prompt would land in front of the payload for the same reason the runtime prompt would. The handler's inline name validation is replaced by the shared `legacyValidateWorkerName`, which the rest of the command family already uses, so an explicitly-passed name and a prompted one are refused on identical terms. `mockOutput` now records `promptTextCalls` so tests can assert on the prompt's message and exercise its `validate` callback. --- .../experimental/workers/new/SIDE_EFFECTS.md | 20 ++- .../experimental/workers/new/new.command.ts | 9 +- .../experimental/workers/new/new.handler.ts | 85 +++++++++---- .../workers/new/new.integration.test.ts | 116 ++++++++++++++---- apps/cli/src/shared/workers/workers.errors.ts | 16 +++ apps/cli/tests/helpers/mocks.ts | 8 +- 6 files changed, 204 insertions(+), 50 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md index d614531471..dd691153f4 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md @@ -1,4 +1,4 @@ -# `supabase experimental workers new ` +# `supabase experimental workers new [name]` > **Local-disk only.** Nothing is deployed and no Management API route is > called; `workers push` is what talks to the platform. @@ -35,6 +35,13 @@ same resolver `start`/`stop`/`status` use) and never climbs to an ancestor. A therefore records the worker in that directory's own `config.toml` — created if absent — rather than in the ancestor project's. +The name is prompted for when the command line does not carry one, and the +prompt refuses a name that is not a DNS label or that `config.toml` already +records — so nothing is asked, and nothing written, for a name the command was +going to refuse. With `-o json|yaml|toml|env` or no interactive terminal there is +nowhere to ask, and the command fails instead of defaulting: unlike the runtime +and size, the name has no default to fall back on. + Writes to `config.toml` are append-only. A worker already recorded under `[workers.]` is refused outright — before the runtime and size prompts, and before anything reaches disk — because editing an entry the user owns is @@ -61,6 +68,7 @@ root. | ---- | ----------------------------------------------------------------------------------- | | `0` | success | | `1` | invalid worker name — the name must be a DNS label | +| `1` | no name given, and nowhere to ask for one — not a terminal, or `-o` is in force | | `1` | bad `--source`: outside the project, or a path the CLI owns | | `1` | destination exists and is not empty | | `1` | the worker is already recorded in `config.toml`, in any form | @@ -83,7 +91,9 @@ root. No custom events — only the `cli_command_executed` that the instrumentation wrapper emits for every command. -Nothing is emitted for a failure the parser catches, such as a missing worker -name or a `--runtime`/`--size` value outside the choice list. The wrapper is -installed by `Command.withHandler`, so a command that never reaches its handler -never reaches the instrumentation either — and `telemetry.json` is not written. +Nothing is emitted for a failure the parser catches, such as a +`--runtime`/`--size` value outside the choice list. The wrapper is installed by +`Command.withHandler`, so a command that never reaches its handler never reaches +the instrumentation either — and `telemetry.json` is not written. A missing name +is _not_ one of those: the argument is optional, so a bare `workers new` reaches +the handler, which asks for the name or fails for want of anywhere to ask. diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts index 66bfd28d45..ce93798dd9 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts @@ -12,7 +12,10 @@ import { legacyWorkersNew } from "./new.handler.ts"; const config = { name: Argument.string("name").pipe( - Argument.withDescription("Worker name. Doubles as its directory, and its hostname."), + Argument.withDescription( + "Worker name. Doubles as its directory, and its hostname. Prompted when omitted.", + ), + Argument.optional, ), runtime: Flag.choice("runtime", WORKER_RUNTIMES).pipe( Flag.withDescription( @@ -51,6 +54,10 @@ export const legacyWorkersNewCommand = Command.make("new", config).pipe( ), Command.withShortDescription("Scaffold a worker locally"), Command.withExamples([ + { + command: "supabase experimental workers new", + description: "Prompt for the name, then for runtime and size", + }, { command: "supabase experimental workers new api", description: "Scaffold supabase/workers/api, prompting for runtime and size", diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index 018f1caaf1..808afd0b63 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -33,18 +33,22 @@ import { } from "../../../../../shared/workers/worker-runtimes.ts"; import { WORKER_STACKS } from "../../../../../shared/workers/worker-stacks.ts"; import { - InvalidWorkerNameError, + MissingWorkerNameError, WorkerDirectoryExistsError, } from "../../../../../shared/workers/workers.errors.ts"; -import { legacyLoadWorkersProjectForEntryWrite } from "../workers.shared.ts"; +import { + legacyLoadWorkersProjectForEntryWrite, + legacyValidateWorkerName, + type LegacyWorkersProject, +} from "../workers.shared.ts"; import type { LegacyWorkersNewFlags } from "./new.command.ts"; /** - * `supabase experimental workers new ` — scaffold `supabase/workers//` from the + * `supabase experimental workers new [name]` — scaffold `supabase/workers//` from the * chosen runtime's starter files and record the choice in `config.toml`. * Nothing is deployed; this is entirely local-disk work. * - * The runtime and size are resolved *before* anything is written, so a + * The name, runtime and size are all resolved *before* anything is written, so a * cancelled prompt leaves nothing behind for this worker at all. */ @@ -53,6 +57,49 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { return [defaultValue, ...values.filter((value) => value !== defaultValue)]; } +/** + * The worker name, asked for when the command line did not carry one. + * + * The name is the one input here that cannot be defaulted — it is the + * directory, the `config.toml` key and the hostname — so a bare + * `supabase workers new` asks rather than failing the parse. The prompt + * validates against everything the command would otherwise refuse a moment + * later, so a mistyped or already-recorded name is corrected in place instead + * of ending the run. + */ +const resolveName = Effect.fnUntraced(function* (options: { + readonly explicit: Option.Option; + /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ + readonly machineOutput: boolean; + readonly project: LegacyWorkersProject; +}) { + if (Option.isSome(options.explicit)) { + return options.explicit.value; + } + + const output = yield* Output; + if (output.format === "text" && output.interactive && !options.machineOutput) { + return yield* output.promptText("What should this worker be called?", { + validate: (value) => { + const invalid = validateWorkerNameMessage(value); + if (invalid !== undefined) { + return invalid; + } + return options.project.section.workers[value] === undefined + ? undefined + : `"${value}" is already configured in ${options.project.configPath}.`; + }, + }); + } + + return yield* Effect.fail( + new MissingWorkerNameError({ + detail: "Worker name is required in non-interactive mode.", + suggestion: "Pass a worker name, for example `supabase workers new api`.", + }), + ); +}); + const resolveRuntime = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ @@ -134,21 +181,21 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun yield* Effect.gen(function* () { const project = yield* legacyLoadWorkersProjectForEntryWrite(); - const name = flags.name; - const invalid = validateWorkerNameMessage(name); - if (invalid !== undefined) { - return yield* Effect.fail( - new InvalidWorkerNameError({ - detail: `"${name}" is not a valid worker name. ${invalid}`, - suggestion: "Worker names become hostnames, so they must be DNS labels.", - }), - ); - } + // `-o` leaves `output.format` as `text`, and the prompts go through Clack, + // which writes its terminal UI to stdout with no stream override — so a + // prompt would land in front of the payload just as the notices did. Read + // before the first prompt rather than beside the last, since the name is + // now asked for too. + const machineOutput = yield* legacyWorkersMachineOutputRequested(); + + const name = yield* resolveName({ explicit: flags.name, machineOutput, project }); + yield* legacyValidateWorkerName(name); // Refused before anything is asked or written. `new` creates a worker; // changing one that already exists is a `config.toml` edit, and the file is // the user's. Checking here rather than only in `planWorkerEntry` means the - // prompts never run for a name that was going to be refused anyway. + // runtime and size prompts never run for a name that was going to be + // refused anyway; the name prompt rejects it up front for the same reason. if (project.section.workers[name] !== undefined) { return yield* Effect.fail( new WorkerAlreadyConfiguredError({ @@ -159,12 +206,8 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun } // Resolved before anything is written, so cancelling either prompt leaves - // nothing behind — the name included. - // `-o` leaves `output.format` as `text`, and `promptSelect` goes through - // Clack, which writes its terminal UI to stdout with no stream override — so - // a prompt would land in front of the payload just as the notices did. With a - // machine format requested there is nowhere to ask, so the defaults stand. - const machineOutput = yield* legacyWorkersMachineOutputRequested(); + // nothing behind — the name included. With a machine format requested there + // is nowhere to ask, so the defaults stand. const runtime = yield* resolveRuntime({ explicit: flags.runtime, machineOutput }); const size = yield* resolveSize({ explicit: flags.size, machineOutput }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index 6c9471aecf..2fcf05b552 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -13,6 +13,7 @@ import { import { InvalidWorkerNameError, InvalidWorkerSourceError, + MissingWorkerNameError, WorkerDirectoryExistsError, } from "../../../../../shared/workers/workers.errors.ts"; import { legacyWorkersNew } from "./new.handler.ts"; @@ -27,7 +28,7 @@ verify_jwt = false function flags(overrides: Partial = {}): LegacyWorkersNewFlags { return { - name: "api", + name: Option.some("api"), runtime: Option.none(), size: Option.none(), source: Option.none(), @@ -54,7 +55,7 @@ describe("legacy workers new", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); const workerDir = join(repo.dir, "supabase", "workers", "api"); expect(existsSync(join(workerDir, "index.mjs"))).toBe(true); @@ -69,6 +70,75 @@ describe("legacy workers new", () => { expect(out.stdoutText).toContain("supabase experimental workers push api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + it.live("asks for the name when the command line carries none", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["orders"], + promptSelectResponses: ["node", "2gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.none() })); + + expect(out.promptTextCalls.map((call) => call.message)).toEqual([ + "What should this worker be called?", + ]); + expect(existsSync(join(repo.dir, "supabase", "workers", "orders", "index.mjs"))).toBe(true); + expect(repo.config()).toContain("[workers.orders]"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The prompt is the last place a mistyped or taken name can be corrected + // without ending the run, so it refuses both there rather than after asking. + it.live("refuses a bad or already-recorded name at the name prompt", () => { + const repo = project({ + "supabase/config.toml": `${CONFIG_WITH_COMMENTS}\n[workers.api]\nruntime = "node"\nsize = "2gb"\n`, + }); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["orders"], + promptSelectResponses: ["node", "2gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.none() })); + + const validate = out.promptTextCalls[0]?.opts?.validate; + expect(validate).toBeDefined(); + expect(validate?.("My_Worker")).toContain("lowercase letters"); + expect(validate?.("api")).toContain("already configured"); + expect(validate?.("orders")).toBeUndefined(); + expect(existsSync(join(repo.dir, "supabase", "workers", "orders", "index.mjs"))).toBe(true); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Nowhere to ask means nothing to scaffold under: the name is the directory, + // the config key and the hostname, and none of those has a default. + it.live.each([ + { label: "not interactive", setup: { interactive: false } }, + // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. + { label: "-o json", setup: { goOutput: "json" as const } }, + ])("refuses a bare new when there is nowhere to ask ($label)", ({ setup }) => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + // An answer is waiting, so a prompt would succeed rather than fail some + // other way. + promptTextResponses: ["orders"], + ...setup, + }); + + return Effect.gen(function* () { + const error = yield* legacyWorkersNew(flags({ name: Option.none() })).pipe(Effect.flip); + + expect(error).toBeInstanceOf(MissingWorkerNameError); + expect(out.promptTextCalls).toEqual([]); + expect(existsSync(join(repo.dir, "supabase", "workers"))).toBe(false); + expect(repo.config()).toBe(CONFIG_WITH_COMMENTS); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("prompts for runtime and size when neither is given", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ @@ -77,7 +147,7 @@ describe("legacy workers new", () => { }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); expect(out.promptSelectCalls.map((call) => call.message)).toEqual([ "Which runtime should this worker use?", @@ -94,7 +164,7 @@ describe("legacy workers new", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, format: "json" }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); expect(out.promptSelectCalls).toHaveLength(0); expect(repo.config()).toContain('runtime = "deno"'); @@ -110,12 +180,12 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("deno"), size: Option.some("4gb") }), + flags({ name: Option.some("api"), runtime: Option.some("deno"), size: Option.some("4gb") }), ); const recorded = repo.config(); const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -136,7 +206,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -154,7 +224,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some("packages/api"), }), @@ -174,7 +244,7 @@ describe("legacy workers new", () => { for (const source of [".", "..", "supabase", "supabase/functions"]) { const error = yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some(source), }), @@ -194,7 +264,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: created.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); expect(existsSync(join(created.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); expect(readFileSync(join(created.dir, "supabase", "config.toml"), "utf8")).toBe( @@ -212,7 +282,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -228,7 +298,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); expect(existsSync(join(repo.dir, "supabase", "workers", "api", "index.mjs"))).toBe(true); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); @@ -240,7 +310,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -257,7 +327,9 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - const error = yield* legacyWorkersNew(flags({ name: "My_Worker" })).pipe(Effect.flip); + const error = yield* legacyWorkersNew(flags({ name: Option.some("My_Worker") })).pipe( + Effect.flip, + ); expect(error).toBeInstanceOf(InvalidWorkerNameError); expect(existsSync(join(repo.dir, "supabase", "workers"))).toBe(false); @@ -286,7 +358,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("deno") }), + flags({ name: Option.some("api"), runtime: Option.some("deno") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerAlreadyConfiguredError); @@ -307,7 +379,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); const jsonPath = join(repo.dir, "supabase", "config.json"); expect(readFileSync(jsonPath, "utf8")).toBe(configJson); @@ -332,7 +404,7 @@ describe("legacy workers new", () => { const { layer } = setupLegacyWorkers({ workdir }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api", runtime: Option.some("node") })); + yield* legacyWorkersNew(flags({ name: Option.some("api"), runtime: Option.some("node") })); // The ancestor project is untouched. expect(repo.config()).toBe(CONFIG_WITH_COMMENTS); @@ -357,7 +429,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerConfigWriteUnsafeError); @@ -374,7 +446,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( - flags({ name: "api", runtime: Option.some("node") }), + flags({ name: Option.some("api"), runtime: Option.some("node") }), ).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerDirectoryExistsError); @@ -396,7 +468,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some("generated"), }), @@ -423,7 +495,7 @@ describe("legacy workers new", () => { }); return Effect.gen(function* () { - yield* legacyWorkersNew(flags({ name: "api" })); + yield* legacyWorkersNew(flags({ name: Option.some("api") })); const payload: unknown = JSON.parse(out.stdoutText); // The defaults stand, because there was nowhere to ask. @@ -439,7 +511,7 @@ describe("legacy workers new", () => { return Effect.gen(function* () { const error = yield* legacyWorkersNew( flags({ - name: "api", + name: Option.some("api"), runtime: Option.some("node"), source: Option.some(join("supabase", "config.toml")), }), diff --git a/apps/cli/src/shared/workers/workers.errors.ts b/apps/cli/src/shared/workers/workers.errors.ts index 44826c9315..eda518866c 100644 --- a/apps/cli/src/shared/workers/workers.errors.ts +++ b/apps/cli/src/shared/workers/workers.errors.ts @@ -21,6 +21,22 @@ export class InvalidWorkerNameError extends Data.TaggedError("InvalidWorkerNameE } } +/** + * A bare `new` had no name to scaffold under, and nowhere to ask for one. + * + * The name is the one input this command cannot default — it is the directory, + * the `config.toml` key and the hostname all at once — so with `-o` in force or + * no interactive terminal there is nothing to do but say so. + */ +export class MissingWorkerNameError extends Data.TaggedError("MissingWorkerNameError")<{ + readonly detail: string; + readonly suggestion: string; +}> { + get [ErrorActionabilityId](): CliErrorActionabilityDeclaration { + return actionability.provideFlags; + } +} + /** * A symlink in the worker source points outside the build context. * diff --git a/apps/cli/tests/helpers/mocks.ts b/apps/cli/tests/helpers/mocks.ts index 6e766d28c2..02bec33028 100644 --- a/apps/cli/tests/helpers/mocks.ts +++ b/apps/cli/tests/helpers/mocks.ts @@ -283,6 +283,10 @@ export function mockOutput( } | undefined; }> = []; + const promptTextCalls: Array<{ + message: string; + opts?: { defaultValue?: string; validate?: (v: string) => string | undefined }; + }> = []; const promptTextResponses = [...(opts.promptTextResponses ?? [])]; const promptSelectResponses = [...(opts.promptSelectResponses ?? [])]; const promptPasswordResponses = [...(opts.promptPasswordResponses ?? [])]; @@ -387,10 +391,11 @@ export function mockOutput( promptText: (() => { let callCount = 0; return ( - _msg: string, + message: string, options?: { defaultValue?: string; validate?: (v: string) => string | undefined }, ) => { callCount++; + promptTextCalls.push({ message, opts: options }); // Exercise the validate callback to cover both branches (line 140) if (options?.validate) { options.validate(""); // truthy branch: returns error message @@ -451,6 +456,7 @@ export function mockOutput( events, promptConfirmCalls, promptSelectCalls, + promptTextCalls, rawChunks, get stdoutText() { return rawChunks From 5c84df0b9163ff6d96b7b632a3fdba97a4474c83 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 28 Aug 2026 23:41:09 -0300 Subject: [PATCH 04/15] fix(cli): gate the workers new prompts on stdin too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `output.interactive` is derived solely from `tty.stdoutIsTty`, so with stdin piped or redirected and stdout still on a terminal it stayed true. A bare `printf 'api\n' | supabase workers new` therefore opened Clack's name prompt and read the worker name off the pipe instead of taking the documented `MissingWorkerNameError` path — and the runtime and size prompts consumed whatever followed rather than falling back to their defaults. The three resolvers now share one `canPromptFor` decision, made once before the first prompt, which pairs `output.interactive` with `tty.stdinIsTty` the way `workers delete` already guards its confirmation. A prompt is only answerable from a keyboard, so both streams have to be a terminal. --- .../experimental/workers/new/SIDE_EFFECTS.md | 26 ++++---- .../experimental/workers/new/new.handler.ts | 63 ++++++++++++------- .../workers/new/new.integration.test.ts | 23 +++++++ 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md index dd691153f4..e52dade14e 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md @@ -38,9 +38,11 @@ absent — rather than in the ancestor project's. The name is prompted for when the command line does not carry one, and the prompt refuses a name that is not a DNS label or that `config.toml` already records — so nothing is asked, and nothing written, for a name the command was -going to refuse. With `-o json|yaml|toml|env` or no interactive terminal there is -nowhere to ask, and the command fails instead of defaulting: unlike the runtime -and size, the name has no default to fall back on. +going to refuse. With `-o json|yaml|toml|env`, a redirected stdout, or a stdin +that is not a terminal, there is nowhere to ask, and the command fails instead +of defaulting: unlike the runtime and size, the name has no default to fall back +on. Every prompt is gated on both streams, so `printf 'api\n' | supabase workers +new` takes that failure path rather than reading the worker name off the pipe. Writes to `config.toml` are append-only. A worker already recorded under `[workers.]` is refused outright — before the runtime and size prompts, @@ -64,15 +66,15 @@ root. ## Exit Codes -| Code | Condition | -| ---- | ----------------------------------------------------------------------------------- | -| `0` | success | -| `1` | invalid worker name — the name must be a DNS label | -| `1` | no name given, and nowhere to ask for one — not a terminal, or `-o` is in force | -| `1` | bad `--source`: outside the project, or a path the CLI owns | -| `1` | destination exists and is not empty | -| `1` | the worker is already recorded in `config.toml`, in any form | -| `1` | the rendered `config.toml` would not parse, or `[workers]` is a sealed inline table | +| Code | Condition | +| ---- | -------------------------------------------------------------------------------------------------- | +| `0` | success | +| `1` | invalid worker name — the name must be a DNS label | +| `1` | no name given, and nowhere to ask for one — stdin or stdout is not a terminal, or `-o` is in force | +| `1` | bad `--source`: outside the project, or a path the CLI owns | +| `1` | destination exists and is not empty | +| `1` | the worker is already recorded in `config.toml`, in any form | +| `1` | the rendered `config.toml` would not parse, or `[workers]` is a sealed inline table | ## Environment Variables diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index 808afd0b63..ee353babda 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -8,6 +8,7 @@ import { } from "../workers.output.ts"; import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { RuntimeInfo } from "../../../../../shared/runtime/runtime-info.service.ts"; +import { Tty } from "../../../../../shared/runtime/tty.service.ts"; import { commitWorkerEntry, planWorkerEntry, @@ -57,6 +58,26 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { return [defaultValue, ...values.filter((value) => value !== defaultValue)]; } +/** + * Whether this run has a terminal to ask on. + * + * `-o json|yaml|toml|env` leaves `output.format` as `text`, and the prompts go + * through Clack, which writes its terminal UI to stdout with no stream + * override — so a machine format is as non-interactive as a redirected stdout, + * whichever flag asked for it. + * + * `output.interactive` only tracks *stdout*, so on its own it still let + * `printf 'api\n' | supabase workers new` feed the pipe straight into the name + * prompt instead of taking the documented non-interactive path. A prompt is + * only answerable from a keyboard, so stdin has to be a terminal too — the same + * pair `workers delete` guards its confirmation with. + */ +const canPromptFor = Effect.fnUntraced(function* (machineOutput: boolean) { + const output = yield* Output; + const tty = yield* Tty; + return output.format === "text" && output.interactive && !machineOutput && tty.stdinIsTty; +}); + /** * The worker name, asked for when the command line did not carry one. * @@ -69,16 +90,16 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { */ const resolveName = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; readonly project: LegacyWorkersProject; }) { if (Option.isSome(options.explicit)) { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; return yield* output.promptText("What should this worker be called?", { validate: (value) => { const invalid = validateWorkerNameMessage(value); @@ -102,8 +123,8 @@ const resolveName = Effect.fnUntraced(function* (options: { const resolveRuntime = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; }) { // `--runtime` is a choice flag, so the parser has already rejected anything // outside the catalog by the time it gets here. @@ -111,8 +132,8 @@ const resolveRuntime = Effect.fnUntraced(function* (options: { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; const selected = yield* output.promptSelect( "Which runtime should this worker use?", defaultFirst([...WORKER_RUNTIMES], DEFAULT_WORKER_RUNTIME).map((runtime) => ({ @@ -129,15 +150,15 @@ const resolveRuntime = Effect.fnUntraced(function* (options: { const resolveSize = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; - /** `-o json|yaml|toml|env` — stdout belongs to the payload, so do not prompt. */ - readonly machineOutput: boolean; + /** Whether there is a terminal to ask on — see `canPromptFor`. */ + readonly canPrompt: boolean; }) { if (Option.isSome(options.explicit)) { return options.explicit.value; } - const output = yield* Output; - if (output.format === "text" && output.interactive && !options.machineOutput) { + if (options.canPrompt) { + const output = yield* Output; const selected = yield* output.promptSelect( "Which instance size should this worker use?", defaultFirst([...WORKER_SIZES], DEFAULT_WORKER_SIZE).map((size) => ({ @@ -181,14 +202,12 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun yield* Effect.gen(function* () { const project = yield* legacyLoadWorkersProjectForEntryWrite(); - // `-o` leaves `output.format` as `text`, and the prompts go through Clack, - // which writes its terminal UI to stdout with no stream override — so a - // prompt would land in front of the payload just as the notices did. Read - // before the first prompt rather than beside the last, since the name is - // now asked for too. + // Decided once, before the first prompt rather than beside the last, since + // the name is now asked for too — every prompt below shares the answer. const machineOutput = yield* legacyWorkersMachineOutputRequested(); + const canPrompt = yield* canPromptFor(machineOutput); - const name = yield* resolveName({ explicit: flags.name, machineOutput, project }); + const name = yield* resolveName({ explicit: flags.name, canPrompt, project }); yield* legacyValidateWorkerName(name); // Refused before anything is asked or written. `new` creates a worker; @@ -206,10 +225,10 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun } // Resolved before anything is written, so cancelling either prompt leaves - // nothing behind — the name included. With a machine format requested there - // is nowhere to ask, so the defaults stand. - const runtime = yield* resolveRuntime({ explicit: flags.runtime, machineOutput }); - const size = yield* resolveSize({ explicit: flags.size, machineOutput }); + // nothing behind — the name included. With nowhere to ask, the defaults + // stand; only the name has nothing to fall back to. + const runtime = yield* resolveRuntime({ explicit: flags.runtime, canPrompt }); + const size = yield* resolveSize({ explicit: flags.size, canPrompt }); // Validated before anything is written: this is the directory the starter // files land in, so a value naming the project root, `supabase/`, or diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index 2fcf05b552..2886c62add 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -119,6 +119,10 @@ describe("legacy workers new", () => { { label: "not interactive", setup: { interactive: false } }, // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. { label: "-o json", setup: { goOutput: "json" as const } }, + // `printf 'orders\n' | supabase workers new`: stdout is still a terminal, so + // `output.interactive` on its own would have fed the pipe straight into the + // name prompt instead of taking this documented path. + { label: "piped stdin", setup: { stdinIsTty: false } }, ])("refuses a bare new when there is nowhere to ask ($label)", ({ setup }) => { const repo = project(); const { layer, out } = setupLegacyWorkers({ @@ -159,6 +163,25 @@ describe("legacy workers new", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The runtime and size prompts do have defaults to fall back on, so a piped + // stdin must leave them unasked rather than consuming the pipe. + it.live("takes the defaults without prompting when stdin is piped", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + stdinIsTty: false, + promptSelectResponses: ["node", "4gb"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew(flags({ name: Option.some("api") })); + + expect(out.promptSelectCalls).toEqual([]); + expect(repo.config()).toContain('runtime = "deno"'); + expect(repo.config()).toContain('size = "2gb"'); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("falls back to the defaults without prompting when not interactive", () => { const repo = project(); const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, format: "json" }); From 0af0f0ae6c1d3685bc97fdc96ec84eb1ee9e6467 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 31 Aug 2026 19:41:01 -0300 Subject: [PATCH 05/15] chore(workers): describe behaviour rather than its history in comments Comments explaining why code is shaped a certain way now state the constraint directly instead of narrating what an earlier version did. The reasoning is unchanged; only the framing is. --- .../experimental/workers/new/new.integration.test.ts | 4 ++-- .../legacy/commands/experimental/workers/workers.output.ts | 4 ++-- apps/cli/src/shared/workers/worker-package.unit.test.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index 2886c62add..a9fbf2872a 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -461,8 +461,8 @@ describe("legacy workers new", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // A plain file used to read as an empty directory, which then failed with a - // bare EEXIST from `makeDirectory` instead of naming what was in the way. + // A plain file must not read as an empty directory: that fails with a bare + // EEXIST from `makeDirectory` instead of naming what is in the way. it.live("refuses a plain file at the destination", () => { const repo = project({ "supabase/workers/api": "not a directory" }); const { layer } = setupLegacyWorkers({ workdir: repo.dir }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts index 3bf81b7c4c..cda3b71759 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts @@ -12,8 +12,8 @@ import { LegacyWorkersEnvNotSupportedError } from "./workers.errors.ts"; * machine-readable. * * The struct-shaped encoders elsewhere reproduce a payload shape their command - * already shipped. `workers` has none to match, so it serialises through the - * generic encoders and shapes its payload as the command reads best. + * is required to match. `workers` has none, so it serialises through the generic + * encoders and shapes its payload as the command reads best. * * Returns whether it emitted anything, so the caller can skip its text * rendering — `output.success` writes to stdout in text mode and would corrupt diff --git a/apps/cli/src/shared/workers/worker-package.unit.test.ts b/apps/cli/src/shared/workers/worker-package.unit.test.ts index d95169a2d7..7e53609a95 100644 --- a/apps/cli/src/shared/workers/worker-package.unit.test.ts +++ b/apps/cli/src/shared/workers/worker-package.unit.test.ts @@ -203,9 +203,9 @@ describe("packageWorkerDirectory", () => { expect(result.fileCount).toBe(0); }); - // A file that cannot be read used to be archived as zero bytes, so `push` - // reported success for a deploy that shipped an empty file. Failing is the - // only honest answer: the archive is the application. + // Archiving an unreadable file as zero bytes would report success for a deploy + // carrying an empty file. Failing is the only honest answer: the archive is the + // application. test("fails rather than archiving a file it cannot read as empty", async () => { const unreadable = join(dir, "secret.txt"); writeFileSync(unreadable, "important"); From a74e3f9555e047675f436f34e6853be6ca8887a0 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 29 Aug 2026 00:11:51 -0300 Subject: [PATCH 06/15] feat(workers): bring the command family's output onto one shape The workers commands each grew their own way of saying "here is what happened" and "here is what to run next". This settles them on the shapes the rest of the legacy shell already uses, with no change to what any command does. - "What to run next" lines in `new`, `push`, `delete` and `status` move to `emitSuccessTrailer`, the way `stop`, `bootstrap`, `migration repair` and `gen signing-key` already emit theirs: printed once at the end of the run rather than inline, so a multi-worker push does not bury each worker's hint under the next worker's output. The commands within them are aqua'd, as every other follow-up hint in this shell writes them. - `list`'s two advisories take the yellow `WARNING:` prefix and the two-line consequence shape `start`'s Docker notice uses. Each was one long sentence that re-flowed at a different width under a table that lines its columns up. - `list` drops the URL column. Every worker's URL is the same host and prefix with the name on the end, and carrying it pushed the table past 130 columns for one derivable field, since `renderGlamourTable` sizes to the widest cell and never wraps. `status` still renders it vertically, and every machine format still carries `url` per worker. - `push` counts its per-worker announcements (`Deploying Worker 1/2:`) and closes a multi-worker run with a summary line. Each worker takes minutes; the name alone said nothing about how much of the run was left. - `push` names the workers a failed run never attempted. The loop stops at the first failure and the error only names the worker that broke, leaving the rest to be reconstructed from argument order. On stderr in every format, machine ones included: that run is a CI run. - Both of `push`'s retry suggestions carry an explicit `--project-ref` when the flag supplied the ref, via the `legacyWorkersProjectRefSuffix` helper `status` and `delete` already use. A suggestion is copy-pasted verbatim, so one that dropped it re-resolved against whatever this checkout was linked to. Adds unit coverage for `legacyRenderWorkerDetails`'s padding and empty-row dropping, and pins the shared `-o env` refusal so a new command that forgets its own up-front check cannot silently emit TOML instead. --- .../workers/delete/SIDE_EFFECTS.md | 16 +- .../workers/delete/delete.handler.ts | 8 +- .../workers/delete/delete.integration.test.ts | 60 +++- .../experimental/workers/list/SIDE_EFFECTS.md | 4 + .../experimental/workers/list/list.handler.ts | 42 ++- .../workers/list/list.integration.test.ts | 84 ++++- .../experimental/workers/new/new.handler.ts | 19 +- .../workers/new/new.integration.test.ts | 29 +- .../experimental/workers/push/SIDE_EFFECTS.md | 11 + .../experimental/workers/push/push.handler.ts | 60 +++- .../workers/push/push.integration.test.ts | 338 +++++++++++++++++- .../workers/status/SIDE_EFFECTS.md | 16 +- .../workers/status/status.handler.ts | 10 +- .../workers/status/status.integration.test.ts | 29 +- .../workers/workers.format.unit.test.ts | 34 ++ .../workers.output.integration.test.ts | 40 +++ apps/cli/src/shared/workers/workers-api.ts | 9 +- 17 files changed, 756 insertions(+), 53 deletions(-) create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.format.unit.test.ts create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.output.integration.test.ts diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md index e9ebcabe0b..0b0d8cfc55 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md @@ -77,11 +77,11 @@ wrapper emits for every command. ## Output Formats -| Mode | stdout | stderr | -| ----------------------------- | --------------------------------------------------------------------------------------------- | --------------------------------------------- | -| text (default) | the confirmation prompt, then what was deleted and kept | that nothing local was kept, when nothing was | -| `--output-format json` | one structured result carrying `worker_name`, `project_ref`, `kept_*` | as above | -| `--output-format stream-json` | the same result as a single terminal event | as above | -| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | -| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | -| `-o env` | refused **before** the DELETE; discovering it at emit time deleted the worker and then failed | the error | +| Mode | stdout | stderr | +| ----------------------------- | --------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- | +| text (default) | the confirmation prompt, then what was deleted and kept | that nothing local was kept when nothing was, and the redeploy hint | +| `--output-format json` | one structured result carrying `worker_name`, `project_ref`, `kept_*` | as above | +| `--output-format stream-json` | the same result as a single terminal event | as above | +| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | +| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | +| `-o env` | refused **before** the DELETE; discovering it at emit time deleted the worker and then failed | the error | diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts index e5fc407967..f767f9d4ac 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts @@ -1,5 +1,6 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; +import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { @@ -50,7 +51,7 @@ import type { LegacyWorkersDeleteFlags } from "./delete.command.ts"; * stdout, so merely redirecting output would otherwise delete unattended. This * refuses instead, and says which flag would have authorised it. */ -export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete")(function* ( +export const legacyWorkersDelete = Effect.fn("legacy.workers.delete")(function* ( flags: LegacyWorkersDeleteFlags, ) { const output = yield* Output; @@ -232,8 +233,9 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete // alone is not enough to redeploy from, so `push` would fail on the very // command this line recommends. if (keptSource !== undefined) { - yield* output.raw( - `Redeploy it with supabase experimental workers push ${name}${refSuffix}.\n`, + // Trailer, like every other "what to run next" line in this shell. + yield* emitSuccessTrailer( + `Redeploy it with ${legacyAqua(`supabase experimental workers push ${name}${refSuffix}`)}.\n`, ); } } else { diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts index 1f6e9cf899..d8ce622e9f 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts @@ -72,7 +72,8 @@ describe("legacy workers delete", () => { // Nothing local is touched — that is what makes `push` a one-command undo. expect(existsSync(join(repo.dir, "supabase", "workers", "api", "index.js"))).toBe(true); expect(readFileSync(join(repo.dir, "supabase", "config.toml"), "utf8")).toBe(CONFIG); - expect(out.stdoutText).toContain("supabase experimental workers push api"); + // The redeploy hint is a success trailer, which lands on stderr. + expect(out.stderrText).toContain("supabase experimental workers push api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -541,6 +542,63 @@ describe("legacy workers delete", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + it.live("pluralizes the live instance count in the confirmation", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["api"], + routes: { + ...routes, + [getRoute]: { + status: 200, + body: { + data: workerResource({ + name: "api", + instances: 3, + instanceCounts: { declared: 3, live: 2, ready: 2, stale: 0 }, + }), + }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersDelete({ name: "api", projectRef: Option.none() }); + + expect(out.stdoutText).toContain("2 running instances will be terminated"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Scaled to zero: there is a tally, and it says nothing is running. Warning + // about terminated instances there would invent a consequence. + it.live("promises no terminations when nothing is running", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + promptTextResponses: ["api"], + routes: { + ...routes, + [getRoute]: { + status: 200, + body: { + data: workerResource({ + name: "api", + instances: 2, + instanceCounts: { declared: 2, live: 0, ready: 0, stale: 0 }, + }), + }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersDelete({ name: "api", projectRef: Option.none() }); + + expect(out.stdoutText).toContain("permanently deletes"); + expect(out.stdoutText).not.toContain("will be terminated"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // An orphan — deployed from another checkout — has no local entry and no local // directory, so there is nothing that was "kept" and `push` has no source to // redeploy from. diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/list/SIDE_EFFECTS.md index 322844cd65..b7ae13668c 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/list/SIDE_EFFECTS.md @@ -67,3 +67,7 @@ wrapper emits for every command. | `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | | `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | | `-o env` | refused before any request; the payload carries a `workers` array a flat `KEY=value` list cannot express | the error | + +The text table omits each worker's URL — it is the same host and prefix on +every row, and carrying it made the table 137 columns wide. Every machine +format still carries `url` per worker, and `workers status` renders it. diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts index c1c5434b58..6b0dbfa6e6 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts @@ -1,5 +1,7 @@ import { Effect } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; +import { legacyAqua, legacyYellow } from "../../../../shared/legacy-colors.ts"; +import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; import { renderGlamourTable } from "../../../../output/legacy-glamour-table.ts"; import { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; @@ -28,7 +30,15 @@ import type { LegacyWorkersListFlags } from "./list.command.ts"; * count from the spec. `status` is where the live tally lives. */ -const HEADERS = ["NAME", "RUNTIME", "SIZE", "STATE", "INSTANCES", "URL"] as const; +/** + * No URL column. Every worker's URL is the same 40-odd characters of host and + * prefix with the name on the end, which pushed the table past 130 columns to + * carry one derivable field — `renderGlamourTable` sizes each column to its + * widest cell and never wraps. `workers status` renders it, vertically, for the + * same reason (see `workers.format.ts`), and every machine format still carries + * `url` per worker. + */ +const HEADERS = ["NAME", "RUNTIME", "SIZE", "STATE", "INSTANCES"] as const; interface WorkerRow { readonly name: string; @@ -68,6 +78,14 @@ function runtimeLabel(row: WorkerRow): string { return runtimeLabelFor(row) ?? "-"; } +/** + * `api is` / `api, box are` — the subject of both advisories below, which only + * ever differ in the verb. + */ +function nameList(names: ReadonlyArray): string { + return `${names.join(", ")} ${names.length === 1 ? "is" : "are"}`; +} + function toCells(row: WorkerRow): ReadonlyArray { return [ row.name, @@ -75,11 +93,10 @@ function toCells(row: WorkerRow): ReadonlyArray { row.deployed === undefined ? "-" : formatApiSize(row.deployed.spec.size), stateLabel(row), row.deployed === undefined ? "-" : String(row.deployed.spec.instances), - row.url ?? "-", ]; } -export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(function* ( +export const legacyWorkersList = Effect.fn("legacy.workers.list")(function* ( flags: LegacyWorkersListFlags, ) { const output = yield* Output; @@ -165,7 +182,7 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f if (rows.length === 0) { yield* output.raw( - "No workers found. Scaffold one with supabase experimental workers new .\n", + `No workers found. Scaffold one with ${legacyAqua("supabase experimental workers new ", process.stdout)}.\n`, ); return; } @@ -178,14 +195,20 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f // the source directory *before* inferring a runtime and fails with // `WorkerSourceMissingError`, so telling that user about runtime guessing // points them at the wrong prerequisite. + // + // Both are written the way this shell writes every other heads-up that is + // not a failure: a yellow `WARNING:` prefix, then the consequence on its own + // line (`start`'s Docker-on-Windows notice is the same two-line shape). The + // single long sentence each of these used to be re-flowed differently at + // every terminal width, right under a table that lines its columns up. const unconfigured = rows .filter((row) => row.deployed !== undefined && !row.configured && row.local) .map((row) => row.name); if (unconfigured.length > 0) { + const configDisplay = displayPath(project.projectRoot, project.configPath); yield* output.raw( - `${unconfigured.join(", ")} ${ - unconfigured.length === 1 ? "is" : "are" - } deployed but absent from supabase/config.toml: pushing from here would have to guess the runtime.\n`, + `${legacyYellow("WARNING:")} ${nameList(unconfigured)} deployed but not in ${configDisplay}.\n` + + `Pushing from here would have to guess the runtime.\n`, "stderr", ); } @@ -195,9 +218,8 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f .map((row) => row.name); if (remoteOnly.length > 0) { yield* output.raw( - `${remoteOnly.join(", ")} ${ - remoteOnly.length === 1 ? "is" : "are" - } deployed but ${remoteOnly.length === 1 ? "has" : "have"} no source in this project: scaffold or restore it before pushing from here.\n`, + `${legacyYellow("WARNING:")} ${nameList(remoteOnly)} deployed with no source in this project.\n` + + `Scaffold or restore before pushing from here.\n`, "stderr", ); } diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts index eb50d0da97..a5370c3226 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts @@ -70,7 +70,9 @@ describe("legacy workers list", () => { expect(rows).toHaveLength(3); // Sorted by name, so `api`, `box`, then the scaffolded-but-undeployed `old`. expect(rows[0]).toContain("2gb (1 vCPU)"); - expect(rows[0]).toContain(`https://${WORKERS_PROJECT_REF}.supabase.co/workers/v1/api`); + // The URL is deliberately not a column: one derivable field pushed the + // table past 130 columns. The machine payload still carries it. + expect(stdout).not.toContain("https://"); expect(rows[1]).toContain("sandbox"); expect(rows[2]).toContain("not deployed"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); @@ -122,6 +124,63 @@ describe("legacy workers list", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // Two of them, so the advisory has to read as a list rather than as one name + // with a stray verb. + it.live("calls out every deployed worker config.toml does not know about", () => { + const created = makeWorkersProject({ + "supabase/config.toml": `project_id = "demo"\n`, + "supabase/workers/stray/index.js": "export default {};\n", + "supabase/workers/spare/index.js": "export default {};\n", + }); + const repo = { + dir: created.dir, + cleanup: () => rmSync(created.dir, { recursive: true, force: true }), + }; + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + routes: { + [listRoute]: { + status: 200, + body: { + data: [ + workerResource({ name: "stray", runtime: "node" }), + workerResource({ name: "spare", runtime: "node" }), + ], + }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersList({ projectRef: Option.none() }); + + expect(out.stderrText).toContain("spare, stray are deployed but not in"); + expect(out.stderrText).toContain("guess the runtime"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Deletion is asynchronous, so a worker can be listed while it is being torn + // down. Reporting its build state would show `active` for something on its + // way out. + it.live("shows a worker being torn down as deleting", () => { + const repo = project(`project_id = "demo"\n\n[workers.api]\nruntime = "node"\n`); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + routes: { + [listRoute]: { + status: 200, + body: { data: [workerResource({ name: "api", runtime: "node", deleting: true })] }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersList({ projectRef: Option.none() }); + + expect(out.stdoutText).toContain("deleting"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // Nothing local at all: `deployOneWorker` checks the source directory before // it ever infers a runtime, so "would have to guess the runtime" named the // wrong prerequisite for this one. @@ -386,6 +445,29 @@ describe("legacy workers list", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + it.live("encodes YAML when -o yaml asks for it", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + goOutput: "yaml", + routes: { + [listRoute]: { + status: 200, + body: { data: [workerResource({ name: "api", runtime: "node" })] }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersList({ projectRef: Option.none() }); + + expect(out.stdoutText).toContain("project_ref:"); + expect(out.stdoutText).toContain("name: api"); + // The table would have gone to stdout too, and broken the document. + expect(out.stdoutText).not.toContain("NAME"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // `pretty` is the human default; `table` and `csv` are accepted by the global // flag for `db query`'s benefit, and every resource command is meant to ignore // them and render text. All three used to fall through to the TOML encoder, diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index ee353babda..8f6276a8e9 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -1,6 +1,8 @@ import { join, relative, sep } from "node:path"; import { Effect, FileSystem, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; +import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; +import { legacyAqua, legacyBold } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersMachineOutput, @@ -67,7 +69,7 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { * whichever flag asked for it. * * `output.interactive` only tracks *stdout*, so on its own it still let - * `printf 'api\n' | supabase workers new` feed the pipe straight into the name + * `printf 'api\n' | supabase experimental workers new` feed the pipe straight into the name * prompt instead of taking the documented non-interactive path. A prompt is * only answerable from a keyboard, so stdin has to be a terminal too — the same * pair `workers delete` guards its confirmation with. @@ -83,7 +85,7 @@ const canPromptFor = Effect.fnUntraced(function* (machineOutput: boolean) { * * The name is the one input here that cannot be defaulted — it is the * directory, the `config.toml` key and the hostname — so a bare - * `supabase workers new` asks rather than failing the parse. The prompt + * `supabase experimental workers new` asks rather than failing the parse. The prompt * validates against everything the command would otherwise refuse a moment * later, so a mistyped or already-recorded name is corrected in place instead * of ending the run. @@ -116,7 +118,7 @@ const resolveName = Effect.fnUntraced(function* (options: { return yield* Effect.fail( new MissingWorkerNameError({ detail: "Worker name is required in non-interactive mode.", - suggestion: "Pass a worker name, for example `supabase workers new api`.", + suggestion: "Pass a worker name, for example `supabase experimental workers new api`.", }), ); }); @@ -190,7 +192,7 @@ const destinationIsFree = Effect.fnUntraced(function* (target: string) { return entries.length === 0; }); -export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(function* ( +export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( flags: LegacyWorkersNewFlags, ) { const fs = yield* FileSystem.FileSystem; @@ -328,7 +330,7 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun // then the details. Guidance goes in a closing sentence rather than a // pseudo-row, since no other command puts a next step inside its output // table. - yield* output.raw(`Created new Worker at ${sourceDisplay}\n`); + yield* output.raw(`Created new Worker at ${legacyBold(sourceDisplay, process.stdout)}\n`); yield* output.raw( legacyRenderWorkerDetails([ ["Runtime", runtime], @@ -336,6 +338,11 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun ["Access", "public"], ]), ); - yield* output.raw(`Deploy it with supabase experimental workers push ${name}.\n`); + // On the success trailer rather than inline, the way `bootstrap` emits its + // "start your app" line: the shell prints trailers once at the end of the + // run, so the next step is the last thing on screen. + yield* emitSuccessTrailer( + `Deploy it with ${legacyAqua(`supabase experimental workers push ${name}`)}.\n`, + ); }).pipe(Effect.ensuring(telemetryState.flush)); }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index a9fbf2872a..8bd78bdb27 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -67,7 +67,8 @@ describe("legacy workers new", () => { // the shape `functions new` established. expect(out.stdoutText).toContain("Created new Worker at supabase/workers/api"); expect(out.stdoutText).toContain("Runtime"); - expect(out.stdoutText).toContain("supabase experimental workers push api"); + // The deploy hint is a success trailer, which lands on stderr. + expect(out.stderrText).toContain("supabase experimental workers push api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); it.live("asks for the name when the command line carries none", () => { @@ -119,7 +120,7 @@ describe("legacy workers new", () => { { label: "not interactive", setup: { interactive: false } }, // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. { label: "-o json", setup: { goOutput: "json" as const } }, - // `printf 'orders\n' | supabase workers new`: stdout is still a terminal, so + // `printf 'orders\n' | supabase experimental workers new`: stdout is still a terminal, so // `output.interactive` on its own would have fed the pipe straight into the // name prompt instead of taking this documented path. { label: "piped stdin", setup: { stdinIsTty: false } }, @@ -527,6 +528,30 @@ describe("legacy workers new", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The prompts only ever offer values this CLI knows, so an unrecognized answer + // means the prompt layer handed back something off-menu. Recording it verbatim + // would put a runtime into config.toml that `push` then refuses; the default + // is the one answer that still scaffolds something deployable. + it.live("falls back to the defaults when a prompt answers off-menu", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ + workdir: repo.dir, + promptSelectResponses: ["cobol", "colossal"], + }); + + return Effect.gen(function* () { + yield* legacyWorkersNew({ + name: Option.some("api"), + runtime: Option.none(), + size: Option.none(), + source: Option.none(), + }); + + expect(repo.config()).toContain(`runtime = "deno"`); + expect(repo.config()).toContain(`size = "2gb"`); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("refuses --source pointed at the project config file", () => { const repo = project(); const { layer } = setupLegacyWorkers({ workdir: repo.dir }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md index 4941f0b37f..30c0305291 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/push/SIDE_EFFECTS.md @@ -72,6 +72,17 @@ payload always carries a `workers` array, which a flat `KEY=value` list cannot express, and discovering that at the end would fail the command with the remote project already changed. +A multi-worker run stops at the first failure, and names the workers it never +attempted on stderr in **every** format, machine ones included: that run is a +CI run, where nobody watched the loop and "what still needs deploying" is the +question the failure raises. The per-worker `Deploying Worker n/N:` announcement +is text-only by contrast, since it is progress rather than an outcome. + +Both retry suggestions — the one on a failed build and the one on a build that +never settled — carry an explicit `--project-ref` when the flag supplied the +ref, since they are copy-pasted verbatim. A suggestion that dropped it would +re-resolve against whatever this checkout happens to be linked to. + The presigned `PUT` above is the one request whose URL is itself a credential. `--debug` logs every request URL, so `legacyHttpClientLayer` redacts query strings that carry a signature. diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts index 088132e053..bf6c06164c 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts @@ -6,6 +6,7 @@ import { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput, legacyWorkersMachineOutputRequested, + legacyWorkersProjectRefSuffix, } from "../workers.output.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; @@ -178,6 +179,12 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { readonly project: LegacyWorkersProject; readonly name: string; readonly projectRef: string; + /** + * ` --project-ref ` when the flag supplied the ref, `""` when the link + * did — the follow-up hint below is copy-pasted verbatim, so it has to carry + * whatever the user typed to reach this project. + */ + readonly refSuffix: string; readonly instances: Option.Option; readonly pollSchedule?: Schedule.Schedule; readonly pollRetrySchedule?: Schedule.Schedule; @@ -325,6 +332,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { const settled = yield* awaitWorkerBuild(api, projectRef, name, { schedule: input.pollSchedule, retrySchedule: input.pollRetrySchedule, + refSuffix: input.refSuffix, onPoll: (polled) => polled.buildState === "building" ? deploying.message("Building worker...") : Effect.void, }).pipe(Effect.tapError(() => deploying.fail())); @@ -336,7 +344,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { detail: `The build for "${name}" failed${ settled.stateReason === undefined ? "" : `: ${settled.stateReason}` }.`, - suggestion: `Fix the issue, then re-run \`supabase experimental workers push ${name}\`.`, + suggestion: `Fix the issue, then re-run \`supabase experimental workers push ${name}${input.refSuffix}\`.`, }), ); } @@ -383,6 +391,28 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { }; }); +/** + * Names the workers a failed run never got to. + * + * The loop stops on the first failure, so everything after it was never + * attempted — and the error itself only names the worker that broke. Left + * unsaid, the user has to reconstruct the remainder from argument order, or + * from the discovery walk's ordering when the push was a bare `push`. + * + * Written on stderr in every format, unlike the per-worker announcements: a + * machine-format run is a CI run, which is exactly where nobody is watching the + * loop and "what still needs deploying" is the question the failure raises. + */ +const reportUnattempted = Effect.fnUntraced(function* (skipped: ReadonlyArray) { + if (skipped.length === 0) { + return; + } + const output = yield* Output; + // A label rather than a sentence, so it reads the same for one name or six + // and carries no verb to agree with the count. + yield* output.raw(`Not attempted: ${skipped.join(", ")}\n`, "stderr"); +}); + /** * `supabase experimental workers push [name...]` — deploy the named workers, or every worker * in the project when none are named, mirroring `supabase functions deploy`. @@ -393,7 +423,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { * the run, because a build that failed is usually the thing to fix before * spending minutes on the rest. */ -export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(function* ( +export const legacyWorkersPush = Effect.fn("legacy.workers.push")(function* ( flags: LegacyWorkersPushFlags, options: { readonly pollSchedule?: Schedule.Schedule; @@ -439,26 +469,46 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f yield* legacyRejectWorkersEnvOutput(); const machineOutput = yield* legacyWorkersMachineOutputRequested(); + // Computed once for the whole run, the way `status` and `delete` do: an + // explicit `--project-ref` has to survive into every hint this push emits. + const refSuffix = legacyWorkersProjectRefSuffix(flags.projectRef); const deployed: Array> = []; - for (const name of names) { + for (const [index, name] of names.entries()) { if (names.length > 1 && !machineOutput) { // stderr, unblanked and labelled, the way `functions deploy` announces // each function: a bare name with a leading blank line put a section // header into whatever was consuming stdout. - yield* output.raw(`Deploying Worker: ${legacyAqua(name)}\n`, "stderr"); + // + // Counted, because each worker's package/upload/build takes minutes and + // the name alone says nothing about how much of the run is left. + yield* output.raw( + `Deploying Worker ${index + 1}/${names.length}: ${legacyAqua(name)}\n`, + "stderr", + ); } deployed.push( yield* deployOneWorker({ project, name, projectRef, + refSuffix, instances: flags.instances, machineOutput, ...(options.pollSchedule === undefined ? {} : { pollSchedule: options.pollSchedule }), ...(options.pollRetrySchedule === undefined ? {} : { pollRetrySchedule: options.pollRetrySchedule }), - }), + }).pipe(Effect.tapError(() => reportUnattempted(names.slice(index + 1)))), + ); + } + + // Only for a run that deployed several: one worker already said so itself, + // and repeating it as a summary reads like a second deploy. + if (names.length > 1 && !machineOutput && output.format === "text") { + yield* output.raw( + `Deployed ${names.length} Workers to project ${projectRef}: ${names + .map((name) => legacyAqua(name, process.stdout)) + .join(", ")}\n`, ); } diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts index 1e7cb049d9..2fd22f494a 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, readdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, readdirSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "@effect/vitest"; import { Effect, Option, Predicate, Schedule } from "effect"; @@ -14,10 +14,13 @@ import { LegacyProjectNotLinkedError } from "../../../../config/legacy-project-r import { LegacyWorkersEnvNotSupportedError } from "../workers.errors.ts"; import { NoWorkersToDeployError, + UnknownWorkerRuntimeError, + UnknownWorkerSizeError, WorkerBuildFailedError, WorkerBuildTimeoutError, WorkerProjectNotFoundError, WorkersUnavailableError, + WorkerSourceEscapingLinkError, WorkerSourceMissingError, WorkerUploadFailedError, } from "../../../../../shared/workers/workers.errors.ts"; @@ -96,6 +99,16 @@ function listableAsCurrentUser(path: string): boolean { } } +/** The same question one level down: can this path still be stat-ed? */ +function stattableAsCurrentUser(path: string): boolean { + try { + statSync(path); + return true; + } catch { + return false; + } +} + function push(flagOverrides: Partial = {}) { // Both schedules are injected: the outer poll and the per-read retry. The // production retry is spaced in seconds, so leaving it in place made the @@ -143,6 +156,7 @@ describe("legacy workers push", () => { expect(out.stdoutText).toContain("Deployed Worker api"); expect(out.stdoutText).toContain("Runtime"); expect(out.stdoutText).toContain(`https://${WORKERS_PROJECT_REF}.supabase.co/workers/v1/api`); + expect(out.stdoutText).toContain("v1"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -193,6 +207,42 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // `[workers.*] runtime` and `size` are plain strings in the config schema, so + // an unrecognized value reaches the handler rather than failing the parse. + // Naming the accepted values beats echoing a schema error, and the refusal + // has to land before anything is packaged or uploaded. + it.live("names the runtimes on offer when config records one it does not know", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "cobol"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(UnknownWorkerRuntimeError); + expect((error as UnknownWorkerRuntimeError).detail).toContain("cobol"); + expect((error as UnknownWorkerRuntimeError).suggestion).toContain("dockerfile, node, deno"); + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("names the sizes on offer when config records one it does not know", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nsize = "huge"\n`, + }); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(UnknownWorkerSizeError); + expect((error as UnknownWorkerSizeError).detail).toContain("huge"); + expect((error as UnknownWorkerSizeError).suggestion).toContain("2gb, 4gb"); + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("sends the recorded size and the requested instance count", () => { const repo = project({ "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\nsize = "4gb"\n`, @@ -301,6 +351,76 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The reason is optional in the API contract, so the detail has to read as a + // sentence without one rather than trailing a bare colon. + it.live("reports a failed build that came with no reason", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ + workdir: repo.dir, + routes: routes({ + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { data: workerResource({ name: "api", runtime: "node", buildState: "failed" }) }, + }, + }), + }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(WorkerBuildFailedError); + expect((error as WorkerBuildFailedError).detail).toBe(`The build for "api" failed.`); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // Every deploy this CLI sends asks for public exposure, but the accepted spec + // is the platform's answer, not the request echoed back. A worker it did not + // expose has no URL to print, and inventing one from the ref would name an + // address that does not resolve. + it.live("omits the URL for a worker the platform did not expose publicly", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + routes: routes({ + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { + data: workerResource({ + name: "api", + runtime: "node", + buildState: "active", + exposure: "private", + }), + }, + }, + }), + }); + + return Effect.gen(function* () { + yield* push(); + + expect(out.stdoutText).toContain("Deployed Worker api"); + expect(out.stdoutText).toContain("private"); + expect(out.stdoutText).not.toContain("https://"); + expect(out.stdoutText).not.toContain("URL"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The schedules every other test injects are a seam: the command itself calls + // the handler with no options at all. The stubbed worker settles on the first + // poll, so the production schedules never get to space anything out. + it.live("deploys when called the way the command wires it, with no test seams", () => { + const repo = project(); + const { layer, out, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* legacyWorkersPush(flags()); + + expect(http.routeKeys).toContain(`POST ${workersRoute("/api/deploy")}`); + expect(out.stdoutText).toContain("Deployed Worker api"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("stops waiting on a build that never settles, and says where to look", () => { const repo = project(); const { layer } = setupLegacyWorkers({ @@ -314,9 +434,9 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - const error = yield* legacyWorkersPush(flags(), { pollSchedule: Schedule.recurs(2) }).pipe( - Effect.flip, - ); + const error = yield* legacyWorkersPush(flags(), { + pollSchedule: Schedule.recurs(2), + }).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerBuildTimeoutError); expect((error as { suggestion: string }).suggestion).toContain( @@ -325,6 +445,84 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // Every "run this next" string here is copy-pasted verbatim. From an unlinked + // checkout — or one linked elsewhere — dropping the `--project-ref` the user + // typed either fails to resolve or silently addresses a same-named worker in + // whatever project this checkout points at. + describe("carries an explicit --project-ref into its hints", () => { + const unlinked = (repoDir: string, routeOverrides = {}) => + setupLegacyWorkers({ + workdir: repoDir, + linked: false, + routes: routes(routeOverrides), + }); + const withRef = { projectRef: Option.some(WORKERS_PROJECT_REF) }; + + it.live("in the failed-build retry suggestion", () => { + const repo = project(); + const { layer } = unlinked(repo.dir, { + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { data: workerResource({ name: "api", runtime: "node", buildState: "failed" }) }, + }, + }); + + return Effect.gen(function* () { + const error = yield* push(withRef).pipe(Effect.flip); + + expect(error).toBeInstanceOf(WorkerBuildFailedError); + expect((error as WorkerBuildFailedError).suggestion).toContain( + `supabase experimental workers push api --project-ref ${WORKERS_PROJECT_REF}`, + ); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("in the give-up-waiting suggestion", () => { + const repo = project(); + const { layer } = unlinked(repo.dir, { + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { data: workerResource({ name: "api", buildState: "building" }) }, + }, + }); + + return Effect.gen(function* () { + const error = yield* legacyWorkersPush(flags(withRef), { + pollSchedule: Schedule.recurs(2), + }).pipe(Effect.flip); + + expect(error).toBeInstanceOf(WorkerBuildTimeoutError); + expect((error as { suggestion: string }).suggestion).toContain( + `supabase experimental workers status api --project-ref ${WORKERS_PROJECT_REF}`, + ); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The mirror image: when the link supplied the ref, repeating it back is + // noise on a command that already resolves to the right project. + it.live("but leaves it off when the link supplied the ref", () => { + const repo = project(); + const { layer } = setupLegacyWorkers({ + workdir: repo.dir, + routes: routes({ + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { data: workerResource({ name: "api", buildState: "failed" }) }, + }, + }), + }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect((error as WorkerBuildFailedError).suggestion).toContain( + "supabase experimental workers push api", + ); + expect((error as WorkerBuildFailedError).suggestion).not.toContain("--project-ref"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + }); + it.live("fails before deploying when the presigned upload is rejected", () => { const repo = project(); const { layer, http } = setupLegacyWorkers({ @@ -344,6 +542,26 @@ describe("legacy workers push", () => { // section, so it has to honour one: loading TOML-only left the section empty, // which meant a guessed runtime and default size and instance count for a // worker that had configured all three. + // The context is already uploaded by the time the deploy is refused, so the + // failure has to be reported as the deploy's, not the upload's. + it.live("reports a rejected deploy after the context has been uploaded", () => { + const repo = project(); + const { layer, http } = setupLegacyWorkers({ + workdir: repo.dir, + routes: routes({ + [`POST ${workersRoute("/api/deploy")}`]: { status: 500, body: { message: "boom" } }, + }), + }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(Predicate.isTagged(error, "WorkersApiUnexpectedStatusError")).toBe(true); + expect(http.routeKeys).toContain("PUT /deploy-context/api.tar.gz"); + expect(http.routeKeys).toContain(`POST ${workersRoute("/api/deploy")}`); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("deploys a worker configured in config.json, not just config.toml", () => { const created = makeWorkersProject({ "supabase/config.json": JSON.stringify({ @@ -623,6 +841,24 @@ describe("legacy workers push", () => { ); }); + // Packaging stores symlinks rather than following them, so a link out of the + // tree would package a path the build cannot resolve. It is refused while + // packaging — before a slot is minted — so nothing is uploaded for a context + // that could never build. + it.live("refuses a source that links outside itself, before minting a slot", () => { + const repo = project(); + symlinkSync("../../config.toml", join(repo.dir, "supabase", "workers", "api", "escape.toml")); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + const error = yield* push().pipe(Effect.flip); + + expect(error).toBeInstanceOf(WorkerSourceEscapingLinkError); + expect((error as WorkerSourceEscapingLinkError).detail).toContain("escape.toml"); + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("rides out a transient failure while polling the build", () => { const repo = project(); const { layer, http } = setupLegacyWorkers({ @@ -702,6 +938,65 @@ describe("legacy workers push", () => { http.routeKeys.indexOf(`POST ${workersRoute("/web/deploy")}`), ); expect(out.stdoutText).toContain("web"); + // Each worker is announced with its place in the run, and the run closes + // by naming everything it deployed. + expect(out.stderrText).toContain("Deploying Worker 1/2: api"); + expect(out.stderrText).toContain("Deploying Worker 2/2: web"); + expect(out.stdoutText).toContain( + `Deployed 2 Workers to project ${WORKERS_PROJECT_REF}: api, web`, + ); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + // The other half of that stat: an entry that is there but cannot be read is a + // real filesystem problem, not a name to skip. Dropping it would deploy a + // subset of the project and report success. Root ignores the permission bits, + // and CI sometimes runs as root, so this asserts the outcome that actually + // applies rather than skipping. + it.live("fails rather than skipping a workers entry it cannot stat", () => { + const repo = project(); + const workersRoot = join(repo.dir, "supabase", "workers"); + // Readable, so the listing still names `api`; not traversable, so stat-ing + // anything inside it fails with a permission error. + chmodSync(workersRoot, 0o600); + const stattable = stattableAsCurrentUser(join(workersRoot, "api")); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + if (stattable) { + yield* push({ names: [] }); + expect(http.routeKeys).toContain(`POST ${workersRoute("/api/deploy")}`); + return; + } + const error = yield* push({ names: [] }).pipe(Effect.flip); + + expect(error).not.toBeInstanceOf(NoWorkersToDeployError); + expect(Predicate.isTagged(error, "PlatformError")).toBe(true); + expect(http.requests).toHaveLength(0); + }).pipe( + Effect.provide(layer), + Effect.ensuring( + Effect.sync(() => { + chmodSync(workersRoot, 0o700); + repo.cleanup(); + }), + ), + ); + }); + + // A dangling link in the workers root is listed by the directory read but has + // nothing to stat. Discovery skips it rather than failing the whole run over a + // path that names no worker. + it.live("skips a dangling link in the workers root while discovering", () => { + const repo = project(); + symlinkSync("nowhere", join(repo.dir, "supabase", "workers", "ghost")); + const { layer, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push({ names: [] }); + + expect(http.routeKeys).toContain(`POST ${workersRoute("/api/deploy")}`); + expect(http.routeKeys.some((key) => key.includes("/ghost"))).toBe(false); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -739,6 +1034,41 @@ describe("legacy workers push", () => { ); }); + it.live("names the workers a failed run never got to", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\n\n[workers.web]\nruntime = "node"\n`, + "supabase/workers/web/index.js": "export default {};\n", + }); + const { layer, out, http } = setupLegacyWorkers({ + workdir: repo.dir, + routes: routes({ + // `api` sorts first, so the run stops before `web` is ever touched. + [`GET ${workersRoute("/api")}`]: { + status: 200, + body: { + data: workerResource({ + name: "api", + runtime: "node", + buildState: "failed", + stateReason: "error building image: exit status 1", + }), + }, + }, + }), + }); + + return Effect.gen(function* () { + const error = yield* push({ names: [] }).pipe(Effect.flip); + + expect(error).toBeInstanceOf(WorkerBuildFailedError); + expect(out.stderrText).toContain("Not attempted: web"); + // Named rather than deployed: the run really did stop. + expect(http.routeKeys).not.toContain(`POST ${workersRoute("/web/deploy")}`); + // No summary either — nothing finished. + expect(out.stdoutText).not.toContain("Deployed 2 Workers"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("fails when there are no workers to deploy at all", () => { const repo = project({ "supabase/config.toml": `project_id = "demo"\n` }); rmSync(join(repo.dir, "supabase", "workers"), { recursive: true, force: true }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md index f58c69978c..47680c8daa 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md @@ -61,11 +61,11 @@ wrapper emits for every command. ## Output Formats -| Mode | stdout | stderr | -| ----------------------------- | ------------------------------------------------------------------------------------------------------ | ---------------------------- | -| text (default) | the details block, plus the build-retry line on a failure | an unreadable instance tally | -| `--output-format json` | one structured result carrying every reported field | as above | -| `--output-format stream-json` | the same result as a single terminal event | as above | -| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | -| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | -| `-o env` | refused before any request; the payload nests an instance tally a flat `KEY=value` list cannot express | the error | +| Mode | stdout | stderr | +| ----------------------------- | ------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------- | +| text (default) | the details block | an unreadable instance tally, and the build-retry hint on a failure | +| `--output-format json` | one structured result carrying every reported field | as above | +| `--output-format stream-json` | the same result as a single terminal event | as above | +| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | +| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | +| `-o env` | refused before any request; the payload nests an instance tally a flat `KEY=value` list cannot express | the error | diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts index 44aff12f4c..2ea6b2ddf9 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts @@ -1,5 +1,7 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; +import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; +import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersMachineOutput, @@ -30,7 +32,7 @@ import type { LegacyWorkersStatusFlags } from "./status.command.ts"; * scrolled away, plus the live instance tally, which is the only place it is * available — the list endpoint stays free of per-worker backend calls. */ -export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status")(function* ( +export const legacyWorkersStatus = Effect.fn("legacy.workers.status")(function* ( flags: LegacyWorkersStatusFlags, ) { const output = yield* Output; @@ -151,8 +153,10 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status // Not while it is being torn down: deletion is asynchronous, so a push here // races the tombstone or resurrects the very worker the user is removing. if (record.buildState === "failed" && record.deleting !== true) { - yield* output.raw( - `Fix the issue, then re-run supabase experimental workers push ${name}${refSuffix}.\n`, + // Trailer, like every other "what to run next" line in this shell: the + // command reports a failed build but exits 0, so the trailer flushes. + yield* emitSuccessTrailer( + `Fix the issue, then re-run ${legacyAqua(`supabase experimental workers push ${name}${refSuffix}`)}.\n`, ); } }).pipe( diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/status.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/status/status.integration.test.ts index bcc0f709cc..ee525a8cd1 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/status.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/status/status.integration.test.ts @@ -223,7 +223,8 @@ describe("legacy workers status", () => { expect(out.stdoutText).toContain("failed"); expect(out.stdoutText).toContain("exit status 1"); - expect(out.stdoutText).toContain("supabase experimental workers push api"); + // The retry hint is a success trailer, which lands on stderr. + expect(out.stderrText).toContain("supabase experimental workers push api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -486,6 +487,32 @@ describe("legacy workers status", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // The URL is derived from the exposure the platform reports, not assumed: a + // worker it did not expose has no address to print, and the row is dropped + // rather than rendered empty. + it.live("omits the URL for a worker that is not publicly exposed", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + routes: { + [getRoute]: { + status: 200, + body: { + data: workerResource({ name: "api", runtime: "node", exposure: "private" }), + }, + }, + }, + }); + + return Effect.gen(function* () { + yield* legacyWorkersStatus({ name: "api", projectRef: Option.none() }); + + expect(out.stdoutText).toContain("private"); + expect(out.stdoutText).not.toContain("URL"); + expect(out.stdoutText).not.toContain("https://"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("refuses -o env before making any request at all", () => { const repo = project(); const { layer, http } = setupLegacyWorkers({ diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.format.unit.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.format.unit.test.ts new file mode 100644 index 0000000000..11fba19cf1 --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.format.unit.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { legacyRenderWorkerDetails } from "./workers.format.ts"; + +describe("legacyRenderWorkerDetails", () => { + it("pads every label to the widest one", () => { + expect( + legacyRenderWorkerDetails([ + ["State", "active"], + ["Runtime", "node"], + ]), + ).toBe(" State active\n Runtime node\n"); + }); + + it("drops rows whose value is empty", () => { + expect( + legacyRenderWorkerDetails([ + ["State", "active"], + ["Image", ""], + ]), + ).toBe(" State active\n"); + }); + + // Several reported fields are optional in the API contract, so a worker can + // answer with nothing worth rendering. Returning "" rather than a bare newline + // keeps the caller from printing an empty block under its headline. + it("renders nothing at all when every value is empty", () => { + expect( + legacyRenderWorkerDetails([ + ["Image", ""], + ["URL", ""], + ]), + ).toBe(""); + }); +}); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.integration.test.ts new file mode 100644 index 0000000000..bd56fba7b2 --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.integration.test.ts @@ -0,0 +1,40 @@ +import { rmSync } from "node:fs"; +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +import { LegacyWorkersEnvNotSupportedError } from "./workers.errors.ts"; +import { + makeWorkersProject, + setupLegacyWorkers, +} from "../../../../../tests/helpers/legacy-workers.ts"; +import { legacyEmitWorkersMachineOutput } from "./workers.output.ts"; + +/** + * Every workers command refuses `-o env` up front, before it touches the + * network, so the encoder's own env branch is a backstop rather than a path a + * user reaches. It is worth pinning anyway: a new command that forgets the + * refusal must not silently emit TOML under a flag that asked for env — it + * raises the same refusal instead. + */ +describe("legacyEmitWorkersMachineOutput", () => { + it.live("refuses -o env rather than falling through to the TOML encoder", () => { + const created = makeWorkersProject({ "supabase/config.toml": `project_id = "demo"\n` }); + const { layer, out } = setupLegacyWorkers({ + workdir: created.dir, + goOutput: "env", + routes: {}, + }); + + return Effect.gen(function* () { + const error = yield* legacyEmitWorkersMachineOutput({ + project_ref: "demo", + workers: [], + }).pipe(Effect.flip); + + expect(error).toBeInstanceOf(LegacyWorkersEnvNotSupportedError); + expect(out.stdoutText).toBe(""); + }).pipe( + Effect.provide(layer), + Effect.ensuring(Effect.sync(() => rmSync(created.dir, { recursive: true, force: true }))), + ); + }); +}); diff --git a/apps/cli/src/shared/workers/workers-api.ts b/apps/cli/src/shared/workers/workers-api.ts index e2ab533fc5..4bdec8bfc4 100644 --- a/apps/cli/src/shared/workers/workers-api.ts +++ b/apps/cli/src/shared/workers/workers-api.ts @@ -453,6 +453,13 @@ export const awaitWorkerBuild = Effect.fnUntraced(function* ( readonly retrySchedule?: Schedule.Schedule; /** Called with each poll's result, for progress reporting. */ readonly onPoll?: (worker: WorkerRecord) => Effect.Effect; + /** + * ` --project-ref ` to append to the suggestion below, when the caller + * reached this project through the flag rather than the link. The suggestion + * is copy-pasted verbatim, so dropping it re-resolves against whatever this + * checkout happens to be linked to. + */ + readonly refSuffix?: string; } = {}, ) { const poll = Effect.gen(function* () { @@ -483,7 +490,7 @@ export const awaitWorkerBuild = Effect.fnUntraced(function* ( return yield* Effect.fail( new WorkerBuildTimeoutError({ detail: `"${name}" was still building when this command stopped waiting.`, - suggestion: `Check on it with \`supabase experimental workers status ${name}\`.`, + suggestion: `Check on it with \`supabase experimental workers status ${name}${options.refSuffix ?? ""}\`.`, }), ); } From 548a185b205b73f2f698c8c4b54ebd588c97a6dd Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 29 Aug 2026 00:12:27 -0300 Subject: [PATCH 07/15] test(cli): guard every legacy boolean flag against a required default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Flag.boolean(name)` builds a bare `Single` param, and a bare `Single` is *required* — omitting it fails the whole command with a missing-flag error before the handler ever runs. Every boolean flag has to be closed off with `Flag.withDefault(false)` or `Flag.optional`, and nothing in the existing suites notices when one is not. Handler integration tests build their flags record directly, so they never touch the parser, and the required-ness is invisible to the type checker because a required boolean flag still infers as `boolean`. The flag only misbehaves when a real invocation omits it, which is exactly the invocation no handler test makes. So this walks the whole legacy command tree, including global flags, and asserts every boolean param carries a default or is optional. It reads the primitive kind through `Primitive.getTypeName` rather than `_tag`, since this repo forbids inspecting effect's runtime representation in tests as well as in source. --- .../legacy-boolean-flag-defaults.unit.test.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 apps/cli/src/legacy/cli/legacy-boolean-flag-defaults.unit.test.ts diff --git a/apps/cli/src/legacy/cli/legacy-boolean-flag-defaults.unit.test.ts b/apps/cli/src/legacy/cli/legacy-boolean-flag-defaults.unit.test.ts new file mode 100644 index 0000000000..b4a032b883 --- /dev/null +++ b/apps/cli/src/legacy/cli/legacy-boolean-flag-defaults.unit.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { Primitive, type Command } from "effect/unstable/cli"; +import { + legacyCommandInternals, + legacyFlattenSubcommands, + legacyUserGlobalFlagParams, +} from "../docs/legacy-docs-introspection.ts"; +import { legacyUnwrapParam } from "../shared/legacy-param-introspection.ts"; +import { legacyRoot } from "./root.ts"; + +/** + * `Flag.boolean(name)` builds a bare `Single` param, and a bare `Single` is + * *required* — omitting it fails the whole command with a missing-flag error + * before the handler ever runs. Every boolean flag therefore has to be closed + * off with `Flag.withDefault(false)` or `Flag.optional`. + * + * Nothing else catches this: handler integration tests build their flags record + * directly, so they never touch the parser, and the required-ness is invisible + * to the type checker because a required boolean flag still infers as + * `boolean`. The flag only misbehaves when a real invocation omits it, which is + * precisely the invocation no handler test makes — so the guard walks the + * command tree instead of waiting for a command to be exercised end to end. + */ + +/** + * The published getter for a primitive's kind — `Primitive.getTypeName`, whose + * own doc example pins `Primitive.boolean` to `"boolean"`. Reading + * `primitiveType._tag` instead would couple this guard to effect's runtime + * representation, which this repo forbids in tests as well as in source. + * + * Derived from `Primitive.boolean` rather than written as the literal + * `"boolean"`: were that name to change upstream, a hardcoded literal would + * match nothing and leave the guard silently passing every command, which is + * the one failure mode a regression test must not have. + */ +const BOOLEAN_TYPE_NAME = Primitive.getTypeName(Primitive.boolean); + +function booleanFlagsRequiringAValue(command: Command.Command.Any): ReadonlyArray { + const internals = legacyCommandInternals(command); + // All three parameter sets a command can be parsed with, not just its own: + // `Command.withSharedFlags` puts inherited flags on `contextConfig`, and the + // root's persistent flags arrive as `globalFlags`. A bare boolean introduced + // through either would break every command that inherits it while a guard + // reading only `config.flags` stayed green. + const params = [ + ...internals.config.flags, + ...internals.contextConfig.flags, + ...legacyUserGlobalFlagParams(command), + ]; + + // Throws rather than skipping if effect's internal shape moves, so this + // cannot quietly degrade into a test that inspects nothing. + const own = params.flatMap((flag) => { + const unwrapped = legacyUnwrapParam(flag); + if (unwrapped === undefined) { + throw new Error(`Unrecognizable flag param on "${command.name}".`); + } + const { single, isOptional } = unwrapped; + return Primitive.getTypeName(single.primitiveType) === BOOLEAN_TYPE_NAME && !isOptional + ? [`${command.name} --${single.name}`] + : []; + }); + + return [...own, ...legacyFlattenSubcommands(command).flatMap(booleanFlagsRequiringAValue)]; +} + +describe("legacy boolean flag wiring", () => { + it("gives every boolean flag a default, so omitting it is not a parse error", () => { + expect(booleanFlagsRequiringAValue(legacyRoot)).toEqual([]); + }); +}); From 1d7e06b6a4f07b15589acbd68584f5a7d1a3e643 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 31 Aug 2026 19:42:13 -0300 Subject: [PATCH 08/15] chore(workers): describe behaviour rather than its history in comments Comments explaining why code is shaped a certain way now state the constraint directly instead of narrating what an earlier version did. The reasoning is unchanged; only the framing is. --- .../workers/delete/delete.integration.test.ts | 12 ++++++------ .../experimental/workers/list/list.handler.ts | 6 +++--- .../workers/list/list.integration.test.ts | 4 ++-- .../workers/push/push.integration.test.ts | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts index d8ce622e9f..3d0c59bc34 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.integration.test.ts @@ -77,9 +77,9 @@ describe("legacy workers delete", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // The refusal used to live at emit time, which on this command is *after* the - // DELETE: `--yes -o env` removed the worker and then exited non-zero with no - // payload, which a script reads as "the delete failed" and may retry. + // The refusal has to precede the DELETE. At emit time `--yes -o env` would + // remove the worker and then exit non-zero with no payload, which a script + // reads as "the delete failed" and may retry. // Deletion never touches local files, so a malformed local config has no // business standing between the user and a worker they named explicitly. it.live("deletes a remote worker despite an unparseable local config", () => { @@ -328,8 +328,8 @@ describe("legacy workers delete", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // `interactive` follows stdout, so a plain `>` redirect reaches this branch - // even from a live terminal — the case that used to delete without asking. + // `interactive` follows stdout, so a plain `>` redirect reaches this branch even + // from a live terminal — the case where deleting without asking would be worst. it.live("refuses when stdout is redirected and no --yes was given", () => { const repo = project(); const { layer, http } = setupLegacyWorkers({ @@ -662,7 +662,7 @@ describe("legacy workers delete", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // Deletion never reads the local source, so a `source` that no longer resolves + // Deletion never reads the local source, so a `source` that does not resolve // inside the project must not block removing the remote worker. it.live("deletes the remote worker even when the configured source is unusable", () => { const repo = project('project_id = "demo"\n\n[workers.api]\nsource = "../../elsewhere"\n'); diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts index 6b0dbfa6e6..5ab6fb40a0 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts @@ -198,9 +198,9 @@ export const legacyWorkersList = Effect.fn("legacy.workers.list")(function* ( // // Both are written the way this shell writes every other heads-up that is // not a failure: a yellow `WARNING:` prefix, then the consequence on its own - // line (`start`'s Docker-on-Windows notice is the same two-line shape). The - // single long sentence each of these used to be re-flowed differently at - // every terminal width, right under a table that lines its columns up. + // line (`start`'s Docker-on-Windows notice is the same two-line shape). A + // single long sentence re-flows differently at every terminal width, right + // under a table that lines its columns up. const unconfigured = rows .filter((row) => row.deployed !== undefined && !row.configured && row.local) .map((row) => row.name); diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts index a5370c3226..0bfb506676 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.integration.test.ts @@ -470,8 +470,8 @@ describe("legacy workers list", () => { // `pretty` is the human default; `table` and `csv` are accepted by the global // flag for `db query`'s benefit, and every resource command is meant to ignore - // them and render text. All three used to fall through to the TOML encoder, - // which is the trap the payload allowlist closes. + // them and render text. Falling through to the TOML encoder is the trap the + // payload allowlist closes. it.live.each(["pretty", "table", "csv"] as const)( "renders text rather than TOML for -o %s", (goOutput) => { diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts index 2fd22f494a..85184aeef7 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts @@ -1159,8 +1159,8 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // The "nothing to deploy" guard counts directory entries, so a tree of empty - // subdirectories used to package to zero files and deploy an image with no + // The "nothing to deploy" guard counts directory entries, so without this a tree + // of empty subdirectories packages to zero files and deploys an image with no // handler in it. it.live("refuses a source holding only empty directories, before minting a slot", () => { const repo = project({ "supabase/workers/api/nested/.keep": "" }); @@ -1216,8 +1216,8 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); - // A malformed config.toml used to fail outside the finalizers, so the run - // skipped the telemetry flush every invocation is supposed to perform. + // A malformed config.toml must fail inside the finalizers, or the run skips the + // telemetry flush every invocation is supposed to perform. it.live("flushes telemetry when the project config cannot be loaded", () => { const repo = project({ "supabase/config.toml": "project_id = [unclosed\n" }); const { layer, telemetry } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); From 9ae1f387f8d9343a7f43976b4be106372762b3e6 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 02:41:13 -0300 Subject: [PATCH 09/15] fix(cli): point the workers new retry at the experimental path `workers` is registered only beneath the `experimental` parent, so the `MissingWorkerNameError` suggestion telling the user to run `supabase workers new api` produced an unknown-command error when copied. Name the real invocation path in the suggestion, and in the handler, doc and test prose that described the piped-stdin case with the same stale path. An assertion on the suggestion keeps the retry path from drifting away from where the command is mounted. --- .../experimental/workers/new/SIDE_EFFECTS.md | 5 +++-- .../experimental/workers/new/new.handler.ts | 18 +++++++++--------- .../workers/new/new.integration.test.ts | 12 +++++++++--- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md index e52dade14e..d331367dbd 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md @@ -41,8 +41,9 @@ records — so nothing is asked, and nothing written, for a name the command was going to refuse. With `-o json|yaml|toml|env`, a redirected stdout, or a stdin that is not a terminal, there is nowhere to ask, and the command fails instead of defaulting: unlike the runtime and size, the name has no default to fall back -on. Every prompt is gated on both streams, so `printf 'api\n' | supabase workers -new` takes that failure path rather than reading the worker name off the pipe. +on. Every prompt is gated on both streams, so +`printf 'api\n' | supabase experimental workers new` takes that failure path +rather than reading the worker name off the pipe. Writes to `config.toml` are append-only. A worker already recorded under `[workers.]` is refused outright — before the runtime and size prompts, diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index ee353babda..024bf64ed6 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -67,10 +67,10 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { * whichever flag asked for it. * * `output.interactive` only tracks *stdout*, so on its own it still let - * `printf 'api\n' | supabase workers new` feed the pipe straight into the name - * prompt instead of taking the documented non-interactive path. A prompt is - * only answerable from a keyboard, so stdin has to be a terminal too — the same - * pair `workers delete` guards its confirmation with. + * `printf 'api\n' | supabase experimental workers new` feed the pipe straight + * into the name prompt instead of taking the documented non-interactive path. A + * prompt is only answerable from a keyboard, so stdin has to be a terminal too + * — the same pair `workers delete` guards its confirmation with. */ const canPromptFor = Effect.fnUntraced(function* (machineOutput: boolean) { const output = yield* Output; @@ -83,10 +83,10 @@ const canPromptFor = Effect.fnUntraced(function* (machineOutput: boolean) { * * The name is the one input here that cannot be defaulted — it is the * directory, the `config.toml` key and the hostname — so a bare - * `supabase workers new` asks rather than failing the parse. The prompt - * validates against everything the command would otherwise refuse a moment - * later, so a mistyped or already-recorded name is corrected in place instead - * of ending the run. + * `supabase experimental workers new` asks rather than failing the parse. The + * prompt validates against everything the command would otherwise refuse a + * moment later, so a mistyped or already-recorded name is corrected in place + * instead of ending the run. */ const resolveName = Effect.fnUntraced(function* (options: { readonly explicit: Option.Option; @@ -116,7 +116,7 @@ const resolveName = Effect.fnUntraced(function* (options: { return yield* Effect.fail( new MissingWorkerNameError({ detail: "Worker name is required in non-interactive mode.", - suggestion: "Pass a worker name, for example `supabase workers new api`.", + suggestion: "Pass a worker name, for example `supabase experimental workers new api`.", }), ); }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts index a9fbf2872a..4b44f58631 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts @@ -119,9 +119,9 @@ describe("legacy workers new", () => { { label: "not interactive", setup: { interactive: false } }, // A TTY, but stdout was claimed by the payload, so a prompt would corrupt it. { label: "-o json", setup: { goOutput: "json" as const } }, - // `printf 'orders\n' | supabase workers new`: stdout is still a terminal, so - // `output.interactive` on its own would have fed the pipe straight into the - // name prompt instead of taking this documented path. + // `printf 'orders\n' | supabase experimental workers new`: stdout is still a + // terminal, so `output.interactive` on its own would have fed the pipe + // straight into the name prompt instead of taking this documented path. { label: "piped stdin", setup: { stdinIsTty: false } }, ])("refuses a bare new when there is nowhere to ask ($label)", ({ setup }) => { const repo = project(); @@ -137,6 +137,12 @@ describe("legacy workers new", () => { const error = yield* legacyWorkersNew(flags({ name: Option.none() })).pipe(Effect.flip); expect(error).toBeInstanceOf(MissingWorkerNameError); + if (!(error instanceof MissingWorkerNameError)) { + return yield* Effect.die("expected MissingWorkerNameError"); + } + // The retry has to name the path the command is actually registered at; + // `supabase workers new` is an unknown command. + expect(error.suggestion).toContain("supabase experimental workers new"); expect(out.promptTextCalls).toEqual([]); expect(existsSync(join(repo.dir, "supabase", "workers"))).toBe(false); expect(repo.config()).toBe(CONFIG_WITH_COMMENTS); From 12c4d1ca1d2fb11b5011b57bcbf7e819a772d620 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 04:14:12 -0300 Subject: [PATCH 10/15] fix(cli): stop telling users to run a command that does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Workers API decode failure suggested `supabase update`, which is not a command in either shell's root — the CLI has no self-update path, which is why the post-command upgrade notice sends users to the docs instead. Point the suggestion at that same upgrade guide, and hoist the URL from `legacy-upgrade-notice.ts` into `shared/cli/version.ts` so both callers read one constant rather than duplicating the link. --- apps/cli/src/legacy/shared/legacy-upgrade-notice.ts | 6 ++---- apps/cli/src/shared/cli/version.ts | 8 ++++++++ apps/cli/src/shared/workers/workers-api.ts | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/apps/cli/src/legacy/shared/legacy-upgrade-notice.ts b/apps/cli/src/legacy/shared/legacy-upgrade-notice.ts index 497ecf638d..027ca96ee8 100644 --- a/apps/cli/src/legacy/shared/legacy-upgrade-notice.ts +++ b/apps/cli/src/legacy/shared/legacy-upgrade-notice.ts @@ -18,14 +18,12 @@ import { lastGlobalFlagValue, rootFlagTokens, } from "../../shared/cli/run.ts"; -import { CLI_VERSION } from "../../shared/cli/version.ts"; +import { CLI_UPGRADE_GUIDE_URL, CLI_VERSION } from "../../shared/cli/version.ts"; import { legacyBold, legacyYellow } from "./legacy-colors.ts"; import { parseDotEnv } from "./legacy-dotenv.ts"; import { legacyCandidateDotenvFilenames } from "./legacy-project-environment.ts"; const LATEST_RELEASE_URL = "https://api.github.com/repos/supabase/cli/releases/latest"; -const UPGRADE_GUIDE_URL = - "https://supabase.com/docs/guides/cli/getting-started#updating-the-supabase-cli"; const CACHE_TTL_MS = 10 * 60 * 60 * 1000; /** No Go equivalent (its client sets no timeout); bounds this pre-exit hook's latency. */ const FETCH_TIMEOUT_MS = 3000; @@ -145,7 +143,7 @@ function comparePrerelease(left: string, right: string): number { export function legacyFormatUpgradeNotice(latestTag: string, currentVersion: string): string { return ( `A new version of Supabase CLI is available: ${legacyYellow(latestTag)} (currently installed v${currentVersion})\n` + - `We recommend updating regularly for new features and bug fixes: ${legacyBold(UPGRADE_GUIDE_URL)}` + `We recommend updating regularly for new features and bug fixes: ${legacyBold(CLI_UPGRADE_GUIDE_URL)}` ); } diff --git a/apps/cli/src/shared/cli/version.ts b/apps/cli/src/shared/cli/version.ts index 1e60d4c7d9..04c39c621f 100644 --- a/apps/cli/src/shared/cli/version.ts +++ b/apps/cli/src/shared/cli/version.ts @@ -5,3 +5,11 @@ declare const SUPABASE_CLI_VERSION: string | undefined; export const CLI_VERSION = typeof SUPABASE_CLI_VERSION === "string" ? SUPABASE_CLI_VERSION : "0.0.0-dev"; + +/** + * Where a user goes to get a newer CLI. There is no self-update command, so + * anything telling a user to upgrade has to send them here rather than name an + * invocation. + */ +export const CLI_UPGRADE_GUIDE_URL = + "https://supabase.com/docs/guides/cli/getting-started#updating-the-supabase-cli"; diff --git a/apps/cli/src/shared/workers/workers-api.ts b/apps/cli/src/shared/workers/workers-api.ts index e2ab533fc5..71a654f526 100644 --- a/apps/cli/src/shared/workers/workers-api.ts +++ b/apps/cli/src/shared/workers/workers-api.ts @@ -12,6 +12,7 @@ import { Effect, Option, Schedule, Schema } from "effect"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientError from "effect/unstable/http/HttpClientError"; import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest"; +import { CLI_UPGRADE_GUIDE_URL } from "../cli/version.ts"; import { WorkerBuildTimeoutError, WorkersApiNetworkError, @@ -193,7 +194,7 @@ const decodeBody = ( new WorkersApiUnexpectedStatusError({ status, detail: `The Workers API returned a response this CLI could not read while trying to ${operation}: ${error.message}.`, - suggestion: "Update the CLI with `supabase update`, then retry.", + suggestion: `Update the CLI, then retry: ${CLI_UPGRADE_GUIDE_URL}`, }), ), ); From 1f103f99a06d490414c8eab0945d65ee808fb7fd Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 17:15:46 -0300 Subject: [PATCH 11/15] fix(workers): restore the experimental segment in worker span names The output work reintroduced `legacy.workers.*` as the `Effect.fn` span name for all five worker handlers, undoing the rename that landed with the move under `experimental`. Traces and the NDJSON exporter recorded a command route that no longer exists, so any filter keyed to the current names missed them. --- .../commands/experimental/workers/delete/delete.handler.ts | 2 +- .../legacy/commands/experimental/workers/list/list.handler.ts | 2 +- .../src/legacy/commands/experimental/workers/new/new.handler.ts | 2 +- .../legacy/commands/experimental/workers/push/push.handler.ts | 2 +- .../commands/experimental/workers/status/status.handler.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts index f767f9d4ac..f607dcc41f 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts @@ -51,7 +51,7 @@ import type { LegacyWorkersDeleteFlags } from "./delete.command.ts"; * stdout, so merely redirecting output would otherwise delete unattended. This * refuses instead, and says which flag would have authorised it. */ -export const legacyWorkersDelete = Effect.fn("legacy.workers.delete")(function* ( +export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete")(function* ( flags: LegacyWorkersDeleteFlags, ) { const output = yield* Output; diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts index 5ab6fb40a0..669972f199 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.handler.ts @@ -96,7 +96,7 @@ function toCells(row: WorkerRow): ReadonlyArray { ]; } -export const legacyWorkersList = Effect.fn("legacy.workers.list")(function* ( +export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(function* ( flags: LegacyWorkersListFlags, ) { const output = yield* Output; diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts index 4f8d53030c..79b195dc54 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts @@ -192,7 +192,7 @@ const destinationIsFree = Effect.fnUntraced(function* (target: string) { return entries.length === 0; }); -export const legacyWorkersNew = Effect.fn("legacy.workers.new")(function* ( +export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(function* ( flags: LegacyWorkersNewFlags, ) { const fs = yield* FileSystem.FileSystem; diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts index bf6c06164c..d3a8fa25bd 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts @@ -423,7 +423,7 @@ const reportUnattempted = Effect.fnUntraced(function* (skipped: ReadonlyArray; diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts index 2ea6b2ddf9..b0319080af 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/status/status.handler.ts @@ -32,7 +32,7 @@ import type { LegacyWorkersStatusFlags } from "./status.command.ts"; * scrolled away, plus the live instance tally, which is the only place it is * available — the list endpoint stays free of per-worker backend calls. */ -export const legacyWorkersStatus = Effect.fn("legacy.workers.status")(function* ( +export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status")(function* ( flags: LegacyWorkersStatusFlags, ) { const output = yield* Output; From de593920d4536086254c9a66c72366f6aa5f4910 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 17:16:43 -0300 Subject: [PATCH 12/15] fix(workers): stop echoing an empty --project-ref into retry suggestions `LegacyProjectRefResolver` treats `--project-ref ""` as absent and falls back to the environment or the linked-project file, but the suffix helper keyed off `Option.isSome` alone. Every suggestion that command emitted ended in a valueless `--project-ref`, which cannot be pasted back and re-run. Match the resolver's reading of the flag instead. --- .../experimental/workers/workers.output.ts | 9 +++++++- .../workers/workers.output.unit.test.ts | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.output.unit.test.ts diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts index cda3b71759..95e83127dc 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts @@ -108,6 +108,13 @@ export const legacyRejectWorkersEnvOutput = Effect.fnUntraced(function* () { * * Keyed off the flag rather than the resolved ref: when the link supplied it, * appending it again is noise on a command that already resolves correctly. + * + * An empty `--project-ref ""` counts as "not supplied", the same reading + * `LegacyProjectRefResolver` gives it before falling back to the environment or + * the linked-project file. Carrying it through would suggest a command ending + * in a valueless `--project-ref`, which cannot be pasted back. */ export const legacyWorkersProjectRefSuffix = (projectRef: Option.Option): string => - Option.isSome(projectRef) ? ` --project-ref ${projectRef.value}` : ""; + Option.isSome(projectRef) && projectRef.value.length > 0 + ? ` --project-ref ${projectRef.value}` + : ""; diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.unit.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.unit.test.ts new file mode 100644 index 0000000000..861459564c --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.unit.test.ts @@ -0,0 +1,22 @@ +import { Option } from "effect"; +import { describe, expect, it } from "vitest"; +import { legacyWorkersProjectRefSuffix } from "./workers.output.ts"; + +describe("legacyWorkersProjectRefSuffix", () => { + it("carries an explicit --project-ref into the suggestion", () => { + expect(legacyWorkersProjectRefSuffix(Option.some("abcdefghijklmnopqrst"))).toBe( + " --project-ref abcdefghijklmnopqrst", + ); + }); + + it("adds nothing when the ref came from the link", () => { + expect(legacyWorkersProjectRefSuffix(Option.none())).toBe(""); + }); + + // `--project-ref ""` resolves from the environment or the linked-project file, + // so echoing the flag back would suggest a command ending in a valueless + // `--project-ref` that cannot be pasted and re-run. + it("adds nothing when the flag was supplied empty", () => { + expect(legacyWorkersProjectRefSuffix(Option.some(""))).toBe(""); + }); +}); From d99e6b5187ccd58e29ddd1eda63bc711c764dc15 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 17:18:07 -0300 Subject: [PATCH 13/15] fix(workers): keep push's per-worker progress out of structured formats `SIDE_EFFECTS.md` claimed the `Deploying Worker n/N:` announcement was text-only, but the gate read `machineOutput`, which tracks `-o` alone. Under `--output-format json` or `stream-json` the flag stays false and the announcement was written to stderr, contradicting the documented matrix. Gate it on the format too, the way the run's closing summary already is. The "not attempted" report keeps reaching every format: it says what still needs deploying, which is exactly what an unwatched CI run has to know. --- .../experimental/workers/push/push.handler.ts | 8 ++++- .../workers/push/push.integration.test.ts | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts index d3a8fa25bd..cc90677741 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.handler.ts @@ -474,13 +474,19 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f const refSuffix = legacyWorkersProjectRefSuffix(flags.projectRef); const deployed: Array> = []; for (const [index, name] of names.entries()) { - if (names.length > 1 && !machineOutput) { + if (names.length > 1 && !machineOutput && output.format === "text") { // stderr, unblanked and labelled, the way `functions deploy` announces // each function: a bare name with a leading blank line put a section // header into whatever was consuming stdout. // // Counted, because each worker's package/upload/build takes minutes and // the name alone says nothing about how much of the run is left. + // + // Text only, on both axes: `machineOutput` tracks `-o`, which leaves + // `output.format` as `text`, so neither check covers the other. This is + // progress rather than an outcome, and `--output-format json` asked for + // a stream of events — unlike the unattempted-workers report below, + // which every format gets because it says what still needs deploying. yield* output.raw( `Deploying Worker ${index + 1}/${names.length}: ${legacyAqua(name)}\n`, "stderr", diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts index 85184aeef7..0123082b4d 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts @@ -1140,6 +1140,38 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // `--output-format json` asked for a stream of events, so progress does not + // belong in it — unlike the "not attempted" report, which every format gets. + it.live("keeps per-worker progress out of json mode", () => { + const repo = project({ + "supabase/config.toml": `project_id = "demo"\n\n[workers.api]\nruntime = "node"\n\n[workers.web]\nruntime = "node"\n`, + "supabase/workers/web/index.js": "export default {};\n", + }); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + format: "json", + routes: { + ...routes(), + [`POST ${workersRoute("/web/uploads")}`]: { status: 201, body: uploadSlot }, + [`POST ${workersRoute("/web/deploy")}`]: { + status: 202, + body: { data: workerResource({ name: "web", runtime: "node", buildState: "building" }) }, + }, + [`GET ${workersRoute("/web")}`]: { + status: 200, + body: { data: workerResource({ name: "web", runtime: "node", buildState: "active" }) }, + }, + }, + }); + + return Effect.gen(function* () { + yield* push({ names: [] }); + + expect(out.stderrText).not.toContain("Deploying Worker"); + expect(out.stdoutText).not.toContain("Deployed 2 Workers"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // `-o env` cannot express the `workers` array. Discovering that at emit time // meant failing with the project already changed, inviting a retry that // deployed all over again. From 71af01ded1c978ef32b6013a112c4ce4e9cc7923 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 17:19:16 -0300 Subject: [PATCH 14/15] docs(workers): mark the status and delete trailers as text-only Both handlers return at the machine emission or at `output.success`, so the build-retry trailer, the redeploy trailer, and the two stderr notices beside them are only reachable in the text branch. The format matrices claimed "as above" for every structured row, which would drive E2E expectations for output those modes never produce. --- .../experimental/workers/delete/SIDE_EFFECTS.md | 14 ++++++++++---- .../experimental/workers/status/SIDE_EFFECTS.md | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md index 0b0d8cfc55..4b425f2fdd 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/SIDE_EFFECTS.md @@ -80,8 +80,14 @@ wrapper emits for every command. | Mode | stdout | stderr | | ----------------------------- | --------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- | | text (default) | the confirmation prompt, then what was deleted and kept | that nothing local was kept when nothing was, and the redeploy hint | -| `--output-format json` | one structured result carrying `worker_name`, `project_ref`, `kept_*` | as above | -| `--output-format stream-json` | the same result as a single terminal event | as above | -| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | -| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | +| `--output-format json` | one structured result carrying `worker_name`, `project_ref`, `kept_*` | neither — both are text-only | +| `--output-format stream-json` | the same result as a single terminal event | neither — both are text-only | +| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | neither — both are text-only | +| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | both, as in text | | `-o env` | refused **before** the DELETE; discovering it at emit time deleted the worker and then failed | the error | + +A structured emission is the end of the run: the handler returns at +`legacyEmitWorkersMachineOutput` or at `output.success`, so nothing in the text +branch below it — the kept-nothing notice and the redeploy trailer — is reached. +`-o pretty`, `table` and `csv` are the exception, since they encode nothing and +fall through to that same text branch. diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md index 47680c8daa..26c7c76a48 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/experimental/workers/status/SIDE_EFFECTS.md @@ -64,8 +64,14 @@ wrapper emits for every command. | Mode | stdout | stderr | | ----------------------------- | ------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------- | | text (default) | the details block | an unreadable instance tally, and the build-retry hint on a failure | -| `--output-format json` | one structured result carrying every reported field | as above | -| `--output-format stream-json` | the same result as a single terminal event | as above | -| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | as above | -| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | as above | +| `--output-format json` | one structured result carrying every reported field | neither — both are text-only | +| `--output-format stream-json` | the same result as a single terminal event | neither — both are text-only | +| `-o json` / `yaml` / `toml` | the same payload in that encoding, and nothing else | neither — both are text-only | +| `-o pretty` / `table` / `csv` | the text rendering — these fall through rather than encoding | both, as in text | | `-o env` | refused before any request; the payload nests an instance tally a flat `KEY=value` list cannot express | the error | + +A structured emission is the end of the run: the handler returns at +`legacyEmitWorkersMachineOutput` or at `output.success`, so nothing in the text +branch below it — the instance tally and the build-retry trailer — is reached. +`-o pretty`, `table` and `csv` are the exception, since they encode nothing and +fall through to that same text branch. From 5bc206a434c7ce11ab999a4c8712f69d3c53fb1b Mon Sep 17 00:00:00 2001 From: kanad Date: Wed, 2 Sep 2026 22:17:54 -0700 Subject: [PATCH 15/15] Update apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts --- .../commands/experimental/workers/push/push.integration.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts index 0123082b4d..567d15c8c3 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts @@ -156,7 +156,6 @@ describe("legacy workers push", () => { expect(out.stdoutText).toContain("Deployed Worker api"); expect(out.stdoutText).toContain("Runtime"); expect(out.stdoutText).toContain(`https://${WORKERS_PROJECT_REF}.supabase.co/workers/v1/api`); - expect(out.stdoutText).toContain("v1"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); });