feat: support --allow-anonymous deploys#8109
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds anonymous deploy capability and a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/utils/deploy/drop-api.ts (1)
117-134: Consider propagating non-transient errors instead of returningfalse.When
response.okisfalse, the function returnsfalsewhich causespWaitForto keep polling. This is appropriate for transient issues, but 4xx errors (e.g., 401, 403, 404) indicate permanent failures that should throw immediately rather than poll until timeout.♻️ Proposed fix to fail fast on client errors
const checkDeploy = async (): Promise<boolean> => { const response = await fetch(`${apiBase}/sites/${siteId}/deploys/${deployId}`, { headers: makeHeaders(userAgent), }) if (!response.ok) { + // 4xx errors are not transient - fail immediately + if (response.status >= 400 && response.status < 500) { + throw new Error(`Deploy status check failed: ${response.status} ${response.statusText}`) + } + // 5xx errors may be transient - continue polling return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/deploy/drop-api.ts` around lines 117 - 134, The checkDeploy function currently returns false whenever response.ok is false which causes pWaitFor to keep polling; change it to fail fast for client errors by inspecting response.status after the fetch in checkDeploy, throwing an Error for 4xx statuses (e.g., 400–499) with a descriptive message (include status and statusText or response body) instead of returning false, while retaining the existing behavior of returning false for transient server errors (5xx) so pWaitFor can continue polling; reference the checkDeploy function and the variables apiBase, siteId, deployId, and deploy when implementing this change.src/commands/deploy/deploy.ts (1)
1200-1209: Consider whether edge function dist path should be included for anonymous deploys.Line 1205 includes
edgeFunctionsDistPathin the hash directories, but lines 1161-1163 should have already blocked the deploy if edge functions exist. If this path can contain stale build artifacts from previous authenticated deploys, including them might cause unexpected behavior.If edge functions are blocked for anonymous deploys, consider excluding the dist path:
- const edgeFunctionsDistPath = await getEdgeFunctionsDistPathIfExists(workingDir) - const filter = getDeployFilesFilter({ site, deployFolder }) const { files, filesShaMap } = await hashFiles({ concurrentHash: DEFAULT_CONCURRENT_HASH, - directories: [deployFolder, edgeFunctionsDistPath].filter(Boolean), + directories: [deployFolder], filter,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 1200 - 1209, The code currently always adds edgeFunctionsDistPath to the directories passed to hashFiles (via edgeFunctionsDistPath from getEdgeFunctionsDistPathIfExists), which may include stale artifacts for anonymous deploys; change the directories array construction passed into hashFiles (the directories: [deployFolder, edgeFunctionsDistPath].filter(Boolean) argument in the call to hashFiles) to only include edgeFunctionsDistPath when the deploy is allowed to include edge functions (e.g., when the deploy is authenticated or when the earlier check that blocks edge functions for anonymous deploys indicates they are permitted); use the same flag/condition used earlier to block edge functions (the code that checks for anonymous deploy) so edgeFunctionsDistPath is excluded for anonymous deploys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/commands/deploy.md`:
- Line 34: The lint warning is caused by straight quotes in the example value
for the `created-via` field; update the docs entry for `created-via` to use
backticks (e.g., `cli`, `drop`) or smart/curly quotes instead of straight quotes
so the static analyzer stops flagging the example value.
In `@src/utils/deploy/drop-api.ts`:
- Line 88: The URL construction in the fetch call using
"files${encodeURI(filePath)}" can produce malformed URLs when filePath (or
normalizedPath) lacks a leading slash; update the code around the fetch (the
Response response = await fetch(...) call) to ensure filePath has a leading "/"
before encoding—e.g., normalize filePath to (filePath.startsWith("/") ? filePath
: "/" + filePath) and then use encodeURI on that value when building
`${apiBase}/deploys/${deployId}/files${encodedPath}` so the resulting URL is
always ".../files/<path>".
In `@tests/integration/commands/deploy/deploy-anonymous.test.ts`:
- Around line 315-318: The handler passed to withFunction currently uses a
synchronous arrow function for 'hello.js' but must be async to match the
expected handler signature; update the handler in the withFunction call (the
handler property) to be an async function (e.g., add the async keyword before
the arrow function) so the exported handler matches the async contract used
throughout the deploy tests.
---
Nitpick comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 1200-1209: The code currently always adds edgeFunctionsDistPath to
the directories passed to hashFiles (via edgeFunctionsDistPath from
getEdgeFunctionsDistPathIfExists), which may include stale artifacts for
anonymous deploys; change the directories array construction passed into
hashFiles (the directories: [deployFolder,
edgeFunctionsDistPath].filter(Boolean) argument in the call to hashFiles) to
only include edgeFunctionsDistPath when the deploy is allowed to include edge
functions (e.g., when the deploy is authenticated or when the earlier check that
blocks edge functions for anonymous deploys indicates they are permitted); use
the same flag/condition used earlier to block edge functions (the code that
checks for anonymous deploy) so edgeFunctionsDistPath is excluded for anonymous
deploys.
In `@src/utils/deploy/drop-api.ts`:
- Around line 117-134: The checkDeploy function currently returns false whenever
response.ok is false which causes pWaitFor to keep polling; change it to fail
fast for client errors by inspecting response.status after the fetch in
checkDeploy, throwing an Error for 4xx statuses (e.g., 400–499) with a
descriptive message (include status and statusText or response body) instead of
returning false, while retaining the existing behavior of returning false for
transient server errors (5xx) so pWaitFor can continue polling; reference the
checkDeploy function and the variables apiBase, siteId, deployId, and deploy
when implementing this change.
🪄 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: cfb706a7-7306-42ef-99b5-e971e7c025be
📒 Files selected for processing (6)
docs/commands/deploy.mdsrc/commands/deploy/deploy.tssrc/commands/deploy/index.tssrc/commands/deploy/option_values.tssrc/utils/deploy/drop-api.tstests/integration/commands/deploy/deploy-anonymous.test.ts
|
|
||
| - `alias` (*string*) - Specifies the alias for deployment, the string at the beginning of the deploy subdomain. Useful for creating predictable deployment URLs. Avoid setting an alias string to the same value as a deployed branch. `alias` doesn’t create a branch deploy and can’t be used in conjunction with the branch subdomain feature. Maximum 37 characters. | ||
| - `alias` (*string*) - Specifies the alias for deployment, the string at the beginning of the deploy subdomain. Useful for creating predictable deployment URLs. Avoid setting an alias string to the same value as a deployed branch. `alias` doesn't create a branch deploy and can't be used in conjunction with the branch subdomain feature. Maximum 37 characters. | ||
| - `allow-anonymous` (*boolean*) - If not logged in, deploy anonymously and create a claimable site instead of requiring authentication |
There was a problem hiding this comment.
Could it be an option to just default to this flow when not signed in? Or would that be annoying for folks who think they're signed in but then realised are not and end up with a drop site without intending?
Just trying to see if we can avoid an extra flag that agents will also need to learn to use.
There was a problem hiding this comment.
yeah I went back and forth on this. I don't want to cause unnecessary issues with deploying things inadvertently. It's the same with the --create-site flag too, should we just create one if the site's not linked. Right now our precedence is to look for intential decisions there.
src/commands/deploy/index.ts
Outdated
| false, | ||
| ) | ||
| .option('--created-via <source>', 'Specify the source of the deploy (e.g., "cli", "drop")') | ||
| .addOption(new Option('--claim-site <siteId>', 'Claim an anonymous deploy site').hideHelp(true)) |
There was a problem hiding this comment.
I think it's a bit odd to have the claim flow as part of the deploy command. We're not deploying anything at this point, right?
I could recommend a new claim command that just takes a --site and --token flags. Even nicer would be if the token has the site ID and any other metadata needed, so people would just do netlify claim <token>.
There was a problem hiding this comment.
I'm down with that
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 `@tests/integration/commands/deploy/deploy-anonymous.test.ts`:
- Around line 1-12: The test file has Prettier/formatting errors; run the
project's formatter (npm run format) or apply Prettier to the file to fix
styling so CI passes, then re-run tests; ensure imports and spacing around the
top-level symbols (callCli, getCLIOptions, startDeployMockApi, withSiteBuilder)
conform to the repo's Prettier rules and commit the formatted changes.
- Around line 356-359: The edge function handler passed to withEdgeFunction is
synchronous; change the handler provided to the withEdgeFunction call (the
object with name 'hello') to use an async arrow function (i.e., make handler
async () => new Response('Hello')) so it matches other tests like deploy.test.ts
and maintains consistency in edge function handlers across the suite.
🪄 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: 530ea82d-a12d-483b-8977-4233555b32e8
📒 Files selected for processing (1)
tests/integration/commands/deploy/deploy-anonymous.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/commands/deploy/deploy.ts (1)
1231-1241: Consider more robust 429 detection.The error detection
String(error).includes('429')is fragile as it depends on the error message format. If the error structure changes, this check could silently fail.♻️ Suggested improvement for more reliable error handling
} catch (error) { - if (String(error).includes('429')) { + const errorMessage = error instanceof Error ? error.message : String(error) + if (errorMessage.includes('429') || (error as { status?: number }).status === 429) { const loginCommand = isInteractive()Alternatively, consider throwing a typed error from
getDropTokenthat includes astatusproperty for more reliable checking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 1231 - 1241, The catch block in deploy.ts currently checks errors with String(error).includes('429'), which is brittle; instead inspect a structured status property (e.g., error.status || error.response?.status) or make getDropToken throw a typed error with a status code and then check that property in the catch. Update the catch in the deploy command (the block surrounding getDropToken handling) to detect 429 by reading error.status or error.response.status, and fall back to the string check only as a last resort, then call logAndThrowError with the same message; ensure isInteractive remains used to build loginCommand.tests/integration/commands/claim/claim.test.ts (1)
22-66: LGTM — Good integration test for the claim command happy path.The test properly validates:
- CLI output contains success message
- Correct request sent to
/drop/claimendpoint- State file updated with claimed site ID
Consider adding test cases for error scenarios in a follow-up:
- Claim with invalid/expired token
- Claim when already claimed
- Network error handling
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/commands/claim/claim.test.ts` around lines 22 - 66, The test file tests/integration/commands/claim/claim.test.ts currently covers the happy path; add follow-up tests that exercise error scenarios: (1) invalid/expired token — configure startDeployMockApi to respond to POST /drop/claim with 401/403 and assert callCli output/error and that .netlify/state.json is not written/unchanged (use withSiteBuilder, callCli, readFile as in the existing test), (2) already claimed — mock a 409 response and assert proper CLI message and no state update, and (3) network error handling — simulate network failure (e.g., have the mock API close or time out before the POST) and assert the CLI surfaces a network error and no state change; reuse helper functions startDeployMockApi, withSiteBuilder, callCli and the same assertions pattern to keep tests consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/commands/claim.md`:
- Around line 14-16: The usage example currently shows "netlify claim" but the
command requires a <siteId> argument; update the usage block in
docs/commands/claim.md so the example line uses "netlify claim <siteId>"
(matching the documented syntax and the example shown later) to accurately
reflect the required argument.
- Around line 24-25: The docs entry for the claim command needs to mark the
token flag as required: update the docs/commands/claim.md flags list so the
`token` entry reads something like `token` (*string*) - The drop token provided
when the site was deployed (required); this should match the required flag
definition used by the claim command (flags.token / --token).
In `@src/commands/deploy/deploy.ts`:
- Around line 1219-1222: The dropApiOptions object is using the wrong header key
casing so it always falls back to 'netlify-cli'; update the userAgent assignment
in the dropApiOptions creation to read
command.netlify.api.defaultHeaders['User-Agent'] (capital A) instead of
['User-agent'] so the actual API client User-Agent is used when present.
---
Nitpick comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 1231-1241: The catch block in deploy.ts currently checks errors
with String(error).includes('429'), which is brittle; instead inspect a
structured status property (e.g., error.status || error.response?.status) or
make getDropToken throw a typed error with a status code and then check that
property in the catch. Update the catch in the deploy command (the block
surrounding getDropToken handling) to detect 429 by reading error.status or
error.response.status, and fall back to the string check only as a last resort,
then call logAndThrowError with the same message; ensure isInteractive remains
used to build loginCommand.
In `@tests/integration/commands/claim/claim.test.ts`:
- Around line 22-66: The test file
tests/integration/commands/claim/claim.test.ts currently covers the happy path;
add follow-up tests that exercise error scenarios: (1) invalid/expired token —
configure startDeployMockApi to respond to POST /drop/claim with 401/403 and
assert callCli output/error and that .netlify/state.json is not
written/unchanged (use withSiteBuilder, callCli, readFile as in the existing
test), (2) already claimed — mock a 409 response and assert proper CLI message
and no state update, and (3) network error handling — simulate network failure
(e.g., have the mock API close or time out before the POST) and assert the CLI
surfaces a network error and no state change; reuse helper functions
startDeployMockApi, withSiteBuilder, callCli and the same assertions pattern to
keep tests consistent.
🪄 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: 2798e588-91d1-4537-a2f6-8599a3a83218
📒 Files selected for processing (11)
docs/commands/claim.mddocs/index.mdsrc/commands/claim/claim.tssrc/commands/claim/index.tssrc/commands/deploy/deploy.tssrc/commands/deploy/index.tssrc/commands/deploy/option_values.tssrc/commands/main.tssrc/utils/deploy/drop-api.tstests/integration/commands/claim/claim.test.tstests/integration/commands/deploy/deploy-anonymous.test.ts
✅ Files skipped from review due to trivial changes (3)
- docs/index.md
- src/commands/deploy/option_values.ts
- src/commands/deploy/index.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/commands/claim.md (1)
14-16:⚠️ Potential issue | 🟡 MinorUsage line is missing the required positional argument.
The usage block should include
<siteId>to match the required argument and the example below.📝 Proposed fix
```bash -netlify claim +netlify claim <siteId></details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/commands/claim.mdaround lines 14 - 16, The usage line in
docs/commands/claim.md currently shows "netlify claim" but is missing the
required positional argument; update the usage block to include the siteId
placeholder (i.e., change the example/usage from "netlify claim" to "netlify
claim ") so it matches the required argument and the example below.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/commands/deploy/deploy.ts (1)</summary><blockquote> `1203-1209`: **Consider using a type predicate for type-safe filtering.** `filter(Boolean)` doesn't narrow the array type in TypeScript. If `edgeFunctionsDistPath` is `undefined`, the result type remains `(string | undefined)[]` rather than narrowing to `string[]`. <details> <summary>♻️ Suggested improvement</summary> ```diff const { files, filesShaMap } = await hashFiles({ concurrentHash: DEFAULT_CONCURRENT_HASH, - directories: [deployFolder, edgeFunctionsDistPath].filter(Boolean), + directories: [deployFolder, edgeFunctionsDistPath].filter((dir): dir is string => Boolean(dir)), filter, normalizer: deployFileNormalizer.bind(null, workingDir), statusCb: options.json ? () => {} : deployProgressCb(), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 1203 - 1209, The array passed to hashFiles uses [deployFolder, edgeFunctionsDistPath].filter(Boolean) which leaves the union type (string | undefined)[]; change the filter to a type predicate so TypeScript narrows to string[] (e.g., replace filter(Boolean) with a predicate like (p): p is string => p !== undefined && p !== null) and keep the existing call site in deploy.ts that constructs the directories argument for hashFiles; this will ensure files/directory types are string[] when calling hashFiles and remove the need for unsafe casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/commands/claim.md`:
- Around line 14-16: The usage line in docs/commands/claim.md currently shows
"netlify claim" but is missing the required positional argument; update the
usage block to include the siteId placeholder (i.e., change the example/usage
from "netlify claim" to "netlify claim <siteId>") so it matches the required
argument and the example below.
---
Nitpick comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 1203-1209: The array passed to hashFiles uses [deployFolder,
edgeFunctionsDistPath].filter(Boolean) which leaves the union type (string |
undefined)[]; change the filter to a type predicate so TypeScript narrows to
string[] (e.g., replace filter(Boolean) with a predicate like (p): p is string
=> p !== undefined && p !== null) and keep the existing call site in deploy.ts
that constructs the directories argument for hashFiles; this will ensure
files/directory types are string[] when calling hashFiles and remove the need
for unsafe casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0374645f-104d-45fe-98b5-2fdf6a17110f
📒 Files selected for processing (5)
docs/commands/claim.mdsrc/commands/claim/index.tssrc/commands/deploy/deploy.tssrc/utils/deploy/drop-api.tstests/integration/commands/claim/claim.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/commands/claim/index.ts
- src/utils/deploy/drop-api.ts
src/commands/deploy/deploy.ts
Outdated
| deployInfo = await createDropDeploy(dropApiOptions, files, dropToken, options.createdVia) | ||
| } catch (error) { | ||
| const dropError = error as DropApiError | ||
| if (dropError.status === 429 || String(error).includes('429')) { |
There was a problem hiding this comment.
This string check feels a bit brittle. Can we not rely on the status property alone?
src/commands/deploy/deploy.ts
Outdated
| if (options.allowAnonymous) { | ||
| const [token] = await getToken(options.auth) | ||
| if (token) { | ||
| log(`${NETLIFYDEVLOG} You are logged in — deploying normally.`) |
There was a problem hiding this comment.
I understand that "deploying normally" is in opposition to anonymously, but this feels a bit out of place. Do we have to say anything here?
There was a problem hiding this comment.
I'd like to make it clear what we are doing. Will change it to deploying to your team. which feels less odd.
| 'Specify team slug when creating a site. Only works with --create-site or --site-name flag.', | ||
| ) | ||
| .option( | ||
| '--allow-anonymous', |
There was a problem hiding this comment.
For consistency with the existing product surface, how would you feel about calling this --drop?
There was a problem hiding this comment.
My thinking... it's not really the same as drop, just using the drop APIs for when it needs to be anonymous but it being anonymous doesn't make it a drop - authed users also use drop. Drop is simply the deploy itself so it'd be more appropriate to call the command itself drop instead of deploy as that's the level that it is which we wouldn't want to do. So really, it's a question of how are we doing this deploy
There was a problem hiding this comment.
Makes sense. There's something I don't love about --allow-anonymous, but I can't quite put my finger on it. I'm sure it's fine, let's go with it.
There was a problem hiding this comment.
if we figure it out, I'll happily alias :)
| 'If not logged in, deploy anonymously and create a claimable site instead of requiring authentication', | ||
| false, | ||
| ) | ||
| .option('--created-via <source>', 'Specify the source of the deploy (e.g., "cli", "drop")') |
There was a problem hiding this comment.
I understand that this information may be useful to us, but it feels we should be able to capture it ourselves as opposed to making it a user-facing flag. We can use the user agent (in addition to other metadata) to know if the deploy comes from the CLI. Can we not extend that mechanism to capture everything we need?
There was a problem hiding this comment.
we don't want to say this is from the CLI if we can say this deploy is from Clawbot or Claude or Codex whatever it is. That's the part that's needed here.
🤖 I have created a release *beep* *boop* --- ## [24.7.0](v24.6.2...v24.7.0) (2026-03-27) ### Features * support --allow-anonymous deploys ([#8109](#8109)) ([9dee49e](9dee49e)) ### Bug Fixes * **deps:** update netlify packages ([#8105](#8105)) ([42a46bf](42a46bf)) --- 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>
2 participants
🎉 Thanks for submitting a pull request! 🎉
Summary
Allow agents to create netlify deploys without first requiring a human to authenticate with Netlify
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)