Skip to content

Comments

feat: add deploy-specific environment variables to dev#7947

Open
ndhoule wants to merge 2 commits intomainfrom
nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment
Open

feat: add deploy-specific environment variables to dev#7947
ndhoule wants to merge 2 commits intomainfrom
nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment

Conversation

@ndhoule
Copy link
Contributor

@ndhoule ndhoule commented Feb 13, 2026

No description provided.

@github-actions
Copy link

github-actions bot commented Feb 13, 2026

📊 Benchmark results

Comparing with ad3487d

  • Dependency count: 1,167 ⬆️ 7.54% increase vs. ad3487d
  • Package size: 379 MB ⬆️ 13.32% increase vs. ad3487d
  • Number of ts-expect-error directives: 362 (no change)

@ndhoule ndhoule force-pushed the nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment branch 6 times, most recently from fdb2610 to 020c389 Compare February 20, 2026 00:52
@coderabbitai
Copy link

coderabbitai bot commented Feb 20, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Deploy-time environment variables are now propagated and available to Functions and Edge Functions during build and dev.
  • Tests

    • Added integration test ensuring injected environment variables are accessible inside Functions and Edge Functions during development.
  • Chores

    • Updated CI workflow and bumped a couple of build-related dependencies.

Walkthrough

Threads a new deployEnvironment payload through build and dev flows: runNetlifyBuild now returns deployEnvironment and the dev startup passes it into server initializers. startFunctionsServer, startProxyServer, and related proxy/edge init functions accept and forward deployEnvironment. Servers filter entries by scope (keeping functions), strip scopes, and pass sanitized variables into FunctionsRegistry and EdgeFunctionsRegistry, which merge deploy-time variables into function and edge environments. Several constructors/signatures were extended and an integration test validating env injection during netlify dev was added.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Most changes align with the PR objective; however, the GitHub Actions workflow update (conventional-commit.yml) replacing the PR title linter appears unrelated to the deploy-specific environment variables feature. Either move the conventional-commit.yml changes to a separate pull request or clarify its relationship to the deploy-specific environment variables feature.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided by the author; the description field is empty and thus cannot be evaluated for relevance to the changeset. Add a brief description explaining the changes, purpose, and any relevant implementation details or design decisions for better context.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add deploy-specific environment variables to dev' clearly and concisely describes the main change in the changeset: introducing deploy-specific environment variables to the dev command.
Linked Issues check ✅ Passed The changeset implements the core objective of EX-1284 by threading deploy-specific environment variables through dev timeline execution, filtering them by scope, and making them available to both functions and edge functions during dev server execution.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndhoule ndhoule force-pushed the nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment branch 4 times, most recently from 221f837 to 6adbe65 Compare February 20, 2026 01:56
@ndhoule ndhoule force-pushed the nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment branch from 6adbe65 to 852cd8d Compare February 20, 2026 02:14
@ndhoule ndhoule marked this pull request as ready for review February 20, 2026 02:48
@ndhoule ndhoule requested a review from a team as a code owner February 20, 2026 02:48
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/lib/edge-functions/registry.ts (1)

368-388: ⚠️ Potential issue | 🟠 Major

Preserve higher‑precedence env values when merging deployEnvironment.
Avoid overwriting keys already set from higher‑precedence sources in the env map.

🔧 Suggested fix
-    for (const { key, value } of deployEnvironment) {
-      env[key] = value
-    }
+    for (const { key, value } of deployEnvironment) {
+      if (!(key in env)) {
+        env[key] = value
+      }
+    }
Based on learnings: Environment variables must be loaded with specific precedence: Process environment > .env files > Netlify site settings > Addon-provided variables > Build-time configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/edge-functions/registry.ts` around lines 368 - 388, The merge
currently lets deployEnvironment values overwrite env entries from
higher‑precedence sources; update getEnvironmentVariables so when iterating
deployEnvironment (the array of { key, value, isSecret }), only assign env[key]
= value if the key is not already present (e.g., check env[key] === undefined or
!(key in env)) to preserve higher precedence values from envConfig (.env, ui,
account, addons, internal) and avoid clobbering them.
src/lib/functions/netlify-function.ts (1)

272-289: ⚠️ Potential issue | 🟠 Major

Avoid deployEnvironment overriding higher‑precedence values.
Guard deployEnvironment entries so they don’t clobber keys already set by higher‑precedence sources (e.g., process env).

🔧 Suggested fix
-    const environment = {
-      // Include function-specific environment variables from config
-      ...configEnvVars,
-      ...Object.fromEntries(this.deployEnvironment.map(({ key, value }) => [key, value])),
-    }
+    const deployEnvVars = Object.fromEntries(
+      this.deployEnvironment
+        .filter(({ key }) => !(key in process.env))
+        .map(({ key, value }) => [key, value]),
+    )
+    const environment = {
+      // Include function-specific environment variables from config
+      ...configEnvVars,
+      ...deployEnvVars,
+    }
Based on learnings: Environment variables must be loaded with specific precedence: Process environment > .env files > Netlify site settings > Addon-provided variables > Build-time configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/functions/netlify-function.ts` around lines 272 - 289, The current
environment construction allows this.deployEnvironment entries to overwrite
higher-precedence variables; update the build in
src/lib/functions/netlify-function.ts so deployEnvironment does not clobber
existing keys by either (a) filtering this.deployEnvironment to only include
entries whose keys are not already present in process.env or configEnvVars, or
(b) changing the spread order so higher-precedence sources (process.env and
configEnvVars) come after deployEnvironment; locate the relevant symbols
configEnvVars, this.deployEnvironment and the environment object and implement
the filter or adjust the spread order to preserve the precedence: Process env >
.env/site/addon > build config.
src/utils/run-build.ts (1)

