fix: improve create sites and login directions#8126
Conversation
…active environments
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated login-request output to include an explicit instruction that the authorization URL must be given to the user before polling Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
📊 Benchmark resultsComparing with 5d18836
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/sites/sites-create.ts`:
- Around line 69-77: The retry logic in createSiteWithRetry currently appends
"-123" to the original nameAttempt which can produce slugs longer than the
allowed length; instead, reuse the normalized name you send to the API (the
trimmed/normalized value used in body.name) and when generating a retry suffix
(e.g., '-123') first truncate that normalized base to (MAX_SLUG_LENGTH -
suffix.length) before concatenating the suffix so retries always produce valid
slugs; update the code paths that build nameAttempt on retry (references:
createSiteWithRetry, nameAttempt, body.name) to perform
normalization->truncate->append suffix consistently.
In `@tests/integration/commands/sites/sites.test.ts`:
- Around line 117-159: The test "should fail after max retries in
non-interactive mode" never forces non-interactive behavior so it can take the
interactive 422 branch; before calling program.parseAsync you should stub the
same signals used by isInteractive() to force non-interactive mode (e.g. set
process.stdin.isTTY = false and set an env flag like process.env.CI = 'true' or
the specific env var your isInteractive() checks). Update the test (around the
program.parseAsync invocation in tests/integration/commands/sites/sites.test.ts)
to set those values so src/commands/sites/sites-create.ts will follow the
non-interactive retry/fail path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8013df9c-fd02-43a2-888b-b21fc0c6db42
📒 Files selected for processing (4)
src/commands/login/login-request.tssrc/commands/sites/sites-create.tstests/integration/commands/sites/sites.test.tstests/unit/commands/login/login-request.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/sites/sites-create.ts (1)
13-25:⚠️ Potential issue | 🟡 MinorReuse
validateSiteNamein the prompt validator.The prompt still only checks the character class. A
>63character interactive name now skips local validation and depends on the API to reject it; if that comes back as422, this new retry flow points the user at “already exists” instead of the real validation error. Reusing the shared validator here keeps the prompt and retry path aligned.💡 Proposed fix
-import { MAX_SITE_NAME_LENGTH } from '../../utils/validation.js' +import { MAX_SITE_NAME_LENGTH, validateSiteName } from '../../utils/validation.js' @@ - validate: (input) => - /^[a-zA-Z\d-]+$/.test(input || undefined) || 'Only alphanumeric characters and hyphens are allowed', + validate: (input) => { + if (input === '') { + return true + } + + try { + validateSiteName(input) + return true + } catch (error_) { + return error_ instanceof Error ? error_.message : 'Invalid project name' + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/sites/sites-create.ts` around lines 13 - 25, The prompt validator in getSiteNameInput only checks character class and should reuse the shared validateSiteName function to ensure length and other rules are enforced locally; update the inquirer prompt in getSiteNameInput to call validateSiteName (import it from its module) instead of the inline /^[a-zA-Z\d-]+$/ check so interactive validation matches the API/retry validation and correctly rejects >63-char names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/commands/sites/sites-create.ts`:
- Around line 13-25: The prompt validator in getSiteNameInput only checks
character class and should reuse the shared validateSiteName function to ensure
length and other rules are enforced locally; update the inquirer prompt in
getSiteNameInput to call validateSiteName (import it from its module) instead of
the inline /^[a-zA-Z\d-]+$/ check so interactive validation matches the
API/retry validation and correctly rejects >63-char names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26b2f2cb-d42f-4636-84e8-a750c069e8ad
📒 Files selected for processing (3)
src/commands/sites/sites-create.tssrc/utils/validation.tstests/integration/commands/sites/sites.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration/commands/sites/sites.test.ts
jaredm563
left a comment
jaredm563
left a comment
There was a problem hiding this comment.
We're only checking for HTTP 422, is that gauraunteed to always be a slug collision? just wondering if its possible to pick up a different error that should be terminal
src/commands/sites/sites-create.ts
Outdated
| } | ||
|
|
||
| if (isInteractive()) { | ||
| warn(`${nameAttempt || 'Site name'}.netlify.app already exists. Please try a different slug.`) |
There was a problem hiding this comment.
site name.netlify.app feels wrong? :)
src/commands/sites/sites-create.ts
Outdated
| let retries = 0 | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
| while (true) { |
There was a problem hiding this comment.
I'd suggest to check for retries and max_name_retries here rather than just while(true)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/sites/sites-create.ts`:
- Around line 152-154: The interactive branch currently forces a non-empty site
name by calling getSiteNameInput(options.name) which rejects blank answers and
prevents the "leave blank for random name" flow; update the call/site logic so
an empty response is allowed and treated as no name (i.e., undefined) before
calling createSiteWithRetry. Concretely, either adjust getSiteNameInput to
accept an option like allowEmpty and return an empty string as undefined, or
after receiving { name: siteName } map empty string to undefined (e.g., const
name = siteName || undefined) and pass that to createSiteWithRetry; reference
isInteractive, options.name, getSiteNameInput, and createSiteWithRetry to locate
the relevant code.
- Around line 83-85: Pre-validate the chosen slug with the shared site-name
helper (the same validation used by getSiteNameInput) before calling the Sites
API to catch >64-char and other local validation failures, and change the catch
logic in tryCreateSiteInteractive (and the non-interactive creation branch) to
only treat 422 as a retryable "name already exists" when the API error payload
explicitly indicates a collision (e.g. check (error_ as APIError).status === 422
&& (error_.body?.title || error_.message || error_.response?.body?.message)
contains a name-taken indicator), otherwise rethrow or surface the original
error; ensure warn(`${attemptName}.netlify.app already exists...`) and the
recursive retry are only executed in that explicit-collision case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64e06256-dd05-46e0-b986-8a38a42b5cb3
📒 Files selected for processing (1)
src/commands/sites/sites-create.ts
| if ((error_ as APIError).status === 422) { | ||
| warn(`${siteName}.netlify.app already exists. Please try a different slug.`) | ||
| await inputSiteName() | ||
| } else { | ||
| return logAndThrowError( | ||
| `createSiteInTeam error: ${(error_ as APIError).status}: ${(error_ as APIError).message}`, | ||
| ) | ||
| warn(`${attemptName}.netlify.app already exists. Please try a different slug.`) | ||
| return tryCreateSiteInteractive(undefined) |
There was a problem hiding this comment.
Only retry on an actual slug-conflict 422.
These branches currently collapse every 422 into “name already exists”. getSiteNameInput() still doesn’t reject 64+ character slugs, so an overlength name will land here and surface the wrong remediation; the non-interactive path will also burn its retries and finish with the same misleading error. Please validate with the shared site-name helper before the API call, or only retry when the API explicitly reports a collision.
Also applies to: 120-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/sites/sites-create.ts` around lines 83 - 85, Pre-validate the
chosen slug with the shared site-name helper (the same validation used by
getSiteNameInput) before calling the Sites API to catch >64-char and other local
validation failures, and change the catch logic in tryCreateSiteInteractive (and
the non-interactive creation branch) to only treat 422 as a retryable "name
already exists" when the API error payload explicitly indicates a collision
(e.g. check (error_ as APIError).status === 422 && (error_.body?.title ||
error_.message || error_.response?.body?.message) contains a name-taken
indicator), otherwise rethrow or surface the original error; ensure
warn(`${attemptName}.netlify.app already exists...`) and the recursive retry are
only executed in that explicit-collision case.
| } else if (isInteractive() && !options.name) { | ||
| const { name: siteName } = await getSiteNameInput(options.name) | ||
| await createSiteWithRetry(siteName) |
There was a problem hiding this comment.
Interactive sites:create still blocks the random-name path.
This branch always sends interactive no---name usage through getSiteNameInput(), but that prompt rejects an empty answer. Users can’t actually “leave blank for a random name” here, so the documented flow is impossible. If random names are still supported, let an empty answer skip body.name instead of treating the prompt as required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/sites/sites-create.ts` around lines 152 - 154, The interactive
branch currently forces a non-empty site name by calling
getSiteNameInput(options.name) which rejects blank answers and prevents the
"leave blank for random name" flow; update the call/site logic so an empty
response is allowed and treated as no name (i.e., undefined) before calling
createSiteWithRetry. Concretely, either adjust getSiteNameInput to accept an
option like allowEmpty and return an empty string as undefined, or after
receiving { name: siteName } map empty string to undefined (e.g., const name =
siteName || undefined) and pass that to createSiteWithRetry; reference
isInteractive, options.name, getSiteNameInput, and createSiteWithRetry to locate
the relevant code.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/commands/sites/sites-create.ts (2)
83-85:⚠️ Potential issue | 🟠 MajorDon't map every
422to “name already exists”.This file still lets invalid slugs reach
createSiteInTeam, so non-conflict422s will be misreported as collisions here. In non-interactive mode that also burns the retry budget before throwing the wrong error. Only enter the retry/re-prompt path once the failure is known to be a real name conflict.Also applies to: 118-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/sites/sites-create.ts` around lines 83 - 85, The current handlers in tryCreateSiteInteractive (and the similar block at lines 118-123) treat any 422 APIError as "name already exists", which misreports other validation failures and can consume retries; update these blocks to detect a true name-conflict before retrying: inspect the APIError payload returned by createSiteInTeam (e.g., error_.status === 422 and error_.body or error_.error fields) for the specific conflict indicator (the provider-specific error code/message that denotes a slug/name collision) and only call warn(...) and retry via tryCreateSiteInteractive when that exact conflict marker is present; otherwise, rethrow or surface the original validation error so non-conflict 422s are not misclassified or counted against the retry budget.
70-71:⚠️ Potential issue | 🟠 MajorThe interactive “random name” path is still unreachable.
The prompt still advertises blank input as “random name”, but this flow never preserves that state: falsy
siteNamevalues get routed straight back intogetSiteNameInput()and prompt again instead of callingcreateSiteInTeamwith an empty body.Also applies to: 132-133, 148-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/sites/sites-create.ts` around lines 70 - 71, The prompt treats blank input as "random name" but the code reruns getSiteNameInput whenever the returned siteName is falsy; update tryCreateSiteInteractive and the other similar blocks (the ones referencing getSiteNameInput around the later occurrences) so they only re-prompt when the returned name is strictly undefined (user cancelled/no value), but allow an empty string ("") to be passed through to createSiteInTeam to trigger the random-name path; in short, change the falsy check to an explicit undefined check for the result of getSiteNameInput (use attemptName === undefined) and call createSiteInTeam with attemptName (including "") when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/commands/sites/sites-create.ts`:
- Around line 83-85: The current handlers in tryCreateSiteInteractive (and the
similar block at lines 118-123) treat any 422 APIError as "name already exists",
which misreports other validation failures and can consume retries; update these
blocks to detect a true name-conflict before retrying: inspect the APIError
payload returned by createSiteInTeam (e.g., error_.status === 422 and
error_.body or error_.error fields) for the specific conflict indicator (the
provider-specific error code/message that denotes a slug/name collision) and
only call warn(...) and retry via tryCreateSiteInteractive when that exact
conflict marker is present; otherwise, rethrow or surface the original
validation error so non-conflict 422s are not misclassified or counted against
the retry budget.
- Around line 70-71: The prompt treats blank input as "random name" but the code
reruns getSiteNameInput whenever the returned siteName is falsy; update
tryCreateSiteInteractive and the other similar blocks (the ones referencing
getSiteNameInput around the later occurrences) so they only re-prompt when the
returned name is strictly undefined (user cancelled/no value), but allow an
empty string ("") to be passed through to createSiteInTeam to trigger the
random-name path; in short, change the falsy check to an explicit undefined
check for the result of getSiteNameInput (use attemptName === undefined) and
call createSiteInTeam with attemptName (including "") when present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e51cf1d6-5559-4c62-8542-8459b4c317cd
📒 Files selected for processing (1)
src/commands/sites/sites-create.ts
🤖 I have created a release *beep* *boop* --- ## [24.8.2](v24.8.1...v24.8.2) (2026-03-31) ### Bug Fixes * **deps:** bump path-to-regexp ([#8123](#8123)) ([5d18836](5d18836)) * improve create sites and login directions ([#8126](#8126)) ([6bfc767](6bfc767)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
4 participants
🎉 Thanks for submitting a pull request! 🎉
Summary
Right now, if there is a new site name conflict it always forces the user to an interactive prompt. With this change we are going to be more consistent with other commands that, if site names have conflicts, it will try with a suffix appended to the name.
Also, adding a little more direction for agents to handle login requests
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)