refactor(workers): consolidate the command family onto shared helpers - #6431
Draft
johnstonmatt wants to merge 54 commits into
Draft
refactor(workers): consolidate the command family onto shared helpers#6431johnstonmatt wants to merge 54 commits into
johnstonmatt wants to merge 54 commits into
Conversation
Makes the `name` argument to `supabase workers new` optional and prompts for it when it is omitted, so a bare `supabase workers new` walks through name, runtime and size rather than failing the parse. The name is the one input this command cannot default — it is the directory, the `[workers.<name>]` key and the hostname all at once. So where the runtime and size prompts fall back to a default when there is nowhere to ask, the name prompt has nothing to fall back to: with `-o json|yaml|toml|env` or no interactive terminal, the command fails with a new `MissingWorkerNameError` pointing at `supabase workers new api`. The prompt validates against everything the command would otherwise refuse a moment later — a non-DNS-label name, and a name `config.toml` already records — so a typo is corrected in place instead of ending the run. That also means the project has to be loaded before the first prompt, and the machine-output check moves up with it: `-o` leaves `output.format` as `text`, and Clack writes its terminal UI to stdout, so a name prompt would land in front of the payload for the same reason the runtime prompt would. The handler's inline name validation is replaced by the shared `legacyValidateWorkerName`, which the rest of the command family already uses, so an explicitly-passed name and a prompted one are refused on identical terms. `mockOutput` now records `promptTextCalls` so tests can assert on the prompt's message and exercise its `validate` callback.
`output.interactive` is derived solely from `tty.stdoutIsTty`, so with stdin piped or redirected and stdout still on a terminal it stayed true. A bare `printf 'api\n' | supabase workers new` therefore opened Clack's name prompt and read the worker name off the pipe instead of taking the documented `MissingWorkerNameError` path — and the runtime and size prompts consumed whatever followed rather than falling back to their defaults. The three resolvers now share one `canPromptFor` decision, made once before the first prompt, which pairs `output.interactive` with `tty.stdinIsTty` the way `workers delete` already guards its confirmation. A prompt is only answerable from a keyboard, so both streams have to be a terminal.
Makes the `name` argument to `supabase workers new` optional and prompts for it when it is omitted, so a bare `supabase workers new` walks through name, runtime and size rather than failing the parse. The name is the one input this command cannot default — it is the directory, the `[workers.<name>]` key and the hostname all at once. So where the runtime and size prompts fall back to a default when there is nowhere to ask, the name prompt has nothing to fall back to: with `-o json|yaml|toml|env` or no interactive terminal, the command fails with a new `MissingWorkerNameError` pointing at `supabase workers new api`. The prompt validates against everything the command would otherwise refuse a moment later — a non-DNS-label name, and a name `config.toml` already records — so a typo is corrected in place instead of ending the run. That also means the project has to be loaded before the first prompt, and the machine-output check moves up with it: `-o` leaves `output.format` as `text`, and Clack writes its terminal UI to stdout, so a name prompt would land in front of the payload for the same reason the runtime prompt would. The handler's inline name validation is replaced by the shared `legacyValidateWorkerName`, which the rest of the command family already uses, so an explicitly-passed name and a prompted one are refused on identical terms. `mockOutput` now records `promptTextCalls` so tests can assert on the prompt's message and exercise its `validate` callback.
`output.interactive` is derived solely from `tty.stdoutIsTty`, so with stdin piped or redirected and stdout still on a terminal it stayed true. A bare `printf 'api\n' | supabase workers new` therefore opened Clack's name prompt and read the worker name off the pipe instead of taking the documented `MissingWorkerNameError` path — and the runtime and size prompts consumed whatever followed rather than falling back to their defaults. The three resolvers now share one `canPromptFor` decision, made once before the first prompt, which pairs `output.interactive` with `tty.stdinIsTty` the way `workers delete` already guards its confirmation. A prompt is only answerable from a keyboard, so both streams have to be a terminal.
Comments explaining why code is shaped a certain way now state the constraint directly instead of narrating what an earlier version did. The reasoning is unchanged; only the framing is.
The workers commands each grew their own way of saying "here is what happened" and "here is what to run next". This settles them on the shapes the rest of the legacy shell already uses, with no change to what any command does. - "What to run next" lines in `new`, `push`, `delete` and `status` move to `emitSuccessTrailer`, the way `stop`, `bootstrap`, `migration repair` and `gen signing-key` already emit theirs: printed once at the end of the run rather than inline, so a multi-worker push does not bury each worker's hint under the next worker's output. The commands within them are aqua'd, as every other follow-up hint in this shell writes them. - `list`'s two advisories take the yellow `WARNING:` prefix and the two-line consequence shape `start`'s Docker notice uses. Each was one long sentence that re-flowed at a different width under a table that lines its columns up. - `list` drops the URL column. Every worker's URL is the same host and prefix with the name on the end, and carrying it pushed the table past 130 columns for one derivable field, since `renderGlamourTable` sizes to the widest cell and never wraps. `status` still renders it vertically, and every machine format still carries `url` per worker. - `push` counts its per-worker announcements (`Deploying Worker 1/2:`) and closes a multi-worker run with a summary line. Each worker takes minutes; the name alone said nothing about how much of the run was left. - `push` names the workers a failed run never attempted. The loop stops at the first failure and the error only names the worker that broke, leaving the rest to be reconstructed from argument order. On stderr in every format, machine ones included: that run is a CI run. - Both of `push`'s retry suggestions carry an explicit `--project-ref` when the flag supplied the ref, via the `legacyWorkersProjectRefSuffix` helper `status` and `delete` already use. A suggestion is copy-pasted verbatim, so one that dropped it re-resolved against whatever this checkout was linked to. Adds unit coverage for `legacyRenderWorkerDetails`'s padding and empty-row dropping, and pins the shared `-o env` refusal so a new command that forgets its own up-front check cannot silently emit TOML instead.
`Flag.boolean(name)` builds a bare `Single` param, and a bare `Single` is *required* — omitting it fails the whole command with a missing-flag error before the handler ever runs. Every boolean flag has to be closed off with `Flag.withDefault(false)` or `Flag.optional`, and nothing in the existing suites notices when one is not. Handler integration tests build their flags record directly, so they never touch the parser, and the required-ness is invisible to the type checker because a required boolean flag still infers as `boolean`. The flag only misbehaves when a real invocation omits it, which is exactly the invocation no handler test makes. So this walks the whole legacy command tree, including global flags, and asserts every boolean param carries a default or is optional. It reads the primitive kind through `Primitive.getTypeName` rather than `_tag`, since this repo forbids inspecting effect's runtime representation in tests as well as in source.
Comments explaining why code is shaped a certain way now state the constraint directly instead of narrating what an earlier version did. The reasoning is unchanged; only the framing is.
`status` reports the deployment; nothing reported the runtime. Once `push` succeeded and `status` said `active`, a misbehaving worker was a black box from the CLI. Reads the project's unified logs stream rather than a worker route — there is no worker-scoped log endpoint — via `v1GetProjectLogs`, which the generated client already carries. `--source app|requests|builds` narrows to one of the three streams; without it all three are returned. `--tail` caps the rows. Three things about that endpoint are load-bearing and non-obvious, so they are documented at each site: - **The filter is `log_attributes`, not the `source` column.** Worker rows carry an empty top-level `source`, because the Workers Logflare source is not enrolled as a category in the generic logs path. `where source = 'worker_guest_logs'` matches nothing. The `in (...)` list over the three known streams is therefore a tenancy guard, not a convenience — with `source` empty it is the only thing excluding a non-worker row that happens to carry a `worker` attribute. - **Both timestamp bounds are always sent, spanning under 24h.** One bound alone yields a one-minute window, silently; neither is an outright error; and a span over 24h is clamped to `start + 24h`, which returns an *older* slice than the one asked for rather than a truncated one. - **A failed query can arrive as HTTP 200** with a populated `error`, so the envelope is checked before `result`. The response is decoded against a local schema rather than the generated `V1GetProjectLogsOutput`: that schema marks `result`/`error` optional but allows neither to be `null`, while the endpoint always sends one of them as an explicit `null`, so decoding any real response against it fails. Rendering is per-stream, because `event_message` differs in kind — on the request stream it is only `"GET /"`, with status and duration in `log_attributes`, so the request line is composed. `severity_text` is ignored: it is `INFO` on every row of every stream, so the level is derived, and guest lines report none rather than a guess. A guest message is tenant-controlled bytes, so escape sequences are stripped before it reaches a terminal while a stack trace's newlines and indentation survive. `mapRequestError`/`unexpectedStatus`/`decodeBody` move out of `workers-api.ts` into `workers-api-status.ts`, unchanged, now that a second seam needs them. The test helper records `urlParams`: `HttpClientRequest` keeps them off the URL, so without this no test could assert the emitted SQL or window.
Matches the only other log-line format this shell prints — the `--debug` HTTP logger, which uses Go's `log.LstdFlags` (`legacy-debug-logger.layer.ts`). Someone reading a tail is asking "what just happened", and the answer gets compared against their own clock. Text output only. The machine payload keeps both unambiguous forms, so nothing that is parsed, sorted, or pasted into an issue depends on the reader's zone: `timestamp` stays ISO-8601 UTC and `timestamp_ms` the raw epoch value. The unit tests derive their expected prefix from the same instant with the same field accessors, rather than hardcoding one: a literal `"14:45:32"` would have passed only on a UTC machine. One case additionally pins the zone choice itself — asserting the output is *not* the UTC rendering — guarded so it stays meaningful on a UTC machine, where the two coincide. Verified green under `TZ=Asia/Tokyo`, `TZ=UTC`, and the ambient zone.
Opt-in, matching `workers push --wait`: long-running behaviour in this family is asked for, never defaulted. **The poll interval is set by the rate limit, not by responsiveness.** The v1 analytics endpoints allow 10 requests per 60 seconds, so the two-second poll a live tail suggests would spend the whole allowance in ten seconds. Six seconds is the arithmetic floor; ten leaves room for the history query, the deployed-worker check, and a retry in the same window. Measured at ~7 requests in the worst 60-second window. The interval is in `--follow`'s help text, because a 10-second tail is visibly not a live stream and would otherwise look broken. The cursor deliberately lags 60 seconds behind the newest line printed. Guest lines are relayed CloudWatch -> subscription filter -> Lambda -> Logflare and arrive late and out of order, so a cursor sitting on the newest timestamp would drop every straggler permanently. Overlap is therefore guaranteed and expected; dedupe on the Logflare-minted `id` is what makes it invisible, bounded so a long tail does not grow the set forever. `followWindow` clamps to the same sub-24h span as a bounded read, so a tail resumed after a laptop suspend cannot ask for a wider window — the server answers those by returning an *older* slice. Every poll sends both timestamp bounds. Advancing only `iso_timestamp_start` is the obvious implementation and is wrong: it yields a one-minute window. Output: - `-o json|yaml|toml` and `--output-format json` are refused up front, beside the `-o env` refusal and for the same reason — each promises one terminal payload and a tail has no last element. - `--output-format stream-json` emits one `log-entry` event per line instead of a single `result`, reusing the existing variant. `stream` splits error/warn to `stderr`; `source` separates backlog from live. - SIGINT exits 130, matching the local `supabase logs` command. - `--tail 0` skips the backlog and makes no history request, since the endpoint rejects `limit 0`. It also suppresses the not-deployed check, which would otherwise read "no rows" as "no worker" when no query was made at all. Both schedules are injectable, as `awaitWorkerBuild`'s are, so the cursor, dedupe and retry paths are tested without a wall clock. The SIGINT test forks the handler and synchronises on the mock's `awaitExit` — `exit` never returns, so the handler cannot be awaited. Stressed over five consecutive runs.
Comments explaining why code is shaped a certain way now state the constraint directly instead of narrating what an earlier version did. The reasoning is unchanged; only the framing is.
`supabase workers push` blocked on the server-side container build on every invocation. That build routinely runs for minutes, so the common case — a deploy that builds fine — was the slowest thing in the loop. The command now returns once the platform accepts the deploy, which is the last thing it can learn without waiting: the deploy response arrives only after the spec and the uploaded context are accepted, and it carries the accepted spec back. `--wait` opts into the build's verdict, for CI and for anyone who needs the image version before continuing. A deploy answered with a spec already in `failed` is still reported as a failure whether or not the build was waited on, rather than exiting zero on a worker that will never come up. Without `--wait` the details block leads with a `State` row — the one row that says the worker is not serving yet — and drops `Image`, since no image exists until the build produces one. A success trailer then names both `workers status` and `--wait` as ways to follow the build. The flag closes with `Flag.withDefault(false)`; the guard added in the previous commit is what keeps a boolean flag from shipping required again.
…/supabase/cli into FUNC-840/select-workers-new-name # Conflicts: # apps/cli/src/legacy/commands/experimental/workers/new/SIDE_EFFECTS.md # apps/cli/src/legacy/commands/experimental/workers/new/new.command.ts # apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts
`workers` is registered only beneath the `experimental` parent, so the `MissingWorkerNameError` suggestion telling the user to run `supabase workers new api` produced an unknown-command error when copied. Name the real invocation path in the suggestion, and in the handler, doc and test prose that described the piped-stdin case with the same stale path. An assertion on the suggestion keeps the retry path from drifting away from where the command is mounted.
The Workers API decode failure suggested `supabase update`, which is not a command in either shell's root — the CLI has no self-update path, which is why the post-command upgrade notice sends users to the docs instead. Point the suggestion at that same upgrade guide, and hoist the URL from `legacy-upgrade-notice.ts` into `shared/cli/version.ts` so both callers read one constant rather than duplicating the link.
…olish # Conflicts: # apps/cli/src/legacy/commands/experimental/workers/new/new.handler.ts # apps/cli/src/legacy/commands/experimental/workers/new/new.integration.test.ts
# Conflicts: # apps/cli/src/shared/workers/workers-api.ts
The guard's rationale pointed at `supabase workers push`, but the family is registered beneath the `experimental` parent, so that invocation does not exist. Name the real path, and describe the flag rather than when it was added.
The output work reintroduced `legacy.workers.*` as the `Effect.fn` span name for all five worker handlers, undoing the rename that landed with the move under `experimental`. Traces and the NDJSON exporter recorded a command route that no longer exists, so any filter keyed to the current names missed them.
`LegacyProjectRefResolver` treats `--project-ref ""` as absent and falls back to the environment or the linked-project file, but the suffix helper keyed off `Option.isSome` alone. Every suggestion that command emitted ended in a valueless `--project-ref`, which cannot be pasted back and re-run. Match the resolver's reading of the flag instead.
`SIDE_EFFECTS.md` claimed the `Deploying Worker n/N:` announcement was text-only, but the gate read `machineOutput`, which tracks `-o` alone. Under `--output-format json` or `stream-json` the flag stays false and the announcement was written to stderr, contradicting the documented matrix. Gate it on the format too, the way the run's closing summary already is. The "not attempted" report keeps reaching every format: it says what still needs deploying, which is exactly what an unwatched CI run has to know.
Both handlers return at the machine emission or at `output.success`, so the build-retry trailer, the redeploy trailer, and the two stderr notices beside them are only reachable in the text branch. The format matrices claimed "as above" for every structured row, which would drive E2E expectations for output those modes never produce.
`--source` already means "the directory a worker's code lives in" across the family: it is the `config.toml` key, the flag on `workers new`, the `Source` row in `status`, and what `push` reads to find a worker. Reusing it on `logs` for a closed set of log streams gave one flag two meanings in one command family. `--kind` names the axis rather than the mechanism, cannot be read as a mode the way `--stream` collides with `--follow`, and reads correctly inside a command already called `logs`. The backend's `log_attributes['source']` key and the per-line `source` field on `log-entry` events are untouched — they are different things, and the rename is what makes them distinguishable. The command is unreleased, so this costs nothing now and would be a breaking flag change later.
Same defect the status and delete matrices carried: the no-logs line and its `status` trailer sit below the machine and structured early returns, so only the text branch reaches them, but every structured row claimed "as above".
`WORKER_LOG_STREAMS[flags.kind.value as WorkerLogKindChoice]` asserted the relationship rather than holding it: the choice list lived in the command and the stream map in the query module, so adding a word to one and not the other compiled, indexed the map as `undefined`, and reached the SQL as an empty stream name. The cast turned out to be unnecessary on its own — `Flag.choice` already infers the literal union — so removing it makes the index site type-checked. The list moves next to the map as `WORKER_LOG_KINDS`, constrained with `satisfies`, so drift now fails at the declaration as well as at the use.
`-o` outranks `--output-format` when both are set, and `-o pretty|table|csv` encode nothing and fall through to the text rendering. Branching on `output.format` alone therefore did the opposite of what the pair asked for: `-o pretty --output-format json --follow` was refused as a single-payload format, and the bounded path emitted JSON for a run that had asked for text. `legacyWorkersRenderFormat` resolves the two flags once, and the handler branches on that instead of consulting `output.format` at each emission.
Ref resolution sat above both finalizers, so an unlinked non-interactive checkout — or a declined project picker — failed before `telemetryState.flush` was installed and wrote no post-run event, even though the command had run. Telemetry now wraps the resolution; only the linked-project cache stays under the ref, since it has nothing to write without one.
The production `ProcessControl.exit` calls `process.exit` synchronously, so calling it from inside the race branch tore the runtime down before any of the command's cleanup: no linked-project cache write, no telemetry flush, and no post-run `cli_command_executed` from the instrumentation wrapper. Every followed run was invisible to telemetry the moment it was interrupted. The branch now records the code with `setExitCode` and returns, so the race completes, the finalizers run, and `runCli` exits with the recorded code the way it already does for a bounded run.
Each poll asked for `--tail` rows, but the query orders newest-first, so a burst larger than that came back as its newest slice alone — and the cursor then advanced past the rows that were never returned, dropping them permanently. `--tail 1 --follow` lost almost everything; the default lost anything above 100 rows in a polling interval. The poll now uses its own page size and walks `end` backwards while pages come back full, so a burst is drained before the cursor moves. Bounded at five requests, because the endpoint allows ten a minute; rows past that bound are not lost, since the cursor still only advances over what was emitted.
Three holes in the branch that skips the history query: The cursor starts at the invocation instant and `followWindow` reaches a grace period behind it, so the first poll replayed up to a minute of the history the run had just been told to skip. The grace is what makes a late-relayed line visible at all, so it stays; a floor on the line's own timestamp is what keeps skipped history out. The deployed-worker check was gated on `--tail > 0`, so a tail with no history query never asked whether the worker existed and a typo waited forever on logs that could not arrive. It now runs for any follow. That check also ran with no spinner: `--tail 0` has no "Fetching logs..." to inherit, and the bounded path cleared it beforehand. It gets its own task.
…message `stripControlSequences` was applied only to `event_message` on the guest stream, on the premise that it was the one untrusted string. It was not: a request `path`, `method` and `status` are chosen by whoever called the worker, a `duration_ms` comes back on the same row, and a build `event` and `reason` are relayed from the builder. All of them were interpolated raw into a line written to a terminal. The strip itself also kept carriage returns. A bare CR returns the cursor to column zero, so even a sanitised guest line could overwrite the timestamp and stream tag printed to its left and forge output that looks like the CLI's own. CRLF now folds to a newline first, so real line breaks survive, and lone CRs go with the other C0 controls. The stream the tag derives from is left alone deliberately: the query only returns rows whose stream is one of three literals, so it cannot carry anything.
Every exported token from `legacy/` carries the `Legacy` prefix, with no exceptions — it is what keeps the two in-tree shells from bleeding into each other at import sites. `WorkerLogLevel` was shipping bare.
The handler records 130 for an interrupted `--follow` and an integration test asserts it, but the compatibility table listed only 0 and 1 — so the record E2E coverage is derived from was incomplete.
`Schema.Number` accepted any finite value, and the payload build calls `new Date(entry.timestampMs).toISOString()` unconditionally — text runs construct it too. An out-of-range timestamp from an upstream projection regression therefore threw `RangeError`, turning a recoverable bad row into a defect. The bound moves onto the schema, where it fails through `decodeBody` as the unreadable-response error the rest of the module already raises.
The follow loop retried every failure on a five-second schedule for up to a minute. A 401, 402 or 404 answers the same way every time, so the reader waited a minute to be told something the first attempt already knew — and the retries spent most of the endpoint's ten-requests-per-minute allowance getting there, so a rate limit could land on top of the real cause. Server-side statuses still ride out, along with 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 reads as terminal: it will not parse better on a second attempt.
`line` was set from `event_message`, which on the request stream is only `"GET /"` — the status and duration live in `log_attributes` — and on the build stream omits the structured failure reason. `log-entry` has no attributes field, so a consumer had no way to recover either. The composition text mode already does moves into `legacyWorkerLogText`, and both callers use it. Coloring and the timestamp/tag prefix stay in the renderer, since neither belongs in a machine event.
…40/select-workers-new-name
…/supabase/cli into FUNC-851/general-output-polish
Three separate breaks, all from the same PR: - `WORKER_LOG_CURSOR_GRACE_SECONDS` is only read by `logWindow` in its own file, so the `export` was dead and knip failed the quality job. Dropped. - `--kind` is a value-consuming long flag, and the repo-wide completeness guard in `legacy-db-target-flags` requires every one of them to be registered or the DB-target scanner mis-reads the token after it. - The colour test supplied a fake stream but not a fake environment. The gate checks `CI` before it asks the stream, so the assertion only held on a developer machine and failed in CI. Stubbed the same four variables `legacy-colors.unit.test.ts` stubs.
…into FUNC-853/workers-logs-command
…ploy-wait-flag # Conflicts: # apps/cli/src/legacy/commands/experimental/workers/push/push.integration.test.ts
`git diff-tree --stdin` given a cwd outside any repository exits before it reads a hash, so the write that follows can land on a dead process and raise `EPIPE: broken pipe, send` instead of the exit-code error the caller reports. Which one surfaces is a race against process startup — green on an idle machine, red on a loaded CI runner. The exit code and stderr are the diagnosis, so a broken pipe on stdin is dropped and the reporting left to them.
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.
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.
`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.
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`.
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.
`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.
`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.
johnstonmatt
changed the base branch from
develop
to
FUNC-848/workers-deploy-wait-flag
September 2, 2026 01:37
johnstonmatt
changed the title
johnstonmatt
changed the title
Base automatically changed from
FUNC-848/workers-deploy-wait-flag
to
develop
September 3, 2026 23:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pure refactor of the
workerscommand family, one consolidation per commit, stacked on #6371. The duplication it removes was hiding a handful of bugs, fixed here as a side effect:legacyWorkersRunowns the shared command scaffold — service acquisition, ref resolution and suffix, and the two finalizers — with the ordering written once. Four handlers resolved the ref above both finalizers, so an unlinked non-interactive run failed before the telemetry flush was installed and wrote no post-run event.legacyEmitWorkersPayloadmakes the "emit a structured payload" decision once, andlegacyWorkersRendersTextnames the "renders human text" condition. Both fix the same precedence bug across ten hand-written call sites:-o pretty --output-format jsonasks for text but was treated as a machine run.legacyWorkersCommand, withlegacyWorkersPushCommandandlegacyWorkersStatusCommandfor the two recurring suggestions, replaces thesupabase experimental workersliteral in roughly thirty call sites. A unit test pins the spelling, since these strings get copy-pasted out of a terminal.legacyWorkerNotDeployedbuilds the sentence that is identical in three commands and takes the way out that is not:statusandlogspoint atpush,deletedeliberately points atlist.workers logs: the follow loop's two always-pairedRefs become oneFollowCursor, and the paging walk moves todrainSince, so the poll body reads as its three steps — read the cursor, drain the window, emit what is new.workers push: the sixty-line source check lifts out ofdeployOneWorkerintoassertDeployableSource, leaving the deploy readable as its steps.Also trims comments that explained a change rather than the code. Rationale about what used to be wrong, and how many callers had it wrong, lives in these commit messages rather than in a docblock every later reader pays for.