192-205: ⚠️ Potential issue | 🟡 Minor

Both 'build' and 'dev' timelines cast deployEnvironment to a type missing the scopes field declared in the overloads.

The function overloads (lines 59–63, 64–68) declare deployEnvironment with scopes: string[], but lines 172 and 203 cast to { key: string; value: string; isSecret: boolean }[], dropping the scopes field. The downstream proxy.ts expects the full type with scopes. Verify whether @netlify/build's startDev and buildSite actually return scopes, then either remove the casts or update the overload signatures accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/run-build.ts` around lines 192 - 205, The deployEnvironment
returned from startDev / buildSite is being cast to { key: string; value:
string; isSecret: boolean }[] (see deployEnvironment usage in run-build.ts)
which drops the scopes field declared in the overloads; inspect the actual
return shape of `@netlify/build`'s startDev and buildSite (confirm whether they
include scopes:string[]), then either remove the unsafe casts in run-build.ts
(so the value retains scopes) or update the overload signatures to match the
true return type; ensure any downstream consumer (e.g., proxy.ts) receives the
full type including scopes and update the type assertions or interfaces
accordingly.
🤖 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/utils/run-build.ts`:
- Around line 170-174: The return value of the build path violates the overload
signature by returning deployEnvironment items without the required scopes
field; update the returned object in runBuild (the block returning configPath:
tempConfigPath, deployEnvironment: [] as { key: string; value: string; isSecret:
boolean }[], generatedFunctions) so its deployEnvironment is typed to include
scopes (e.g. { key: string; value: string; isSecret: boolean; scopes: string[]
}[]) or construct the empty array with the correct shape (empty items with
scopes: string[]), ensuring the return matches the declared overload type for
deployEnvironment.

In `@tests/integration/commands/dev/dev.test.ts`:
- Around line 837-931: The test hard-codes port 4445 which causes collisions
under describe.concurrent; change dev.targetPort to 0 in withNetlifyToml and
make the generated dev server (index.mjs created by withContentFile) listen on
the assigned port from the environment (e.g., server.listen(process.env.PORT) or
process.env.PORT || 0) so the test harness can bind a random free port; keep the
fetches using server.url (withDevServer) unchanged. Update references in the
diff: modify the dev.targetPort in withNetlifyToml and replace
server.listen(4445) in the index.mjs content so the runtime uses
process.env.PORT (or equivalent) provided by the test harness.

---

Outside diff comments:
In `@src/lib/edge-functions/registry.ts`:
- Around line 368-388: The merge currently lets deployEnvironment values
overwrite env entries from higher‑precedence sources; update
getEnvironmentVariables so when iterating deployEnvironment (the array of { key,
value, isSecret }), only assign env[key] = value if the key is not already
present (e.g., check env[key] === undefined or !(key in env)) to preserve higher
precedence values from envConfig (.env, ui, account, addons, internal) and avoid
clobbering them.

In `@src/lib/functions/netlify-function.ts`:
- Around line 272-289: The current environment construction allows
this.deployEnvironment entries to overwrite higher-precedence variables; update
the build in src/lib/functions/netlify-function.ts so deployEnvironment does not
clobber existing keys by either (a) filtering this.deployEnvironment to only
include entries whose keys are not already present in process.env or
configEnvVars, or (b) changing the spread order so higher-precedence sources
(process.env and configEnvVars) come after deployEnvironment; locate the
relevant symbols configEnvVars, this.deployEnvironment and the environment
object and implement the filter or adjust the spread order to preserve the
precedence: Process env > .env/site/addon > build config.

In `@src/utils/run-build.ts`:
- Around line 192-205: The deployEnvironment returned from startDev / buildSite
is being cast to { key: string; value: string; isSecret: boolean }[] (see
deployEnvironment usage in run-build.ts) which drops the scopes field declared
in the overloads; inspect the actual return shape of `@netlify/build`'s startDev
and buildSite (confirm whether they include scopes:string[]), then either remove
the unsafe casts in run-build.ts (so the value retains scopes) or update the
overload signatures to match the true return type; ensure any downstream
consumer (e.g., proxy.ts) receives the full type including scopes and update the
type assertions or interfaces accordingly.

@ndhoule ndhoule changed the title feat: add deploy-specific environment variables to netlify dev feat: add deploy-specific environment variables to dev Feb 20, 2026
@ndhoule ndhoule force-pushed the nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment branch from 8e467a6 to d0ef90d Compare February 20, 2026 17:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @.github/workflows/conventional-commit.yml:
- Around line 19-20: Remove the dead "Install Dependencies" step that runs `npm
install `@commitlint/config-conventional`` from the workflow; the
`amannn/action-semantic-pull-request` action uses its own `with:` inputs (like
`types` and `scopes`) so the commitlint package installation is
unnecessary—delete the step named "Install Dependencies" (the run: npm install
`@commitlint/config-conventional`) and keep the existing
`amannn/action-semantic-pull-request` configuration intact.

@ndhoule ndhoule force-pushed the nathanhoule/ex-1284-update-netlify-cli-to-load-deploy-specific-environment branch from d0ef90d to 4f3cde9 Compare February 20, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant