(size/L)feat:Added the option for bruno version edit in the collection overview#8424
Open
sachin-bruno wants to merge 2 commits into
Open
Conversation
sachin-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
Contributor
WalkthroughAdds collection version support across editing, display, serialization, and migration paths. The UI can now change a collection’s version, and BRU/YAML formats map the value through ChangesCollection Version Edit Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
sachin-bruno
force-pushed
the
sachin-bruno/feature-collection-version-edit
branch
from
042d893 to
aeb5890
Compare
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/bruno-electron/src/utils/collection-import.js (1)
96-116: 🗄️ Data Integrity & Integration | ��� Major | ⚡ Quick winDon't overwrite an imported YAML
versionhere either.This helper has the same regression: when
brunoConfig.versionexists on an OpenCollection import butcollectionVersiondoes not, Line 97 backfills'1', and Lines 113-115 then persist that fallback as the YAML version. Existing collection versions get rewritten on import.Suggested fix
- } else if (!brunoConfig.collectionVersion) { - brunoConfig.collectionVersion = '1'; + } else if (!brunoConfig.collectionVersion) { + brunoConfig.collectionVersion = brunoConfig.opencollection + ? (brunoConfig.version || '1') + : '1'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/utils/collection-import.js` around lines 96 - 116, The YAML import path in collection-import.js is still overwriting an existing imported version by backfilling collectionVersion to '1' and then copying it into brunoConfig.version. Update getBrunoJsonConfig and the format === 'yml' branch so that an existing brunoConfig.version is preserved for OpenCollection imports, only defaulting when neither version nor collectionVersion is present, and avoid assigning the fallback value back into the exported YAML version.packages/bruno-electron/src/ipc/collection.js (1)
1395-1427: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve imported OpenCollection versions instead of forcing
'1'.If the source YAML already populated
brunoConfig.versionbut notcollectionVersion, Line 1395 backfillscollectionVersion = '1', and Lines 1424-1426 immediately copy that fallback back intoversion. That rewrites an existinginfo.versionto"1"during import. SeedcollectionVersionfrom the existing YAMLversionfirst, and only default to'1'when neither field exists.Suggested fix
- } else if (!brunoConfig.collectionVersion) { - // Imports create new collection files; default the user-facing version when the - // source didn't carry one (a present version, e.g. OpenCollection info.version, is kept). - brunoConfig.collectionVersion = '1'; + } else if (!brunoConfig.collectionVersion) { + brunoConfig.collectionVersion = brunoConfig.opencollection + ? (brunoConfig.version || '1') + : '1'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-electron/src/ipc/collection.js` around lines 1395 - 1427, The import path in collection handling is overwriting an existing OpenCollection/YAML version by defaulting brunoConfig.collectionVersion to '1' when only brunoConfig.version is present, then later copying that fallback back into version. Update the logic in getBrunoJsonConfig and the subsequent format === 'yml' mapping so collectionVersion is seeded from an existing version first, and only falls back to '1' when both version and collectionVersion are missing. Preserve the original imported version for brunoConfig.opencollection imports and keep the delete of collectionVersion after mapping unchanged.
🧹 Nitpick comments (1)
tests/collection/change-version/change-version.spec.ts (1)
43-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid styling-class selectors in this E2E assertion.
preview.locator('.old')and.newtie the spec to modal internals instead of observable behavior, so a markup/style refactor can break the test without changing the feature. Prefer asserting the visible preview text or exposing dedicateddata-testids for the old/new values. As per coding guidelines, "E2E tests must verify user-visible behaviour, not implementation details." As per path instructions, "Replace brittle text/index selectors with role, label, test id, or stable user-facing selectors."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/collection/change-version/change-version.spec.ts` around lines 43 - 47, The change-version E2E assertion is using brittle styling-class selectors for the preview values, so update the test in change-version.spec.ts to avoid preview.locator('.old') and .new. Use user-visible text assertions or stable dedicated data-testid hooks on the change-version-preview elements so the test targets observable behavior instead of modal internals.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/components/CollectionSettings/Overview/Info/index.js`:
- Line 24: The collection version row is using getCollectionVersion() in a way
that hides the “Not Set” state for legacy YAML collections because the helper
falls back to a default version. Update the Info component’s collectionVersion
handling to read from the explicitly persisted version field (or change
getCollectionVersion() so it only returns a saved value and no YAML fallback),
and keep “Not Set” as the display until the user saves a version. Make sure the
fix preserves the existing behavior for OpenCollection without auto-migrating
legacy data.
- Around line 52-67: The Version row in the Overview Info component is
implemented as a clickable div, which prevents proper keyboard activation.
Update the interactive wrapper around the Version label and Change action in
Info to use a real button (or equivalent semantic control) while preserving the
existing onClick that opens ChangeCollectionVersion. Keep the current
layout/styling and data-testid hooks intact so the action remains accessible via
Tab, Enter, and Space.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.js`:
- Around line 44-55: The submit guard in ChangeCollectionVersion/index.js is
blocking the “clear version” flow because canSubmit requires
trimmedVersion.length > 0; update the canSubmit logic so an empty trimmedVersion
is allowed when it differs from currentVersion, while still preventing no-op
submits. Keep the change localized to the change handler flow around
trimmedVersion, canSubmit, and handleConfirm so saveCollectionVersion() can
receive an empty string and clear the stored collectionVersion.
In `@packages/bruno-app/src/utils/collections/index.js`:
- Around line 1264-1267: The version helper for OpenCollection YAML collections
is reintroducing a hardcoded fallback, which prevents missing versions from
staying unset. Update getCollectionVersion in collections/index.js so it returns
only the stored brunoConfig.version and does not substitute '1'; keep the
defaulting behavior only in the new-collection creation flow where
brunoConfig.version is initially seeded. Make sure the logic around
isOpenCollectionFormat and the version-clearing reducer still allows the value
to remain absent.
In `@packages/bruno-converters/tests/opencollection/collection-version.spec.js`:
- Around line 1-45: The collection-version tests are missing coverage for the
cleared-version case, even though both converter directions treat an empty
string as unset. Update the existing openCollectionToBruno and
brunoToOpenCollection specs to assert that info.version: '' imports to an
undefined brunoConfig.collectionVersion, and that an empty collectionVersion
exports to no info.version. Use the current converter entry points
brunoToOpenCollection and openCollectionToBruno so refactors can’t reintroduce
empty-string versions.
In `@packages/bruno-filestore/src/formats/yml/stringifyCollection.spec.ts`:
- Around line 51-77: The version serialization tests in stringifyCollection
should also cover the empty-string clear-version path, since stringifyCollection
special-cases version === ''. Add an explicit assertion in
stringifyCollection.spec that passes version: '' through stringifyCollection and
verifies parseCollection reads back an empty string (or the expected cleared
value), alongside the existing undefined and free-form version cases.
---
Outside diff comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 1395-1427: The import path in collection handling is overwriting
an existing OpenCollection/YAML version by defaulting
brunoConfig.collectionVersion to '1' when only brunoConfig.version is present,
then later copying that fallback back into version. Update the logic in
getBrunoJsonConfig and the subsequent format === 'yml' mapping so
collectionVersion is seeded from an existing version first, and only falls back
to '1' when both version and collectionVersion are missing. Preserve the
original imported version for brunoConfig.opencollection imports and keep the
delete of collectionVersion after mapping unchanged.
In `@packages/bruno-electron/src/utils/collection-import.js`:
- Around line 96-116: The YAML import path in collection-import.js is still
overwriting an existing imported version by backfilling collectionVersion to '1'
and then copying it into brunoConfig.version. Update getBrunoJsonConfig and the
format === 'yml' branch so that an existing brunoConfig.version is preserved for
OpenCollection imports, only defaulting when neither version nor
collectionVersion is present, and avoid assigning the fallback value back into
the exported YAML version.
---
Nitpick comments:
In `@tests/collection/change-version/change-version.spec.ts`:
- Around line 43-47: The change-version E2E assertion is using brittle
styling-class selectors for the preview values, so update the test in
change-version.spec.ts to avoid preview.locator('.old') and .new. Use
user-visible text assertions or stable dedicated data-testid hooks on the
change-version-preview elements so the test targets observable behavior instead
of modal internals.
🪄 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: 98ac1ada-7dc2-47c4-9586-a796587d9bd9
📒 Files selected for processing (33)
packages/bruno-app/src/components/CollectionSettings/Overview/Info/StyledWrapper.jspackages/bruno-app/src/components/CollectionSettings/Overview/Info/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/CollectionVersionInfo/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/CollectionVersionInfo/index.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-converters/src/opencollection/bruno-to-opencollection.tspackages/bruno-converters/src/opencollection/opencollection-to-bruno.tspackages/bruno-converters/src/opencollection/types.tspackages/bruno-converters/tests/opencollection/collection-version.spec.jspackages/bruno-electron/src/app/collections.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/utils/collection-import.jspackages/bruno-filestore/src/formats/yml/parseCollection.spec.tspackages/bruno-filestore/src/formats/yml/parseCollection.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.spec.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.tstests/collection/change-version-bru/change-version-bru.spec.tstests/collection/change-version-bru/fixtures/collection/bruno.jsontests/collection/change-version-bru/init-user-data/preferences.jsontests/collection/change-version/change-version.spec.tstests/collection/change-version/fixtures/collection/opencollection.ymltests/collection/change-version/init-user-data/preferences.jsontests/collection/generate-docs/fixtures/collection/bruno.jsontests/collection/generate-docs/generate-docs.spec.tstests/collection/migrate-to-yml/fixtures/collection/bruno.jsontests/collection/migrate-to-yml/migrate-to-yml.spec.ts
sundram-bruno
approved these changes
Jun 30, 2026
sachin-bruno
force-pushed
the
sachin-bruno/feature-collection-version-edit
branch
from
fef1ad2 to
65a9d08
Compare
bijin-bruno
reviewed
Jul 1, 2026
|
|
||
| const handleCopy = () => { | ||
| if (!trimmedVersion) return; | ||
| navigator.clipboard?.writeText(trimmedVersion); |
Collaborator
There was a problem hiding this comment.
Do we need optional chaining here? if navigator.clipboard is not available, it doesn't make sense to setCopied as true.
bijin-bruno
reviewed
Jul 1, 2026
Comment on lines
+73
to
+74
| title="Change Collection Version" | ||
| customHeader={<ModalTitle>Change Collection Version</ModalTitle>} |
Collaborator
There was a problem hiding this comment.
Suggested change
| title="Change Collection Version" | |
| customHeader={<ModalTitle>Change Collection Version</ModalTitle>} | |
| customHeader={<ModalTitle>Change Collection Version</ModalTitle>} |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/collection/change-version-bru/change-version-bru.spec.ts (1)
26-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the reused Playwright locators.
These steps keep re-resolving the same test ids, which makes failures noisier than necessary. Please store the modal/value/button locators once and reuse them across the steps. As per path instructions, "Use locator variables for locators".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/collection/change-version-bru/change-version-bru.spec.ts` around lines 26 - 47, The Playwright test in change-version-bru.spec.ts keeps re-querying the same test ids across steps, so hoist the repeated locators into variables and reuse them throughout the scenario. Update the relevant step setup around the overview/version flow to store the version value, change button, current value, input, and submit button locators once, then reference those variables in each step instead of calling getByTestId repeatedly.Source: Path instructions
tests/collection/change-version/change-version.spec.ts (1)
43-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid CSS-class selectors in this E2E assertion.
.oldand.neware presentation hooks, so a style-only rename will break the test without changing user behavior. Please expose stable test ids for the preview values (or assert the full preview text) and use those here instead. As per coding guidelines, "Adddata-testidto testable elements for Playwright"; as per path instructions, "Replace brittle text/index selectors with role, label, test id, or stable user-facing selectors".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/collection/change-version/change-version.spec.ts` around lines 43 - 47, The E2E assertion in the change-version preview is relying on brittle CSS class selectors, which should be replaced with stable test hooks. Update the preview component that renders the old/new values so it exposes testable identifiers (for example in the change-version preview UI), then change the assertions in change-version.spec.ts to target those stable identifiers or assert the full preview text instead of using .old and .new.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-electron/src/ipc/collection.js`:
- Around line 1395-1398: `collectionVersion` is being checked with truthiness in
the import/migration flow, which incorrectly treats `0` as missing and can reset
or drop valid numeric versions. Update the null handling in the `collection.js`
paths around `brunoConfig.collectionVersion` and `info.version` to use explicit
null/undefined checks instead of falsy checks, so imported versions round-trip
correctly without changing `0`.
- Around line 1711-1722: The save flow in collection.js should not continue when
`collectionRoot` is missing and `parseCollection()` fails while recovering the
YAML root. Update the `rootToWrite` recovery path so that the `catch` in the
`opencollection.yml` read/parse block surfaces the failure instead of falling
back to `collectionRoot`, and prevent `stringifyCollection()` from writing when
the root cannot be reconstructed. Use the `rootToWrite`, `parseCollection()`,
and `stringifyCollection()` logic in this handler to fail the save with an error
rather than overwriting the existing document payload.
---
Nitpick comments:
In `@tests/collection/change-version-bru/change-version-bru.spec.ts`:
- Around line 26-47: The Playwright test in change-version-bru.spec.ts keeps
re-querying the same test ids across steps, so hoist the repeated locators into
variables and reuse them throughout the scenario. Update the relevant step setup
around the overview/version flow to store the version value, change button,
current value, input, and submit button locators once, then reference those
variables in each step instead of calling getByTestId repeatedly.
In `@tests/collection/change-version/change-version.spec.ts`:
- Around line 43-47: The E2E assertion in the change-version preview is relying
on brittle CSS class selectors, which should be replaced with stable test hooks.
Update the preview component that renders the old/new values so it exposes
testable identifiers (for example in the change-version preview UI), then change
the assertions in change-version.spec.ts to target those stable identifiers or
assert the full preview text instead of using .old and .new.
🪄 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: d7714d57-cbd4-4111-930b-f5af375dde18
📒 Files selected for processing (33)
packages/bruno-app/src/components/CollectionSettings/Overview/Info/StyledWrapper.jspackages/bruno-app/src/components/CollectionSettings/Overview/Info/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/CollectionVersionInfo/index.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/CollectionVersionInfo/index.spec.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-converters/src/opencollection/bruno-to-opencollection.tspackages/bruno-converters/src/opencollection/opencollection-to-bruno.tspackages/bruno-converters/src/opencollection/types.tspackages/bruno-converters/tests/opencollection/collection-version.spec.jspackages/bruno-electron/src/app/collections.jspackages/bruno-electron/src/ipc/collection.jspackages/bruno-electron/src/utils/collection-import.jspackages/bruno-filestore/src/formats/yml/parseCollection.spec.tspackages/bruno-filestore/src/formats/yml/parseCollection.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.spec.tspackages/bruno-filestore/src/formats/yml/stringifyCollection.tstests/collection/change-version-bru/change-version-bru.spec.tstests/collection/change-version-bru/fixtures/collection/bruno.jsontests/collection/change-version-bru/init-user-data/preferences.jsontests/collection/change-version/change-version.spec.tstests/collection/change-version/fixtures/collection/opencollection.ymltests/collection/change-version/init-user-data/preferences.jsontests/collection/generate-docs/fixtures/collection/bruno.jsontests/collection/generate-docs/generate-docs.spec.tstests/collection/migrate-to-yml/fixtures/collection/bruno.jsontests/collection/migrate-to-yml/migrate-to-yml.spec.ts
✅ Files skipped from review due to trivial changes (8)
- tests/collection/change-version/init-user-data/preferences.json
- tests/collection/change-version-bru/fixtures/collection/bruno.json
- tests/collection/change-version/fixtures/collection/opencollection.yml
- tests/collection/migrate-to-yml/fixtures/collection/bruno.json
- packages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/StyledWrapper.js
- tests/collection/change-version-bru/init-user-data/preferences.json
- packages/bruno-app/src/components/CollectionSettings/Overview/Info/StyledWrapper.js
- packages/bruno-filestore/src/formats/yml/parseCollection.spec.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- tests/collection/migrate-to-yml/migrate-to-yml.spec.ts
- tests/collection/generate-docs/fixtures/collection/bruno.json
- packages/bruno-filestore/src/formats/yml/parseCollection.ts
- packages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.spec.js
- packages/bruno-converters/src/opencollection/opencollection-to-bruno.ts
- packages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/index.js
- packages/bruno-filestore/src/formats/yml/stringifyCollection.ts
- packages/bruno-electron/src/utils/collection-import.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/StyledWrapper.js
- packages/bruno-converters/tests/opencollection/collection-version.spec.js
- packages/bruno-electron/src/app/collections.js
- packages/bruno-app/src/components/CollectionSettings/Overview/Info/index.js
- tests/collection/generate-docs/generate-docs.spec.ts
- packages/bruno-converters/src/opencollection/types.ts
- packages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/CollectionVersionInfo/index.spec.js
- packages/bruno-filestore/src/formats/yml/stringifyCollection.spec.ts
- packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
- packages/bruno-app/src/utils/collections/index.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/ChangeCollectionVersion/index.js
- packages/bruno-converters/src/opencollection/bruno-to-opencollection.ts
- packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
Comment on lines
+1395
to
+1398
| } else if (!brunoConfig.collectionVersion) { | ||
| // Imports create new collection files; default the user-facing version when the | ||
| // source didn't carry one (a present version, e.g. OpenCollection info.version, is kept). | ||
| brunoConfig.collectionVersion = '1'; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use nullish checks for collectionVersion.
These truthiness checks treat 0 as “missing”, so an imported/migrated numeric version can be reset to '1' or dropped entirely before it reaches info.version. Please gate on null/undefined instead of falsiness so numeric versions round-trip correctly.
Proposed fix
- } else if (!brunoConfig.collectionVersion) {
+ } else if (brunoConfig.collectionVersion == null) {
// Imports create new collection files; default the user-facing version when the
// source didn't carry one (a present version, e.g. OpenCollection info.version, is kept).
brunoConfig.collectionVersion = '1';
}
...
- if (brunoConfig.collectionVersion) {
+ if (brunoConfig.collectionVersion != null) {
brunoConfig.version = brunoConfig.collectionVersion;
}
...
- if (ymlBrunoConfig.collectionVersion) {
+ if (ymlBrunoConfig.collectionVersion != null) {
ymlBrunoConfig.version = ymlBrunoConfig.collectionVersion;
}Also applies to: 1424-1426, 2700-2703
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1395 - 1398,
`collectionVersion` is being checked with truthiness in the import/migration
flow, which incorrectly treats `0` as missing and can reset or drop valid
numeric versions. Update the null handling in the `collection.js` paths around
`brunoConfig.collectionVersion` and `info.version` to use explicit
null/undefined checks instead of falsy checks, so imported versions round-trip
correctly without changing `0`.
Comment on lines
+1711
to
+1722
| if (!rootToWrite) { | ||
| const ocYmlPath = path.join(collectionPath, 'opencollection.yml'); | ||
| if (fs.existsSync(ocYmlPath)) { | ||
| try { | ||
| const existing = fs.readFileSync(ocYmlPath, 'utf8'); | ||
| rootToWrite = parseCollection(existing, { format }).collectionRoot; | ||
| } catch (e) { | ||
| rootToWrite = collectionRoot; | ||
| } | ||
| } | ||
| } | ||
| const content = await stringifyCollection(rootToWrite, transformedBrunoConfig, { format }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail the save if the YAML root cannot be recovered.
If collectionRoot is omitted and parseCollection() throws here, rootToWrite stays unset and the handler still writes opencollection.yml. That turns a config-only update into a destructive overwrite of the existing root/documentation payload instead of surfacing the read/parse failure.
Proposed fix
let rootToWrite = collectionRoot;
if (!rootToWrite) {
const ocYmlPath = path.join(collectionPath, 'opencollection.yml');
if (fs.existsSync(ocYmlPath)) {
- try {
- const existing = fs.readFileSync(ocYmlPath, 'utf8');
- rootToWrite = parseCollection(existing, { format }).collectionRoot;
- } catch (e) {
- rootToWrite = collectionRoot;
- }
+ const existing = fs.readFileSync(ocYmlPath, 'utf8');
+ rootToWrite = parseCollection(existing, { format }).collectionRoot;
}
}
+ if (!rootToWrite) {
+ throw new Error('Unable to update YAML config without a collection root');
+ }
const content = await stringifyCollection(rootToWrite, transformedBrunoConfig, { format });📝 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
| if (!rootToWrite) { | |
| const ocYmlPath = path.join(collectionPath, 'opencollection.yml'); | |
| if (fs.existsSync(ocYmlPath)) { | |
| try { | |
| const existing = fs.readFileSync(ocYmlPath, 'utf8'); | |
| rootToWrite = parseCollection(existing, { format }).collectionRoot; | |
| } catch (e) { | |
| rootToWrite = collectionRoot; | |
| } | |
| } | |
| } | |
| const content = await stringifyCollection(rootToWrite, transformedBrunoConfig, { format }); | |
| let rootToWrite = collectionRoot; | |
| if (!rootToWrite) { | |
| const ocYmlPath = path.join(collectionPath, 'opencollection.yml'); | |
| if (fs.existsSync(ocYmlPath)) { | |
| const existing = fs.readFileSync(ocYmlPath, 'utf8'); | |
| rootToWrite = parseCollection(existing, { format }).collectionRoot; | |
| } | |
| } | |
| if (!rootToWrite) { | |
| throw new Error('Unable to update YAML config without a collection root'); | |
| } | |
| const content = await stringifyCollection(rootToWrite, transformedBrunoConfig, { format }); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 1714-1714: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(ocYmlPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/bruno-electron/src/ipc/collection.js` around lines 1711 - 1722, The
save flow in collection.js should not continue when `collectionRoot` is missing
and `parseCollection()` fails while recovering the YAML root. Update the
`rootToWrite` recovery path so that the `catch` in the `opencollection.yml`
read/parse block surfaces the failure instead of falling back to
`collectionRoot`, and prevent `stringifyCollection()` from writing when the root
cannot be reconstructed. Use the `rootToWrite`, `parseCollection()`, and
`stringifyCollection()` logic in this handler to fail the save with an error
rather than overwriting the existing document payload.
bijin-bruno
requested changes
Jul 1, 2026
Collaborator
There was a problem hiding this comment.
- modal resizes with long string input.
- Add input validation for version?
| <span className="version-value" data-testid="version-value">{`Version: ${version}`}</span> | ||
| ) : null} | ||
| ) : ( | ||
| <span className="version-value unset" data-testid="version-value">Not Set</span> |
Collaborator
There was a problem hiding this comment.
Not Set is probably unclear without a label
| }; | ||
|
|
||
| const config = toOpenCollectionConfig(collection.brunoConfig as BrunoConfig); | ||
| const brunoConfig = collection.brunoConfig as BrunoConfig | undefined; |
Collaborator
There was a problem hiding this comment.
Is there any case were collection.brunoConfig is undefined ?
|
|
||
| const isOpenCollectionFormat = Boolean(brunoConfig?.opencollection); | ||
| const collectionVersion = isOpenCollectionFormat ? brunoConfig?.version : brunoConfig?.collectionVersion; | ||
| if (openCollection.info && collectionVersion != null && collectionVersion !== '') { |
Collaborator
There was a problem hiding this comment.
openCollection.info is always truthy here
| version: Yup.string().when('opencollection', { | ||
| is: (opencollection) => Boolean(opencollection), | ||
| then: Yup.string().notRequired(), | ||
| otherwise: Yup.string().oneOf(['1']).notRequired() |
Collaborator
There was a problem hiding this comment.
Might need to allow more values
Comment on lines
+2
to
+9
| "maximized": false, | ||
| "lastOpenedCollections": ["{{collectionPath}}"], | ||
| "preferences": { | ||
| "onboarding": { | ||
| "hasLaunchedBefore": true, | ||
| "hasSeenWelcomeModal": true | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Keep only the fields which we want to override
Suggested change
| "maximized": false, | |
| "lastOpenedCollections": ["{{collectionPath}}"], | |
| "preferences": { | |
| "onboarding": { | |
| "hasLaunchedBefore": true, | |
| "hasSeenWelcomeModal": true | |
| } | |
| } | |
| "lastOpenedCollections": ["{{collectionPath}}"], |
Comment on lines
+178
to
+181
| // bru only: the user-facing collection version (bruno.json key, sibling of the marker). | ||
| collectionVersion?: string; | ||
| // present only for OpenCollection (yml) collections; its presence marks the format. | ||
| opencollection?: string; |
Collaborator
There was a problem hiding this comment.
We should avoid lang specific fields inside the app (BrunoConfig).
eg: if we have BrunoConfig.collectionVersion, it should map to collectionVersion in .bru and info.version in .yml/OpenCollection.
The internal model should remain format-agnostic, with the mapping handled by the respective parsers/serializers.
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
A collection has no visible, editable version in the Bruno UI. yml (opencollection) collections define info.version in their schema, but Bruno never reads, displays, or lets users set it. .bru collections have no collection-version field at all (bruno.json carries only the bru schema version). Users can't see what version a collection is at, or set one, in either format.
Proposed Solution
Add a Version section to the collection Overview page (Collection Settings → Overview) for both formats:
Display the current collection version. yml reads info.version; .bru reads a new collectionVersion field in bruno.json, added after the existing version field (which remains the bru schema version). Collections that have never set a version show "Not set".
A “Change” text button opens the Change Collection Version modal: the current version, a free-form New Version text field, and a preview line ("Updates info.version in collection YAML" for yml / "Updates collectionVersion in bruno.json" for .bru, from → ). Update Version saves.
New collections default the version to 1 (yml info.version and .bru collectionVersion both default to "1"). Existing collections are not auto-migrated; the value is written only when the user sets one.
https://usebruno.atlassian.net/browse/BRU-2545
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.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests