fix: window normlize path comparison#7240
Conversation
WalkthroughThis PR standardizes path comparisons across collection and environment handling code by normalizing file paths before equality checks. Changes span utility helpers and their consumers to ensure consistent cross-platform path handling without altering functional behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
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 |
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 (1)
packages/bruno-electron/src/ipc/collection.js (1)
103-106:⚠️ Potential issue | 🟡 Minor
isTransientPathnot normalized — inconsistent with the rest of this PR.
transientBaseis constructed viapath.join(OS backslashes on Windows), whilefilePathmay arrive with forward slashes. The rawstartsWithcan returnfalsewhen it should returntrue, causingfindCollectionPathByItemPathto skip themetadata.jsonlookup and return the temp directory path instead of the real collection path — breakinggetCollectionFormatand downstream operations for transient requests on Windows.Note that
renderer:delete-transient-requests(line 1052) already normalises both sides correctly;isTransientPathshould do the same.🛠️ Proposed fix
const isTransientPath = (filePath) => { const transientBase = getTransientDirectoryBase(); - return filePath.startsWith(transientBase + path.sep) || filePath.startsWith(transientBase); + const normalizedFilePath = path.normalize(filePath); + const normalizedBase = path.normalize(transientBase); + return normalizedFilePath.startsWith(normalizedBase + path.sep) || normalizedFilePath === normalizedBase; };🤖 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 103 - 106, isTransientPath uses raw startsWith with transientBase (from getTransientDirectoryBase) but doesn't normalize filePath, causing mismatches on Windows; update isTransientPath to normalize both transientBase and filePath (e.g., via path.normalize or replacing forward slashes) and ensure comparisons use normalized forms (and keep the existing path.sep check behavior). Locate the isTransientPath function and modify it to call path.normalize (or equivalent) on filePath and transientBase before doing startsWith so findCollectionPathByItemPath and related code (e.g., renderer:delete-transient-requests behavior) behave consistently across platforms.
🧹 Nitpick comments (2)
packages/bruno-electron/src/app/collection-watcher.js (1)
35-72: LGTM — all three local predicates are consistently updated.Side note:
isBrunoConfigFileandisEnvironmentsFolderhere duplicateisBrunoConfigFile/isBruEnvironmentConfigfromutils/filesystem.js. Both copies are now in sync, but consolidating to the shared exports whenever convenient would reduce future drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/app/collection-watcher.js` around lines 35 - 72, This duplicates predicates isBrunoConfigFile and isEnvironmentsFolder already exported from utils/filesystem.js (the latter named isBruEnvironmentConfig there); replace the local implementations in collection-watcher.js with imports from utils/filesystem.js and use those imported functions (isBrunoConfigFile, isBruEnvironmentConfig) instead of the local isBrunoConfigFile and isEnvironmentsFolder to avoid drift and keep logic centralized.packages/bruno-electron/src/utils/filesystem.js (1)
441-475: LGTM — normalization is symmetric and consistent across all four predicates.
path.normalizeis the right call for separator-style differences on Windows. One note for posterity: it does not case-fold, so if acollectionPathever arrives with a different casing than the watcher-reporteddirname(e.g. user-typed path in a config), the equality check would still fail.path.normalize(x).toLowerCase()guarded byisWindowsOS()would close that gap if it proves necessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/utils/filesystem.js` around lines 441 - 475, The four path-equality predicates (isDotEnvFile, isBrunoConfigFile, isBruEnvironmentConfig, isCollectionRootBruFile) use path.normalize but don't case-fold on Windows; update each to, when running on Windows (use an isWindowsOS() helper), compare path.normalize(dirname).toLowerCase() === path.normalize(target).toLowerCase() (where target is collectionPath or envDirectory) so casing differences don't break equality; keep non-Windows behavior unchanged.
🤖 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 `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 103-106: isTransientPath uses raw startsWith with transientBase
(from getTransientDirectoryBase) but doesn't normalize filePath, causing
mismatches on Windows; update isTransientPath to normalize both transientBase
and filePath (e.g., via path.normalize or replacing forward slashes) and ensure
comparisons use normalized forms (and keep the existing path.sep check
behavior). Locate the isTransientPath function and modify it to call
path.normalize (or equivalent) on filePath and transientBase before doing
startsWith so findCollectionPathByItemPath and related code (e.g.,
renderer:delete-transient-requests behavior) behave consistently across
platforms.
---
Nitpick comments:
In `@packages/bruno-electron/src/app/collection-watcher.js`:
- Around line 35-72: This duplicates predicates isBrunoConfigFile and
isEnvironmentsFolder already exported from utils/filesystem.js (the latter named
isBruEnvironmentConfig there); replace the local implementations in
collection-watcher.js with imports from utils/filesystem.js and use those
imported functions (isBrunoConfigFile, isBruEnvironmentConfig) instead of the
local isBrunoConfigFile and isEnvironmentsFolder to avoid drift and keep logic
centralized.
In `@packages/bruno-electron/src/utils/filesystem.js`:
- Around line 441-475: The four path-equality predicates (isDotEnvFile,
isBrunoConfigFile, isBruEnvironmentConfig, isCollectionRootBruFile) use
path.normalize but don't case-fold on Windows; update each to, when running on
Windows (use an isWindowsOS() helper), compare
path.normalize(dirname).toLowerCase() === path.normalize(target).toLowerCase()
(where target is collectionPath or envDirectory) so casing differences don't
break equality; keep non-Windows behavior unchanged.
Description
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