From d4e37d5ac83233379039c7669694bc73e3b376b0 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 28 Aug 2026 23:31:40 -0300 Subject: [PATCH 1/2] 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 2/2] 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" });