From 6c8acc046a4bce65c3aa5fa49310809e3842d556 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 21:54:05 -0300 Subject: [PATCH 1/7] refactor(workers): give the family one command scaffold Five handlers opened with the same three service acquisitions, the same ref resolution and suffix, and closed with the same two finalizers. The ordering is the whole point of that block and is easy to get subtly wrong: four of them resolved the ref above both finalizers, so an unlinked non-interactive checkout failed before `telemetryState.flush` was installed and wrote no post-run event. `legacyWorkersRun` owns the ordering once. Telemetry wraps the resolution; the linked-project cache stays under the ref, having nothing to write without one. `logs` already had the fix and now shares the same path. --- .../workers/delete/delete.handler.ts | 332 +++++++++--------- .../experimental/workers/list/list.handler.ts | 238 ++++++------- .../experimental/workers/logs/logs.handler.ts | 28 +- .../experimental/workers/push/push.handler.ts | 176 +++++----- .../workers/status/status.handler.ts | 228 ++++++------ .../workers/workers.run.integration.test.ts | 82 +++++ .../experimental/workers/workers.run.ts | 42 +++ 7 files changed, 588 insertions(+), 538 deletions(-) create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.run.ts 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 f607dcc41f..ab5b92ce7e 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 @@ -7,7 +7,6 @@ import { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput, legacyWorkersMachineOutputRequested, - legacyWorkersProjectRefSuffix, } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; @@ -19,15 +18,13 @@ import { WorkersApiUnexpectedStatusError, } from "../../../../../shared/workers/workers.errors.ts"; import { legacyResolveYes } from "../../../../../shared/legacy/global-flags.ts"; -import { LegacyProjectRefResolver } from "../../../../config/legacy-project-ref.service.ts"; import { Tty } from "../../../../../shared/runtime/tty.service.ts"; -import { LegacyLinkedProjectCache } from "../../../../telemetry/legacy-linked-project-cache.service.ts"; -import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyDescribeWorkerForReporting, legacyLoadWorkersProjectForReporting, legacyValidateWorkerName, } from "../workers.shared.ts"; +import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersDeleteFlags } from "./delete.command.ts"; /** @@ -56,197 +53,184 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete ) { const output = yield* Output; const api = yield* LegacyPlatformApi; - const resolver = yield* LegacyProjectRefResolver; - const linkedProjectCache = yield* LegacyLinkedProjectCache; - const telemetryState = yield* LegacyTelemetryState; const tty = yield* Tty; // `--yes` OR `SUPABASE_YES`, matching `projects delete` and every other // command that guards a destructive step behind a prompt. const yes = yield* legacyResolveYes; - // The ref is resolved outside the finalizers because caching it is one of - // them; everything that can fail on its own — loading `config.toml`, - // validating the name, resolving the worker — belongs inside, so those - // failures still flush telemetry. Same shape as `config/push`. - const projectRef = yield* resolver.resolve(flags.projectRef); - // Every retry this command suggests is for a *destructive* re-run, so the ref - // has to survive the copy-paste. - const refSuffix = legacyWorkersProjectRefSuffix(flags.projectRef); - - yield* Effect.gen(function* () { - const project = yield* legacyLoadWorkersProjectForReporting(); - const name = yield* legacyValidateWorkerName(flags.name); - const worker = yield* legacyDescribeWorkerForReporting(project, name); - - // Before the first API call, not at emit time: the emit branch is reached - // *after* the DELETE, so `--yes -o env` deleted the worker and only then - // exited non-zero with no payload — which a script reads as a failed delete. - yield* legacyRejectWorkersEnvOutput(); - - const fetching = yield* output.task("Fetching worker..."); - // The lookup is a courtesy, not a prerequisite: it supplies the instance - // tally the confirmation quotes and the "already gone" verdict. The API - // grants the read and the delete separately — `edge_functions:read` for - // `GET`, `edge_functions:write` for `DELETE` — so a credential holding only - // the latter could not delete a worker it is entitled to delete. A refused - // read now leaves the worker *unknown* and the delete goes ahead. - const lookup = yield* getWorker(api, projectRef, name).pipe( - Effect.map((found) => ({ readable: true, worker: Option.getOrUndefined(found) })), - Effect.catchIf( - (error) => error instanceof WorkersApiUnexpectedStatusError && error.status === 403, - () => Effect.succeed({ readable: false, worker: undefined }), - ), - Effect.tapError(() => fetching.fail()), - ); - yield* fetching.clear(); - - const deployed = lookup.worker; - const machineOutput = yield* legacyWorkersMachineOutputRequested(); - - // `--yes` is the scripted path, and `deleteWorker` already treats a DELETE - // 404 as done — "a delete that races another one is still a delete that - // happened". The pre-flight GET contradicted that for teardown: a script run - // twice exited non-zero the second time, for a worker in exactly the state - // it asked for. Interactively the error stays: somebody typed this command - // and wants to hear the worker was not there. - if (lookup.readable && deployed === undefined && !yes) { - return yield* Effect.fail( - new WorkerNotDeployedError({ - detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, - // `status`'s wording, inherited, pointed the wrong way here: somebody - // deleting "api" and hearing "nothing is deployed" does not want to - // deploy it — they want to see what *is* deployed. - suggestion: `See what is deployed with \`supabase experimental workers list${refSuffix}\`.`, - }), + yield* legacyWorkersRun(flags.projectRef, ({ projectRef, refSuffix }) => + Effect.gen(function* () { + const project = yield* legacyLoadWorkersProjectForReporting(); + const name = yield* legacyValidateWorkerName(flags.name); + const worker = yield* legacyDescribeWorkerForReporting(project, name); + + // Before the first API call, not at emit time: the emit branch is reached + // *after* the DELETE, so `--yes -o env` deleted the worker and only then + // exited non-zero with no payload — which a script reads as a failed delete. + yield* legacyRejectWorkersEnvOutput(); + + const fetching = yield* output.task("Fetching worker..."); + // The lookup is a courtesy, not a prerequisite: it supplies the instance + // tally the confirmation quotes and the "already gone" verdict. The API + // grants the read and the delete separately — `edge_functions:read` for + // `GET`, `edge_functions:write` for `DELETE` — so a credential holding only + // the latter could not delete a worker it is entitled to delete. A refused + // read now leaves the worker *unknown* and the delete goes ahead. + const lookup = yield* getWorker(api, projectRef, name).pipe( + Effect.map((found) => ({ readable: true, worker: Option.getOrUndefined(found) })), + Effect.catchIf( + (error) => error instanceof WorkersApiUnexpectedStatusError && error.status === 403, + () => Effect.succeed({ readable: false, worker: undefined }), + ), + Effect.tapError(() => fetching.fail()), ); - } - - if (!yes) { - // `-o json` leaves `output.format` as `text`, so the format check alone - // still let the warning and the prompt run — onto the stdout the user had - // asked to carry a payload. 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 experimental workers delete api` feed the pipe straight - // into the prompt and delete without `--yes`. The confirmation is only - // meaningful from a keyboard, so stdin has to be a terminal too — the same - // pair `projects delete` guards its prompt with. - if (output.format !== "text" || machineOutput || !output.interactive || !tty.stdinIsTty) { + yield* fetching.clear(); + + const deployed = lookup.worker; + const machineOutput = yield* legacyWorkersMachineOutputRequested(); + + // `--yes` is the scripted path, and `deleteWorker` already treats a DELETE + // 404 as done — "a delete that races another one is still a delete that + // happened". The pre-flight GET contradicted that for teardown: a script run + // twice exited non-zero the second time, for a worker in exactly the state + // it asked for. Interactively the error stays: somebody typed this command + // and wants to hear the worker was not there. + if (lookup.readable && deployed === undefined && !yes) { return yield* Effect.fail( - new WorkerDeleteConfirmationRequiredError({ - detail: `Deleting "${name}" from project ${projectRef} needs confirmation, and there is no interactive terminal to ask on.`, - suggestion: `Re-run \`supabase experimental workers delete ${name} --yes${refSuffix}\` to confirm without a prompt.`, + new WorkerNotDeployedError({ + detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, + // `status`'s wording, inherited, pointed the wrong way here: somebody + // deleting "api" and hearing "nothing is deployed" does not want to + // deploy it — they want to see what *is* deployed. + suggestion: `See what is deployed with \`supabase experimental workers list${refSuffix}\`.`, }), ); } - // The live tally when the API reports one, labelled "declared" when it - // does not. `spec.instances` is the target, which for a worker still - // provisioning differs from what is running — and a destructive prompt is - // the wrong place to overstate. - // Absent when the read was refused: the prompt still asks for the name, - // it just cannot quote a count it was not allowed to see. - const live = deployed?.instances?.live; - const declared = deployed?.spec.instances; - const terminating = - live !== undefined - ? live > 0 - ? ` ${live} running instance${live === 1 ? "" : "s"} will be terminated.` - : "" - : declared !== undefined && declared > 0 - ? ` ${declared} declared instance${declared === 1 ? "" : "s"} will be terminated.` - : ""; - yield* output.raw( - `This permanently deletes "${name}" from project ${projectRef}.${terminating}\n`, - ); - const typed = yield* output.promptText(`Type ${name} to confirm`); - // Trimmed: a trailing space from a paste is not a different answer, and - // making someone re-run a destructive command over one is just friction. - if (typed.trim() !== name) { - return yield* Effect.fail( - new WorkerDeleteNotConfirmedError({ - detail: `The confirmation did not match "${name}", so nothing was deleted.`, - suggestion: `Re-run \`supabase experimental workers delete ${name}${refSuffix}\` and type the name exactly, or pass --yes.`, - }), - ); - } - } - - // Skipped only when the fetch actually said there is nothing there. An - // unreadable worker still gets the DELETE — that request is the one the - // credential is entitled to make, and the API treats a 404 on it as done. - if (deployed !== undefined || !lookup.readable) { - const deleting = yield* output.task("Deleting worker..."); - yield* deleteWorker(api, projectRef, name).pipe(Effect.tapError(() => deleting.fail())); - yield* deleting.clear(); - } - - // A worker deployed from another checkout has neither a local entry nor a - // local directory, so there is nothing here that was kept. - const keptSource = worker.sourceExists - ? displayPath(project.projectRoot, worker.sourceDir) - : undefined; - const keptEntry = worker.entry !== undefined; - - const payload = { - worker_name: name, - project_ref: projectRef, - ...(keptSource === undefined ? {} : { kept_source: keptSource }), - kept_config_entry: keptEntry, - }; - - // `-o` asks for a machine-readable stdout, so nothing human may be written - // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - if (output.format !== "text") { - yield* output.success("", payload); - return; - } - - { - if (deployed === undefined && lookup.readable) { + if (!yes) { + // `-o json` leaves `output.format` as `text`, so the format check alone + // still let the warning and the prompt run — onto the stdout the user had + // asked to carry a payload. 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 experimental workers delete api` feed the pipe straight + // into the prompt and delete without `--yes`. The confirmation is only + // meaningful from a keyboard, so stdin has to be a terminal too — the same + // pair `projects delete` guards its prompt with. + if (output.format !== "text" || machineOutput || !output.interactive || !tty.stdinIsTty) { + return yield* Effect.fail( + new WorkerDeleteConfirmationRequiredError({ + detail: `Deleting "${name}" from project ${projectRef} needs confirmation, and there is no interactive terminal to ask on.`, + suggestion: `Re-run \`supabase experimental workers delete ${name} --yes${refSuffix}\` to confirm without a prompt.`, + }), + ); + } + + // The live tally when the API reports one, labelled "declared" when it + // does not. `spec.instances` is the target, which for a worker still + // provisioning differs from what is running — and a destructive prompt is + // the wrong place to overstate. + // Absent when the read was refused: the prompt still asks for the name, + // it just cannot quote a count it was not allowed to see. + const live = deployed?.instances?.live; + const declared = deployed?.spec.instances; + const terminating = + live !== undefined + ? live > 0 + ? ` ${live} running instance${live === 1 ? "" : "s"} will be terminated.` + : "" + : declared !== undefined && declared > 0 + ? ` ${declared} declared instance${declared === 1 ? "" : "s"} will be terminated.` + : ""; yield* output.raw( - `Nothing was deployed for ${legacyAqua(name, process.stdout)} in project ${projectRef}, so there was nothing to delete.\n`, + `This permanently deletes "${name}" from project ${projectRef}.${terminating}\n`, ); + const typed = yield* output.promptText(`Type ${name} to confirm`); + // Trimmed: a trailing space from a paste is not a different answer, and + // making someone re-run a destructive command over one is just friction. + if (typed.trim() !== name) { + return yield* Effect.fail( + new WorkerDeleteNotConfirmedError({ + detail: `The confirmation did not match "${name}", so nothing was deleted.`, + suggestion: `Re-run \`supabase experimental workers delete ${name}${refSuffix}\` and type the name exactly, or pass --yes.`, + }), + ); + } + } + + // Skipped only when the fetch actually said there is nothing there. An + // unreadable worker still gets the DELETE — that request is the one the + // credential is entitled to make, and the API treats a 404 on it as done. + if (deployed !== undefined || !lookup.readable) { + const deleting = yield* output.task("Deleting worker..."); + yield* deleteWorker(api, projectRef, name).pipe(Effect.tapError(() => deleting.fail())); + yield* deleting.clear(); + } + + // A worker deployed from another checkout has neither a local entry nor a + // local directory, so there is nothing here that was kept. + const keptSource = worker.sourceExists + ? displayPath(project.projectRoot, worker.sourceDir) + : undefined; + const keptEntry = worker.entry !== undefined; + + const payload = { + worker_name: name, + project_ref: projectRef, + ...(keptSource === undefined ? {} : { kept_source: keptSource }), + kept_config_entry: keptEntry, + }; + + // `-o` asks for a machine-readable stdout, so nothing human may be written + // to it — `output.success` logs to stdout in text mode. + if (yield* legacyEmitWorkersMachineOutput(payload)) { return; } - yield* output.raw( - `Deleted Worker ${legacyAqua(name, process.stdout)} from project ${projectRef}\n`, - ); + if (output.format !== "text") { + yield* output.success("", payload); + return; + } - // "Deleted" reads more final than it is *when there is something left* — - // so only say so when there is. For an orphan there is nothing local to - // keep, and pointing at `push` would send the user at a command that has - // no source to deploy. - const kept = [ - ...(keptSource === undefined ? [] : [keptSource]), - ...(keptEntry ? ["its supabase/config.toml entry"] : []), - ]; - if (kept.length > 0) { - yield* output.raw(legacyRenderWorkerDetails([["Kept", kept.join(", ")]])); - // Only when the source is still there: a retained `config.toml` entry - // alone is not enough to redeploy from, so `push` would fail on the very - // command this line recommends. - if (keptSource !== undefined) { - // 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`, + { + if (deployed === undefined && lookup.readable) { + yield* output.raw( + `Nothing was deployed for ${legacyAqua(name, process.stdout)} in project ${projectRef}, so there was nothing to delete.\n`, ); + return; } - } else { + yield* output.raw( - `Nothing for "${name}" exists in this project on disk, so nothing was kept.\n`, - "stderr", + `Deleted Worker ${legacyAqua(name, process.stdout)} from project ${projectRef}\n`, ); + + // "Deleted" reads more final than it is *when there is something left* — + // so only say so when there is. For an orphan there is nothing local to + // keep, and pointing at `push` would send the user at a command that has + // no source to deploy. + const kept = [ + ...(keptSource === undefined ? [] : [keptSource]), + ...(keptEntry ? ["its supabase/config.toml entry"] : []), + ]; + if (kept.length > 0) { + yield* output.raw(legacyRenderWorkerDetails([["Kept", kept.join(", ")]])); + // Only when the source is still there: a retained `config.toml` entry + // alone is not enough to redeploy from, so `push` would fail on the very + // command this line recommends. + if (keptSource !== undefined) { + // 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 { + yield* output.raw( + `Nothing for "${name}" exists in this project on disk, so nothing was kept.\n`, + "stderr", + ); + } } - } - }).pipe( - Effect.ensuring(linkedProjectCache.cache(projectRef)), - Effect.ensuring(telemetryState.flush), + }), ); }); 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 669972f199..a81390a73f 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 @@ -9,10 +9,8 @@ import { LegacyCliSettings } from "../../../../config/legacy-cli-settings.servic import { formatApiSize } from "../../../../../shared/workers/worker-runtimes.ts"; import { workerUrl } from "../../../../../shared/workers/worker-url.ts"; import { listWorkers, type WorkerRecord } from "../../../../../shared/workers/workers-api.ts"; -import { LegacyProjectRefResolver } from "../../../../config/legacy-project-ref.service.ts"; -import { LegacyLinkedProjectCache } from "../../../../telemetry/legacy-linked-project-cache.service.ts"; -import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyDiscoverWorkerNames, legacyLoadWorkersProject } from "../workers.shared.ts"; +import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersListFlags } from "./list.command.ts"; /** @@ -101,130 +99,120 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f ) { const output = yield* Output; const api = yield* LegacyPlatformApi; - const resolver = yield* LegacyProjectRefResolver; - const linkedProjectCache = yield* LegacyLinkedProjectCache; - const telemetryState = yield* LegacyTelemetryState; const settings = yield* LegacyCliSettings; - // The ref is resolved outside the finalizers because caching it is one of - // them; everything that can fail on its own — loading `config.toml`, - // validating the name, resolving the worker — belongs inside, so those - // failures still flush telemetry. Same shape as `config/push`. - const projectRef = yield* resolver.resolve(flags.projectRef); - - yield* Effect.gen(function* () { - const project = yield* legacyLoadWorkersProject(); - - // Up front, like the rest of the family: this payload always carries a - // `workers` array, so `-o env` can never encode it, and finding that out at - // emit time means failing after the fetch has already been paid for. - yield* legacyRejectWorkersEnvOutput(); - - const fetching = yield* output.task("Fetching workers..."); - const deployed = yield* listWorkers(api, projectRef).pipe( - Effect.tapError(() => fetching.fail()), - ); - yield* fetching.clear(); - - const byName = new Map(deployed.map((worker) => [worker.name, worker])); - const configuredNames = Object.keys(project.section.workers); - // Three sources: config entries, deployed workers, and directories under the - // workers root. The last are deployable — `legacyDiscoverWorkerNames` is the - // walk a bare `push` does — so the inventory has to show them. - const discoveredNames = yield* legacyDiscoverWorkerNames(project); - const names = [...new Set([...configuredNames, ...discoveredNames, ...byName.keys()])].sort(); - - const rows: Array = names.map((name) => { - const record = byName.get(name); - return { - name, - configured: configuredNames.includes(name), - local: configuredNames.includes(name) || discoveredNames.includes(name), - deployed: record, - localRuntime: project.section.workers[name]?.runtime, - url: - record !== undefined && record.spec.exposure === "public" - ? workerUrl(projectRef, settings.projectHost, name) - : undefined, - }; - }); - - const payload = { - project_ref: projectRef, - workers: rows.map((row) => ({ - name: row.name, - configured: row.configured, - local: row.local, - deployed: row.deployed !== undefined, - // Read the same way `runtimeLabel` reads it, so `-o json` and the text - // table cannot disagree: for a deployed worker an absent `spec.runtime` - // *means* dockerfile, and falling back to the local config there - // reported a stale runtime the deployment had moved off. - runtime: runtimeLabelFor(row), - size: row.deployed?.spec.size, - state: stateLabel(row), - instances: row.deployed?.spec.instances, - url: row.url, - })), - }; - - // `-o` is independent of `--output-format`: it leaves `output.format` as - // `text`, so this has to be checked before the text branch below, not - // inside the structured one. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - if (output.format !== "text") { - yield* output.success("", payload); - return; - } - - if (rows.length === 0) { - yield* output.raw( - `No workers found. Scaffold one with ${legacyAqua("supabase experimental workers new ", process.stdout)}.\n`, - ); - return; - } - - yield* output.raw(renderGlamourTable([...HEADERS], rows.map(toCells))); - - // Two different problems, and they need different advice. A worker with a - // local directory but no entry can be pushed — the runtime is the only - // unknown. One with nothing local at all cannot: `deployOneWorker` checks - // 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( - `${legacyYellow("WARNING:")} ${nameList(unconfigured)} deployed but not in ${configDisplay}.\n` + - `Pushing from here would have to guess the runtime.\n`, - "stderr", - ); - } - - const remoteOnly = rows - .filter((row) => row.deployed !== undefined && !row.local) - .map((row) => row.name); - if (remoteOnly.length > 0) { - yield* output.raw( - `${legacyYellow("WARNING:")} ${nameList(remoteOnly)} deployed with no source in this project.\n` + - `Scaffold or restore before pushing from here.\n`, - "stderr", + yield* legacyWorkersRun(flags.projectRef, ({ projectRef }) => + Effect.gen(function* () { + const project = yield* legacyLoadWorkersProject(); + + // Up front, like the rest of the family: this payload always carries a + // `workers` array, so `-o env` can never encode it, and finding that out at + // emit time means failing after the fetch has already been paid for. + yield* legacyRejectWorkersEnvOutput(); + + const fetching = yield* output.task("Fetching workers..."); + const deployed = yield* listWorkers(api, projectRef).pipe( + Effect.tapError(() => fetching.fail()), ); - } - }).pipe( - Effect.ensuring(linkedProjectCache.cache(projectRef)), - Effect.ensuring(telemetryState.flush), + yield* fetching.clear(); + + const byName = new Map(deployed.map((worker) => [worker.name, worker])); + const configuredNames = Object.keys(project.section.workers); + // Three sources: config entries, deployed workers, and directories under the + // workers root. The last are deployable — `legacyDiscoverWorkerNames` is the + // walk a bare `push` does — so the inventory has to show them. + const discoveredNames = yield* legacyDiscoverWorkerNames(project); + const names = [...new Set([...configuredNames, ...discoveredNames, ...byName.keys()])].sort(); + + const rows: Array = names.map((name) => { + const record = byName.get(name); + return { + name, + configured: configuredNames.includes(name), + local: configuredNames.includes(name) || discoveredNames.includes(name), + deployed: record, + localRuntime: project.section.workers[name]?.runtime, + url: + record !== undefined && record.spec.exposure === "public" + ? workerUrl(projectRef, settings.projectHost, name) + : undefined, + }; + }); + + const payload = { + project_ref: projectRef, + workers: rows.map((row) => ({ + name: row.name, + configured: row.configured, + local: row.local, + deployed: row.deployed !== undefined, + // Read the same way `runtimeLabel` reads it, so `-o json` and the text + // table cannot disagree: for a deployed worker an absent `spec.runtime` + // *means* dockerfile, and falling back to the local config there + // reported a stale runtime the deployment had moved off. + runtime: runtimeLabelFor(row), + size: row.deployed?.spec.size, + state: stateLabel(row), + instances: row.deployed?.spec.instances, + url: row.url, + })), + }; + + // `-o` is independent of `--output-format`: it leaves `output.format` as + // `text`, so this has to be checked before the text branch below, not + // inside the structured one. + if (yield* legacyEmitWorkersMachineOutput(payload)) { + return; + } + + if (output.format !== "text") { + yield* output.success("", payload); + return; + } + + if (rows.length === 0) { + yield* output.raw( + `No workers found. Scaffold one with ${legacyAqua("supabase experimental workers new ", process.stdout)}.\n`, + ); + return; + } + + yield* output.raw(renderGlamourTable([...HEADERS], rows.map(toCells))); + + // Two different problems, and they need different advice. A worker with a + // local directory but no entry can be pushed — the runtime is the only + // unknown. One with nothing local at all cannot: `deployOneWorker` checks + // 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( + `${legacyYellow("WARNING:")} ${nameList(unconfigured)} deployed but not in ${configDisplay}.\n` + + `Pushing from here would have to guess the runtime.\n`, + "stderr", + ); + } + + const remoteOnly = rows + .filter((row) => row.deployed !== undefined && !row.local) + .map((row) => row.name); + if (remoteOnly.length > 0) { + yield* output.raw( + `${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/logs/logs.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts index b611a638db..0e576f60e5 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts @@ -2,11 +2,7 @@ import { Effect, Option, Ref, Schedule } 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 { - legacyEmitWorkersMachineOutput, - legacyRejectWorkersEnvOutput, - legacyWorkersProjectRefSuffix, -} from "../workers.output.ts"; +import { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { legacyRenderWorkerLogLine, legacyWorkerLogLevel, @@ -34,14 +30,12 @@ import { WorkersApiNetworkError, WorkersApiUnexpectedStatusError, } from "../../../../../shared/workers/workers.errors.ts"; -import { LegacyProjectRefResolver } from "../../../../config/legacy-project-ref.service.ts"; -import { LegacyLinkedProjectCache } from "../../../../telemetry/legacy-linked-project-cache.service.ts"; -import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyValidateWorkerName } from "../workers.shared.ts"; import { legacyWorkersMachineOutputRequested, legacyWorkersRenderFormat, } from "../workers.output.ts"; +import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersLogsFlags } from "./logs.command.ts"; /** @@ -158,20 +152,10 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f ) { const output = yield* Output; const api = yield* LegacyPlatformApi; - const resolver = yield* LegacyProjectRefResolver; - const linkedProjectCache = yield* LegacyLinkedProjectCache; - const telemetryState = yield* LegacyTelemetryState; const processControl = yield* ProcessControl; - // Telemetry wraps the ref resolution as well: an unlinked non-interactive - // checkout fails inside `resolve`, and by then the command has run. Only the - // linked-project cache stays under the ref, since it has nothing to write - // without one. - yield* Effect.gen(function* () { - const projectRef = yield* resolver.resolve(flags.projectRef); - const refSuffix = legacyWorkersProjectRefSuffix(flags.projectRef); - - yield* Effect.gen(function* () { + yield* legacyWorkersRun(flags.projectRef, ({ projectRef, refSuffix }) => + Effect.gen(function* () { const name = yield* legacyValidateWorkerName(flags.name); // Up front, like the rest of the family: this payload always carries a `logs` @@ -449,6 +433,6 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f Effect.flatMap((signal) => processControl.setExitCode(signal === "SIGINT" ? 130 : 0)), ), ); - }).pipe(Effect.ensuring(linkedProjectCache.cache(projectRef))); - }).pipe(Effect.ensuring(telemetryState.flush)); + }), + ); }); 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 d3ddf06c50..2f09609886 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 @@ -7,7 +7,6 @@ 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"; @@ -44,9 +43,6 @@ import { WorkerBuildFailedError, WorkerSourceMissingError, } from "../../../../../shared/workers/workers.errors.ts"; -import { LegacyProjectRefResolver } from "../../../../config/legacy-project-ref.service.ts"; -import { LegacyLinkedProjectCache } from "../../../../telemetry/legacy-linked-project-cache.service.ts"; -import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyDescribeWorker, legacyDiscoverWorkerNames, @@ -54,6 +50,7 @@ import { legacyValidateWorkerName, type LegacyWorkersProject, } from "../workers.shared.ts"; +import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersPushFlags } from "./push.command.ts"; /** @@ -477,107 +474,94 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f } = {}, ) { const output = yield* Output; - const resolver = yield* LegacyProjectRefResolver; - const linkedProjectCache = yield* LegacyLinkedProjectCache; - const telemetryState = yield* LegacyTelemetryState; - // The ref is resolved outside the finalizers because caching it is one of - // them; everything that can fail on its own — loading `config.toml`, - // validating names, discovering workers — belongs inside, so a malformed - // config still flushes telemetry. Same shape as `config/push`. - const projectRef = yield* resolver.resolve(flags.projectRef); - - yield* Effect.gen(function* () { - const project = yield* legacyLoadWorkersProject(); - - const requested = - flags.names.length > 0 - ? yield* Effect.forEach(flags.names, legacyValidateWorkerName) - : yield* legacyDiscoverWorkerNames(project); - - if (requested.length === 0) { - return yield* Effect.fail( - new NoWorkersToDeployError({ - detail: `No workers were named, and none were found in ${displayPath( - project.projectRoot, - project.workersDir, - )}.`, - suggestion: "Scaffold one with `supabase experimental workers new `.", - }), - ); - } - - const names = [...new Set(requested)]; + yield* legacyWorkersRun(flags.projectRef, ({ projectRef, refSuffix }) => + Effect.gen(function* () { + const project = yield* legacyLoadWorkersProject(); + + const requested = + flags.names.length > 0 + ? yield* Effect.forEach(flags.names, legacyValidateWorkerName) + : yield* legacyDiscoverWorkerNames(project); + + if (requested.length === 0) { + return yield* Effect.fail( + new NoWorkersToDeployError({ + detail: `No workers were named, and none were found in ${displayPath( + project.projectRoot, + project.workersDir, + )}.`, + suggestion: "Scaffold one with `supabase experimental workers new `.", + }), + ); + } - // Before the first deploy, not after the last one: this payload always - // carries a `workers` array, so `-o env` can never encode it, and finding - // that out at the end means failing with the remote project already changed. - yield* legacyRejectWorkersEnvOutput(); + const names = [...new Set(requested)]; + + // Before the first deploy, not after the last one: this payload always + // carries a `workers` array, so `-o env` can never encode it, and finding + // that out at the end means failing with the remote project already changed. + yield* legacyRejectWorkersEnvOutput(); + + const machineOutput = yield* legacyWorkersMachineOutputRequested(); + const deployed: Array> = []; + for (const [index, name] of names.entries()) { + if (names.length > 1 && !machineOutput && output.format === "text") { + // stderr, unblanked and labelled, the way `functions deploy` announces + // each function: a bare name with a leading blank line put a section + // header into whatever was consuming stdout. + // + // Counted, because each worker's package/upload/build takes minutes and + // the name alone says nothing about how much of the run is left. + // + // Text only, on both axes: `machineOutput` tracks `-o`, which leaves + // `output.format` as `text`, so neither check covers the other. This is + // progress rather than an outcome, and `--output-format json` asked for + // a stream of events — unlike the unattempted-workers report below, + // which every format gets because it says what still needs deploying. + yield* output.raw( + `Deploying Worker ${index + 1}/${names.length}: ${legacyAqua(name)}\n`, + "stderr", + ); + } + deployed.push( + yield* deployOneWorker({ + project, + name, + projectRef, + refSuffix, + instances: flags.instances, + wait: flags.wait, + machineOutput, + ...(options.pollSchedule === undefined ? {} : { pollSchedule: options.pollSchedule }), + ...(options.pollRetrySchedule === undefined + ? {} + : { pollRetrySchedule: options.pollRetrySchedule }), + }).pipe(Effect.tapError(() => reportUnattempted(names.slice(index + 1)))), + ); + } - 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 [index, name] of names.entries()) { + // 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") { - // stderr, unblanked and labelled, the way `functions deploy` announces - // each function: a bare name with a leading blank line put a section - // header into whatever was consuming stdout. - // - // Counted, because each worker's package/upload/build takes minutes and - // the name alone says nothing about how much of the run is left. - // - // Text only, on both axes: `machineOutput` tracks `-o`, which leaves - // `output.format` as `text`, so neither check covers the other. This is - // progress rather than an outcome, and `--output-format json` asked for - // a stream of events — unlike the unattempted-workers report below, - // which every format gets because it says what still needs deploying. yield* output.raw( - `Deploying Worker ${index + 1}/${names.length}: ${legacyAqua(name)}\n`, - "stderr", + `Deployed ${names.length} Workers to project ${projectRef}: ${names + .map((name) => legacyAqua(name, process.stdout)) + .join(", ")}\n`, ); } - deployed.push( - yield* deployOneWorker({ - project, - name, - projectRef, - refSuffix, - instances: flags.instances, - wait: flags.wait, - 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`, - ); - } - const payload = { project_ref: projectRef, workers: deployed }; + const payload = { project_ref: projectRef, workers: deployed }; - // `-o` asks for a machine-readable stdout, so nothing human may be written - // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } + // `-o` asks for a machine-readable stdout, so nothing human may be written + // to it — `output.success` logs to stdout in text mode. + if (yield* legacyEmitWorkersMachineOutput(payload)) { + return; + } - if (output.format !== "text") { - yield* output.success("", payload); - } - }).pipe( - Effect.ensuring(linkedProjectCache.cache(projectRef)), - Effect.ensuring(telemetryState.flush), + if (output.format !== "text") { + yield* output.success("", payload); + } + }), ); }); 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 b0319080af..dbfe0ab265 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 @@ -3,11 +3,7 @@ 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, - legacyRejectWorkersEnvOutput, - legacyWorkersProjectRefSuffix, -} from "../workers.output.ts"; +import { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { LegacyCliSettings } from "../../../../config/legacy-cli-settings.service.ts"; import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; @@ -15,14 +11,12 @@ import { formatApiSize } from "../../../../../shared/workers/worker-runtimes.ts" import { workerUrl } from "../../../../../shared/workers/worker-url.ts"; import { getWorker } from "../../../../../shared/workers/workers-api.ts"; import { WorkerNotDeployedError } from "../../../../../shared/workers/workers.errors.ts"; -import { LegacyProjectRefResolver } from "../../../../config/legacy-project-ref.service.ts"; -import { LegacyLinkedProjectCache } from "../../../../telemetry/legacy-linked-project-cache.service.ts"; -import { LegacyTelemetryState } from "../../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyDescribeWorkerForReporting, legacyLoadWorkersProjectForReporting, legacyValidateWorkerName, } from "../workers.shared.ts"; +import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersStatusFlags } from "./status.command.ts"; /** @@ -37,130 +31,122 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status ) { const output = yield* Output; const api = yield* LegacyPlatformApi; - const resolver = yield* LegacyProjectRefResolver; - const linkedProjectCache = yield* LegacyLinkedProjectCache; - const telemetryState = yield* LegacyTelemetryState; const settings = yield* LegacyCliSettings; - // The ref is resolved outside the finalizers because caching it is one of - // them; everything that can fail on its own — loading `config.toml`, - // validating the name, resolving the worker — belongs inside, so those - // failures still flush telemetry. Same shape as `config/push`. - const projectRef = yield* resolver.resolve(flags.projectRef); - const refSuffix = legacyWorkersProjectRefSuffix(flags.projectRef); - - yield* Effect.gen(function* () { - const project = yield* legacyLoadWorkersProjectForReporting(); - const name = yield* legacyValidateWorkerName(flags.name); - const worker = yield* legacyDescribeWorkerForReporting(project, name); - - // Up front, like the rest of the family: discovering an unencodable format - // at emit time means failing after the fetch has already been paid for. - yield* legacyRejectWorkersEnvOutput(); + yield* legacyWorkersRun(flags.projectRef, ({ projectRef, refSuffix }) => + Effect.gen(function* () { + const project = yield* legacyLoadWorkersProjectForReporting(); + const name = yield* legacyValidateWorkerName(flags.name); + const worker = yield* legacyDescribeWorkerForReporting(project, name); - const fetching = yield* output.task("Fetching worker..."); - const found = yield* getWorker(api, projectRef, name).pipe( - Effect.tapError(() => fetching.fail()), - ); - yield* fetching.clear(); + // Up front, like the rest of the family: discovering an unencodable format + // at emit time means failing after the fetch has already been paid for. + yield* legacyRejectWorkersEnvOutput(); - if (Option.isNone(found)) { - return yield* Effect.fail( - new WorkerNotDeployedError({ - detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, - suggestion: `Deploy it with \`supabase experimental workers push ${name}${refSuffix}\`.`, - }), + const fetching = yield* output.task("Fetching worker..."); + const found = yield* getWorker(api, projectRef, name).pipe( + Effect.tapError(() => fetching.fail()), ); - } + yield* fetching.clear(); - const record = found.value; - const url = - record.spec.exposure === "public" - ? workerUrl(projectRef, settings.projectHost, name) - : undefined; - // Reported only when an entry or the directory establishes it. With neither, - // the path is an inference about a worker that may have been deployed from - // another checkout. - // - // `sourceResolved` matters for the entry half: when the configured `source` - // could not be resolved, `sourceDir` is the *default* directory standing in - // for it, and printing that would name a path the entry does not. - const sourceDisplay = - (worker.entry !== undefined && worker.sourceResolved) || worker.sourceExists - ? displayPath(project.projectRoot, worker.sourceDir) - : undefined; + if (Option.isNone(found)) { + return yield* Effect.fail( + new WorkerNotDeployedError({ + detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, + suggestion: `Deploy it with \`supabase experimental workers push ${name}${refSuffix}\`.`, + }), + ); + } - const payload = { - worker_name: name, - project_ref: projectRef, - runtime: record.spec.runtime ?? "dockerfile", - size: record.spec.size, - exposure: record.spec.exposure, - build_state: record.buildState, - state_reason: record.stateReason, - image_version: record.imageVersion, - deleting: record.deleting, - declared_instances: record.spec.instances, - instances: record.instances, - instances_error: record.instancesError, - ...(sourceDisplay === undefined ? {} : { source: sourceDisplay }), - ...(url === undefined ? {} : { url }), - }; + const record = found.value; + const url = + record.spec.exposure === "public" + ? workerUrl(projectRef, settings.projectHost, name) + : undefined; + // Reported only when an entry or the directory establishes it. With neither, + // the path is an inference about a worker that may have been deployed from + // another checkout. + // + // `sourceResolved` matters for the entry half: when the configured `source` + // could not be resolved, `sourceDir` is the *default* directory standing in + // for it, and printing that would name a path the entry does not. + const sourceDisplay = + (worker.entry !== undefined && worker.sourceResolved) || worker.sourceExists + ? displayPath(project.projectRoot, worker.sourceDir) + : undefined; - // `-o` asks for a machine-readable stdout, so nothing human may be written - // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } + const payload = { + worker_name: name, + project_ref: projectRef, + runtime: record.spec.runtime ?? "dockerfile", + size: record.spec.size, + exposure: record.spec.exposure, + build_state: record.buildState, + state_reason: record.stateReason, + image_version: record.imageVersion, + deleting: record.deleting, + declared_instances: record.spec.instances, + instances: record.instances, + instances_error: record.instancesError, + ...(sourceDisplay === undefined ? {} : { source: sourceDisplay }), + ...(url === undefined ? {} : { url }), + }; - // One structured emission, in the structured branch only. Calling - // `output.success` before this check emitted the payload twice: the JSON - // layer appends each success to stdout, so `JSON.parse` failed, and - // `stream-json` saw two terminal result events. - if (output.format !== "text") { - yield* output.success("", payload); - return; - } + // `-o` asks for a machine-readable stdout, so nothing human may be written + // to it — `output.success` logs to stdout in text mode. + if (yield* legacyEmitWorkersMachineOutput(payload)) { + return; + } - const details: Array = [ - ["State", record.deleting === true ? "deleting" : record.buildState], - ["Reason", record.stateReason ?? ""], - ["Runtime", record.spec.runtime ?? "dockerfile"], - ["Size", formatApiSize(record.spec.size)], - ["Image", record.imageVersion ?? ""], - ["Access", record.spec.exposure], - [ - // Every number in the tally line comes from the tally: mixing - // `instances.ready` with `spec.instances` compares a snapshot against - // the desired count, which mid-scale renders fractions like `3/1 ready`. - "Instances", - record.instances !== undefined - ? `${record.instances.ready}/${record.instances.declared} ready, ${record.instances.live} live, ${record.instances.stale} stale` - : `${record.spec.instances} declared`, - ], - ["URL", url ?? ""], - ["Project", projectRef], - // `legacyRenderWorkerDetails` drops empty-valued rows, so an unknown - // source omits the row rather than printing a guess. - ["Source", sourceDisplay ?? ""], - ]; + // One structured emission, in the structured branch only. Calling + // `output.success` before this check emitted the payload twice: the JSON + // layer appends each success to stdout, so `JSON.parse` failed, and + // `stream-json` saw two terminal result events. + if (output.format !== "text") { + yield* output.success("", payload); + return; + } - yield* output.raw(legacyRenderWorkerDetails(details)); + const details: Array = [ + ["State", record.deleting === true ? "deleting" : record.buildState], + ["Reason", record.stateReason ?? ""], + ["Runtime", record.spec.runtime ?? "dockerfile"], + ["Size", formatApiSize(record.spec.size)], + ["Image", record.imageVersion ?? ""], + ["Access", record.spec.exposure], + [ + // Every number in the tally line comes from the tally: mixing + // `instances.ready` with `spec.instances` compares a snapshot against + // the desired count, which mid-scale renders fractions like `3/1 ready`. + "Instances", + record.instances !== undefined + ? `${record.instances.ready}/${record.instances.declared} ready, ${record.instances.live} live, ${record.instances.stale} stale` + : `${record.spec.instances} declared`, + ], + ["URL", url ?? ""], + ["Project", projectRef], + // `legacyRenderWorkerDetails` drops empty-valued rows, so an unknown + // source omits the row rather than printing a guess. + ["Source", sourceDisplay ?? ""], + ]; - if (record.instances === undefined && record.instancesError !== undefined) { - yield* output.raw(`Instance counts could not be read: ${record.instancesError}\n`, "stderr"); - } - // 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) { - // 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( - Effect.ensuring(linkedProjectCache.cache(projectRef)), - Effect.ensuring(telemetryState.flush), + yield* output.raw(legacyRenderWorkerDetails(details)); + + if (record.instances === undefined && record.instancesError !== undefined) { + yield* output.raw( + `Instance counts could not be read: ${record.instancesError}\n`, + "stderr", + ); + } + // 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) { + // 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`, + ); + } + }), ); }); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts new file mode 100644 index 0000000000..90f80b8064 --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts @@ -0,0 +1,82 @@ +import { rmSync } from "node:fs"; +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Exit, Option } from "effect"; +import { + makeWorkersProject, + setupLegacyWorkers, +} from "../../../../../tests/helpers/legacy-workers.ts"; +import { legacyWorkersDelete } from "./delete/delete.handler.ts"; +import { legacyWorkersList } from "./list/list.handler.ts"; +import { legacyWorkersLogs } from "./logs/logs.handler.ts"; +import { legacyWorkersPush } from "./push/push.handler.ts"; +import { legacyWorkersStatus } from "./status/status.handler.ts"; + +const CONFIG = 'project_id = "demo"\n\n[workers.api]\nruntime = "node"\n'; + +function project() { + const created = makeWorkersProject({ + "supabase/config.toml": CONFIG, + "supabase/workers/api/index.js": "export default {};\n", + }); + return { dir: created.dir, cleanup: () => rmSync(created.dir, { recursive: true, force: true }) }; +} + +/** + * Every project-scoped command, with only the flags the run needs to get as far + * as resolving a ref. + */ +const COMMANDS = [ + ["list", () => legacyWorkersList({ projectRef: Option.none() })], + ["status", () => legacyWorkersStatus({ name: "api", projectRef: Option.none() })], + ["delete", () => legacyWorkersDelete({ name: "api", projectRef: Option.none() })], + [ + "push", + () => + legacyWorkersPush({ + names: ["api"], + instances: Option.none(), + wait: false, + projectRef: Option.none(), + }), + ], + [ + "logs", + () => + legacyWorkersLogs({ + name: "api", + projectRef: Option.none(), + kind: Option.none(), + follow: false, + tail: 100, + }), + ], +] as const; + +/** + * The ordering `legacyWorkersRun` exists to hold. + * + * The command has run by the time an unlinked checkout fails inside `resolve`, + * so its post-run event still has to be written. Four of these resolved the ref + * above the finalizer and wrote nothing. + */ +describe("legacyWorkersRun", () => { + for (const [name, run] of COMMANDS) { + it.live(`flushes telemetry when ${name} cannot resolve a project ref`, () => { + const repo = project(); + const { layer, telemetry, http } = setupLegacyWorkers({ + workdir: repo.dir, + linked: false, + routes: {}, + }); + + return Effect.gen(function* () { + const exit = yield* (run() as Effect.Effect).pipe(Effect.exit); + + expect(Exit.isFailure(exit)).toBe(true); + expect(telemetry.flushed).toBe(true); + // Nothing was spent before the ref failed to resolve. + expect(http.requests).toHaveLength(0); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(repo.cleanup))); + }); + } +}); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts new file mode 100644 index 0000000000..7b761b63a3 --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts @@ -0,0 +1,42 @@ +import { Effect, type Option } from "effect"; +import { LegacyProjectRefResolver } from "../../../config/legacy-project-ref.service.ts"; +import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-project-cache.service.ts"; +import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; +import { legacyWorkersProjectRefSuffix } from "./workers.output.ts"; + +/** What every project-scoped workers command needs before it can do anything. */ +export interface LegacyWorkersRunContext { + readonly projectRef: string; + /** The `--project-ref` a suggestion must carry, or `""` — see the helper. */ + readonly refSuffix: string; +} + +/** + * The lifecycle every project-scoped workers command shares. + * + * Telemetry wraps the ref resolution as well, because an unlinked + * non-interactive checkout fails inside `resolve` and the command has run by + * then. The linked-project cache stays under the ref, having nothing to write + * without one. + * + * A helper rather than a per-command preamble because the ordering is the whole + * point and is easy to get subtly wrong: four commands had the resolution above + * both finalizers and silently wrote no post-run event. + */ +export const legacyWorkersRun = ( + projectRefFlag: Option.Option, + body: (context: LegacyWorkersRunContext) => Effect.Effect, +) => + Effect.gen(function* () { + const resolver = yield* LegacyProjectRefResolver; + const linkedProjectCache = yield* LegacyLinkedProjectCache; + const telemetryState = yield* LegacyTelemetryState; + + return yield* Effect.gen(function* () { + const projectRef = yield* resolver.resolve(projectRefFlag); + return yield* body({ + projectRef, + refSuffix: legacyWorkersProjectRefSuffix(projectRefFlag), + }).pipe(Effect.ensuring(linkedProjectCache.cache(projectRef))); + }).pipe(Effect.ensuring(telemetryState.flush)); + }); From f534abd7a03ae3c1311e78985ba6fec008913e44 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 21:58:25 -0300 Subject: [PATCH 2/7] refactor(workers): emit every payload through one helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The machine-output check, the structured emission and the fall-through to text were written out in all five handlers. Four of them branched on `output.format` alone, which ignores `-o`'s priority over `--output-format` — so `-o pretty --output-format json` emitted JSON from a run that had asked for text. `legacyEmitWorkersPayload` makes the decision once, keeping the existing "returns whether it emitted" contract so callers still skip their text rendering the same way. The precedence is pinned in one place rather than once per command. --- .../workers/delete/delete.handler.ts | 9 +-- .../experimental/workers/list/list.handler.ts | 9 +-- .../experimental/workers/logs/logs.handler.ts | 17 ++--- .../experimental/workers/push/push.handler.ts | 8 +-- .../workers/status/status.handler.ts | 13 +--- .../workers.output.integration.test.ts | 65 ++++++++++++++++++- .../experimental/workers/workers.output.ts | 29 +++++++++ 7 files changed, 105 insertions(+), 45 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts index ab5b92ce7e..489097d437 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 @@ -4,7 +4,7 @@ import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { - legacyEmitWorkersMachineOutput, + legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput, legacyWorkersMachineOutputRequested, } from "../workers.output.ts"; @@ -184,12 +184,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete // `-o` asks for a machine-readable stdout, so nothing human may be written // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - if (output.format !== "text") { - yield* output.success("", payload); + if (yield* legacyEmitWorkersPayload(payload)) { return; } 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 a81390a73f..e0ca2fc0f9 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 @@ -3,7 +3,7 @@ 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 { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { LegacyCliSettings } from "../../../../config/legacy-cli-settings.service.ts"; import { formatApiSize } from "../../../../../shared/workers/worker-runtimes.ts"; @@ -161,12 +161,7 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f // `-o` is independent of `--output-format`: it leaves `output.format` as // `text`, so this has to be checked before the text branch below, not // inside the structured one. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - if (output.format !== "text") { - yield* output.success("", payload); + if (yield* legacyEmitWorkersPayload(payload)) { return; } diff --git a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts index 0e576f60e5..de6ddb00b0 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts @@ -2,7 +2,7 @@ import { Effect, Option, Ref, Schedule } 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 { legacyEmitWorkersMachineOutput, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; +import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { legacyRenderWorkerLogLine, legacyWorkerLogLevel, @@ -287,18 +287,9 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f logs: entries.map(toPayloadEntry), }; - // `-o` asks for a machine-readable stdout, so nothing human may be written to - // it — `output.success` logs to stdout in text mode. Unreachable while - // following, which refuses these formats up front. - if (!flags.follow && (yield* legacyEmitWorkersMachineOutput(payload))) { - return; - } - - // One structured emission, in the structured branch only, and only for a - // bounded read. A tail has no terminal payload to put here — it emits a - // `log-entry` event per line through `emitLines` instead. - if (!flags.follow && renderFormat !== "text") { - yield* output.success("", payload); + // Only for a bounded read: a tail has no terminal payload to put here, and + // emits a `log-entry` event per line through `emitLines` instead. + if (!flags.follow && (yield* legacyEmitWorkersPayload(payload))) { return; } 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 2f09609886..2bedeb4fee 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 @@ -4,7 +4,7 @@ import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { - legacyEmitWorkersMachineOutput, + legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput, legacyWorkersMachineOutputRequested, } from "../workers.output.ts"; @@ -555,13 +555,9 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f // `-o` asks for a machine-readable stdout, so nothing human may be written // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { + if (yield* legacyEmitWorkersPayload(payload)) { return; } - - if (output.format !== "text") { - yield* output.success("", payload); - } }), ); }); 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 dbfe0ab265..839f4ff46c 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 @@ -3,7 +3,7 @@ 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, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; +import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { LegacyCliSettings } from "../../../../config/legacy-cli-settings.service.ts"; import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; @@ -94,16 +94,7 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status // `-o` asks for a machine-readable stdout, so nothing human may be written // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - // One structured emission, in the structured branch only. Calling - // `output.success` before this check emitted the payload twice: the JSON - // layer appends each success to stdout, so `JSON.parse` failed, and - // `stream-json` saw two terminal result events. - if (output.format !== "text") { - yield* output.success("", payload); + if (yield* legacyEmitWorkersPayload(payload)) { return; } 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 index bd56fba7b2..c48a2f13c3 100644 --- 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 @@ -6,7 +6,7 @@ import { makeWorkersProject, setupLegacyWorkers, } from "../../../../../tests/helpers/legacy-workers.ts"; -import { legacyEmitWorkersMachineOutput } from "./workers.output.ts"; +import { legacyEmitWorkersMachineOutput, legacyEmitWorkersPayload } from "./workers.output.ts"; /** * Every workers command refuses `-o env` up front, before it touches the @@ -38,3 +38,66 @@ describe("legacyEmitWorkersMachineOutput", () => { ); }); }); + +/** + * The one place the two format flags are reconciled, so the precedence between + * them is pinned here rather than once per command. + */ +describe("legacyEmitWorkersPayload", () => { + const PAYLOAD = { project_ref: "demo", workers: [] }; + + function setup(options: Parameters[0]) { + const created = makeWorkersProject({ "supabase/config.toml": `project_id = "demo"\n` }); + const it = setupLegacyWorkers({ ...options, workdir: created.dir, routes: {} }); + return { + ...it, + cleanup: () => rmSync(created.dir, { recursive: true, force: true }), + }; + } + + const structured = (out: ReturnType["out"]) => + out.messages.filter((message) => message.type === "success"); + + it.live("hands a plain text run back to its caller", () => { + const { layer, out, cleanup } = setup({ workdir: "" }); + + return Effect.gen(function* () { + expect(yield* legacyEmitWorkersPayload(PAYLOAD)).toBe(false); + expect(structured(out)).toHaveLength(0); + expect(out.stdoutText).toBe(""); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(cleanup))); + }); + + it.live("emits one structured result for --output-format json", () => { + const { layer, out, cleanup } = setup({ workdir: "", format: "json" }); + + return Effect.gen(function* () { + expect(yield* legacyEmitWorkersPayload(PAYLOAD)).toBe(true); + expect(structured(out)).toHaveLength(1); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(cleanup))); + }); + + it.live("writes the encoded payload and nothing else for -o json", () => { + const { layer, out, cleanup } = setup({ workdir: "", goOutput: "json" }); + + return Effect.gen(function* () { + expect(yield* legacyEmitWorkersPayload(PAYLOAD)).toBe(true); + // Not `output.success`, which would put human text on the same stdout. + expect(structured(out)).toHaveLength(0); + expect(JSON.parse(out.stdoutText)).toMatchObject({ project_ref: "demo" }); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(cleanup))); + }); + + // `-o` outranks `--output-format`, and `-o pretty` encodes nothing — so this + // pair asks for the text rendering. Branching on `output.format` alone emitted + // JSON instead, which is the opposite of what was asked for. + it.live("lets -o pretty override --output-format json", () => { + const { layer, out, cleanup } = setup({ workdir: "", goOutput: "pretty", format: "json" }); + + return Effect.gen(function* () { + expect(yield* legacyEmitWorkersPayload(PAYLOAD)).toBe(false); + expect(structured(out)).toHaveLength(0); + expect(out.stdoutText).toBe(""); + }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(cleanup))); + }); +}); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts index 9e5a25cffe..36c76c4215 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts @@ -117,6 +117,35 @@ export const legacyRejectWorkersEnvOutput = Effect.fnUntraced(function* () { } }); +/** + * Emit `payload` in whichever machine format the run asked for. + * + * Returns whether it emitted, so a caller can skip its text rendering — the + * same contract as {@link legacyEmitWorkersMachineOutput}, which this wraps. + * + * The two format flags are resolved here rather than at each call site. + * Branching on `output.format` alone ignored `-o`'s priority, so + * `-o pretty --output-format json` emitted JSON from a run that asked for text. + * + * The structured emission happens exactly once, and only in the structured + * branch: calling `output.success` ahead of the machine check emitted the + * payload twice, which broke `JSON.parse` and gave `stream-json` two terminal + * result events. + */ +export const legacyEmitWorkersPayload = Effect.fnUntraced(function* ( + payload: Record, +) { + if (yield* legacyEmitWorkersMachineOutput(payload)) { + return true; + } + if ((yield* legacyWorkersRenderFormat()) === "text") { + return false; + } + const output = yield* Output; + yield* output.success("", payload); + return true; +}); + /** * The `--project-ref` a retry suggestion has to carry, or `""` when the ref came * from the link. From d57f81a1f9ec7210369504baf93b75eb59c6526b Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 22:01:06 -0300 Subject: [PATCH 3/7] refactor(workers): build the family's command strings in one place `supabase experimental workers` was a literal in roughly thirty call sites. The move under `experimental` had to rewrite every one, and two follow-up commits exist because some were missed or left pointing at a command that no longer existed. `legacyWorkersCommand` owns the path, with `legacyWorkersPushCommand` and `legacyWorkersStatusCommand` for the two suggestions that recur. A unit test pins the exact spelling, since these strings are copy-pasted out of a terminal. --- .../workers/delete/delete.command.ts | 5 ++-- .../workers/delete/delete.handler.ts | 9 +++--- .../experimental/workers/list/list.command.ts | 3 +- .../experimental/workers/list/list.handler.ts | 3 +- .../experimental/workers/logs/logs.command.ts | 9 +++--- .../experimental/workers/logs/logs.handler.ts | 5 ++-- .../experimental/workers/new/new.command.ts | 9 +++--- .../experimental/workers/push/push.command.ts | 9 +++--- .../experimental/workers/push/push.handler.ts | 13 +++++--- .../workers/status/status.command.ts | 3 +- .../workers/status/status.handler.ts | 5 ++-- .../experimental/workers/workers.commands.ts | 27 +++++++++++++++++ .../workers/workers.commands.unit.test.ts | 30 +++++++++++++++++++ 13 files changed, 101 insertions(+), 29 deletions(-) create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts create mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.command.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.command.ts index 455b8c81a0..47242e5b8e 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.command.ts @@ -1,5 +1,6 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { withLegacyCommandInstrumentation } from "../../../../telemetry/legacy-command-instrumentation.ts"; @@ -25,11 +26,11 @@ export const legacyWorkersDeleteCommand = Command.make("delete", config).pipe( Command.withShortDescription("Delete a worker from Supabase"), Command.withExamples([ { - command: "supabase experimental workers delete api", + command: legacyWorkersCommand("delete api"), description: "Delete a worker, confirming by typing its name", }, { - command: "supabase experimental workers delete api --yes", + command: legacyWorkersCommand("delete api --yes"), description: "Skip the confirmation prompt (scripts and CI)", }, ]), 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 489097d437..68e5bb2dbe 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,6 +1,7 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; +import { legacyWorkersCommand, legacyWorkersPushCommand } from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { @@ -102,7 +103,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete // `status`'s wording, inherited, pointed the wrong way here: somebody // deleting "api" and hearing "nothing is deployed" does not want to // deploy it — they want to see what *is* deployed. - suggestion: `See what is deployed with \`supabase experimental workers list${refSuffix}\`.`, + suggestion: `See what is deployed with \`${legacyWorkersCommand(`list${refSuffix}`)}\`.`, }), ); } @@ -122,7 +123,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete return yield* Effect.fail( new WorkerDeleteConfirmationRequiredError({ detail: `Deleting "${name}" from project ${projectRef} needs confirmation, and there is no interactive terminal to ask on.`, - suggestion: `Re-run \`supabase experimental workers delete ${name} --yes${refSuffix}\` to confirm without a prompt.`, + suggestion: `Re-run \`${legacyWorkersCommand(`delete ${name} --yes${refSuffix}`)}\` to confirm without a prompt.`, }), ); } @@ -153,7 +154,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete return yield* Effect.fail( new WorkerDeleteNotConfirmedError({ detail: `The confirmation did not match "${name}", so nothing was deleted.`, - suggestion: `Re-run \`supabase experimental workers delete ${name}${refSuffix}\` and type the name exactly, or pass --yes.`, + suggestion: `Re-run \`${legacyWorkersCommand(`delete ${name}${refSuffix}`)}\` and type the name exactly, or pass --yes.`, }), ); } @@ -216,7 +217,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete if (keptSource !== undefined) { // 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`, + `Redeploy it with ${legacyAqua(legacyWorkersPushCommand(name, refSuffix))}.\n`, ); } } else { diff --git a/apps/cli/src/legacy/commands/experimental/workers/list/list.command.ts b/apps/cli/src/legacy/commands/experimental/workers/list/list.command.ts index 0efc9c1008..39325fdb00 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/list/list.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/list/list.command.ts @@ -1,5 +1,6 @@ import { Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { withLegacyCommandInstrumentation } from "../../../../telemetry/legacy-command-instrumentation.ts"; @@ -21,7 +22,7 @@ export const legacyWorkersListCommand = Command.make("list", config).pipe( Command.withShortDescription("List this project's workers"), Command.withExamples([ { - command: "supabase experimental workers list", + command: legacyWorkersCommand("list"), description: "See every worker in the linked project", }, ]), 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 e0ca2fc0f9..e103d765d1 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 @@ -10,6 +10,7 @@ import { formatApiSize } from "../../../../../shared/workers/worker-runtimes.ts" import { workerUrl } from "../../../../../shared/workers/worker-url.ts"; import { listWorkers, type WorkerRecord } from "../../../../../shared/workers/workers-api.ts"; import { legacyDiscoverWorkerNames, legacyLoadWorkersProject } from "../workers.shared.ts"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { legacyWorkersRun } from "../workers.run.ts"; import type { LegacyWorkersListFlags } from "./list.command.ts"; @@ -167,7 +168,7 @@ export const legacyWorkersList = Effect.fn("legacy.experimental.workers.list")(f if (rows.length === 0) { yield* output.raw( - `No workers found. Scaffold one with ${legacyAqua("supabase experimental workers new ", process.stdout)}.\n`, + `No workers found. Scaffold one with ${legacyAqua(legacyWorkersCommand("new "), process.stdout)}.\n`, ); return; } diff --git a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.command.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.command.ts index d6f98556a2..485aec3ff0 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.command.ts @@ -1,5 +1,6 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { @@ -69,19 +70,19 @@ export const legacyWorkersLogsCommand = Command.make("logs", config).pipe( Command.withShortDescription("Show a worker's logs"), Command.withExamples([ { - command: "supabase experimental workers logs api", + command: legacyWorkersCommand("logs api"), description: "Print the last 100 log lines across all streams", }, { - command: "supabase experimental workers logs api --kind requests --tail 20", + command: legacyWorkersCommand("logs api --kind requests --tail 20"), description: "Print the 20 most recent HTTP requests the worker served", }, { - command: "supabase experimental workers logs api --follow", + command: legacyWorkersCommand("logs api --follow"), description: "Print recent logs, then keep printing new lines until interrupted", }, { - command: "supabase experimental workers logs api --tail 0 --follow", + command: legacyWorkersCommand("logs api --tail 0 --follow"), description: "Skip the backlog and print only lines that arrive from now on", }, ]), diff --git a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts index de6ddb00b0..3b22a6bcc1 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts @@ -1,6 +1,7 @@ import { Effect, Option, Ref, Schedule } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; +import { legacyWorkersPushCommand, legacyWorkersStatusCommand } from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { @@ -274,7 +275,7 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f return yield* Effect.fail( new WorkerNotDeployedError({ detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, - suggestion: `Deploy it with \`supabase experimental workers push ${name}${refSuffix}\`.`, + suggestion: `Deploy it with \`${legacyWorkersPushCommand(name, refSuffix)}\`.`, }), ); } @@ -297,7 +298,7 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f // Deployed (the check above would have failed otherwise) and silent. yield* output.raw(`No logs for "${name}" in the last 24 hours.\n`); yield* emitSuccessTrailer( - `Check it is running with ${legacyAqua(`supabase experimental workers status ${name}${refSuffix}`)}.\n`, + `Check it is running with ${legacyAqua(legacyWorkersStatusCommand(name, refSuffix))}.\n`, ); return; } diff --git a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts index ce93798dd9..b28a10d6a1 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts @@ -1,6 +1,7 @@ import { Layer } from "effect"; import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../../../shared/runtime/command-runtime.layer.ts"; import { WORKER_RUNTIMES, WORKER_SIZES } from "../../../../../shared/workers/worker-runtimes.ts"; @@ -55,19 +56,19 @@ export const legacyWorkersNewCommand = Command.make("new", config).pipe( Command.withShortDescription("Scaffold a worker locally"), Command.withExamples([ { - command: "supabase experimental workers new", + command: legacyWorkersCommand("new"), description: "Prompt for the name, then for runtime and size", }, { - command: "supabase experimental workers new api", + command: legacyWorkersCommand("new api"), description: "Scaffold supabase/workers/api, prompting for runtime and size", }, { - command: "supabase experimental workers new api --runtime node", + command: legacyWorkersCommand("new api --runtime node"), description: "Scaffold supabase/workers/api on the node runtime", }, { - command: "supabase experimental workers new api --source packages/api", + command: legacyWorkersCommand("new api --source packages/api"), description: "Scaffold the worker outside the workers directory", }, ]), diff --git a/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts b/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts index 6f9261c2ff..f46f8b8a53 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/push/push.command.ts @@ -1,5 +1,6 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { withLegacyCommandInstrumentation } from "../../../../telemetry/legacy-command-instrumentation.ts"; @@ -51,19 +52,19 @@ export const legacyWorkersPushCommand = Command.make("push", config).pipe( Command.withShortDescription("Build and deploy workers"), Command.withExamples([ { - command: "supabase experimental workers push", + command: legacyWorkersCommand("push"), description: "Deploy every worker in the project", }, { - command: "supabase experimental workers push api", + command: legacyWorkersCommand("push api"), description: "Deploy a single worker", }, { - command: "supabase experimental workers push api web", + command: legacyWorkersCommand("push api web"), description: "Deploy several workers by name", }, { - command: "supabase experimental workers push api --wait", + command: legacyWorkersCommand("push api --wait"), description: "Deploy and block until the build succeeds or fails", }, ]), 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 2bedeb4fee..98d3b76289 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 @@ -8,6 +8,11 @@ import { legacyRejectWorkersEnvOutput, legacyWorkersMachineOutputRequested, } from "../workers.output.ts"; +import { + legacyWorkersCommand, + legacyWorkersPushCommand, + legacyWorkersStatusCommand, +} from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { LegacyCliSettings } from "../../../../config/legacy-cli-settings.service.ts"; @@ -156,7 +161,7 @@ function missingSourceSuggestion(input: { readonly entry: WorkerEntry | undefined; }): string { if (input.entry === undefined) { - return `Scaffold it with \`supabase experimental workers new ${input.name}\`.`; + return `Scaffold it with \`${legacyWorkersCommand(`new ${input.name}`)}\`.`; } if (input.entry.source !== undefined) { return `Create ${input.sourceDisplay}, or correct \`source\` under [workers.${input.name}] in ${input.configPath}.`; @@ -358,7 +363,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}${input.refSuffix}\`.`, + suggestion: `Fix the issue, then re-run \`${legacyWorkersPushCommand(name, input.refSuffix)}\`.`, }), ); } @@ -409,7 +414,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { // width and buried both commands mid-sentence. yield* emitSuccessTrailer( `\nYour build was submitted successfully.\n` + - `Run ${legacyAqua(`supabase experimental workers status ${name}${input.refSuffix}`)} to check on it.\n` + + `Run ${legacyAqua(legacyWorkersStatusCommand(name, input.refSuffix))} to check on it.\n` + `Add ${legacyAqua("--wait")} to block on the build next time.\n`, ); } @@ -491,7 +496,7 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f project.projectRoot, project.workersDir, )}.`, - suggestion: "Scaffold one with `supabase experimental workers new `.", + suggestion: `Scaffold one with \`${legacyWorkersCommand("new ")}\`.`, }), ); } diff --git a/apps/cli/src/legacy/commands/experimental/workers/status/status.command.ts b/apps/cli/src/legacy/commands/experimental/workers/status/status.command.ts index 78b02d0d86..9e29801abd 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/status/status.command.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/status/status.command.ts @@ -1,5 +1,6 @@ import { Argument, Command, Flag } from "effect/unstable/cli"; import type * as CliCommand from "effect/unstable/cli/Command"; +import { legacyWorkersCommand } from "../workers.commands.ts"; import { withJsonErrorHandling } from "../../../../../shared/output/json-error-handling.ts"; import { legacyManagementApiRuntimeLayer } from "../../../../shared/legacy-management-api-runtime.layer.ts"; import { withLegacyCommandInstrumentation } from "../../../../telemetry/legacy-command-instrumentation.ts"; @@ -22,7 +23,7 @@ export const legacyWorkersStatusCommand = Command.make("status", config).pipe( Command.withShortDescription("Show a worker in detail"), Command.withExamples([ { - command: "supabase experimental workers status api", + command: legacyWorkersCommand("status api"), description: "Inspect a specific worker", }, ]), 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 839f4ff46c..081e81486d 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,6 +1,7 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; +import { legacyWorkersPushCommand } from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; @@ -53,7 +54,7 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status return yield* Effect.fail( new WorkerNotDeployedError({ detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, - suggestion: `Deploy it with \`supabase experimental workers push ${name}${refSuffix}\`.`, + suggestion: `Deploy it with \`${legacyWorkersPushCommand(name, refSuffix)}\`.`, }), ); } @@ -135,7 +136,7 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status // 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`, + `Fix the issue, then re-run ${legacyAqua(legacyWorkersPushCommand(name, refSuffix))}.\n`, ); } }), diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts new file mode 100644 index 0000000000..f382331604 --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts @@ -0,0 +1,27 @@ +/** + * How this family is invoked, in one place. + * + * The path was a literal in roughly thirty call sites, so moving the family + * under `experimental` had to rewrite every one — and two follow-up commits + * exist because some were missed or left pointing at a command that no longer + * existed. Every user-facing string that names one of these commands builds it + * from here. + */ +const PATH = "supabase experimental workers"; + +/** One of this family's commands, spelled the way a user would type it. */ +export const legacyWorkersCommand = (rest: string) => `${PATH} ${rest}`; + +/** + * The `push` that deploys or redeploys `name`. + * + * `refSuffix` is the caller's `--project-ref` when the flag supplied one — a + * suggestion is copy-pasted verbatim, so one that drops it re-resolves against + * whatever this checkout is linked to. + */ +export const legacyWorkersPushCommand = (name: string, refSuffix = "") => + legacyWorkersCommand(`push ${name}${refSuffix}`); + +/** The `status` that reports on `name`. */ +export const legacyWorkersStatusCommand = (name: string, refSuffix = "") => + legacyWorkersCommand(`status ${name}${refSuffix}`); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts new file mode 100644 index 0000000000..095c645d1f --- /dev/null +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { + legacyWorkersCommand, + legacyWorkersPushCommand, + legacyWorkersStatusCommand, +} from "./workers.commands.ts"; + +/** + * These strings are copy-pasted out of the terminal, so the exact spelling is + * the contract. Pinned here so moving the family again is one deliberate edit + * rather than a sweep that misses call sites — which is how the last move went. + */ +describe("legacyWorkersCommand", () => { + it("prefixes the family path", () => { + expect(legacyWorkersCommand("list")).toBe("supabase experimental workers list"); + }); + + it("carries an explicit --project-ref into a push suggestion", () => { + expect(legacyWorkersPushCommand("api", " --project-ref demo")).toBe( + "supabase experimental workers push api --project-ref demo", + ); + }); + + // The suffix is empty when the ref came from the link, where repeating it is + // noise on a command that already resolves correctly. + it("omits the ref when there is none to carry", () => { + expect(legacyWorkersPushCommand("api")).toBe("supabase experimental workers push api"); + expect(legacyWorkersStatusCommand("api")).toBe("supabase experimental workers status api"); + }); +}); From cdb42bfa69a51075985cf65dd24222801d3c6552 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 22:02:06 -0300 Subject: [PATCH 4/7] refactor(workers): build the not-deployed error once Three commands raised `WorkerNotDeployedError` with a byte-identical detail and their own suggestion. `legacyWorkerNotDeployed` owns the sentence that is the same everywhere and takes the way out that is not: `status` and `logs` point at `push`, while `delete` deliberately points at `list`. --- .../workers/delete/delete.handler.ts | 15 +++++++------- .../experimental/workers/logs/logs.handler.ts | 12 +++++++---- .../workers/status/status.handler.ts | 8 ++++---- .../experimental/workers/workers.commands.ts | 20 +++++++++++++++++++ 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts index 68e5bb2dbe..5e0653285c 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,7 +1,11 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; -import { legacyWorkersCommand, legacyWorkersPushCommand } from "../workers.commands.ts"; +import { + legacyWorkerNotDeployed, + legacyWorkersCommand, + legacyWorkersPushCommand, +} from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { @@ -15,7 +19,6 @@ import { deleteWorker, getWorker } from "../../../../../shared/workers/workers-a import { WorkerDeleteConfirmationRequiredError, WorkerDeleteNotConfirmedError, - WorkerNotDeployedError, WorkersApiUnexpectedStatusError, } from "../../../../../shared/workers/workers.errors.ts"; import { legacyResolveYes } from "../../../../../shared/legacy/global-flags.ts"; @@ -98,11 +101,9 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete // and wants to hear the worker was not there. if (lookup.readable && deployed === undefined && !yes) { return yield* Effect.fail( - new WorkerNotDeployedError({ - detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, - // `status`'s wording, inherited, pointed the wrong way here: somebody - // deleting "api" and hearing "nothing is deployed" does not want to - // deploy it — they want to see what *is* deployed. + legacyWorkerNotDeployed({ + name, + projectRef, suggestion: `See what is deployed with \`${legacyWorkersCommand(`list${refSuffix}`)}\`.`, }), ); diff --git a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts index 3b22a6bcc1..fc1be564aa 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts @@ -1,7 +1,11 @@ import { Effect, Option, Ref, Schedule } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; -import { legacyWorkersPushCommand, legacyWorkersStatusCommand } from "../workers.commands.ts"; +import { + legacyWorkerNotDeployed, + legacyWorkersPushCommand, + legacyWorkersStatusCommand, +} from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; import { @@ -27,7 +31,6 @@ import { getWorker } from "../../../../../shared/workers/workers-api.ts"; import { WorkerLogsQueryFailedError, WorkerLogsRateLimitedError, - WorkerNotDeployedError, WorkersApiNetworkError, WorkersApiUnexpectedStatusError, } from "../../../../../shared/workers/workers.errors.ts"; @@ -273,8 +276,9 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f yield* checking.clear(); if (Option.isNone(deployed)) { return yield* Effect.fail( - new WorkerNotDeployedError({ - detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, + legacyWorkerNotDeployed({ + name, + projectRef, suggestion: `Deploy it with \`${legacyWorkersPushCommand(name, refSuffix)}\`.`, }), ); 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 081e81486d..ff6e28b3a2 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,7 +1,7 @@ import { Effect, Option } from "effect"; import { Output } from "../../../../../shared/output/output.service.ts"; import { emitSuccessTrailer } from "../../../../../shared/cli/success-trailer.ts"; -import { legacyWorkersPushCommand } from "../workers.commands.ts"; +import { legacyWorkerNotDeployed, legacyWorkersPushCommand } from "../workers.commands.ts"; import { legacyAqua } from "../../../../shared/legacy-colors.ts"; import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput } from "../workers.output.ts"; @@ -11,7 +11,6 @@ import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; import { formatApiSize } from "../../../../../shared/workers/worker-runtimes.ts"; import { workerUrl } from "../../../../../shared/workers/worker-url.ts"; import { getWorker } from "../../../../../shared/workers/workers-api.ts"; -import { WorkerNotDeployedError } from "../../../../../shared/workers/workers.errors.ts"; import { legacyDescribeWorkerForReporting, legacyLoadWorkersProjectForReporting, @@ -52,8 +51,9 @@ export const legacyWorkersStatus = Effect.fn("legacy.experimental.workers.status if (Option.isNone(found)) { return yield* Effect.fail( - new WorkerNotDeployedError({ - detail: `Nothing is deployed for "${name}" in project ${projectRef}.`, + legacyWorkerNotDeployed({ + name, + projectRef, suggestion: `Deploy it with \`${legacyWorkersPushCommand(name, refSuffix)}\`.`, }), ); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts index f382331604..7099eb4679 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts @@ -1,3 +1,5 @@ +import { WorkerNotDeployedError } from "../../../../shared/workers/workers.errors.ts"; + /** * How this family is invoked, in one place. * @@ -25,3 +27,21 @@ export const legacyWorkersPushCommand = (name: string, refSuffix = "") => /** The `status` that reports on `name`. */ export const legacyWorkersStatusCommand = (name: string, refSuffix = "") => legacyWorkersCommand(`status ${name}${refSuffix}`); + +/** + * "There is no such deployment", with the caller's own way out. + * + * The detail is the same wherever it is raised; the suggestion is not. `status` + * and `logs` point at `push`, but `delete` deliberately does not — somebody + * removing "api" and hearing "nothing is deployed" wants to see what *is* + * deployed, not to deploy it. + */ +export const legacyWorkerNotDeployed = (options: { + readonly name: string; + readonly projectRef: string; + readonly suggestion: string; +}) => + new WorkerNotDeployedError({ + detail: `Nothing is deployed for "${options.name}" in project ${options.projectRef}.`, + suggestion: options.suggestion, + }); From c89e62bb214569526f87f372cb18b856a658718c Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 22:04:28 -0300 Subject: [PATCH 5/7] refactor(workers): name the "renders human text" condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five more call sites — push's progress lines and per-worker output, delete's confirmation guard, new's prompt gate — each spelled out `output.format` plus `machineOutput` by hand, and each got the precedence wrong the same way: `-o pretty --output-format json` asks for text but was treated as a machine run. `legacyWorkersRendersText` answers it once. `push` threads that instead of `machineOutput`, and `new` now emits through the shared payload helper. --- .../workers/delete/delete.handler.ts | 6 +-- .../experimental/workers/new/new.handler.ts | 21 +++------- .../experimental/workers/push/push.handler.ts | 14 +++---- .../workers.output.integration.test.ts | 40 ++++++++++++++++++- .../experimental/workers/workers.output.ts | 15 +++++++ 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/delete/delete.handler.ts index 5e0653285c..a192f186df 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 @@ -11,7 +11,7 @@ import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput, - legacyWorkersMachineOutputRequested, + legacyWorkersRendersText, } from "../workers.output.ts"; import { LegacyPlatformApi } from "../../../../auth/legacy-platform-api.service.ts"; import { displayPath } from "../../../../../shared/workers/worker-paths.ts"; @@ -91,7 +91,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete yield* fetching.clear(); const deployed = lookup.worker; - const machineOutput = yield* legacyWorkersMachineOutputRequested(); + const rendersText = yield* legacyWorkersRendersText(); // `--yes` is the scripted path, and `deleteWorker` already treats a DELETE // 404 as done — "a delete that races another one is still a delete that @@ -120,7 +120,7 @@ export const legacyWorkersDelete = Effect.fn("legacy.experimental.workers.delete // into the prompt and delete without `--yes`. The confirmation is only // meaningful from a keyboard, so stdin has to be a terminal too — the same // pair `projects delete` guards its prompt with. - if (output.format !== "text" || machineOutput || !output.interactive || !tty.stdinIsTty) { + if (!rendersText || !output.interactive || !tty.stdinIsTty) { return yield* Effect.fail( new WorkerDeleteConfirmationRequiredError({ detail: `Deleting "${name}" from project ${projectRef} needs confirmation, and there is no interactive terminal to ask on.`, 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 79b195dc54..19b1d8ff21 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 @@ -4,10 +4,7 @@ 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, - legacyWorkersMachineOutputRequested, -} from "../workers.output.ts"; +import { legacyEmitWorkersPayload, legacyWorkersRendersText } 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"; @@ -74,10 +71,10 @@ function defaultFirst(values: ReadonlyArray, defaultValue: T): Array { * 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 canPromptForThisRun = Effect.fnUntraced(function* () { const output = yield* Output; const tty = yield* Tty; - return output.format === "text" && output.interactive && !machineOutput && tty.stdinIsTty; + return (yield* legacyWorkersRendersText()) && output.interactive && tty.stdinIsTty; }); /** @@ -206,8 +203,7 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun // 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 canPrompt = yield* canPromptForThisRun(); const name = yield* resolveName({ explicit: flags.name, canPrompt, project }); yield* legacyValidateWorkerName(name); @@ -314,14 +310,7 @@ export const legacyWorkersNew = Effect.fn("legacy.experimental.workers.new")(fun config_path: project.configPath, }; - // `-o` asks for a machine-readable stdout, so nothing human may be written - // to it — `output.success` logs to stdout in text mode. - if (yield* legacyEmitWorkersMachineOutput(payload)) { - return; - } - - if (output.format !== "text") { - yield* output.success("", payload); + if (yield* legacyEmitWorkersPayload(payload)) { return; } 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 98d3b76289..74e109c749 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,7 +6,7 @@ import { legacyRenderWorkerDetails } from "../workers.format.ts"; import { legacyEmitWorkersPayload, legacyRejectWorkersEnvOutput, - legacyWorkersMachineOutputRequested, + legacyWorkersRendersText, } from "../workers.output.ts"; import { legacyWorkersCommand, @@ -199,7 +199,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { readonly pollSchedule?: Schedule.Schedule; readonly pollRetrySchedule?: Schedule.Schedule; /** Suppresses this step's human output when `-o` owns stdout. */ - readonly machineOutput: boolean; + readonly rendersText: boolean; }) { const fs = yield* FileSystem.FileSystem; const output = yield* Output; @@ -377,7 +377,7 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { // Suppressed when `-o` is in play: the payload owns stdout, and these lines // would land in the middle of it. - if (output.format === "text" && !input.machineOutput) { + if (input.rendersText) { // Declarative line first, then the details — the shape every other command // that reports a completed remote change uses. `legacyRenderWorkerDetails` drops // empty-valued rows, so optional fields need no conditional spreads. @@ -508,10 +508,10 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f // that out at the end means failing with the remote project already changed. yield* legacyRejectWorkersEnvOutput(); - const machineOutput = yield* legacyWorkersMachineOutputRequested(); + const rendersText = yield* legacyWorkersRendersText(); const deployed: Array> = []; for (const [index, name] of names.entries()) { - if (names.length > 1 && !machineOutput && output.format === "text") { + if (names.length > 1 && rendersText) { // 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. @@ -537,7 +537,7 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f refSuffix, instances: flags.instances, wait: flags.wait, - machineOutput, + rendersText, ...(options.pollSchedule === undefined ? {} : { pollSchedule: options.pollSchedule }), ...(options.pollRetrySchedule === undefined ? {} @@ -548,7 +548,7 @@ export const legacyWorkersPush = Effect.fn("legacy.experimental.workers.push")(f // 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") { + if (names.length > 1 && rendersText) { yield* output.raw( `Deployed ${names.length} Workers to project ${projectRef}: ${names .map((name) => legacyAqua(name, process.stdout)) 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 index c48a2f13c3..8e80195f0a 100644 --- 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 @@ -6,7 +6,11 @@ import { makeWorkersProject, setupLegacyWorkers, } from "../../../../../tests/helpers/legacy-workers.ts"; -import { legacyEmitWorkersMachineOutput, legacyEmitWorkersPayload } from "./workers.output.ts"; +import { + legacyEmitWorkersMachineOutput, + legacyEmitWorkersPayload, + legacyWorkersRendersText, +} from "./workers.output.ts"; /** * Every workers command refuses `-o env` up front, before it touches the @@ -101,3 +105,37 @@ describe("legacyEmitWorkersPayload", () => { }).pipe(Effect.provide(layer), Effect.ensuring(Effect.sync(cleanup))); }); }); + +/** + * The predicate five call sites were spelling out by hand — progress lines, + * prompts, per-worker output. Neither flag answers it alone. + */ +describe("legacyWorkersRendersText", () => { + const CASES = [ + ["no flags", {}, true], + ["--output-format json", { format: "json" }, false], + ["--output-format stream-json", { format: "stream-json" }, false], + ["-o json", { goOutput: "json" }, false], + ["-o yaml", { goOutput: "yaml" }, false], + // These encode nothing and fall through to the text rendering. + ["-o pretty", { goOutput: "pretty" }, true], + ["-o csv", { goOutput: "csv" }, true], + // `-o` outranks `--output-format`, in both directions. + ["-o pretty with --output-format json", { goOutput: "pretty", format: "json" }, true], + ["-o json with --output-format text", { goOutput: "json" }, false], + ] as const; + + for (const [label, options, expected] of CASES) { + it.live(`is ${expected} for ${label}`, () => { + const created = makeWorkersProject({ "supabase/config.toml": `project_id = "demo"\n` }); + const { layer } = setupLegacyWorkers({ ...options, workdir: created.dir, routes: {} }); + + return Effect.gen(function* () { + expect(yield* legacyWorkersRendersText()).toBe(expected); + }).pipe( + Effect.provide(layer), + Effect.ensuring(Effect.sync(() => rmSync(created.dir, { recursive: true, force: true }))), + ); + }); + } +}); diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts index 36c76c4215..6061a24a8e 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts @@ -117,6 +117,21 @@ export const legacyRejectWorkersEnvOutput = Effect.fnUntraced(function* () { } }); +/** + * Whether this run renders human text on stdout. + * + * Neither flag answers this alone: `-o json|yaml|toml|env` leaves + * `output.format` as `text`, while `--output-format json` says nothing about + * `-o`. Every caller was spelling the pair out by hand, and three of them got + * the precedence wrong. + */ +export const legacyWorkersRendersText = Effect.fnUntraced(function* () { + if (yield* legacyWorkersMachineOutputRequested()) { + return false; + } + return (yield* legacyWorkersRenderFormat()) === "text"; +}); + /** * Emit `payload` in whichever machine format the run asked for. * From 0173b129d9d70bb4e92820ca8c9cd8026f54ee3e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 22:12:02 -0300 Subject: [PATCH 6/7] refactor(workers logs): make the follow loop one step at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pollOnce` was a 60-line closure doing three jobs, and carried two `Ref`s that were only ever read and written together — one cursor split in half, where advancing the timestamp without recording the ids replays the overlap. `FollowCursor` is that one value, and the paging walk moves to `drainSince`, so the poll body reads as its three steps: read the cursor, drain the window, emit what is new. Trims the comments here and in the helpers this pass added. Rationale that belongs to a change rather than to the code — what used to be wrong, and how many callers had it wrong — lives in the commit that made it, not in a docblock the next reader pays for. --- .../experimental/workers/logs/logs.handler.ts | 230 +++++++++--------- .../experimental/workers/workers.commands.ts | 26 +- .../workers/workers.commands.unit.test.ts | 30 --- .../workers.output.integration.test.ts | 9 +- .../experimental/workers/workers.output.ts | 40 +-- .../workers/workers.run.integration.test.ts | 30 +-- .../experimental/workers/workers.run.ts | 15 +- 7 files changed, 142 insertions(+), 238 deletions(-) delete mode 100644 apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts diff --git a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts index fc1be564aa..3cf8c3db89 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/logs/logs.handler.ts @@ -64,25 +64,10 @@ import type { LegacyWorkersLogsFlags } from "./logs.command.ts"; */ const SEEN_ID_LIMIT = 5000; -/** - * How many rows one poll asks for per request. - * - * Independent of `--tail`, which bounds only the history a run opens with. - * Sharing them meant `--tail 1 --follow` polled with `limit 1`: the query orders - * newest-first, so a burst came back as its newest row alone and the cursor then - * advanced past the rest, dropping them for good. The default `--tail 100` had - * the same hole above 100 rows in a polling interval. - */ +/** Rows per poll request. Independent of `--tail`, which bounds history only. */ const FOLLOW_PAGE_SIZE = 1000; -/** - * How many requests one poll may spend draining a burst. - * - * A bound rather than an open loop: the endpoint allows 10 requests a minute, so - * an unbounded drain could spend a whole window's allowance on one poll. Rows - * beyond it are not lost — the cursor only advances past what was emitted, so - * the next poll re-asks for them. - */ +/** Requests one poll may spend draining a burst, against a 10/minute budget. */ const FOLLOW_MAX_PAGES = 5; /** @@ -97,17 +82,12 @@ const FOLLOW_READ_RETRY = Schedule.spaced("5 seconds").pipe( ); /** - * Which poll failures are worth spending another request on. - * - * A tail should ride out a 429 or a momentary blip, but 401, 402 and 404 answer - * the same way every time. Retrying those held the error back for a minute and - * spent most of the endpoint's ten-requests-per-minute allowance getting nowhere, - * so the reader waited longer and then hit a rate limit on top of the real cause. + * Which poll failures are worth another request. * - * Server-side statuses are retried and client-side ones are not, with the - * exception of 408 and 429, which are the server asking for exactly that. A - * decode failure carries the response's own status, so a malformed 200 body is - * correctly read as terminal: it will not parse any better on a second attempt. + * Server-side statuses, plus 408 and 429 — the server asking for a retry. + * Definitive answers (401, 402, 404) surface immediately rather than burning a + * minute and most of the rate limit first. A decode failure carries the + * response's own status, so a malformed 200 body reads as terminal. */ function isRetryableFollowFailure(error: unknown): boolean { if (error instanceof WorkersApiUnexpectedStatusError) { @@ -122,6 +102,71 @@ function isRetryableFollowFailure(error: unknown): boolean { ); } +/** Where the tail has got to: the newest line printed, and the ids printed. */ +interface FollowCursor { + readonly newestMs: number; + readonly seen: ReadonlySet; +} + +/** + * Move the cursor past `fresh`. The timestamp and the id set are only correct + * together: advancing one without the other replays the overlap or loses it. + * + * The id set is bounded — only ids inside the grace window can be re-offered, + * so forgetting the oldest cannot resurrect them. + */ +function advanceCursor(cursor: FollowCursor, fresh: ReadonlyArray): FollowCursor { + const seen = new Set(cursor.seen); + for (const row of fresh) { + seen.add(row.id); + } + return { + newestMs: fresh.reduce((newest, row) => Math.max(newest, row.timestampMs), cursor.newestMs), + seen: seen.size <= SEEN_ID_LIMIT ? seen : new Set([...seen].slice(seen.size - SEEN_ID_LIMIT)), + }; +} + +/** + * Every row since `cursorMs`, across as many requests as it takes. + * + * The query orders newest-first, so one request answers with only the newest + * page. Walk `end` backwards while pages come back full; a short page means the + * window is drained. Bounded by the endpoint's ten-per-minute allowance — rows + * past the bound are not lost, since the cursor only advances over what was + * emitted. + */ +const drainSince = Effect.fnUntraced(function* (input: { + readonly api: LegacyPlatformApi["Service"]; + readonly projectRef: string; + readonly name: string; + readonly streams: ReadonlyArray; + readonly cursorMs: number; +}) { + const collected: Array = []; + let end = new Date(); + for (let page = 0; page < FOLLOW_MAX_PAGES; page += 1) { + const rows = yield* fetchWorkerLogs(input.api, input.projectRef, { + name: input.name, + streams: input.streams, + tail: FOLLOW_PAGE_SIZE, + window: followWindow(end, input.cursorMs), + }); + collected.push(...rows); + if (rows.length < FOLLOW_PAGE_SIZE) { + break; + } + // Rows arrive oldest-first, so the next page ends where this one began. A + // full page sharing one timestamp cannot narrow the window: stop rather than + // re-request it, and let the next poll's grace window cover the remainder. + const nextEnd = new Date(rows[0]!.timestampMs); + if (nextEnd.getTime() >= end.getTime()) { + break; + } + end = nextEnd; + } + return collected; +}); + /** * Test seams for the follow loop. * @@ -236,8 +281,7 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f ? [WORKER_LOG_STREAMS[flags.kind.value]] : ALL_WORKER_LOG_STREAMS; - // Before any request, so a slow history query or deployed-worker check - // cannot widen what `followFloorMs` below treats as "already there". + // Before any request, so a slow one cannot widen `followFloorMs` below. const startedAtMs = Date.now(); // `--tail 0` is "no history". On its own that is a no-op, but it is the shape @@ -257,18 +301,11 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f return rows; }); - // Nothing came back, which is two different situations wearing the same face: - // a worker that is not deployed at all, and one that is deployed and quiet. - // Only worth one extra request, and only in this branch. - // - // `--tail 0` makes no history query, so zero rows says nothing either way — - // but a tail still has to know the worker exists, or a typo waits forever on - // logs that can never arrive. A bounded `--tail 0` run prints nothing by - // definition and is left alone. + // Zero rows is two situations wearing one face: not deployed, or deployed + // and quiet. Worth one extra request to tell them apart. `--tail 0` queried + // nothing, so it only needs the check when it is going on to tail. if (entries.length === 0 && (flags.tail > 0 || flags.follow)) { - // Its own task: with `--tail 0` there is no "Fetching logs..." to inherit, - // and clearing that one before this request left text mode silent across - // a call that can take a moment. + // Its own task: `--tail 0` has no "Fetching logs..." to inherit. const checking = yield* output.task("Checking worker..."); const deployed = yield* getWorker(api, projectRef, name).pipe( Effect.tapError(() => checking.fail()), @@ -323,106 +360,57 @@ export const legacyWorkersLogs = Effect.fn("legacy.experimental.workers.logs")(f } // --- follow --------------------------------------------------------------- - // - // The cursor is the newest timestamp printed, and the set of ids already - // printed. Both live inside this generator rather than being captured while - // the Effect was built: an Effect is a reusable description and may run more - // than once, and shared cursor state across runs would drop lines. - const seenIds = yield* Ref.make(new Set(entries.map((entry) => entry.id))); - const newestSeenMs = yield* Ref.make( - entries.length === 0 ? Date.now() : entries[entries.length - 1]!.timestampMs, - ); + // Inside the generator, not captured while the Effect was built: an Effect + // may run more than once, and shared cursor state would drop lines. + const cursorRef = yield* Ref.make({ + newestMs: entries.at(-1)?.timestampMs ?? Date.now(), + seen: new Set(entries.map((entry) => entry.id)), + }); - // `--tail 0` asked for no history, and `followWindow` deliberately reaches - // a grace period behind the cursor so a line relayed late is still caught. - // Both are wanted, and together they let pre-invocation lines through — so - // keep the wide window and filter on when the line was actually written. + // `followWindow` reaches a grace period behind the cursor so a late relay + // is still caught — which for `--tail 0` would reopen the history it was + // told to skip. Keep the wide window; filter on when the line was written. const followFloorMs = flags.tail === 0 ? startedAtMs : Number.NEGATIVE_INFINITY; const pollOnce = Effect.gen(function* () { - const cursor = yield* Ref.get(newestSeenMs); - - // One request only ever answers with the newest page of its window, so a - // burst bigger than a page needs several. Walk `end` backwards while - // pages come back full; a short page means the window is drained. - const collected: Array = []; - let end = new Date(); - for (let page = 0; page < FOLLOW_MAX_PAGES; page += 1) { - const rows = yield* fetchWorkerLogs(api, projectRef, { - name, - streams, - tail: FOLLOW_PAGE_SIZE, - window: followWindow(end, cursor), - }); - collected.push(...rows); - if (rows.length < FOLLOW_PAGE_SIZE) { - break; - } - // Rows arrive oldest-first, so the next page ends where this one began. - const nextEnd = new Date(rows[0]!.timestampMs); - // A full page whose rows all share one timestamp cannot narrow the - // window. Stop rather than re-request it; the cursor has not advanced - // past those rows, so the next poll's grace window still covers them. - if (nextEnd.getTime() >= end.getTime()) { - break; - } - end = nextEnd; - } + const cursor = yield* Ref.get(cursorRef); + const rows = yield* drainSince({ + api, + projectRef, + name, + streams, + cursorMs: cursor.newestMs, + }); - // Windows always overlap - the server rounds them to the minute and the - // cursor deliberately lags - so dedupe is what makes the overlap invisible - // rather than a source of repeats. - const printed = yield* Ref.get(seenIds); - const fresh = collected - .filter((row) => !printed.has(row.id) && row.timestampMs >= followFloorMs) - // Each page is oldest-first but the pages themselves walk backwards, so - // the concatenation is not ordered until this runs. + // Windows always overlap — the server rounds them to the minute and the + // cursor deliberately lags — so the dedupe is what makes the overlap + // invisible rather than a source of repeats. Pages walk backwards, so + // the concatenation is not in order until this sorts it. + const fresh = rows + .filter((row) => !cursor.seen.has(row.id) && row.timestampMs >= followFloorMs) .sort((left, right) => left.timestampMs - right.timestampMs); if (fresh.length === 0) { return; } yield* emitLines(fresh, "live"); - yield* Ref.update(seenIds, (previous) => { - const next = new Set(previous); - for (const row of fresh) { - next.add(row.id); - } - // Bounded so a tail left running for hours does not grow it without - // limit. Only ids inside the grace window can still be re-offered, so - // forgetting the oldest cannot resurrect them. - if (next.size <= SEEN_ID_LIMIT) { - return next; - } - return new Set([...next].slice(next.size - SEEN_ID_LIMIT)); - }); - yield* Ref.set( - newestSeenMs, - fresh.reduce((newest, row) => Math.max(newest, row.timestampMs), cursor), - ); + yield* Ref.set(cursorRef, advanceCursor(cursor, fresh)); }); - // A 429 or a blip should not end a tail the user is watching; the schedule is - // spaced in seconds, so retrying rides out a transient failure without - // spending the rate limit. Anything definitive surfaces on the first - // attempt — see `isRetryableFollowFailure`. + // A blip should not end a tail someone is watching. See + // `isRetryableFollowFailure` for what does not get a second attempt. const poll = pollOnce.pipe( Effect.retry({ schedule: readRetrySchedule, while: isRetryableFollowFailure }), ); - // `repeat` runs the body before applying the schedule, so the first poll is - // immediate. That is wanted: it catches anything that landed while the history - // query was in flight, and the rows it repeats are discarded by the id dedupe. - // Measured cost is ~7 requests in the worst 60-second window, against a limit - // of 10. + // `repeat` runs the body first, so the opening poll is immediate — it + // catches whatever landed while the history query was in flight. ~7 + // requests in the worst 60-second window, against a limit of 10. yield* Effect.raceFirst( poll.pipe(Effect.repeat({ schedule: pollSchedule })), - // `setExitCode`, not `exit`: the production `exit` calls `process.exit` - // synchronously, which tears the runtime down from inside this race - // branch — before the linked-project cache is written, before telemetry - // is flushed, and before the instrumentation wrapper emits its post-run - // event. Recording the code lets the race complete normally so the - // finalizers run, and `runCli` exits with it once they have. + // `setExitCode`, not `exit`: `exit` calls `process.exit` synchronously, + // tearing the runtime down before this command's finalizers run. Record + // the code and let the race complete; `runCli` exits with it. processControl .awaitSignal() .pipe( diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts index 7099eb4679..32c43b104e 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.ts @@ -1,25 +1,14 @@ import { WorkerNotDeployedError } from "../../../../shared/workers/workers.errors.ts"; -/** - * How this family is invoked, in one place. - * - * The path was a literal in roughly thirty call sites, so moving the family - * under `experimental` had to rewrite every one — and two follow-up commits - * exist because some were missed or left pointing at a command that no longer - * existed. Every user-facing string that names one of these commands builds it - * from here. - */ +/** How this family is invoked. Every string that names one builds from here. */ const PATH = "supabase experimental workers"; /** One of this family's commands, spelled the way a user would type it. */ export const legacyWorkersCommand = (rest: string) => `${PATH} ${rest}`; /** - * The `push` that deploys or redeploys `name`. - * - * `refSuffix` is the caller's `--project-ref` when the flag supplied one — a - * suggestion is copy-pasted verbatim, so one that drops it re-resolves against - * whatever this checkout is linked to. + * The `push` that deploys or redeploys `name`. A suggestion is copy-pasted + * verbatim, so it carries `--project-ref` when the flag supplied one. */ export const legacyWorkersPushCommand = (name: string, refSuffix = "") => legacyWorkersCommand(`push ${name}${refSuffix}`); @@ -29,12 +18,9 @@ export const legacyWorkersStatusCommand = (name: string, refSuffix = "") => legacyWorkersCommand(`status ${name}${refSuffix}`); /** - * "There is no such deployment", with the caller's own way out. - * - * The detail is the same wherever it is raised; the suggestion is not. `status` - * and `logs` point at `push`, but `delete` deliberately does not — somebody - * removing "api" and hearing "nothing is deployed" wants to see what *is* - * deployed, not to deploy it. + * "There is no such deployment", with the caller's own way out: `status` and + * `logs` point at `push`, `delete` at `list` — somebody removing "api" wants to + * see what *is* deployed, not to deploy it. */ export const legacyWorkerNotDeployed = (options: { readonly name: string; diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts deleted file mode 100644 index 095c645d1f..0000000000 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.commands.unit.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { - legacyWorkersCommand, - legacyWorkersPushCommand, - legacyWorkersStatusCommand, -} from "./workers.commands.ts"; - -/** - * These strings are copy-pasted out of the terminal, so the exact spelling is - * the contract. Pinned here so moving the family again is one deliberate edit - * rather than a sweep that misses call sites — which is how the last move went. - */ -describe("legacyWorkersCommand", () => { - it("prefixes the family path", () => { - expect(legacyWorkersCommand("list")).toBe("supabase experimental workers list"); - }); - - it("carries an explicit --project-ref into a push suggestion", () => { - expect(legacyWorkersPushCommand("api", " --project-ref demo")).toBe( - "supabase experimental workers push api --project-ref demo", - ); - }); - - // The suffix is empty when the ref came from the link, where repeating it is - // noise on a command that already resolves correctly. - it("omits the ref when there is none to carry", () => { - expect(legacyWorkersPushCommand("api")).toBe("supabase experimental workers push api"); - expect(legacyWorkersStatusCommand("api")).toBe("supabase experimental workers status api"); - }); -}); 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 index 8e80195f0a..77c5f082c4 100644 --- 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 @@ -114,15 +114,10 @@ describe("legacyWorkersRendersText", () => { const CASES = [ ["no flags", {}, true], ["--output-format json", { format: "json" }, false], - ["--output-format stream-json", { format: "stream-json" }, false], + // `-o json` leaves `output.format` as `text`, so the format alone says yes. ["-o json", { goOutput: "json" }, false], - ["-o yaml", { goOutput: "yaml" }, false], - // These encode nothing and fall through to the text rendering. - ["-o pretty", { goOutput: "pretty" }, true], - ["-o csv", { goOutput: "csv" }, true], - // `-o` outranks `--output-format`, in both directions. + // `-o pretty` encodes nothing and outranks `--output-format`. ["-o pretty with --output-format json", { goOutput: "pretty", format: "json" }, true], - ["-o json with --output-format text", { goOutput: "json" }, false], ] as const; for (const [label, options, expected] of CASES) { diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts index 6061a24a8e..57baee0a51 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.output.ts @@ -81,17 +81,11 @@ export const legacyWorkersMachineOutputRequested = Effect.fnUntraced(function* ( }); /** - * The format a run actually renders in, with `-o` given priority over - * `--output-format`. + * The format a run renders in, with `-o` given priority over `--output-format`. + * `-o pretty|table|csv` encode nothing and fall through to the text rendering. * - * `-o pretty|table|csv` encode nothing and fall through to the text rendering, - * and an explicit `-o` outranks `--output-format` when both are set. Branching - * on `output.format` alone therefore emitted JSON for `-o pretty - * --output-format json`, which asked for exactly the opposite. - * - * `-o json|yaml|toml|env` are absent from the result on purpose: those are - * handled by `legacyEmitWorkersMachineOutput`, which runs before any of this and - * owns its own stdout. + * `-o json|yaml|toml|env` are absent by design: `legacyEmitWorkersMachineOutput` + * runs first and owns its own stdout. */ export const legacyWorkersRenderFormat = Effect.fnUntraced(function* () { const output = yield* Output; @@ -118,12 +112,9 @@ export const legacyRejectWorkersEnvOutput = Effect.fnUntraced(function* () { }); /** - * Whether this run renders human text on stdout. - * - * Neither flag answers this alone: `-o json|yaml|toml|env` leaves - * `output.format` as `text`, while `--output-format json` says nothing about - * `-o`. Every caller was spelling the pair out by hand, and three of them got - * the precedence wrong. + * Whether this run renders human text on stdout. Neither flag answers alone: + * `-o json|yaml|toml|env` leaves `output.format` as `text`, and + * `--output-format` says nothing about `-o`. */ export const legacyWorkersRendersText = Effect.fnUntraced(function* () { if (yield* legacyWorkersMachineOutputRequested()) { @@ -133,19 +124,12 @@ export const legacyWorkersRendersText = Effect.fnUntraced(function* () { }); /** - * Emit `payload` in whichever machine format the run asked for. - * - * Returns whether it emitted, so a caller can skip its text rendering — the - * same contract as {@link legacyEmitWorkersMachineOutput}, which this wraps. - * - * The two format flags are resolved here rather than at each call site. - * Branching on `output.format` alone ignored `-o`'s priority, so - * `-o pretty --output-format json` emitted JSON from a run that asked for text. + * Emit `payload` in whichever machine format the run asked for, returning + * whether it did — so a caller can skip its text rendering. * - * The structured emission happens exactly once, and only in the structured - * branch: calling `output.success` ahead of the machine check emitted the - * payload twice, which broke `JSON.parse` and gave `stream-json` two terminal - * result events. + * Exactly one structured emission, and only in the structured branch: calling + * `output.success` ahead of the machine check emitted the payload twice, which + * broke `JSON.parse` and gave `stream-json` two terminal result events. */ export const legacyEmitWorkersPayload = Effect.fnUntraced(function* ( payload: Record, diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts index 90f80b8064..49ff993ddd 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.run.integration.test.ts @@ -21,34 +21,20 @@ function project() { return { dir: created.dir, cleanup: () => rmSync(created.dir, { recursive: true, force: true }) }; } -/** - * Every project-scoped command, with only the flags the run needs to get as far - * as resolving a ref. - */ +const REF = { projectRef: Option.none() }; + +/** Every project-scoped command, with the least it needs to reach `resolve`. */ const COMMANDS = [ - ["list", () => legacyWorkersList({ projectRef: Option.none() })], - ["status", () => legacyWorkersStatus({ name: "api", projectRef: Option.none() })], - ["delete", () => legacyWorkersDelete({ name: "api", projectRef: Option.none() })], + ["list", () => legacyWorkersList(REF)], + ["status", () => legacyWorkersStatus({ ...REF, name: "api" })], + ["delete", () => legacyWorkersDelete({ ...REF, name: "api" })], [ "push", - () => - legacyWorkersPush({ - names: ["api"], - instances: Option.none(), - wait: false, - projectRef: Option.none(), - }), + () => legacyWorkersPush({ ...REF, names: ["api"], instances: Option.none(), wait: false }), ], [ "logs", - () => - legacyWorkersLogs({ - name: "api", - projectRef: Option.none(), - kind: Option.none(), - follow: false, - tail: 100, - }), + () => legacyWorkersLogs({ ...REF, name: "api", kind: Option.none(), follow: false, tail: 100 }), ], ] as const; diff --git a/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts b/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts index 7b761b63a3..0c0198741c 100644 --- a/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts +++ b/apps/cli/src/legacy/commands/experimental/workers/workers.run.ts @@ -4,24 +4,19 @@ import { LegacyLinkedProjectCache } from "../../../telemetry/legacy-linked-proje import { LegacyTelemetryState } from "../../../telemetry/legacy-telemetry-state.service.ts"; import { legacyWorkersProjectRefSuffix } from "./workers.output.ts"; -/** What every project-scoped workers command needs before it can do anything. */ export interface LegacyWorkersRunContext { readonly projectRef: string; - /** The `--project-ref` a suggestion must carry, or `""` — see the helper. */ + /** What a suggestion must carry — see `legacyWorkersProjectRefSuffix`. */ readonly refSuffix: string; } /** * The lifecycle every project-scoped workers command shares. * - * Telemetry wraps the ref resolution as well, because an unlinked - * non-interactive checkout fails inside `resolve` and the command has run by - * then. The linked-project cache stays under the ref, having nothing to write - * without one. - * - * A helper rather than a per-command preamble because the ordering is the whole - * point and is easy to get subtly wrong: four commands had the resolution above - * both finalizers and silently wrote no post-run event. + * The ordering is the point: telemetry wraps the ref resolution, since an + * unlinked checkout fails inside `resolve` once the command has already run. + * The linked-project cache stays under the ref, having nothing to write without + * one. */ export const legacyWorkersRun = ( projectRefFlag: Option.Option, From e4e0a3e55efc6b27826ccdf9d8e41d2e3911ae64 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 1 Sep 2026 22:13:53 -0300 Subject: [PATCH 7/7] refactor(workers push): lift the source check out of deployOneWorker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `deployOneWorker` opened with a sixty-line bare block — the author's own marker that it was a separate job. `assertDeployableSource` is that job: does this path hold something worth deploying. What is left reads as its steps — describe, check the source, resolve the spec, package, upload, deploy, report. Costs a few lines in parameter plumbing and buys a function that can be read, and tested, without the deploy around it. --- .../experimental/workers/push/push.handler.ts | 131 ++++++++++-------- 1 file changed, 73 insertions(+), 58 deletions(-) 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 74e109c749..775e453ec1 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 @@ -50,6 +50,7 @@ import { } from "../../../../../shared/workers/workers.errors.ts"; import { legacyDescribeWorker, + type LegacyResolvedWorker, legacyDiscoverWorkerNames, legacyLoadWorkersProject, legacyValidateWorkerName, @@ -183,6 +184,71 @@ function addYourCode(sourceDisplay: string): string { return `Add your worker's code to ${sourceDisplay}, then run this command again.`; } +/** + * Refuse a source directory there is nothing to deploy from. + * + * Ahead of `resolveRuntime`, which classifies the directory and announces what + * it guessed — inferring a runtime for a path that does not exist reports on it + * and only then fails on it. + */ +const assertDeployableSource = Effect.fnUntraced(function* (input: { + readonly name: string; + readonly sourceDir: string; + readonly sourceDisplay: string; + readonly configPath: string; + readonly entry: LegacyResolvedWorker["entry"]; +}) { + const fs = yield* FileSystem.FileSystem; + const { sourceDisplay } = input; + + const missing = new WorkerSourceMissingError({ + detail: `There is no worker source at ${sourceDisplay}.`, + suggestion: missingSourceSuggestion({ + name: input.name, + sourceDisplay, + configPath: input.configPath, + entry: input.entry, + }), + }); + + // Only "no such path" means it was never scaffolded. A permission or I/O error + // is a different problem with a different fix, so it propagates as itself. + const info = yield* fs + .stat(input.sourceDir) + .pipe( + Effect.catchTag("PlatformError", (error) => + Predicate.isTagged(error.reason, "NotFound") + ? Effect.fail(missing) + : Effect.fail(error), + ), + ); + + // Occupied but not a directory. "There is no worker source" is false twice + // over, and `workers new` refuses this destination too — so the scaffold + // suggestion would answer with a second error. + if (info.type !== "Directory") { + return yield* Effect.fail( + new WorkerSourceMissingError({ + detail: `${sourceDisplay} is not a directory.`, + suggestion: `Replace it with a directory holding your worker's code, then run this command again.`, + }), + ); + } + + // An empty directory packages and deploys happily, producing an image with + // nothing in it. Read errors propagate rather than reading as empty: a + // directory the CLI cannot open is not one with nothing in it. + const contents = yield* fs.readDirectory(input.sourceDir); + if (contents.length === 0) { + return yield* Effect.fail( + new WorkerSourceMissingError({ + detail: `${sourceDisplay} is empty, so there is nothing to deploy.`, + suggestion: addYourCode(sourceDisplay), + }), + ); + } +}); + const deployOneWorker = Effect.fnUntraced(function* (input: { readonly project: LegacyWorkersProject; readonly name: string; @@ -201,7 +267,6 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { /** Suppresses this step's human output when `-o` owns stdout. */ readonly rendersText: boolean; }) { - const fs = yield* FileSystem.FileSystem; const output = yield* Output; const api = yield* LegacyPlatformApi; const settings = yield* LegacyCliSettings; @@ -211,63 +276,13 @@ const deployOneWorker = Effect.fnUntraced(function* (input: { const sourceDisplay = displayPath(project.projectRoot, worker.sourceDir); - // Checked before the runtime is resolved, not after: with no recorded - // runtime, `resolveRuntime` classifies the directory and announces what it - // guessed. Doing that first meant reporting an inference about a path that - // does not exist, and only then failing on the path. - { - const sourceMissing = new WorkerSourceMissingError({ - detail: `There is no worker source at ${sourceDisplay}.`, - suggestion: missingSourceSuggestion({ - name, - sourceDisplay, - configPath: displayPath(project.projectRoot, project.configPath), - entry: worker.entry, - }), - }); - // Only "no such path" means the worker was never scaffolded. A permission - // or I/O error on the directory is a different problem with a different - // fix, and answering it with "there is no worker source, run `workers new`" - // both misdiagnoses it and points at a directory that already exists — so - // every other reason propagates as itself. - const info = yield* fs - .stat(worker.sourceDir) - .pipe( - Effect.catchTag("PlatformError", (error) => - Predicate.isTagged(error.reason, "NotFound") - ? Effect.fail(sourceMissing) - : Effect.fail(error), - ), - ); - // Something is there, it is just not a directory. Reporting that as "there - // is no worker source" is false twice over: the path is occupied, and - // `workers new` refuses a destination that exists and is not a directory, - // so the scaffold suggestion would answer with a second error. - if (info.type !== "Directory") { - return yield* Effect.fail( - new WorkerSourceMissingError({ - detail: `${sourceDisplay} is not a directory.`, - suggestion: `Replace it with a directory holding your worker's code, then run this command again.`, - }), - ); - } - // An empty directory packages and deploys perfectly happily, producing an - // image with nothing in it — a success message for a worker that cannot - // serve anything. Refuse before uploading rather than after. - // - // Read errors propagate rather than reading as empty: a directory the CLI - // cannot open is not a directory with nothing in it, and the two want - // opposite things from the user. - const contents = yield* fs.readDirectory(worker.sourceDir); - if (contents.length === 0) { - return yield* Effect.fail( - new WorkerSourceMissingError({ - detail: `${sourceDisplay} is empty, so there is nothing to deploy.`, - suggestion: addYourCode(sourceDisplay), - }), - ); - } - } + yield* assertDeployableSource({ + name, + sourceDir: worker.sourceDir, + sourceDisplay, + configPath: displayPath(project.projectRoot, project.configPath), + entry: worker.entry, + }); const runtime = yield* resolveRuntime({ name,