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([]); + }); +}); 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..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 @@ -72,13 +72,14 @@ 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))); }); - // 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", () => { @@ -327,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({ @@ -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. @@ -604,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/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..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 @@ -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). 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); 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..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 @@ -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,10 +445,33 @@ 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, - // 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/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..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 @@ -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 }); @@ -829,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": "" }); @@ -886,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() }); 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 ?? ""}\`.`, }), ); }