From c1aa682c037e5ff051c1bb160d0b84b905ddccd6 Mon Sep 17 00:00:00 2001 From: Sanjai Kumar <161328623+sanjaikumar-bruno@users.noreply.github.com> Date: Thu, 28 Aug 2025 18:09:37 +0530 Subject: [PATCH] fix: environment persistence and UI (#5404) --- .../ReduxStore/slices/collections/actions.js | 42 +++++++++-- .../ReduxStore/slices/collections/index.js | 32 +++++++- .../bruno-app/src/utils/collections/index.js | 7 +- packages/bruno-app/src/utils/environments.js | 31 ++++++++ packages/bruno-js/src/bru.js | 8 +- packages/bruno-js/tests/setEnvVar.spec.js | 74 +++++++++++++++++++ 6 files changed, 183 insertions(+), 11 deletions(-) create mode 100644 packages/bruno-app/src/utils/environments.js create mode 100644 packages/bruno-js/tests/setEnvVar.spec.js diff --git a/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js b/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js index 75b5ecc55..db9cbe43e 100644 --- a/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js +++ b/packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js @@ -41,7 +41,8 @@ import { initRunRequestEvent, updateRunnerConfiguration as _updateRunnerConfiguration, updateActiveConnections, - saveRequest as _saveRequest + saveRequest as _saveRequest, + saveEnvironment as _saveEnvironment } from './index'; import { each } from 'lodash'; @@ -59,6 +60,7 @@ import { calculateDraggedItemNewPathname } from 'utils/collections/index'; import { sanitizeName } from 'utils/common/regex'; +import { buildPersistedEnvVariables } from 'utils/environments'; import { safeParseJSON, safeStringifyJSON } from 'utils/common/index'; import { addTab } from 'providers/ReduxStore/slices/tabs'; import { updateSettingsSelectedTab } from './index'; @@ -1167,8 +1169,16 @@ export const copyEnvironment = (name, baseEnvUid, collectionUid) => (dispatch, g const sanitizedName = sanitizeName(name); const { ipcRenderer } = window; + + // strip "ephemeral" metadata + const variablesToCopy = (baseEnv.variables || []) + .filter((v) => !v.ephemeral) + .map(({ ephemeral, ...rest }) => { + return rest; + }); + ipcRenderer - .invoke('renderer:create-environment', collection.pathname, sanitizedName, baseEnv.variables) + .invoke('renderer:create-environment', collection.pathname, sanitizedName, variablesToCopy) .then( dispatch( updateLastAction({ @@ -1249,12 +1259,27 @@ export const saveEnvironment = (variables, environmentUid, collectionUid) => (di return reject(new Error('Environment not found')); } - environment.variables = variables; + /* + Modal Save writes what the user sees: + - Non-ephemeral vars are saved as-is (without metadata) + - Ephemeral vars: + - if persistedValue exists, save that (explicit persisted case) + - otherwise save the current UI value (treat as user-authored) + */ + const persisted = buildPersistedEnvVariables(variables, { mode: 'save' }); + environment.variables = persisted; const { ipcRenderer } = window; + const envForValidation = cloneDeep(environment); + environmentSchema .validate(environment) - .then(() => ipcRenderer.invoke('renderer:save-environment', collection.pathname, environment)) + .then(() => ipcRenderer.invoke('renderer:save-environment', collection.pathname, envForValidation)) + .then(() => { + // Immediately sync Redux to the saved (persisted) set so old ephemerals + // aren’t around when the watcher event arrives. + dispatch(_saveEnvironment({ variables: persisted, environmentUid, collectionUid })); + }) .then(resolve) .catch(reject); }); @@ -1311,12 +1336,15 @@ export const mergeAndPersistEnvironment = } }); - environment.variables = merged; + // Save only non-ephemeral vars, or ephemerals explicitly persisted this run + const persistedNames = new Set(Object.keys(persistentEnvVariables)); + const environmentToSave = cloneDeep(environment); + environmentToSave.variables = buildPersistedEnvVariables(merged, { mode: 'merge', persistedNames }); const { ipcRenderer } = window; environmentSchema - .validate(environment) - .then(() => ipcRenderer.invoke('renderer:save-environment', collection.pathname, environment)) + .validate(environmentToSave) + .then(() => ipcRenderer.invoke('renderer:save-environment', collection.pathname, environmentToSave)) .then(resolve) .catch(reject); }); diff --git a/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js b/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js index f1fcdef3b..2d6df3ec6 100644 --- a/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js +++ b/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js @@ -284,7 +284,20 @@ export const collectionsSlice = createSlice({ const variable = find(activeEnvironment.variables, (v) => v.name === key); if (variable) { - variable.value = value; + // For updates coming from scripts, treat them as ephemeral overlays. + if (variable.value !== value) { + /* + Overlay (persist: false): keep new value in Redux for UI and mark ephemeral + so it isn't written to disk. persistedValue stores the previous on-disk value; + save/persist uses that base unless the key is explicitly persisted. + */ + const previousValue = variable.value; + variable.value = value; + variable.ephemeral = true; + if (variable.persistedValue === undefined) { + variable.persistedValue = previousValue; + } + } } else { // __name__ is a private variable used to store the name of the environment // this is not a user defined variable and hence should not be updated @@ -295,7 +308,8 @@ export const collectionsSlice = createSlice({ secret: false, enabled: true, type: 'text', - uid: uuid() + uid: uuid(), + ephemeral: true, }); } } @@ -2275,7 +2289,21 @@ export const collectionsSlice = createSlice({ const existingEnv = collection.environments.find((e) => e.uid === environment.uid); if (existingEnv) { + const prevEphemerals = (existingEnv.variables || []).filter((v) => v.ephemeral); existingEnv.variables = environment.variables; + /* + Apply temporary (ephemeral) values only to variables that actually exist in the file. This prevents deleted temporaries from “popping back” after a save. If a variable is present in the file, we temporarily override the UI value while also remembering the on-disk value in persistedValue for future saves. + */ + prevEphemerals.forEach((ev) => { + const target = existingEnv.variables?.find((v) => v.name === ev.name); + if (target) { + if (target.value !== ev.value) { + if (target.persistedValue === undefined) target.persistedValue = target.value; + target.value = ev.value; + } + target.ephemeral = true; + } + }); } else { collection.environments.push(environment); collection.environments.sort((a, b) => a.name.localeCompare(b.name)); diff --git a/packages/bruno-app/src/utils/collections/index.js b/packages/bruno-app/src/utils/collections/index.js index c7e0817b0..b6dbaff86 100644 --- a/packages/bruno-app/src/utils/collections/index.js +++ b/packages/bruno-app/src/utils/collections/index.js @@ -1,5 +1,6 @@ import {cloneDeep, isEqual, sortBy, filter, map, isString, findIndex, find, each, get } from 'lodash'; import { uuid } from 'utils/common'; +import { buildPersistedEnvVariables } from 'utils/environments'; import { sortByNameThenSequence } from 'utils/common/index'; import path from 'utils/common/path'; import { isRequestTagsIncluded } from '@usebruno/common'; @@ -502,7 +503,11 @@ export const transformCollectionToSaveToExportAsFile = (collection, options = {} collectionToSave.version = '1'; collectionToSave.items = []; collectionToSave.activeEnvironmentUid = collection.activeEnvironmentUid; - collectionToSave.environments = collection.environments || []; + // Save environments without runtime metadata (ephemeral/persistedValue) + collectionToSave.environments = (collection.environments || []).map((env) => ({ + ...env, + variables: buildPersistedEnvVariables(env?.variables, { mode: 'save' }) + })); collectionToSave.root = { request: {} diff --git a/packages/bruno-app/src/utils/environments.js b/packages/bruno-app/src/utils/environments.js new file mode 100644 index 000000000..50fcfcdf7 --- /dev/null +++ b/packages/bruno-app/src/utils/environments.js @@ -0,0 +1,31 @@ +const isPersistableEnvVarForMerge = (persistedNames) => (v) => { + return !v?.ephemeral || v?.persistedValue !== undefined || (v?.name && persistedNames.has(v.name)); +}; + +const toPersistedEnvVarForMerge = (persistedNames) => (v) => { + const { ephemeral, persistedValue, ...rest } = v || {}; + if (v?.ephemeral && persistedValue !== undefined && !(v?.name && persistedNames.has(v.name))) { + return { ...rest, value: persistedValue }; + } + return rest; +}; + +const toPersistedEnvVarForSave = (v) => { + const { ephemeral, persistedValue, ...rest } = v || {}; + return v?.ephemeral ? (persistedValue !== undefined ? { ...rest, value: persistedValue } : rest) : rest; +}; + +/* + High-level builder for persisted variables + - mode 'save': write what the user sees + - mode 'merge': write only allowed vars (non-ephemeral, ephemerals with persistedValue, or explicitly persisted this run) +*/ +export const buildPersistedEnvVariables = (variables, { mode, persistedNames } = {}) => { + const src = Array.isArray(variables) ? variables : []; + if (mode === 'merge') { + const names = persistedNames instanceof Set ? persistedNames : new Set(); + return src.filter(isPersistableEnvVarForMerge(names)).map(toPersistedEnvVarForMerge(names)); + } + // default to save mode + return src.map(toPersistedEnvVarForSave); +}; diff --git a/packages/bruno-js/src/bru.js b/packages/bruno-js/src/bru.js index 2b677a88f..2bcb0a7d9 100644 --- a/packages/bruno-js/src/bru.js +++ b/packages/bruno-js/src/bru.js @@ -125,6 +125,12 @@ class Bru { throw new Error('Creating a env variable without specifying a name is not allowed.'); } + if (variableNameRegex.test(key) === false) { + throw new Error( + `Variable name: "${key}" contains invalid characters! Names must only contain alpha-numeric characters, "-", "_", "."` + ); + } + // When persist is true, only string values are allowed if (options?.persist && typeof value !== 'string') { throw new Error(`Persistent environment variables must be strings. Received ${typeof value} for key "${key}".`); @@ -133,7 +139,7 @@ class Bru { this.envVariables[key] = value; if (options?.persist) { - this.persistentEnvVariables[key] = value + this.persistentEnvVariables[key] = value; } else { if (this.persistentEnvVariables[key]) { delete this.persistentEnvVariables[key]; diff --git a/packages/bruno-js/tests/setEnvVar.spec.js b/packages/bruno-js/tests/setEnvVar.spec.js new file mode 100644 index 000000000..d1929055c --- /dev/null +++ b/packages/bruno-js/tests/setEnvVar.spec.js @@ -0,0 +1,74 @@ +const Bru = require('../src/bru'); + +describe('Bru.setEnvVar', () => { + const makeBru = () => + new Bru( + /* envVariables */ {}, + /* runtimeVariables */ {}, + /* processEnvVars */ {}, + /* collectionPath */ '/', + /* historyLogger */ undefined, + /* setVisualizations */ undefined, + /* secretVariables */ {}, + /* collectionVariables */ {}, + /* folderVariables */ {}, + /* requestVariables */ {}, + /* globalEnvironmentVariables */ {}, + /* oauth2CredentialVariables */ {}, + /* iterationDetails */ {}, + /* collectionName */ 'Test' + ); + + test('updates envVariables and does not mark persistent when persist=false', () => { + const bru = makeBru(); + bru.setEnvVar('non_persist', 'value', { persist: false }); + expect(bru.envVariables.non_persist).toBe('value'); + expect(bru.persistentEnvVariables.non_persist).toBeUndefined(); + }); + + test('updates envVariables and tracks persistent when persist=true (string only)', () => { + const bru = makeBru(); + bru.setEnvVar('persist_me', 'value', { persist: true }); + expect(bru.envVariables.persist_me).toBe('value'); + expect(bru.persistentEnvVariables.persist_me).toBe('value'); + }); + + test('updates envVariables when options are omitted (defaults to non-persistent)', () => { + const bru = makeBru(); + bru.setEnvVar('no_options', 'value'); + expect(bru.envVariables.no_options).toBe('value'); + expect(bru.persistentEnvVariables.no_options).toBeUndefined(); + }); + + test('throws when persist=true but value is not a string', () => { + const bru = makeBru(); + expect(() => bru.setEnvVar('persist_me', 123, { persist: true })).toThrow( + /Persistent environment variables must be strings/ + ); + }); + + test('changing existing key to non-persistent removes prior persisted entry', () => { + const bru = makeBru(); + bru.setEnvVar('same_key', 'old', { persist: true }); + expect(bru.persistentEnvVariables.same_key).toBe('old'); + + bru.setEnvVar('same_key', 'new'); + expect(bru.envVariables.same_key).toBe('new'); + expect(bru.persistentEnvVariables.same_key).toBeUndefined(); + }); + + test('changing existing key to persistent updates persisted value', () => { + const bru = makeBru(); + bru.setEnvVar('same_key', 'old'); + expect(bru.persistentEnvVariables.same_key).toBeUndefined(); + + bru.setEnvVar('same_key', 'new', { persist: true }); + expect(bru.envVariables.same_key).toBe('new'); + expect(bru.persistentEnvVariables.same_key).toBe('new'); + }); + + test('validates key name - invalid characters are rejected', () => { + const bru = makeBru(); + expect(() => bru.setEnvVar('invalid key', 'v')).toThrow(/contains invalid characters/); + }); +});