fix(import): dedupe sibling folder/file names case-insensitively#7822
Open
EmilienDaulhiac wants to merge 1 commit intousebruno:mainfrom
Open
fix(import): dedupe sibling folder/file names case-insensitively#7822EmilienDaulhiac wants to merge 1 commit intousebruno:mainfrom
EmilienDaulhiac wants to merge 1 commit intousebruno:mainfrom
Conversation
Collections from Postman/Insomnia/OpenAPI/WSDL that contain siblings with
duplicate names (`Returns` and `Returns`) or case-only variants (`OAuth2`
and `oAuth2`) previously either crashed with EEXIST on the onboarding-
service import path, or silently merged two folders into one on case-
insensitive filesystems (macOS APFS/HFS+, Windows NTFS by default) — both
leaving the user with a corrupted import.
Adds a per-directory `Set<string>` of already-claimed sibling names,
matched case-insensitively, and a `getUniqueSiblingName` helper that
appends ` - N` on collision. Applied in both `parseCollectionItems`
implementations (IPC handler + onboarding util), and the collection-
import.js mkdirSync now uses `{ recursive: true }` for parity.
Also makes the converter-level dedup case-insensitive in
postman-to-bruno, insomnia-to-bruno and wsdl-to-bruno so the tree the
user sees in the UI matches what is written to disk.
Tests: new unit tests for the helper (7), and converter regression
tests for Postman (3) and Insomnia (2) covering exact-duplicate and
case-only collisions for both folders and requests.
EmilienDaulhiac
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
WalkthroughThis PR fixes case-insensitive sibling name collisions in Postman, Insomnia, and WSDL collection imports. It updates converters to detect duplicates case-insensitively, introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/bruno-converters/src/wsdl/wsdl-to-bruno.js (1)
116-128:⚠️ Potential issue | 🟡 MinorAdd a WSDL regression test for case-only operation names.
This converter now changes duplicate detection semantics, but the provided tests only cover Postman/Insomnia paths. Please add a small WSDL case with sibling operations like
GetUser/getuserand assert the suffixed Bruno names.As per coding guidelines, "Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/src/wsdl/wsdl-to-bruno.js` around lines 116 - 128, Add a WSDL-specific regression test that exercises the duplicate-name logic in addSuffixToDuplicateName (packages/bruno-converters/src/wsdl/wsdl-to-bruno.js): create a minimal WSDL string with sibling operations that differ only by case (e.g., GetUser and getuser), run the wsdl-to-bruno conversion path used by your test harness, and assert the converted Bruno operation names include the suffix on the later-occurring case-duplicate (e.g., first remains "GetUser", second becomes "getuser_1"). Ensure the test covers the WSDL converter code path (not just Postman/Insomnia) and fails if names are not suffixed as expected.packages/bruno-converters/src/insomnia/insomnia-to-bruno.js (1)
23-35:⚠️ Potential issue | 🟠 MajorApply the same dedupe to Insomnia v5 folders.
The helper is now case-insensitive, but the v5 folder branch still assigns
name: item.name || 'Untitled Folder'directly. A v5 export with sibling folders likeOAuth2/oAuth2will still produce case-colliding folder names in the converter output.🐛 Proposed fix
} else if (item.children && Array.isArray(item.children)) { + const folderName = item.name || 'Untitled Folder'; + const folderSiblings = allItems.map((sibling) => ({ + name: sibling.name || 'Untitled Folder' + })); + const name = addSuffixToDuplicateName({ name: folderName }, index, folderSiblings); + // Process folder return { uid: uuid(), - name: item.name || 'Untitled Folder', + name, type: 'folder', items: parseCollectionItems(item.children, item.children) };Also applies to: 217-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/src/insomnia/insomnia-to-bruno.js` around lines 23 - 35, The v5 folder conversion still sets name directly as `name: item.name || 'Untitled Folder'`, bypassing case-insensitive dedupe; update the v5 folder branch to use the existing addSuffixToDuplicateName helper instead (call addSuffixToDuplicateName(item, index, allItems) or the appropriate sibling-folder array used in the v5 conversion) so folders like `OAuth2` / `oAuth2` get distinct suffixed names; ensure you pass the same allItems collection and index semantics as used elsewhere so behavior matches the v4/path request dedupe.packages/bruno-converters/src/postman/postman-to-bruno.js (1)
368-380:⚠️ Potential issue | 🟡 MinorUse
Setinstead of plain objects for case-insensitive name deduplication.When
folderNameorrequestName(from Postman imports) is lowercased and stored in a plain object, it can collide with inherited properties likeconstructor,__proto__, ortoString. This breaks deduplication — a request named "Constructor" would incorrectly pass thewhile (folderMap[folderName.toLowerCase()])check. Since these maps only verify existence (not retrieve values), aSetavoids the pitfall entirely.Proposed fix
- const folderMap = {}; - const requestMap = {}; + const folderNames = new Set(); + const requestNames = new Set(); - while (folderMap[folderName.toLowerCase()]) { + while (folderNames.has(folderName.toLowerCase())) { folderName = `${baseFolderName}_${count}`; count++; } - folderMap[folderName.toLowerCase()] = brunoFolderItem; + folderNames.add(folderName.toLowerCase()); - while (requestMap[requestName.toLowerCase()]) { + while (requestNames.has(requestName.toLowerCase())) { requestName = `${baseRequestName}_${count}`; count++; } - requestMap[requestName.toLowerCase()] = brunoRequestItem; + requestNames.add(requestName.toLowerCase());Also applies to: 435, 449, 809
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/src/postman/postman-to-bruno.js` around lines 368 - 380, The dedupe maps folderMap and requestMap are plain objects which can collide with inherited property names when using folderName.toLowerCase() or requestName.toLowerCase(); replace folderMap and requestMap with Set instances and update all checks and inserts (e.g., the while loop that checks folderMap[folderName.toLowerCase()] and similar checks for requestMap) to use Set.prototype.has() and Set.prototype.add(), ensuring isItemAFolder/item.forEach name deduplication uses lowercase keys consistently; also update the same pattern found around the request-name handling (the requestName de-duplication and any other occurrences referenced in the diff) to use Sets.
🤖 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 1169-1179: The per-directory name dedupe Set used in
parseCollectionItems (usedNamesLowercase) is created empty and therefore doesn't
reserve filenames that already exist on disk, allowing new items to overwrite
existing files; fix this by seeding usedNamesLowercase with the current
directory's existing filenames (lowercased) before calling Promise.all so
getUniqueSiblingName can avoid names already present on disk for the given
currentPath; use fs.readdir/fs.promises.readdir (or existing helper) to read
currentPath and add entries (normalized/lowercased and retaining extensions)
into usedNamesLowercase prior to iterating items.
---
Outside diff comments:
In `@packages/bruno-converters/src/insomnia/insomnia-to-bruno.js`:
- Around line 23-35: The v5 folder conversion still sets name directly as `name:
item.name || 'Untitled Folder'`, bypassing case-insensitive dedupe; update the
v5 folder branch to use the existing addSuffixToDuplicateName helper instead
(call addSuffixToDuplicateName(item, index, allItems) or the appropriate
sibling-folder array used in the v5 conversion) so folders like `OAuth2` /
`oAuth2` get distinct suffixed names; ensure you pass the same allItems
collection and index semantics as used elsewhere so behavior matches the v4/path
request dedupe.
In `@packages/bruno-converters/src/postman/postman-to-bruno.js`:
- Around line 368-380: The dedupe maps folderMap and requestMap are plain
objects which can collide with inherited property names when using
folderName.toLowerCase() or requestName.toLowerCase(); replace folderMap and
requestMap with Set instances and update all checks and inserts (e.g., the while
loop that checks folderMap[folderName.toLowerCase()] and similar checks for
requestMap) to use Set.prototype.has() and Set.prototype.add(), ensuring
isItemAFolder/item.forEach name deduplication uses lowercase keys consistently;
also update the same pattern found around the request-name handling (the
requestName de-duplication and any other occurrences referenced in the diff) to
use Sets.
In `@packages/bruno-converters/src/wsdl/wsdl-to-bruno.js`:
- Around line 116-128: Add a WSDL-specific regression test that exercises the
duplicate-name logic in addSuffixToDuplicateName
(packages/bruno-converters/src/wsdl/wsdl-to-bruno.js): create a minimal WSDL
string with sibling operations that differ only by case (e.g., GetUser and
getuser), run the wsdl-to-bruno conversion path used by your test harness, and
assert the converted Bruno operation names include the suffix on the
later-occurring case-duplicate (e.g., first remains "GetUser", second becomes
"getuser_1"). Ensure the test covers the WSDL converter code path (not just
Postman/Insomnia) and fails if names are not suffixed as expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b51c013f-0de8-439c-b51e-0e259e8812fd
📒 Files selected for processing (9)
packages/bruno-converters/src/insomnia/insomnia-to-bruno.jspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/src/wsdl/wsdl-to-bruno.jspackages/bruno-converters/tests/insomnia/dedupe-names.spec.jspackages/bruno-converters/tests/postman/postman-to-bruno/dedupe-folder-names.spec.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/utils/collection-import.jspackages/bruno-electron/src/utils/filesystem.jspackages/bruno-electron/tests/utils/filesystem.spec.js
Comment on lines
1169
to
+1179
| const parseCollectionItems = async (items = [], currentPath) => { | ||
| const usedNamesLowercase = new Set(); | ||
|
|
||
| await Promise.all(items.map(async (item) => { | ||
| if (['http-request', 'graphql-request', 'grpc-request', 'ws-request'].includes(item.type)) { | ||
| let sanitizedFilename = sanitizeName(getFilenameWithFormat(item, format)); | ||
| const sanitizedFilename = sanitizeName(getFilenameWithFormat(item, format)); | ||
| const ext = path.extname(sanitizedFilename); | ||
| const base = ext ? sanitizedFilename.slice(0, -ext.length) : sanitizedFilename; | ||
| const uniqueFilename = getUniqueSiblingName(base, ext, usedNamesLowercase); | ||
| const content = await stringifyRequestViaWorker(item, { format }); | ||
| const filePath = path.join(currentPath, sanitizedFilename); | ||
| const filePath = path.join(currentPath, uniqueFilename); |
Contributor
There was a problem hiding this comment.
Seed dedupe with existing directory entries to avoid metadata overwrites.
The per-directory Set starts empty, so it does not reserve files already written in that directory. In bru imports, a request named collection can still write collection.bru over the root collection file, and a child request named folder can overwrite folder.bru inside a folder.
🐛 Proposed fix
const parseCollectionItems = async (items = [], currentPath) => {
- const usedNamesLowercase = new Set();
+ const usedNamesLowercase = new Set(
+ fs.existsSync(currentPath)
+ ? fs.readdirSync(currentPath).map((name) => name.toLowerCase())
+ : []
+ );
await Promise.all(items.map(async (item) => {Also applies to: 1183-1205
🤖 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 1169 - 1179, The
per-directory name dedupe Set used in parseCollectionItems (usedNamesLowercase)
is created empty and therefore doesn't reserve filenames that already exist on
disk, allowing new items to overwrite existing files; fix this by seeding
usedNamesLowercase with the current directory's existing filenames (lowercased)
before calling Promise.all so getUniqueSiblingName can avoid names already
present on disk for the given currentPath; use fs.readdir/fs.promises.readdir
(or existing helper) to read currentPath and add entries (normalized/lowercased
and retaining extensions) into usedNamesLowercase prior to iterating items.
2 tasks
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
1 participant
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.
BRU-3177
Fixes #7821
Description
Collections imported from Postman/Insomnia/OpenAPI/WSDL that contain siblings with duplicate names or case-only variants (e.g.
OAuth2vsoAuth2) used to either crash withEEXIST(onboarding path) or silently merge two source folders into one on-disk directory (main UI path, case-insensitive filesystems like macOS APFS and default Windows NTFS). Either way the user ended up with a corrupted import.This PR addresses both failure modes:
parseCollectionItemsimplementations (IPC handler and onboarding util), track sibling names per parent directory via aSet<string>keyed ontoLowerCase(), and resolve collisions via a newgetUniqueSiblingNamehelper inutils/filesystem.js. Appends- Non collision (matches the existingfindUniqueFolderNameconvention used for top-level collection names). Applies to folders, requests, and.jssidecar files. Covers all importers since they share the writer path.postman-to-bruno.jsnow keysfolderMap/requestMapcase-insensitively;insomnia-to-bruno.jsandwsdl-to-bruno.jscompareaddSuffixToDuplicateNamecase-insensitively. This keeps the in-memory tree consistent with what gets written to disk.{ recursive: true }to themkdirSynccall inutils/collection-import.jsfor parity with the IPC handler (same fix as fix(import): resolve EEXIST error when importing OpenAPI collections with paths folder arrangement #7499 for the OpenAPI path).The dedup is case-insensitive on all platforms, not gated on OS — collections get shared across machines, and a Linux-clean import that breaks for a macOS teammate on re-import is a worse failure mode than seeing a numeric suffix on Linux.
Tests
packages/bruno-electron/tests/utils/filesystem.spec.js— 7 tests for the helper (no-collision, same-case collision, case-only collision, counter propagation past pre-reserved suffixes, file-extension handling, folder/file shared namespace, null/undefined ext handling).packages/bruno-converters/tests/postman/postman-to-bruno/dedupe-folder-names.spec.js— 3 tests (exact dupe folders, case-only dupe folders, case-only dupe requests).packages/bruno-converters/tests/insomnia/dedupe-names.spec.js— 2 tests (case-only dupe folders, case-only dupe requests).All existing converter tests (890) still pass.
Contribution Checklist:
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests