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 index b4a032b883..98643ae4bf 100644 --- 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 @@ -20,6 +20,10 @@ import { legacyRoot } from "./root.ts"; * `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. + * + * `workers push --wait`, added in this stack, is the flag that prompted it: it + * first shipped with neither closer and made a plain `supabase workers push` + * fail to parse at all. */ /** diff --git a/apps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md index b10b4598d1..8163c24dc1 100644 --- a/apps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/workers/push/SIDE_EFFECTS.md @@ -32,20 +32,24 @@ | `GET` | `/v2/projects/{ref}/workers/{name}` | Bearer token | none | `build_state`, `state_reason`, `image_version`, `spec` | | `GET` | `/v1/projects/{ref}` | Bearer token | none | linked-project cache miss only — name, org, region | -`GET` is polled until `build_state` leaves `building`. +`GET /v2/projects/{ref}/workers/{name}` is requested **only with `--wait`**, and +is then polled until `build_state` leaves `building`. Without it the command +returns on the deploy response, which carries the accepted spec and a +`build_state` of `building`. ## Exit Codes -| Code | Condition | -| ---- | ------------------------------------------------------- | -| `0` | success | -| `1` | no workers named and none found in the project | -| `1` | a worker's source is missing, not a directory, or empty | -| `1` | a worker's source directory cannot be read | -| `1` | a worker's source links to a path outside itself | -| `1` | build context upload failed | -| `1` | the build reached `failed`, or never left `building` | -| `1` | API error, or project not enrolled in the alpha | +| Code | Condition | +| ---- | ------------------------------------------------------------------- | +| `0` | success | +| `1` | no workers named and none found in the project | +| `1` | a worker's source is missing, not a directory, or empty | +| `1` | a worker's source directory cannot be read | +| `1` | a worker's source links to a path outside itself | +| `1` | build context upload failed | +| `1` | the deploy was answered with `build_state: failed` | +| `1` | with `--wait`: the build reached `failed`, or never left `building` | +| `1` | API error, or project not enrolled in the alpha | ## Environment Variables @@ -72,16 +76,20 @@ 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. +Without `--wait` the deploy returns with the build still running, so the +follow-up hint (`workers status`, and `--wait`) is emitted as a success trailer: +stderr, once, at the end of the run rather than between workers. **Text output +only** — like the rest of the human deploy report it sits behind +`output.format === "text"` and the `-o` check, so `--output-format json`, +`stream-json` and every legacy `-o` mode emit no hint. Machine callers read +`build_state` from the payload instead. The hint carries an explicit +`--project-ref` when the flag supplied one, since it is copy-pasted verbatim. + 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 +attempted on stderr in **every** format, machine ones included — unlike the +trailer above, `reportUnattempted` has no format guard: 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. +question the failure raises. The presigned `PUT` above is the one request whose URL is itself a credential. `--debug` logs every request URL, so `legacyHttpClientLayer` redacts query diff --git a/apps/cli/src/legacy/commands/workers/push/push.command.ts b/apps/cli/src/legacy/commands/workers/push/push.command.ts index 9262f028a8..20cae0e513 100644 --- a/apps/cli/src/legacy/commands/workers/push/push.command.ts +++ b/apps/cli/src/legacy/commands/workers/push/push.command.ts @@ -24,6 +24,17 @@ const config = { ), Flag.optional, ), + wait: Flag.boolean("wait").pipe( + // Off by default: the deploy POST is answered once the platform has accepted + // the spec and the uploaded context, and the server-side container build + // that follows routinely runs for minutes. Blocking on it made the common + // case — a deploy that builds fine — the slowest thing in the loop, so the + // wait is opt-in for the callers that actually need the build's verdict. + Flag.withDescription( + "Wait for the server-side build to finish, and fail if it does not succeed. Off by default: the command returns once the deploy is accepted.", + ), + Flag.withDefault(false), + ), projectRef: Flag.string("project-ref").pipe( Flag.withDescription("Project ref of the Supabase project."), Flag.optional, @@ -51,6 +62,10 @@ export const legacyWorkersPushCommand = Command.make("push", config).pipe( command: "supabase workers push api web", description: "Deploy several workers by name", }, + { + command: "supabase workers push api --wait", + description: "Deploy and block until the build succeeds or fails", + }, ]), Command.withHandler((flags) => legacyWorkersPush(flags).pipe( diff --git a/apps/cli/src/legacy/commands/workers/push/push.handler.ts b/apps/cli/src/legacy/commands/workers/push/push.handler.ts index b1c05d7ce8..b16fe8f06f 100644 --- a/apps/cli/src/legacy/commands/workers/push/push.handler.ts +++ b/apps/cli/src/legacy/commands/workers/push/push.handler.ts @@ -1,6 +1,7 @@ import { Effect, FileSystem, Option, Predicate, type Schedule } from "effect"; import type { PlatformError } from "effect/PlatformError"; import { Output } from "../../../../shared/output/output.service.ts"; +import { emitSuccessTrailer } from "../../../../shared/cli/success-trailer.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersMachineOutput, @@ -67,6 +68,11 @@ import type { LegacyWorkersPushFlags } from "./push.command.ts"; * code takes the same path, with the base image and a copy synthesized in place * of your Dockerfile. Every runtime this CLI offers has code to package, so * there is no path here that skips the upload. + * + * The command returns once the platform accepts the deploy. The container build + * that follows runs for minutes, and blocking on it made every successful + * deploy as slow as the slowest one — so `--wait` opts into the build's + * verdict, for CI and for anyone who needs the image version before continuing. */ const resolveRuntime = Effect.fnUntraced(function* (options: { @@ -183,6 +189,8 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { */ readonly refSuffix: string; readonly instances: Option.Option; + /** `--wait`: block on the server-side build instead of returning once it starts. */ + readonly wait: boolean; readonly pollSchedule?: Schedule.Schedule; readonly pollRetrySchedule?: Schedule.Schedule; /** Suppresses this step's human output when `-o` owns stdout. */ @@ -322,18 +330,27 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { }; const deploying = yield* output.task("Deploying worker..."); - yield* deployWorker(api, projectRef, name, { spec, contextUploadId }).pipe( + // The response to the deploy itself is the last thing this command can learn + // without waiting: the platform answers it only after accepting the spec and + // the uploaded context, and it carries the accepted spec back. Everything + // after this point is the server-side container build. + const accepted = yield* deployWorker(api, projectRef, name, { spec, contextUploadId }).pipe( Effect.tapError(() => deploying.fail()), ); - 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())); - + const settled = input.wait + ? 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())) + : accepted; + + // Checked whether or not the build was waited on. A deploy answered with a + // spec already in `failed` is a refusal the command should report as one, + // rather than exiting zero on a worker that will never come up. if (settled.buildState === "failed") { yield* deploying.clear(); return yield* Effect.fail( @@ -364,13 +381,38 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { ); yield* output.raw( legacyRenderWorkerDetails([ + // Labelled `State`, and placed first, the way `workers status` renders + // the same field: without `--wait` it is the one row that says the + // worker is not serving yet, so it should not be hunted for at the + // bottom of the block. + ["State", settled.buildState], ["Runtime", runtime], ["Size", formatApiSize(settled.spec.size)], + // Empty without `--wait`: no image exists until the build produces one, + // and `legacyRenderWorkerDetails` drops an empty-valued row. ["Image", settled.imageVersion ?? ""], ["Access", settled.spec.exposure], ["URL", url ?? ""], ]), ); + if (settled.buildState === "building") { + // A success trailer rather than an inline stderr line: this is a "what to + // run next" hint, which `stop`, `bootstrap`, `migration repair` and + // `gen signing-key` all route through `emitSuccessTrailer` so it prints + // once at the end of the run instead of scrolling away. It matters here + // more than for those: pushing several workers would otherwise bury each + // worker's hint under the next worker's packaging and deploy output. + // + // One short sentence per line, with the command and the flag aqua'd the + // way every other follow-up hint in this shell writes them. The single + // wrapped paragraph this replaced re-flowed differently at every terminal + // width and buried both commands mid-sentence. + yield* emitSuccessTrailer( + `\nYour build was submitted successfully.\n` + + `Run ${legacyAqua(`supabase workers status ${name}${input.refSuffix}`)} to check on it.\n` + + `Add ${legacyAqua("--wait")} to block on the build next time.\n`, + ); + } } return { @@ -419,6 +461,10 @@ const reportUnattempted = Effect.fnUntraced(function* (skipped: ReadonlyArray = {}): LegacyWorkersPu return { names: ["api"], instances: Option.none(), + // Mirrors the command default: the deploy returns once accepted, and only + // the scenarios that are about the build itself opt into waiting. + wait: false, projectRef: Option.none(), ...overrides, }; @@ -120,7 +123,7 @@ function push(flagOverrides: Partial = {}) { } describe("legacy workers push", () => { - it.live("packages, uploads, deploys and waits for the build to settle", () => { + it.live("packages, uploads and deploys, returning once the deploy is accepted", () => { const repo = project(); const { layer, out, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); @@ -131,7 +134,6 @@ describe("legacy workers push", () => { `POST ${workersRoute("/api/uploads")}`, "PUT /deploy-context/api.tar.gz", `POST ${workersRoute("/api/deploy")}`, - `GET ${workersRoute("/api")}`, ]); const deploy = http.requests.find((request) => request.url.endsWith("/deploy")); @@ -156,7 +158,29 @@ 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`); + // No image exists yet, so the row is dropped rather than rendered empty. + expect(out.stdoutText).not.toContain("Image"); + expect(out.stderrText).toContain("supabase workers status api"); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + + it.live("waits for the build to settle when --wait is passed", () => { + const repo = project(); + const { layer, out, http } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); + + return Effect.gen(function* () { + yield* push({ wait: true }); + + expect(http.routeKeys).toEqual([ + `POST ${workersRoute("/api/uploads")}`, + "PUT /deploy-context/api.tar.gz", + `POST ${workersRoute("/api/deploy")}`, + `GET ${workersRoute("/api")}`, + ]); + + expect(out.stdoutText).toContain("Deployed Worker api"); expect(out.stdoutText).toContain("v1"); + expect(out.stderrText).not.toContain("supabase workers status api"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); @@ -315,7 +339,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - yield* push(); + yield* push({ wait: true }); const polls = http.routeKeys.filter((key) => key === `GET ${workersRoute("/api")}`); expect(polls).toHaveLength(3); @@ -341,7 +365,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - const error = yield* push().pipe(Effect.flip); + const error = yield* push({ wait: true }).pipe(Effect.flip); expect(error).toBeInstanceOf(WorkerBuildFailedError); expect((error as WorkerBuildFailedError).detail).toContain("error building image"); @@ -356,8 +380,8 @@ describe("legacy workers push", () => { const { layer } = setupLegacyWorkers({ workdir: repo.dir, routes: routes({ - [`GET ${workersRoute("/api")}`]: { - status: 200, + [`POST ${workersRoute("/api/deploy")}`]: { + status: 202, body: { data: workerResource({ name: "api", runtime: "node", buildState: "failed" }) }, }, }), @@ -380,13 +404,13 @@ describe("legacy workers push", () => { const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, routes: routes({ - [`GET ${workersRoute("/api")}`]: { - status: 200, + [`POST ${workersRoute("/api/deploy")}`]: { + status: 202, body: { data: workerResource({ name: "api", runtime: "node", - buildState: "active", + buildState: "building", exposure: "private", }), }, @@ -405,8 +429,8 @@ describe("legacy workers push", () => { }); // 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. + // the handler with no options at all. A deploy that does not wait never polls, + // so the production schedules stay unused and this stays fast. 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() }); @@ -432,7 +456,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - const error = yield* legacyWorkersPush(flags(), { + const error = yield* legacyWorkersPush(flags({ wait: true }), { pollSchedule: Schedule.recurs(2), }).pipe(Effect.flip); @@ -454,11 +478,24 @@ describe("legacy workers push", () => { }); const withRef = { projectRef: Option.some(WORKERS_PROJECT_REF) }; + it.live("in the still-building trailer", () => { + const repo = project(); + const { layer, out } = unlinked(repo.dir); + + return Effect.gen(function* () { + yield* push(withRef); + + expect(out.stderrText).toContain( + `supabase workers status api --project-ref ${WORKERS_PROJECT_REF}`, + ); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + it.live("in the failed-build retry suggestion", () => { const repo = project(); const { layer } = unlinked(repo.dir, { - [`GET ${workersRoute("/api")}`]: { - status: 200, + [`POST ${workersRoute("/api/deploy")}`]: { + status: 202, body: { data: workerResource({ name: "api", runtime: "node", buildState: "failed" }) }, }, }); @@ -483,7 +520,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - const error = yield* legacyWorkersPush(flags(withRef), { + const error = yield* legacyWorkersPush(flags({ ...withRef, wait: true }), { pollSchedule: Schedule.recurs(2), }).pipe(Effect.flip); @@ -498,21 +535,13 @@ describe("legacy workers push", () => { // 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" }) }, - }, - }), - }); + const { layer, out } = setupLegacyWorkers({ workdir: repo.dir, routes: routes() }); return Effect.gen(function* () { - const error = yield* push().pipe(Effect.flip); + yield* push(); - expect((error as WorkerBuildFailedError).suggestion).toContain("supabase workers push api"); - expect((error as WorkerBuildFailedError).suggestion).not.toContain("--project-ref"); + expect(out.stderrText).toContain("supabase workers status api"); + expect(out.stderrText).not.toContain("--project-ref"); }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); }); @@ -867,7 +896,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - yield* push(); + yield* push({ wait: true }); // The blip was retried rather than aborting a deploy already in flight. expect( @@ -1035,8 +1064,8 @@ describe("legacy workers push", () => { workdir: repo.dir, routes: routes({ // `api` sorts first, so the run stops before `web` is ever touched. - [`GET ${workersRoute("/api")}`]: { - status: 200, + [`POST ${workersRoute("/api/deploy")}`]: { + status: 202, body: { data: workerResource({ name: "api", @@ -1110,7 +1139,7 @@ describe("legacy workers push", () => { }); return Effect.gen(function* () { - yield* push(); + yield* push({ wait: true }); const success = out.messages.findLast( (message) => message.type === "success" && message.data !== undefined, @@ -1132,6 +1161,37 @@ describe("legacy workers push", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); }); + // Without `--wait` the payload reports the accepted deploy rather than a + // finished one: the build has not produced an image, and saying `active` + // would tell a script the worker is already serving. + it.live("reports the build as still running in json mode without --wait", () => { + const repo = project(); + const { layer, out } = setupLegacyWorkers({ + workdir: repo.dir, + format: "json", + routes: routes(), + }); + + return Effect.gen(function* () { + yield* push(); + + const success = out.messages.findLast( + (message) => message.type === "success" && message.data !== undefined, + ); + expect(success?.data?.["workers"]).toEqual([ + { + worker_name: "api", + runtime: "node", + size: "2gb-1vcpu", + exposure: "public", + instances: 1, + build_state: "building", + url: `https://${WORKERS_PROJECT_REF}.supabase.co/workers/v1/api`, + }, + ]); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + // `-o env` cannot express the `workers` array. Discovering that at emit time // meant failing with the project already changed, inviting a retry that // deployed all over again.