feat(workers logs): add supabase experimental workers logs - #6410
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a0ae82d95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6a0ae82 to
c66065d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c66065d71f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@213f80408ee6dd2d903f71ade960bed8b2bda1b5Preview package for commit |
…/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
`-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
`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.
kanadgupta
left a comment
There was a problem hiding this comment.
everything looks sound per my testing! claude found a couple of little cleanups that seem good to know about but i'll defer to you on how to handle them, cool with merging either way. thanks!
[!NOTE]
This review was drafted by an AI (Claude) and is pending human vetting. It was refreshed after the fix commits landed (headb772491); the original pass reviewedc66065d.What this PR does
Adds
supabase experimental workers logs <name>: a bounded read of the project's unified logs viav1GetProjectLogs(filtered to the three worker streams throughlog_attributes),--kindnarrowing,--tail, and a polling--followwith a lagging cursor, id-dedupe, and a bounded multi-page burst drain. Rendering is per-stream with derived levels, every externally influenced field is escape-stripped before it reaches a terminal, and the shared Workers status/error mapping is extracted intoworkers-api-status.ts(a genuine DRY win reused by both API seams). Structure conforms to the repo's conventions: pure SQL/window/format modules with unit tests, scenario-oriented integration tests with injected schedules (no sleeps), no e2e — consistent with the testing pyramid and e2e scope policy.What the new commits addressed
CI is now green, and I verified each fix rather than taking the commit messages on faith. Everything the first review pass (mine and the bots') flagged as serious has landed:
--followCtrl+C finalizers: the race now recordssetExitCode(130)instead of calling the synchronousexit, so theensuringfinalizers (linked-project cache, telemetry flush) and the instrumentation wrapper run beforerunCliexits — with an integration test asserting exactly that (records exit 130 on SIGINT and still runs its finalizers).- Poll page size: decoupled from
--tail(FOLLOW_PAGE_SIZE = 1000) plus a bounded multi-page drain (FOLLOW_MAX_PAGES), with burst-drain tests.- Red unit-test check: the colour tests now stub
NO_COLOR/CLICOLOR/CLICOLOR_FORCE/CI, and the previously wrong env-independence claim in the header comment was corrected.- Red knip check:
WORKER_LOG_CURSOR_GRACE_SECONDSis no longer exported (residue noted below).- Sanitisation:
stripControlSequencesnow covers requeststatus/method/path/duration, buildevent/reason, and CR/CRLF handling — closing the attacker-supplied-path hole and the Codex carriage-return one in one commit.- Also landed: telemetry flush wraps ref resolution,
-onow outranks--output-formatvialegacyWorkersRenderFormat(applied family-wide),--tail 0 --followfilters pre-invocation lines and validates the worker, retries are gated onisRetryableFollowFailure, stream-json carries the composed line,ts_msis schema-bounded to a representableDate,--sourcebecame--kindwith choices tied to the stream map viasatisfies(no cast),LegacyWorkerLogLevelprefix, SIDE_EFFECTS documents exit 130.The two Codex threads the author closed as "leave as-is" (404 project-vs-alpha ambiguity; non-2xx query classification) both have reasonable recorded rationale; I have no objection to either.
Fix before merge
None found in this pass.
Follow-up candidates
- Drain-loop honesty (
logs.handler.ts, inline): whenFOLLOW_MAX_PAGESis exhausted mid-burst the cursor advances past the un-drained middle, so "rows beyond it are not lost" only holds within the 60s grace; capping the cursor at the drain boundary would make it true. Relatedly, the "~7 requests in the worst 60-second window" and SIDE_EFFECTS' "6 requests a minute" claims are stale now that a poll can spend up to 5 requests. Only bites above ~1000 rows/poll — follow-up material for an experimental command.- Dead
followWindowseams (worker-logs.sql.ts, inline): thegraceSeconds/spanMinutesoptions are still used by no caller or test, andfollowWindowstill has no direct unit tests despite being the file's trickiest arithmetic. Delete the seams or use them.One note: the tip commit (
b772491) fixes an EPIPE race inpackages/config/scripts/semantic-release-path-filter.ts— a sound test-stability fix, but unrelated to this PR; ideally it rides its own PR or the base so it doesn't merge or revert with the workers work.Ticket
Delivered. The command does what the branch name promises — bounded reads,
--kind,--tail,--follow, machine formats, error mapping — CI is green, and the follow-mode correctness issues from the first pass are fixed and regression-tested.Overall: 0 fix-before-merge issue(s), 2 follow-up candidate(s).
## Summary Makes the `name` argument to `supabase experimental workers new` optional and prompts for it when it is omitted, so a bare `supabase experimental 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 experimental 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. ## Stack Bottom of the workers stack, on `develop`. Above it: output polish (supabase#6389), `workers logs` (supabase#6410), and `push --wait` (supabase#6371). ## Linked issue FUNC-840 (Linear). Supabase maintainer, exempt from the `open-for-contribution` flow. ## Checklist - [x] The PR title follows [Conventional Commits](https://www.conventionalcommits.org/)
…base#6389) ## Summary 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. **No command changes what it does** — this is output, plus the coverage that pins it. - **Success trailers.** "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. - **`list` advisories.** Both 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, directly 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 — `renderGlamourTable` sizes each column to its widest cell and never wraps. `status` still renders it vertically, and every machine format still carries `url` per worker. - **`push` progress.** Per-worker announcements are counted (`Deploying Worker 1/2:`) and a multi-worker run closes with a summary. Each worker takes minutes; the name alone said nothing about how much of the run was left. - **`push` names what it 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. - **`--project-ref` survives into `push`'s retry suggestions**, 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. Also adds unit coverage for `legacyRenderWorkerDetails`, pins the shared `-o env` refusal, and adds a guard (own commit) asserting no legacy boolean flag ships required — `Flag.boolean` alone builds a *required* param, and nothing in the existing suites notices. ## Stack On top of the `workers new` name prompt (supabase#6349). Above it: `workers logs` (supabase#6410), then `push --wait` (supabase#6371) last, so the output work can ship independently of both. ## Linked issue [FUNC-851](https://linear.app/supabase/issue/FUNC-851/general-output-polish). Supabase maintainer, exempt from the `open-for-contribution` flow. ## Checklist - [x] The PR title follows [Conventional Commits](https://www.conventionalcommits.org/) --------- Co-authored-by: kanad <git@kanad.dev>
…ogs-command # Conflicts: # apps/cli/src/shared/workers/workers-api.ts
…indow `followWindow`'s `graceSeconds`/`spanMinutes` options and `logWindow`'s `spanMinutes` parameter had no caller and no test — both call sites pass nothing — so they were configurability for its own sake over the module's trickiest arithmetic. Both now read the module constants directly. `followWindow` had no direct unit tests at all. It has four now, including the suspend case the 24h clamp exists for: a cursor left days behind would otherwise ask for an over-wide span, which the server answers by rewriting `end` to `start + 24h` — returning an older slice rather than a truncated one, so a resumed tail would silently replay yesterday. The expectations are written as literals rather than read from the constants under test, so they cannot stay green through the change they exist to catch.
The `FOLLOW_MAX_PAGES` comment claimed rows past the bound were re-asked for on
the next poll. They are not. The drain walks `end` backwards, so the pages it
does fetch are the newest ones, and the cursor then advances to the newest row
printed — past a region it never reached. Only the part of that region inside
the next window's grace comes back.
Lowering the cursor cannot fix it: `followWindow` moves the window's floor, not
its ceiling, so a poll anchored at `now` would re-fetch the same newest pages
and never walk down to the gap. The bound stays, and the loss is now reported
instead of silent — once per run, on stderr, in every output format, since a
`stream-json` consumer cannot infer a hole from the events it receives.
Corrects the rate arithmetic in the same pass. Both the handler comment ("~7
requests in the worst 60-second window") and SIDE_EFFECTS ("6 requests a
minute") predate the multi-page drain and assume one request per poll. A quiet
tail does spend 6 a minute; a poll draining a burst spends up to 5, so a
sustained backlog reaches 30 against a limit of 10 and is throttled by the
retry rather than budgeted for.
…gnosis" This reverts b772491. The fix is sound but has nothing to do with workers logs; it landed here to stabilise this branch's CI. Moved to #6455 off develop so it does not merge or revert with the workers work. Raised in review on #6410. Until #6455 lands, the `packages/config` release-script test it stabilises can flake on a loaded runner.
…supabase#6371) ## Summary `supabase experimental workers push` blocks on the server-side container build. That build routinely runs for minutes, so the common case — a deploy that builds fine — is the slowest thing in the loop. `--no-wait` returns once the platform accepts the deploy, which is the last thing the command 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. Waiting stays the default, so a plain push still reports the build's verdict and existing invocations are unchanged. - Under `--no-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 points at `experimental workers status` for the build's outcome. Text output only; machine callers read `build_state` from the payload. - A deploy answered with a spec already in `failed` is reported as a failure whether or not the build was waited on, rather than exiting zero on a worker that will never come up. The second commit is a separate fix that fell out of the review: the wait now runs only when the deploy response left `build_state` at `building`. `V2DeployAWorkerOutput` permits a terminal `active` or `failed` on the deploy response itself, and polling on top of that could only contradict it — `awaitWorkerBuild` reads a post-deploy 404 as "still building", so an already-failed deploy could burn the full poll budget and surface as a timeout rather than the failure the platform had already reported. ## Stack Top of the workers stack, on top of `workers logs` (supabase#6410). Everything below it — the name prompt (supabase#6349), the output polish (supabase#6389) and `workers logs` — is independent of this flag, so this PR can be rejected on its own without holding any of them up. ## Linked issue FUNC-848 (Linear). Supabase maintainer, exempt from the `open-for-contribution` flow. ## Checklist - [x] The PR title follows [Conventional Commits](https://www.conventionalcommits.org/)
Summary
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.--kind app|requests|buildsnarrows to one of the three streams; without it all three are returned, tagged per line.--tailcaps the rows.--followkeeps printing until interrupted.Three non-obvious things about that endpoint
Each is documented at its call site, because none is guessable from the API surface:
log_attributes, not thesourcecolumn. Worker rows carry an empty top-levelsource, because the Workers Logflare source is not enrolled as a category in the generic logs path, sowhere source = 'worker_guest_logs'matches nothing. Thein (...)list over the three known streams is a tenancy guard rather than a convenience — withsourceempty it is the only thing excluding a non-worker row that happens to carry aworkerattribute.start + 24h, returning an older slice than the one asked for rather than a truncated one.error, so the envelope is checked beforeresult.The response is decoded against a local schema rather than the generated
V1GetProjectLogsOutput: that schema marksresult/erroroptional but permits neither to benull, while the endpoint always sends one of them as an explicitnull. Decoding a real response against it always fails — worth fixing in the spec separately.Rendering
Per-stream, because
event_messagediffers in kind: on the request stream it is only"GET /", with status and duration inlog_attributes, so the request line is composed.severity_textis ignored — it isINFOon every row of every stream — so the level is derived, and app lines report none rather than a guess. An app message is tenant-controlled bytes, so escape sequences are stripped before it reaches a terminal while a stack trace's newlines and indentation survive.--followThe 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. It polls every 10 seconds, measured at ~7 requests in the worst 60-second window.
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; dedupe on the Logflare-minted
idis what makes it invisible.-o json|yaml|tomland--output-format jsonare refused up front — each promises one terminal payload and a tail has no last element.--output-format stream-jsonemits onelog-entryevent per line. SIGINT exits 130.Stack
On top of the workers output polish (#6389), with
push --wait(#6371) stacked above so it can be rejected independently. Below those: theworkers newname prompt (#6349).Note
Replaces #6408, which GitHub marked merged during a stack reorder. It was never merged to
develop; the branch and its commits are intact here.