feat(cli): prompt for worker name if not provided - #6349
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12693d1a8b
ℹ️ 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@9ae1f387f8d9343a7f43976b4be106372762b3e6Preview package for commit |
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. |
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.
fecde22 to
d4e37d5
Compare
`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.
edc7199 to
5c84df0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c84df0b91
ℹ️ 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".
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/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.
Summary
Makes the
nameargument tosupabase experimental workers newoptional and prompts for itwhen it is omitted, so a bare
supabase experimental workers newwalks through name, runtimeand 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 andsize 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|envor nointeractive terminal, the command fails with a new
MissingWorkerNameErrorpointing 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.tomlalready 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:
-oleavesoutput.formatastext, and Clack writes itsterminal 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.
mockOutputnow recordspromptTextCallsso tests can assert on the prompt'smessage and exercise its
validatecallback.Stack
Bottom of the workers stack, on
develop. Above it: output polish (#6389),workers logs(#6410), andpush --wait(#6371).Linked issue
FUNC-840 (Linear). Supabase maintainer, exempt from the
open-for-contributionflow.Checklist