Skip to content

fix: ephemeral environment variables being saved to filesystem#6723

Merged
bijin-bruno merged 2 commits intousebruno:mainfrom
sanjaikumar-bruno:refactor/ephemeral-env-vars-save-logic
Jan 8, 2026
Merged

fix: ephemeral environment variables being saved to filesystem#6723
bijin-bruno merged 2 commits intousebruno:mainfrom
sanjaikumar-bruno:refactor/ephemeral-env-vars-save-logic

Conversation

@sanjaikumar-bruno
Copy link
Member

@sanjaikumar-bruno sanjaikumar-bruno commented Jan 7, 2026

BRU-2444

Fixed a bug where ephemeral environment variables (created by scripts) without a persistedValue were being saved to the filesystem. They are now filtered out on save.

Refactored environments.js to improve readability and maintainability, following the updated repo's coding standards.

Screen.Recording.2026-01-07.at.7.05.30.PM.mov

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

Walkthrough

The PR modifies ephemeral environment variable persistence during save operations. Variables with a persistedValue are saved as their original persisted value, while those without are filtered out rather than saved with their current UI value.

Changes

Cohort / File(s) Summary
Ephemeral variable save logic
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js, packages/bruno-app/src/utils/environments.js
Adds isPersistableEnvVarForSave() helper to filter ephemeral variables during save. Modifies buildPersistedEnvVariables() to exclude ephemeral vars without persisted values, while preserving original persisted values for those that have them. Filtering now occurs in save mode, not just merge mode.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Poem

Ephemeral whispers, now held in place,
Persisted echoes find their space,
Scripts may flutter, but leave no trace,
Unless you've marked them—sealed with grace. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: preventing ephemeral environment variables without persistedValue from being saved to the filesystem.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sanjaikumar-bruno sanjaikumar-bruno changed the title refactor: enhance environment variable persistence logic fix: ephemeral environment variables being saved to filesystem Jan 7, 2026
Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
packages/bruno-app/src/utils/environments.js (1)

25-26: Consider clarifying the comment for save mode.

The current comment focuses on what's filtered out, but could be clearer about what's kept. Consider:

-// mode 'save': filters out ephemeral vars without persistedValue (script-created, never on disk)
+// mode 'save': saves non-ephemeral vars and ephemeral vars with persistedValue; filters out script-created ephemeral vars

This makes it explicit that both non-ephemeral variables and ephemeral variables with persistedValue are saved.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efe94d9 and 55fcdee.

📒 Files selected for processing (2)
  • packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
  • packages/bruno-app/src/utils/environments.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
  • packages/bruno-app/src/utils/environments.js
🧠 Learnings (2)
📚 Learning: 2026-01-07T18:53:34.291Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6735
File: packages/bruno-electron/src/ipc/collection.js:936-939
Timestamp: 2026-01-07T18:53:34.291Z
Learning: In the bruno repository, the Redux reducer determines collection format by checking for the `opencollection` property in brunoConfig. The `version` property is not used for format detection, so having both `version` and `opencollection` properties together doesn't cause issues since `opencollection` takes precedence.

Applied to files:

  • packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
  • packages/bruno-app/src/utils/environments.js
🧬 Code graph analysis (1)
packages/bruno-app/src/utils/environments.js (1)
packages/bruno-app/src/utils/collections/index.js (3)
  • variables (1086-1086)
  • variables (1111-1111)
  • variables (1326-1326)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (1)

1754-1777: LGTM! Clear documentation of ephemeral variable save behavior.

The implementation correctly filters ephemeral variables using the updated buildPersistedEnvVariables function with mode: 'save', ensuring only variables with persistedValue or non-ephemeral variables are persisted. The immediate Redux sync at line 1773 is good practice to prevent stale ephemeral variables from lingering when the watcher event arrives.

packages/bruno-app/src/utils/environments.js (2)

15-18: Logic is correct for filtering persistable variables.

The function properly handles edge cases:

  • Returns false for falsy inputs
  • Keeps non-ephemeral variables
  • Keeps ephemeral variables with persistedValue (including null, which is treated as distinct from undefined)

This correctly implements the bug fix to prevent script-created ephemeral variables (those without persistedValue) from being saved.


27-40: Excellent refactor that fixes the ephemeral variable bug.

The explicit filter-then-map pattern for save mode is a significant improvement:

  • Correctness: Ephemeral variables without persistedValue are now filtered out, preventing script-created variables from being saved to disk
  • Readability: The two-step filter→map pattern is more explicit than the previous approach
  • Consistency: Both modes now follow the same structural pattern

The default fallback to save mode (line 36) is appropriate.

@bijin-bruno bijin-bruno merged commit 5b1b1b5 into usebruno:main Jan 8, 2026
8 checks passed
@sanjaikumar-bruno sanjaikumar-bruno deleted the refactor/ephemeral-env-vars-save-logic branch January 9, 2026 05:10
sanjaikumar-bruno added a commit to sanjaikumar-bruno/bruno that referenced this pull request Feb 2, 2026
bijin-bruno pushed a commit that referenced this pull request Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants