fix: change transient request directory from os temp to app data#7194
Open
chirag-bruno wants to merge 1 commit intousebruno:release/v3.1.2from
Open
fix: change transient request directory from os temp to app data#7194chirag-bruno wants to merge 1 commit intousebruno:release/v3.1.2from
chirag-bruno wants to merge 1 commit intousebruno:release/v3.1.2from
Conversation
Move transient request files from os.tmpdir() to app.getPath('userData')/transient
to prevent data loss when the OS clears temporary files.
Changes:
- Add helper functions for transient directory paths
- Update findCollectionPathByItemPath to use new transient path check
- Update renderer:mount-collection to create temp dirs in app data
- Update renderer:delete-transient-requests prefix validation
chirag-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
WalkthroughThis PR refactors transient collection path handling by introducing a new transient directory API ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/bruno-electron/src/ipc/collection.js (2)
1852-1857:existsSyncguard beforemkdirSync({ recursive: true })is redundant.
fs.mkdirSyncwith{ recursive: true }does not throw when the directory already exists, so theexistsSynccheck is a no-op.♻️ Proposed simplification
- // Ensure the transient base directory exists - const transientBase = getTransientDirectoryBase(); - if (!fs.existsSync(transientBase)) { - fs.mkdirSync(transientBase, { recursive: true }); - } - tempDirectoryPath = fs.mkdtempSync(getTransientCollectionPrefix()); + // Ensure the transient base directory exists before creating a temp subdir + fs.mkdirSync(getTransientDirectoryBase(), { recursive: true }); + tempDirectoryPath = fs.mkdtempSync(getTransientCollectionPrefix());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/collection.js` around lines 1852 - 1857, Remove the redundant existsSync guard before creating the transient base directory: instead of checking fs.existsSync(transientBase) call fs.mkdirSync(transientBase, { recursive: true }) directly (keep getTransientDirectoryBase() to compute transientBase), then proceed to create the temp dir with tempDirectoryPath = fs.mkdtempSync(getTransientCollectionPrefix()); this simplifies the block around getTransientDirectoryBase, fs.mkdirSync, tempDirectoryPath and fs.mkdtempSync by eliminating the no-op existsSync check.
1024-1032: LGTM — but note: upgrade sessions with staleos.tmpdir()temp dirs will silently skip cleanup.Post-upgrade, any renderer holding a
tempDirectoryfrom the oldos.tmpdir()location will have its requests silently skipped by the new prefix check. The behaviour is safe, but transient files from those sessions will linger in the old temp path until the OS clears them. No action needed unless you want to add a one-time migration sweep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/collection.js` around lines 1024 - 1032, The current handler for 'renderer:delete-transient-requests' uses getTransientCollectionPrefix() (brunoTempPrefix) and rejects tempDirectory values that don't start with it, causing older renderer sessions referencing the previous os.tmpdir() location to be skipped; update the validation in ipcMain.handle('renderer:delete-transient-requests') to accept legacy temp paths by either checking normalizedTempDir against both brunoTempPrefix and the previous temp-prefix (constructable from os.tmpdir() + the transient suffix) or call a one-time migration utility (e.g., migrateOldTransientDir) that will sweep/translate old tempDirectory locations into the new prefix before running the delete logic, ensuring normalizedTempDir and brunoTempPrefix (and the legacy prefix) are used when populating results.deleted / results.skipped / results.errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 96-99: The isTransientPath function incorrectly uses
filePath.startsWith(transientBase) which matches sibling paths; change the
second condition to an equality check (filePath === transientBase) so the
function returns true only when filePath is exactly the transient base or is
inside it (keep the existing startsWith(transientBase + path.sep) check),
referencing the isTransientPath function and getTransientDirectoryBase and
path.sep.
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 1852-1857: Remove the redundant existsSync guard before creating
the transient base directory: instead of checking fs.existsSync(transientBase)
call fs.mkdirSync(transientBase, { recursive: true }) directly (keep
getTransientDirectoryBase() to compute transientBase), then proceed to create
the temp dir with tempDirectoryPath =
fs.mkdtempSync(getTransientCollectionPrefix()); this simplifies the block around
getTransientDirectoryBase, fs.mkdirSync, tempDirectoryPath and fs.mkdtempSync by
eliminating the no-op existsSync check.
- Around line 1024-1032: The current handler for
'renderer:delete-transient-requests' uses getTransientCollectionPrefix()
(brunoTempPrefix) and rejects tempDirectory values that don't start with it,
causing older renderer sessions referencing the previous os.tmpdir() location to
be skipped; update the validation in
ipcMain.handle('renderer:delete-transient-requests') to accept legacy temp paths
by either checking normalizedTempDir against both brunoTempPrefix and the
previous temp-prefix (constructable from os.tmpdir() + the transient suffix) or
call a one-time migration utility (e.g., migrateOldTransientDir) that will
sweep/translate old tempDirectory locations into the new prefix before running
the delete logic, ensuring normalizedTempDir and brunoTempPrefix (and the legacy
prefix) are used when populating results.deleted / results.skipped /
results.errors.
Comment on lines
+96
to
+99
| const isTransientPath = (filePath) => { | ||
| const transientBase = getTransientDirectoryBase(); | ||
| return filePath.startsWith(transientBase + path.sep) || filePath.startsWith(transientBase); | ||
| }; |
Contributor
There was a problem hiding this comment.
isTransientPath second condition is too broad — can match unrelated sibling paths.
filePath.startsWith(transientBase) will return true for any path that shares the prefix string, including a directory like <transientBase>-other that is not inside the transient dir. The rest of the file consistently uses the startsWith(base + sep) || equals(base) idiom (e.g., line 133).
🐛 Proposed fix
const isTransientPath = (filePath) => {
const transientBase = getTransientDirectoryBase();
- return filePath.startsWith(transientBase + path.sep) || filePath.startsWith(transientBase);
+ return filePath.startsWith(transientBase + path.sep) || filePath === transientBase;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isTransientPath = (filePath) => { | |
| const transientBase = getTransientDirectoryBase(); | |
| return filePath.startsWith(transientBase + path.sep) || filePath.startsWith(transientBase); | |
| }; | |
| const isTransientPath = (filePath) => { | |
| const transientBase = getTransientDirectoryBase(); | |
| return filePath.startsWith(transientBase + path.sep) || filePath === transientBase; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-electron/src/ipc/collection.js` around lines 96 - 99, The
isTransientPath function incorrectly uses filePath.startsWith(transientBase)
which matches sibling paths; change the second condition to an equality check
(filePath === transientBase) so the function returns true only when filePath is
exactly the transient base or is inside it (keep the existing
startsWith(transientBase + path.sep) check), referencing the isTransientPath
function and getTransientDirectoryBase and path.sep.
sanish-bruno
approved these changes
Feb 18, 2026
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
3 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.
Description
Updated transient request target directory
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit