Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request extends multiple Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/commands/env/env-unset.ts (1)
71-81:⚠️ Potential issue | 🟠 MajorSame issue:
--siteoption not wired up.The
siteIdat line 74 doesn't consideroptions.site.Proposed fix
export const envUnset = async (key: string, options: OptionValues, command: BaseCommand) => { const { context, force } = options const { api, cachedConfig, site } = command.netlify - const siteId = site.id + const siteId = options.site || site.id if (!siteId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env-unset.ts` around lines 71 - 81, The envUnset function currently always uses command.netlify.site.id for siteId and ignores the CLI --site option; update envUnset to prefer options.site when provided (e.g., use options.site || command.netlify.site.id) so the function honors the --site flag before falling back to command.netlify.site.id, and then use that computed siteId when calling getSiteInfo and subsequent API calls.src/commands/env/env-import.ts (1)
53-63:⚠️ Potential issue | 🟠 MajorSame issue:
--siteoption not wired up.The
siteIdat line 55 doesn't consideroptions.site.Proposed fix
export const envImport = async (fileName: string, options: OptionValues, command: BaseCommand) => { const { api, cachedConfig, site } = command.netlify - const siteId = site.id + const siteId = options.site || site.id if (!siteId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env-import.ts` around lines 53 - 63, The envImport function currently always reads siteId from command.netlify.site.id and ignores the CLI --site option; update envImport so siteId is derived from the CLI option when present (e.g. use options.site || site.id or the equivalent nullish coalescing) before the existing falsy check, and ensure subsequent calls (like getSiteInfo(api, siteId, cachedConfig)) use that computed siteId; reference the envImport function, the siteId variable, the options.site value, and command.netlify.site to locate where to apply the change.src/commands/env/env-get.ts (1)
8-18:⚠️ Potential issue | 🟠 MajorSame issue:
--siteoption not wired up.The
siteIdderivation at line 11 doesn't consideroptions.site.Proposed fix
export const envGet = async (name: string, options: OptionValues, command: BaseCommand) => { const { context, scope } = options const { api, cachedConfig, site } = command.netlify - const siteId = site.id + const siteId = options.site || site.id if (!siteId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env-get.ts` around lines 8 - 18, The envGet function currently derives siteId from command.netlify.site.id without checking the CLI --site option; change the siteId derivation to prefer options.site (e.g. const siteId = options.site || site.id) so the --site flag is honored, then pass that siteId into getSiteInfo(api, siteId, cachedConfig); update any related null-check/log to use the new siteId variable and keep references to envGet, getSiteInfo, and command.netlify consistent.src/commands/env/env-set.ts (1)
111-119:⚠️ Potential issue | 🟠 MajorSame issue:
--siteoption not wired up.Similar to
env-list.ts, thesiteIdis derived fromsite.idwithout consideringoptions.site. The fix should be applied consistently across all env commands.Proposed fix
export const envSet = async (key: string, value: string, options: OptionValues, command: BaseCommand) => { const { context, force, scope, secret } = options const { api, cachedConfig, site } = command.netlify - const siteId = site.id + const siteId = options.site || site.id if (!siteId) { log('No project id found, please run inside a project folder or `netlify link`') return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env-set.ts` around lines 111 - 119, The envSet handler derives siteId from command.netlify.site.id and ignores the --site CLI flag; update envSet to resolve siteId from options.site first (falling back to command.netlify.site.id) before calling getSiteInfo so the --site option is honored; modify the siteId assignment logic in envSet (and apply the same pattern to other env command handlers) to use options.site when present and then call getSiteInfo(api, siteId, cachedConfig).src/commands/env/env-list.ts (1)
44-55:⚠️ Potential issue | 🟠 MajorWire up the
--siteoption to determine the target site.The
--siteflag is defined and documented but not used in the handler. Line 47 derivessiteIdonly fromsite.id, so passing--site my-other-projecthas no effect.Fix
export const envList = async (options: OptionValues, command: BaseCommand) => { const { context, scope } = options const { api, cachedConfig, site } = command.netlify - const siteId = site.id + const siteId = options.site || site.id if (!siteId) { log('No project id found, please run inside a project folder or `netlify link`') return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/env/env-list.ts` around lines 44 - 55, The handler envList currently always uses command.netlify.site.id and ignores the --site flag; update envList to prefer the CLI option value (options.site) when present and fall back to command.netlify.site.id otherwise, i.e. compute siteId = options.site || command.netlify.site?.id, validate it the same way as before, and then pass that siteId into getSiteInfo(api, siteId, cachedConfig) so the --site flag correctly targets the requested site.
🧹 Nitpick comments (1)
src/commands/env/env.ts (1)
22-22: Optional: centralizesiteoption definitions to reduce drift.The same option string/description is repeated in 5 places. A small helper (or shared constants) would keep future updates consistent.
♻️ Proposed refactor
+const SITE_OPTION_DESCRIPTION = 'A project name or ID to target' +const SITE_OPTION_LONG = '--site <name-or-id>' +const SITE_OPTION_SHORT_AND_LONG = '-s, --site <name-or-id>' + export const createEnvCommand = (program: BaseCommand) => { program .command('env:get') @@ - .option('--site <name-or-id>', 'A project name or ID to target') + .option(SITE_OPTION_LONG, SITE_OPTION_DESCRIPTION) @@ program .command('env:import') @@ - .option('-s, --site <name-or-id>', 'A project name or ID to target') + .option(SITE_OPTION_SHORT_AND_LONG, SITE_OPTION_DESCRIPTION) @@ program .command('env:list') @@ - .option('--site <name-or-id>', 'A project name or ID to target') + .option(SITE_OPTION_LONG, SITE_OPTION_DESCRIPTION) @@ program .command('env:set') @@ - .option('--site <name-or-id>', 'A project name or ID to target') + .option(SITE_OPTION_LONG, SITE_OPTION_DESCRIPTION) @@ program .command('env:unset') @@ - .option('-s, --site <name-or-id>', 'A project name or ID to target') + .option(SITE_OPTION_SHORT_AND_LONG, SITE_OPTION_DESCRIPTION)Also applies to: 49-49, 65-65, 96-96, 133-133
🤖 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 22, Multiple commands repeat the same .option('--site <name-or-id>', 'A project name or ID to target'); extract that literal into a shared symbol and reuse it: add a constant (e.g., SITE_OPTION_FLAGS = "--site <name-or-id>" and SITE_OPTION_DESC = "A project name or ID to target") or a small helper function addSiteOption(command) that calls command.option(SITE_OPTION_FLAGS, SITE_OPTION_DESC), then replace the five inline .option(...) occurrences with either command.option(SITE_OPTION_FLAGS, SITE_OPTION_DESC) or addSiteOption(command) to centralize the definition and avoid drift.
🤖 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/env/env-get.ts`:
- Around line 8-18: The envGet function currently derives siteId from
command.netlify.site.id without checking the CLI --site option; change the
siteId derivation to prefer options.site (e.g. const siteId = options.site ||
site.id) so the --site flag is honored, then pass that siteId into
getSiteInfo(api, siteId, cachedConfig); update any related null-check/log to use
the new siteId variable and keep references to envGet, getSiteInfo, and
command.netlify consistent.
In `@src/commands/env/env-import.ts`:
- Around line 53-63: The envImport function currently always reads siteId from
command.netlify.site.id and ignores the CLI --site option; update envImport so
siteId is derived from the CLI option when present (e.g. use options.site ||
site.id or the equivalent nullish coalescing) before the existing falsy check,
and ensure subsequent calls (like getSiteInfo(api, siteId, cachedConfig)) use
that computed siteId; reference the envImport function, the siteId variable, the
options.site value, and command.netlify.site to locate where to apply the
change.
In `@src/commands/env/env-list.ts`:
- Around line 44-55: The handler envList currently always uses
command.netlify.site.id and ignores the --site flag; update envList to prefer
the CLI option value (options.site) when present and fall back to
command.netlify.site.id otherwise, i.e. compute siteId = options.site ||
command.netlify.site?.id, validate it the same way as before, and then pass that
siteId into getSiteInfo(api, siteId, cachedConfig) so the --site flag correctly
targets the requested site.
In `@src/commands/env/env-set.ts`:
- Around line 111-119: The envSet handler derives siteId from
command.netlify.site.id and ignores the --site CLI flag; update envSet to
resolve siteId from options.site first (falling back to command.netlify.site.id)
before calling getSiteInfo so the --site option is honored; modify the siteId
assignment logic in envSet (and apply the same pattern to other env command
handlers) to use options.site when present and then call getSiteInfo(api,
siteId, cachedConfig).
In `@src/commands/env/env-unset.ts`:
- Around line 71-81: The envUnset function currently always uses
command.netlify.site.id for siteId and ignores the CLI --site option; update
envUnset to prefer options.site when provided (e.g., use options.site ||
command.netlify.site.id) so the function honors the --site flag before falling
back to command.netlify.site.id, and then use that computed siteId when calling
getSiteInfo and subsequent API calls.
---
Nitpick comments:
In `@src/commands/env/env.ts`:
- Line 22: Multiple commands repeat the same .option('--site <name-or-id>', 'A
project name or ID to target'); extract that literal into a shared symbol and
reuse it: add a constant (e.g., SITE_OPTION_FLAGS = "--site <name-or-id>" and
SITE_OPTION_DESC = "A project name or ID to target") or a small helper function
addSiteOption(command) that calls command.option(SITE_OPTION_FLAGS,
SITE_OPTION_DESC), then replace the five inline .option(...) occurrences with
either command.option(SITE_OPTION_FLAGS, SITE_OPTION_DESC) or
addSiteOption(command) to centralize the definition and avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68d3ec07-c0f5-4ae3-b316-501478c5f891
⛔ Files ignored due to path filters (2)
tests/integration/commands/env/__snapshots__/env.test.ts.snapis excluded by!**/*.snaptests/integration/commands/envelope/__snapshots__/envelope.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
docs/commands/env.mdsrc/commands/env/env-clone.tssrc/commands/env/env-get.tssrc/commands/env/env-import.tssrc/commands/env/env-list.tssrc/commands/env/env-set.tssrc/commands/env/env-unset.tssrc/commands/env/env.tssrc/commands/env/utils.ts
eduardoboucas
approved these changes
Mar 25, 2026
eduardoboucas
pushed a commit
that referenced
this pull request
Mar 25, 2026
🤖 I have created a release *beep* *boop* --- ## [24.6.0](v24.5.1...v24.6.0) (2026-03-25) ### Features * log output to stderr on `deploy --json --verbose` ([#8090](#8090)) ([13e973c](13e973c)) ### Bug Fixes * AX for env setting ([#8091](#8091)) ([d77ff79](d77ff79)) --- 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
in ax testing, we see two things: