fix: improve create sites and login directions (#8126)

🎉 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:

- [ ] Open a
[bug/issue](https://github.com/netlify/cli/issues/new/choose) before
writing your code 🧑‍💻. This ensures we can discuss the changes and get
feedback from everyone that should be involved. If you\`re fixing a typo
or something that\`s on fire 🔥 (e.g. incident related), you can skip
this step.
- [ ] Read the [contribution guidelines](../CONTRIBUTING.md) 📖. This
ensures your code follows our style guide and
      passes our tests.
- [ ] Update or add tests (if any source code was changed or added) 🧪
- [ ] Update or add documentation (if features were changed or added) 📝
- [ ] Make sure the status checks below are successful ✅

**A picture of a cute animal (not mandatory, but encouraged)**

---------

Co-authored-by: Sean Roberts <sean.roberts@netlify.com>

Author: sean-roberts

src/commands/login/login-request.ts

@@ -22,11 +22,13 @@ export const loginRequest = async (message: string, apiOpts: NetlifyOptions['api
     url,
     check_command: `netlify login --check ${ticketId}`,
     agent_next_steps:
-      'Poll the check_command for up to ten minutes to see if the user has logged in, or wait for them to tell you and then use check_command after.',
+      'Give the URL to the user so they can authorize. Then poll the check_command for up to ten minutes to see if the user has logged in, or wait for them to tell you and then use check_command after.',
   })
 
   log(`Ticket ID: ${ticketId}`)
   log(`Authorize URL: ${url}`)
   log()
   log(`After authorizing, run: netlify login --check ${ticketId}`)
+  log()
+  log('After user opens the authorization URL and approves, the login will be complete.')
 }

src/commands/sites/sites-create.ts

@@ -10,6 +10,7 @@ import { isInteractive } from '../../utils/scripted-commands.js'
 import { resolveTeamForNonInteractive } from '../../utils/team.js'
 import { track } from '../../utils/telemetry/index.js'
 import type { SiteInfo } from '../../utils/types.js'
+import { MAX_SITE_NAME_LENGTH } from '../../utils/validation.js'
 import type BaseCommand from '../base-command.js'
 import { link } from '../link/link.js'
 
@@ -64,32 +65,77 @@ export const sitesCreate = async (options: OptionValues, command: BaseCommand) =
 
   let site!: SiteInfo
 
-  // Allow the user to reenter site name if selected one isn't available
-  const inputSiteName = async (name?: string) => {
-    const { name: siteName } = await getSiteNameInput(name)
+  const MAX_NAME_RETRIES = 2
 
+  const tryCreateSiteInteractive = async (nameToTry: string | undefined): Promise<void> => {
+    const { name: attemptName } = await getSiteNameInput(nameToTry)
     const body: { name?: string } = {}
-    if (typeof siteName === 'string') {
-      body.name = siteName.trim()
+    if (attemptName.trim()) {
+      body.name = attemptName.trim()
     }
+
     try {
-      // FIXME(serhalp): `id` and `name` should be required in `netlify` package type
       site = (await api.createSiteInTeam({
         accountSlug: accountSlug,
         body,
       })) as unknown as SiteInfo
     } catch (error_) {
       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)
       }
+
+      return logAndThrowError(`createSiteInTeam error: ${(error_ as APIError).status}: ${(error_ as APIError).message}`)
     }
   }
 
+  const createSiteWithRetry = async (siteName: string | undefined) => {
+    if (!isInteractive() && siteName) {
+      for (let attempt = 0; attempt <= MAX_NAME_RETRIES; attempt++) {
+        let nameToTry = siteName
+
+        if (attempt > 0) {
+          const suffix = `-${Math.floor(Math.random() * 900 + 100).toString()}`
+          const normalizedBase = siteName.trim()
+          const maxBaseLength = MAX_SITE_NAME_LENGTH - suffix.length
+          const truncatedBase = normalizedBase.slice(0, maxBaseLength)
+          nameToTry = `${truncatedBase}${suffix}`
+          warn(`${siteName}.netlify.app already exists. Trying ${nameToTry}...`)
+        }
+
+        const body: { name?: string } = {}
+        if (nameToTry.trim()) {
+          body.name = nameToTry.trim()
+        }
+
+        try {
+          site = (await api.createSiteInTeam({
+            accountSlug: accountSlug,
+            body,
+          })) as unknown as SiteInfo
+          return
+        } catch (error_) {
+          if ((error_ as APIError).status === 422) {
+            if (attempt === MAX_NAME_RETRIES) {
+              return logAndThrowError(`Project name "${nameToTry}" is already taken. Please try a different name.`)
+            }
+            continue
+          }
+
+          return logAndThrowError(
+            `createSiteInTeam error: ${(error_ as APIError).status}: ${(error_ as APIError).message}`,
+          )
+        }
+      }
+    }
+
+    if (isInteractive()) {
+      return tryCreateSiteInteractive(siteName)
+    }
+
+    return logAndThrowError('Failed to create site: name already taken')
+  }
+
   if (!isInteractive() && !options.name) {
     try {
       site = (await api.createSiteInTeam({
@@ -99,8 +145,11 @@ export const sitesCreate = async (options: OptionValues, command: BaseCommand) =
     } catch (error_) {
       return logAndThrowError(`Failed to create site: ${(error_ as APIError).status}: ${(error_ as APIError).message}`)
     }
+  } else if (isInteractive() && !options.name) {
+    const { name: siteName } = await getSiteNameInput(options.name)
+    await createSiteWithRetry(siteName)
   } else {
-    await inputSiteName(options.name)
+    await createSiteWithRetry(options.name)
   }
 
   log()

src/utils/validation.ts

@@ -2,7 +2,7 @@ import { InvalidArgumentError } from 'commander'
 
 import { BANG, chalk } from './command-helpers.js'
 
-const MAX_SITE_NAME_LENGTH = 63
+export const MAX_SITE_NAME_LENGTH = 63
 
 export const validateSiteName = (value: string): string => {
   if (value.length > MAX_SITE_NAME_LENGTH) {

tests/integration/commands/sites/sites.test.ts

@@ -45,6 +45,8 @@ const routes = [
 ]
 
 const OLD_ENV = process.env
+const OLD_STDIN_TTY = process.stdin.isTTY
+const OLD_STDOUT_TTY = process.stdout.isTTY
 
 describe('sites command', () => {
   beforeEach(() => {
@@ -61,6 +63,8 @@ describe('sites command', () => {
     Object.defineProperty(process, 'env', {
       value: OLD_ENV,
     })
+    process.stdin.isTTY = OLD_STDIN_TTY
+    process.stdout.isTTY = OLD_STDOUT_TTY
   })
 
   describe('sites:create', () => {
@@ -113,5 +117,127 @@ describe('sites command', () => {
         logJsonSpy.mockRestore()
       })
     })
+
+    test('should fail after max retries in non-interactive mode', async () => {
+      const routesWithPersistentConflict = [
+        {
+          path: 'accounts',
+          response: [{ slug: 'test-account' }],
+        },
+        {
+          path: 'sites',
+          response: [],
+        },
+        {
+          path: 'user',
+          response: { name: 'test user', slug: 'test-user', email: 'user@test.com' },
+        },
+        {
+          path: 'test-account/sites',
+          method: 'POST' as const,
+          status: 422,
+          response: { message: 'site name already exists' },
+        },
+      ]
+
+      await withMockApi(routesWithPersistentConflict, async ({ apiUrl, requests }) => {
+        Object.assign(process.env, getEnvironmentVariables({ apiUrl }))
+        process.env.CI = 'true'
+        process.stdin.isTTY = false
+        process.stdout.isTTY = false
+
+        const program = new BaseCommand('netlify')
+        program.exitOverride()
+        createSitesCreateCommand(program)
+
+        const warnSpy = vi.spyOn(await import('../../../../src/utils/command-helpers.js'), 'warn')
+
+        await expect(async () => {
+          await program.parseAsync([
+            '',
+            '',
+            'sites:create',
+            '--name',
+            'taken-site',
+            '--account-slug',
+            'test-account',
+            '--disable-linking',
+          ])
+        }).rejects.toThrowError(/already taken/)
+
+        const siteCreateRequests = requests.filter(
+          (r) => r.path === '/api/v1/test-account/sites' && r.method === 'POST',
+        )
+        expect(siteCreateRequests).toHaveLength(3)
+
+        expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('taken-site.netlify.app already exists'))
+        expect(warnSpy).toHaveBeenCalledWith(expect.stringMatching(/Trying taken-site-\d{3}\.\.\./))
+
+        warnSpy.mockRestore()
+      })
+    })
+
+    test('should truncate long site names when retrying to avoid exceeding max length', async () => {
+      const routesWithPersistentConflict = [
+        {
+          path: 'accounts',
+          response: [{ slug: 'test-account' }],
+        },
+        {
+          path: 'sites',
+          response: [],
+        },
+        {
+          path: 'user',
+          response: { name: 'test user', slug: 'test-user', email: 'user@test.com' },
+        },
+        {
+          path: 'test-account/sites',
+          method: 'POST' as const,
+          status: 422,
+          response: { message: 'site name already exists' },
+        },
+      ]
+
+      await withMockApi(routesWithPersistentConflict, async ({ apiUrl, requests }) => {
+        Object.assign(process.env, getEnvironmentVariables({ apiUrl }))
+        process.env.CI = 'true'
+        process.stdin.isTTY = false
+        process.stdout.isTTY = false
+
+        const program = new BaseCommand('netlify')
+        program.exitOverride()
+        createSitesCreateCommand(program)
+
+        const longName = 'a'.repeat(60)
+
+        await expect(async () => {
+          await program.parseAsync([
+            '',
+            '',
+            'sites:create',
+            '--name',
+            longName,
+            '--account-slug',
+            'test-account',
+            '--disable-linking',
+          ])
+        }).rejects.toThrowError(/already taken/)
+
+        const siteCreateRequests = requests.filter(
+          (r) => r.path === '/api/v1/test-account/sites' && r.method === 'POST',
+        )
+
+        expect(siteCreateRequests).toHaveLength(3)
+
+        siteCreateRequests.slice(1).forEach((request) => {
+          const bodyName = (request.body as { name?: string }).name
+          expect(bodyName).toBeDefined()
+          if (bodyName) {
+            expect(bodyName.length).toBeLessThanOrEqual(63)
+          }
+        })
+      })
+    })
   })
 })

tests/unit/commands/login/login-request.test.ts

@@ -41,6 +41,7 @@ describe('loginRequest', () => {
       'Authorize URL: https://app.netlify.com/authorize?response_type=ticket&ticket=test-ticket-123',
     )
     expect(output).toContain('netlify login --check test-ticket-123')
+    expect(output).toContain('After user opens the authorization URL and approves, the login will be complete.')
   })
 
   test('passes message to createTicket', async () => {