fix: allow --site-id as a silent flag to prevent common guessing mistakes from agents#8180
fix: allow --site-id as a silent flag to prevent common guessing mistakes from agents#8180sean-roberts wants to merge 5 commits intomainfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a hidden CLI option Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 95b5e52
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/env/env.ts (1)
23-23: Consider extracting the hidden--site-idoption builderThe same
new Option('--site-id <name-or-id>').hideHelp(true)is repeated five times. A small helper reduces drift risk if behavior (e.g., conflicts/default/parsing) is adjusted later.Refactor sketch
+const hiddenSiteIdOption = () => new Option('--site-id <name-or-id>').hideHelp(true) ... - .addOption(new Option('--site-id <name-or-id>').hideHelp(true)) + .addOption(hiddenSiteIdOption()) ...Also applies to: 51-51, 68-68, 100-100, 138-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env.ts` at line 23, Extract the repeated Option builder into a small helper (e.g., siteIdOption or makeHiddenSiteIdOption) that returns new Option('--site-id <name-or-id>').hideHelp(true), then replace each inline occurrence of new Option('--site-id <name-or-id>').hideHelp(true) (the five occurrences) with a call to that helper; update any necessary imports/exports if the helper is used across modules and ensure the helper is defined near the command builders (e.g., in the same module) so future changes to parsing/conflicts/defaults only need to be made in one place.
🤖 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/env/env.ts`:
- Line 23: The env commands ignore the parsed --site-id because NetlifySite.id
only reads persistent state; update the resolution so the runtime flag is
honored: either modify the NetlifySite.get id() getter to return
actionCommand.siteId (fallback to state.get('siteId')) when present, or change
the env handlers (env-get, env-list, env-import) to use the current
command/options.siteId (or actionCommand.siteId) when resolving the target site
instead of always using command.netlify.site.id; reference symbols:
Option('--site-id <name-or-id>'), BaseCommand.init(), actionCommand.siteId,
NetlifySite.get id(), and the env-get/env-list/env-import handlers to implement
the change.
---
Nitpick comments:
In `@src/commands/env/env.ts`:
- Line 23: Extract the repeated Option builder into a small helper (e.g.,
siteIdOption or makeHiddenSiteIdOption) that returns new Option('--site-id
<name-or-id>').hideHelp(true), then replace each inline occurrence of new
Option('--site-id <name-or-id>').hideHelp(true) (the five occurrences) with a
call to that helper; update any necessary imports/exports if the helper is used
across modules and ensure the helper is defined near the command builders (e.g.,
in the same module) so future changes to parsing/conflicts/defaults only need to
be made in one place.
🪄 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: abcadc2b-2282-4f09-905a-adc42583f418
📒 Files selected for processing (1)
src/commands/env/env.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/base-command.ts`:
- Around line 772-776: The getter for netlify.site.id currently returns
actionCommand.siteId first, which can contain an unresolved site name seeded
from --site or --site-id; change the logic so the canonical ID from
state.get('siteId') is preferred (state.get('siteId') || actionCommand.siteId)
or ensure any assignment to actionCommand.siteId (from flags.site or
flags['site-id']) is normalized by calling getSiteByName(...) and storing the
resolved site.id before returning; update the getter and the code path that sets
actionCommand.siteId (or the setter id) so actionCommand.siteId never holds an
unresolved site name.
🪄 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: 8318a836-0dcc-4e96-9900-17c9da26efa4
📒 Files selected for processing (1)
src/commands/base-command.ts
| return actionCommand.siteId || state.get('siteId') | ||
| }, | ||
| set id(id) { | ||
| state.set('siteId', id) | ||
| actionCommand.siteId = id |
There was a problem hiding this comment.
Don't let netlify.site.id return an unresolved site name.
Line 772 now prefers actionCommand.siteId, but that field is seeded from both --site and --site-id. In this file, only flags.site goes through the later getSiteByName() fallback, so --site-id my-site-name can still make netlify.site.id return "my-site-name" instead of the canonical site ID. That means the new alias will still fail anywhere downstream expects an actual ID.
Suggested fix
- let siteData = siteInfo
- if (!siteData.url && flags.site) {
- const result = await getSiteByName(api, flags.site)
+ let siteData = siteInfo
+ const siteSelector =
+ typeof flags.siteId === 'string' ? flags.siteId : typeof flags.site === 'string' ? flags.site : undefined
+ if (!siteData.url && siteSelector) {
+ const result = await getSiteByName(api, siteSelector)
if (result == null) {
- return logAndThrowError(`Project with name "${flags.site}" not found`)
+ return logAndThrowError(`Project with name "${siteSelector}" not found`)
}
siteData = result
+ actionCommand.siteId = result.id
} get id() {
- return actionCommand.siteId || state.get('siteId')
+ return siteData?.id || actionCommand.siteId || state.get('siteId')
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/base-command.ts` around lines 772 - 776, The getter for
netlify.site.id currently returns actionCommand.siteId first, which can contain
an unresolved site name seeded from --site or --site-id; change the logic so the
canonical ID from state.get('siteId') is preferred (state.get('siteId') ||
actionCommand.siteId) or ensure any assignment to actionCommand.siteId (from
flags.site or flags['site-id']) is normalized by calling getSiteByName(...) and
storing the resolved site.id before returning; update the getter and the code
path that sets actionCommand.siteId (or the setter id) so actionCommand.siteId
never holds an unresolved site name.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/commands/base-command.ts (1)
730-732:⚠️ Potential issue | 🟠 MajorNormalize
--site-idvalues before returningnetlify.site.id.Line 730 now propagates
siteData.idintoactionCommand.siteId, and Line 776 returns that value first, but the name-resolution fallback above still only runs forflags.site. With--site-id my-site-name,siteData.idcan still be the unresolved selector, so downstream code readingnetlify.site.idmay still send a site name where an actual site ID is required. That means the new alias still misses the common “site name passed to--site-id” case it is meant to soften.Suggested fix
let siteData = siteInfo - if (!siteData.url && flags.site) { - const result = await getSiteByName(api, flags.site) + const siteSelector = + typeof flags.siteId === 'string' ? flags.siteId : typeof flags.site === 'string' ? flags.site : undefined + if (!siteData.url && siteSelector) { + const result = await getSiteByName(api, siteSelector) if (result == null) { - return logAndThrowError(`Project with name "${flags.site}" not found`) + return logAndThrowError(`Project with name "${siteSelector}" not found`) } siteData = result } if (siteData.id) { actionCommand.siteId = siteData.id }get id() { - return actionCommand.siteId || state.get('siteId') + return siteData.id || actionCommand.siteId || state.get('siteId') },Also applies to: 775-780
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/base-command.ts` around lines 730 - 732, The code assigns siteData.id into actionCommand.siteId but doesn't normalize selector-style values (e.g., a site name passed to --site-id), so netlify.site.id may still be a name; ensure you run the same name-resolution/fallback used for flags.site on actionCommand.siteId (or on siteData.id before assignment) so any selector/name is resolved to the canonical site ID before returning netlify.site.id; apply the same fix to the other branch referenced around lines 775-780 so both flags.site and --site-id are normalized consistently.
🤖 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/base-command.ts`:
- Around line 730-732: The code assigns siteData.id into actionCommand.siteId
but doesn't normalize selector-style values (e.g., a site name passed to
--site-id), so netlify.site.id may still be a name; ensure you run the same
name-resolution/fallback used for flags.site on actionCommand.siteId (or on
siteData.id before assignment) so any selector/name is resolved to the canonical
site ID before returning netlify.site.id; apply the same fix to the other branch
referenced around lines 775-780 so both flags.site and --site-id are normalized
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a8b20a3-33ba-4e4a-af68-28c29bdcb9db
📒 Files selected for processing (1)
src/commands/base-command.ts
1 participant
🎉 Thanks for submitting a pull request! 🎉
Summary
Agents keep hallucinating that --site-id is the flag name for env:* APIs. The actual flag is --site. This will silently support --site-id to avoid this issue
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)