feat: Set JavaScript sandbox to safe mode by default for new collections#4824
Conversation
cd0d20f to
1010ffe
Compare
WalkthroughRemoved the interactive JsSandboxMode modal and its styled wrapper, defaulted collection sandbox mode to 'safe', initialized that default on collection expand via a Redux dispatch, and simplified test helpers and test cases by removing sandbox-accept flows and related options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js (1)
5-5: Remove unused import.The
JsSandboxModeModalimport is no longer used since the modal rendering logic was removed.Apply this diff:
-import JsSandboxModeModal from '../JsSandboxModeModal'; import StyledWrapper from './StyledWrapper';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js(1 hunks)packages/bruno-app/src/components/SecuritySettings/JsSandboxModeModal/StyledWrapper.js(0 hunks)packages/bruno-app/src/components/SecuritySettings/JsSandboxModeModal/index.js(0 hunks)packages/bruno-electron/src/ipc/collection.js(3 hunks)
💤 Files with no reviewable changes (2)
- packages/bruno-app/src/components/SecuritySettings/JsSandboxModeModal/index.js
- packages/bruno-app/src/components/SecuritySettings/JsSandboxModeModal/StyledWrapper.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.jspackages/bruno-electron/src/ipc/collection.js
🧠 Learnings (1)
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
packages/bruno-electron/src/ipc/collection.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js (1)
packages/bruno-app/src/components/SecuritySettings/index.js (1)
jsSandboxMode(9-9)
packages/bruno-electron/src/ipc/collection.js (5)
packages/bruno-electron/src/ipc/network/index.js (1)
collectionPath(338-338)packages/bruno-cli/src/commands/run.js (1)
collectionPath(295-295)packages/bruno-electron/src/utils/collection-import.js (2)
collectionPath(13-13)collectionPath(28-28)packages/bruno-electron/src/ipc/network/prepare-request.js (1)
collectionPath(309-309)packages/bruno-cli/src/runner/prepare-request.js (1)
collectionPath(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - macOS
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (4)
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js (1)
9-9: LGTM!The fallback to 'safe' mode aligns with the PR objective to set JavaScript sandbox to safe mode by default for new collections. This is consistent with the parent component's implementation.
packages/bruno-electron/src/ipc/collection.js (3)
163-165: LGTM!Correctly establishes the default security config with safe mode for newly created collections. The placement before stats retrieval and UI signaling is appropriate.
225-227: LGTM!Consistent with the create-collection handler. Ensures cloned collections also default to safe mode regardless of the source collection's security config.
943-945: LGTM!Completes the consistent pattern across all collection creation flows (create, clone, import). Imported collections will default to safe mode, enhancing security posture.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/websockets/variable-interpolation/variable-interpolation.spec.ts (3)
53-55: Consider removing redundant timeout.The
waitForTimeout(300)on line 55 may be unnecessary since line 58 already waits for the request to become visible with a proper Playwright assertion.Apply this diff to remove the redundant timeout:
// Click to expand the collection await page.locator('#sidebar-collection-name').filter({ hasText: 'variable-interpolation' }).click(); - await page.waitForTimeout(300); // Open the requestAs per coding guidelines, prefer proper locator waits over arbitrary timeouts.
30-30: Consider replacing arbitrary timeouts with proper waits.Lines 30 and 77 use
waitForTimeout(), which the coding guidelines recommend avoiding. Consider using proper Playwright assertions instead:
- Line 30: Instead of waiting 200ms for environment to be applied, wait for a UI element that confirms the environment is active.
- Line 77: Instead of waiting 1000ms for echo server messages, use a proper wait for message count or specific content.
Example for line 77:
- // Wait a bit for messages to be sent and received (echo server echoes back) - await page.waitForTimeout(1000); - // Verify the sent message contains interpolated value // Should send {"test": "test-data"} (not {"test": "{{data}}"}) const messages = locators.messages(); // Find the outgoing message with interpolated content // The echo server will echo back the same message, so we should see it twice const sentMessage = messages.filter({ hasText: 'test-data' }).first(); - await expect(sentMessage).toBeAttached({ timeout: 2000 }); + await expect(sentMessage).toBeVisible({ timeout: 3000 });As per coding guidelines, prefer proper locator waits over arbitrary timeouts.
Also applies to: 77-77
13-45: Consider using test.step for better report readability.Both test functions contain multiple logical steps (open collection, select environment, connect, verify). Using
test.stepwould make the test reports easier to read and debug.Example refactor for the first test:
test('interpolates variables in WebSocket URL', async ({ pageWithUserData: page, restartApp }) => { await test.step('Setup: Open collection and request', async () => { const locators = buildWebsocketCommonLocators(page); await openCollection(page, 'variable-interpolation'); await expect(page.getByTitle(BRU_REQ_NAME)).toBeVisible(); await page.getByTitle(BRU_REQ_NAME).click(); }); await test.step('Select test environment', async () => { await page.locator('div.current-environment').click(); await expect(page.locator('.dropdown-item').filter({ hasText: 'Test' })).toBeVisible(); await page.locator('.dropdown-item').filter({ hasText: 'Test' }).click(); await expect(page.locator('.current-environment').filter({ hasText: /Test/ })).toBeVisible(); }); await test.step('Connect WebSocket and verify interpolated URL', async () => { const locators = buildWebsocketCommonLocators(page); await locators.connectionControls.connect().click(); await expect(locators.connectionControls.disconnect()).toBeAttached({ timeout: MAX_CONNECTION_TIME }); await expect(locators.messages().first().getByText(/Connected to wss:\/\/echo\.websocket\.org/)).toBeAttached({ timeout: 2000 }); }); });As per coding guidelines,
test.stepimproves generated reports significantly.Also applies to: 47-95
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
tests/collection/close-all-collections/close-all-collections.spec.ts(5 hunks)tests/collection/create/create-collection.spec.ts(0 hunks)tests/collection/default-ignores/default-ignores.spec.ts(3 hunks)tests/collection/draft/draft-indicator.spec.ts(2 hunks)tests/collection/draft/draft-values-in-requests.spec.ts(3 hunks)tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts(4 hunks)tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts(4 hunks)tests/collection/moving-requests/tag-persistence.spec.ts(0 hunks)tests/collection/moving-tabs/move-tabs.spec.ts(0 hunks)tests/environments/create-environment/collection-env-create.spec.ts(1 hunks)tests/environments/create-environment/global-env-create.spec.ts(1 hunks)tests/environments/import-environment/collection-env-import.spec.ts(0 hunks)tests/environments/import-environment/global-env-import.spec.ts(0 hunks)tests/global-environments/non-string-values.spec.ts(2 hunks)tests/import/insomnia/import-insomnia-v4-environments.spec.ts(2 hunks)tests/import/insomnia/import-insomnia-v5-environments.spec.ts(2 hunks)tests/import/openapi/duplicate-operation-names-fix.spec.ts(0 hunks)tests/import/openapi/import-openapi-with-examples.spec.ts(3 hunks)tests/import/openapi/operation-name-with-newlines-fix.spec.ts(0 hunks)tests/import/openapi/path-based-grouping.spec.ts(0 hunks)tests/import/postman/import-postman-with-examples.spec.ts(2 hunks)tests/import/wsdl/import-wsdl.spec.ts(3 hunks)tests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.ts(2 hunks)tests/preferences/autosave/autosave.spec.ts(2 hunks)tests/request/copy-request/copy-folder.spec.ts(2 hunks)tests/request/copy-request/copy-request.spec.ts(2 hunks)tests/request/copy-request/keyboard-shortcuts.spec.ts(1 hunks)tests/request/delete-request/delete-request-sequence-updation.spec.ts(1 hunks)tests/request/encoding/curl-encoding.spec.ts(0 hunks)tests/request/newlines/newlines-persistence.spec.ts(2 hunks)tests/request/save/save.spec.ts(0 hunks)tests/response/large-response-crash-prevention.spec.ts(1 hunks)tests/response/response-actions.spec.ts(1 hunks)tests/runner/collection-run.ts(0 hunks)tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts(2 hunks)tests/utils/page/actions.ts(4 hunks)tests/variable-tooltip/variable-tooltip.spec.ts(6 hunks)tests/websockets/variable-interpolation/variable-interpolation.spec.ts(3 hunks)
💤 Files with no reviewable changes (11)
- tests/collection/create/create-collection.spec.ts
- tests/environments/import-environment/global-env-import.spec.ts
- tests/request/encoding/curl-encoding.spec.ts
- tests/collection/moving-tabs/move-tabs.spec.ts
- tests/import/openapi/operation-name-with-newlines-fix.spec.ts
- tests/import/openapi/duplicate-operation-names-fix.spec.ts
- tests/import/openapi/path-based-grouping.spec.ts
- tests/environments/import-environment/collection-env-import.spec.ts
- tests/collection/moving-requests/tag-persistence.spec.ts
- tests/runner/collection-run.ts
- tests/request/save/save.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/environments/create-environment/global-env-create.spec.tstests/request/copy-request/keyboard-shortcuts.spec.tstests/import/openapi/import-openapi-with-examples.spec.tstests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.tstests/preferences/autosave/autosave.spec.tstests/request/copy-request/copy-folder.spec.tstests/global-environments/non-string-values.spec.tstests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/collection/close-all-collections/close-all-collections.spec.tstests/response/large-response-crash-prevention.spec.tstests/environments/create-environment/collection-env-create.spec.tstests/import/insomnia/import-insomnia-v4-environments.spec.tstests/collection/draft/draft-values-in-requests.spec.tstests/import/wsdl/import-wsdl.spec.tstests/collection/default-ignores/default-ignores.spec.tstests/request/copy-request/copy-request.spec.tstests/utils/page/actions.tstests/request/newlines/newlines-persistence.spec.tstests/response/response-actions.spec.tstests/variable-tooltip/variable-tooltip.spec.tstests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.tstests/request/delete-request/delete-request-sequence-updation.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/websockets/variable-interpolation/variable-interpolation.spec.tstests/import/insomnia/import-insomnia-v5-environments.spec.tstests/collection/draft/draft-indicator.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/environments/create-environment/global-env-create.spec.tstests/request/copy-request/keyboard-shortcuts.spec.tstests/import/openapi/import-openapi-with-examples.spec.tstests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.tstests/preferences/autosave/autosave.spec.tstests/request/copy-request/copy-folder.spec.tstests/global-environments/non-string-values.spec.tstests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/collection/close-all-collections/close-all-collections.spec.tstests/response/large-response-crash-prevention.spec.tstests/environments/create-environment/collection-env-create.spec.tstests/import/insomnia/import-insomnia-v4-environments.spec.tstests/collection/draft/draft-values-in-requests.spec.tstests/import/wsdl/import-wsdl.spec.tstests/collection/default-ignores/default-ignores.spec.tstests/request/copy-request/copy-request.spec.tstests/utils/page/actions.tstests/request/newlines/newlines-persistence.spec.tstests/response/response-actions.spec.tstests/variable-tooltip/variable-tooltip.spec.tstests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.tstests/request/delete-request/delete-request-sequence-updation.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/websockets/variable-interpolation/variable-interpolation.spec.tstests/import/insomnia/import-insomnia-v5-environments.spec.tstests/collection/draft/draft-indicator.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
tests/environments/create-environment/global-env-create.spec.tstests/import/openapi/import-openapi-with-examples.spec.tstests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.tstests/preferences/autosave/autosave.spec.tstests/request/copy-request/copy-folder.spec.tstests/global-environments/non-string-values.spec.tstests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/collection/close-all-collections/close-all-collections.spec.tstests/response/large-response-crash-prevention.spec.tstests/environments/create-environment/collection-env-create.spec.tstests/import/insomnia/import-insomnia-v4-environments.spec.tstests/collection/draft/draft-values-in-requests.spec.tstests/import/wsdl/import-wsdl.spec.tstests/collection/default-ignores/default-ignores.spec.tstests/request/copy-request/copy-request.spec.tstests/request/newlines/newlines-persistence.spec.tstests/response/response-actions.spec.tstests/variable-tooltip/variable-tooltip.spec.tstests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/websockets/variable-interpolation/variable-interpolation.spec.tstests/import/insomnia/import-insomnia-v5-environments.spec.tstests/collection/draft/draft-indicator.spec.ts
🧬 Code graph analysis (18)
tests/request/copy-request/keyboard-shortcuts.spec.ts (2)
tests/utils/page/actions.ts (1)
createCollection(693-693)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/request/copy-request/copy-folder.spec.ts (2)
tests/utils/page/actions.ts (1)
createCollection(693-693)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/global-environments/non-string-values.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts (1)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/collection/close-all-collections/close-all-collections.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/import/insomnia/import-insomnia-v4-environments.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/collection/draft/draft-values-in-requests.spec.ts (2)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
openCollection(2328-2345)openCollection(2328-2345)packages/bruno-electron/src/app/collections.js (1)
openCollection(100-131)
tests/import/wsdl/import-wsdl.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/collection/default-ignores/default-ignores.spec.ts (1)
packages/bruno-electron/src/app/collections.js (1)
openCollection(100-131)
tests/request/copy-request/copy-request.spec.ts (1)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/utils/page/actions.ts (2)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (4)
openCollection(2328-2345)openCollection(2328-2345)createCollection(2296-2316)createCollection(2296-2316)packages/bruno-electron/src/app/collections.js (1)
openCollection(100-131)
tests/response/response-actions.spec.ts (1)
tests/utils/page/actions.ts (1)
createCollection(693-693)
tests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)
tests/utils/page/actions.ts (1)
createCollection(693-693)
tests/request/delete-request/delete-request-sequence-updation.spec.ts (1)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/import/postman/import-postman-with-examples.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(692-692)
tests/collection/draft/draft-indicator.spec.ts (2)
tests/utils/page/actions.ts (1)
openCollection(692-692)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
openCollection(2328-2345)openCollection(2328-2345)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (28)
tests/websockets/variable-interpolation/variable-interpolation.spec.ts (1)
3-3: LGTM! Import updated correctly.The import change from
openCollectionAndAcceptSandboxtoopenCollectionaligns with the new API that no longer requires sandbox mode handling.tests/collection/draft/draft-values-in-requests.spec.ts (1)
2-2: LGTM! API simplification aligns with default sandbox behavior.The replacement of
openCollectionAndAcceptSandboxwithopenCollectionis consistent with the PR's objective to default sandbox mode to 'safe'. This simplifies the test code without changing behavior.Also applies to: 15-15, 122-122
tests/preferences/autosave/autosave.spec.ts (1)
18-18: LGTM! Simplified collection creation aligns with defaults.Removing the explicit
openWithSandboxMode: 'safe'option is appropriate since the default sandbox mode is now 'safe'. This reduces test boilerplate.Also applies to: 128-128
tests/request/delete-request/delete-request-sequence-updation.spec.ts (1)
13-13: LGTM! Collection creation simplified.The removal of the sandbox mode option is consistent with the PR's default behavior changes.
tests/environments/create-environment/global-env-create.spec.ts (1)
27-27: LGTM! Import flow simplified.The removal of explicit sandbox mode configuration from
importCollectionaligns with the default 'safe' sandbox behavior.tests/request/copy-request/copy-request.spec.ts (1)
10-10: LGTM! Test setup streamlined.Both
createCollectioncalls now rely on the default 'safe' sandbox mode, reducing explicit configuration.Also applies to: 58-58
tests/environments/create-environment/collection-env-create.spec.ts (1)
25-25: LGTM! Import simplified with default sandbox.The removal of the sandbox mode option from
importCollectionis consistent with the new default behavior.tests/ssl/custom-ca-certs/tests/wss-success/wss-success.spec.ts (1)
2-2: LGTM! WebSocket SSL test simplified.The API change from
openCollectionAndAcceptSandboxtoopenCollectionremoves the need for explicit sandbox mode handling in WebSocket SSL tests.Also applies to: 15-15
tests/import/postman/import-postman-with-examples.spec.ts (1)
3-3: LGTM! Import flow simplified.The replacement of
openCollectionAndAcceptSandboxwithopenCollectionis consistent with the PR's objective to default sandbox mode to 'safe'.Also applies to: 85-85
tests/request/copy-request/keyboard-shortcuts.spec.ts (1)
10-10: LGTM! API update aligns with simplified collection creation.The removal of the
openWithSandboxModeoption is consistent with the PR's objective to default sandbox mode to safe for new collections.tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts (1)
12-12: LGTM! Consistent API simplification across all test cases.All
createCollectioncalls correctly updated to use the simplified three-argument signature, aligning with the default safe sandbox mode.Also applies to: 48-48, 109-109, 144-144
tests/import/openapi/import-openapi-with-examples.spec.ts (1)
3-3: LGTM! API migration correctly applied.The replacement of
openCollectionAndAcceptSandboxwithopenCollectionis consistent with the removal of explicit sandbox-accept flows in the test suite.Also applies to: 81-81, 209-209
tests/global-environments/non-string-values.spec.ts (1)
2-2: LGTM! API update correctly applied.The migration to
openCollectionis consistent with the simplified collection-opening flow introduced in this PR.Also applies to: 14-14
tests/request/newlines/newlines-persistence.spec.ts (1)
2-2: LGTM! Simplified API usage is correct.The update to
openCollectionaligns with the removal of explicit sandbox mode handling in collection opening flows.Also applies to: 28-28
tests/import/insomnia/import-insomnia-v4-environments.spec.ts (1)
3-3: LGTM! API migration is consistent.The replacement of
openCollectionAndAcceptSandboxwithopenCollectioncorrectly reflects the simplified collection-opening API.Also applies to: 45-45
tests/response/response-actions.spec.ts (1)
21-21: LGTM! Simplified collection creation API correctly applied.The removal of the sandbox mode parameter aligns with the default safe mode behavior introduced in this PR.
tests/interpolation/dynamic-variable/set-var-dynamic-variable.spec.ts (1)
2-2: LGTM! API migration correctly implemented.The update to
openCollectionis consistent with the repository-wide refactoring to simplify sandbox mode handling.Also applies to: 14-14
tests/variable-tooltip/variable-tooltip.spec.ts (1)
25-26: LGTM! Correctly updated to new collection opening flow.The changes properly split collection creation and opening into two distinct steps, aligning with the removal of sandbox-accept flows across the test suite.
tests/response/large-response-crash-prevention.spec.ts (1)
18-18: LGTM! Correctly removed sandbox mode option.The simplified collection creation is appropriate here since
createUntitledRequestis called immediately afterward and likely handles collection selection.tests/import/wsdl/import-wsdl.spec.ts (1)
48-48: LGTM! Correctly migrated to new openCollection API.The change properly replaces the sandbox-accept flow with the simplified
openCollectionhelper.tests/collection/draft/draft-indicator.spec.ts (1)
17-17: LGTM! Correctly updated to new collection opening API.The migration from
openCollectionAndAcceptSandboxtoopenCollectionis correct and aligns with the removal of sandbox-mode handling.tests/collection/default-ignores/default-ignores.spec.ts (1)
87-87: LGTM! Correctly migrated to new openCollection API.The change properly replaces the sandbox-accept flow with the simplified helper.
tests/collection/close-all-collections/close-all-collections.spec.ts (1)
102-102: LGTM! Correctly updated to new collection opening API.All instances of
openCollectionAndAcceptSandboxhave been properly replaced withopenCollection, maintaining consistency across the test suite.tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)
12-12: Verify collection is properly opened before creating requests.This test creates a collection and immediately calls
createUntitledRequestwithout explicitly opening the collection. For consistency with other tests in this PR (e.g.,tests/variable-tooltip/variable-tooltip.spec.ts:25-26), consider addingawait openCollection(page, 'source-collection')after line 12.The same pattern appears at lines 25, 69, and 83. Verify whether
createUntitledRequestworks correctly without an explicitopenCollectioncall, or if these locations should also be updated.tests/request/copy-request/copy-folder.spec.ts (1)
10-10: This test's collection initialization pattern is correct for its use case.The test only manipulates collection sidebar UI (creating folders and requests, then copying/pasting). Unlike tests that perform collection-level operations like creating environments (e.g.,
variable-tooltip.spec.ts), this test doesn't require an explicitopenCollectioncall. ThecreateCollectionfunction appropriately positions the collection in the sidebar, and subsequent locator interactions work without needing to open the collection in the editor pane.tests/import/insomnia/import-insomnia-v5-environments.spec.ts (1)
3-3: LGTM! Clean migration to simplified API.The import and usage correctly reflect the new
openCollectionAPI that no longer requires explicit sandbox mode handling, aligning with the PR's objective to default collections to safe mode.Also applies to: 42-42
tests/utils/page/actions.ts (2)
53-70: LGTM! Correctly simplified to remove sandbox mode options.The removal of the options parameter and sandbox-related logic correctly aligns with the PR's objective to default collections to safe mode without requiring explicit configuration.
690-720: LGTM! Export statements correctly reflect the API changes.The updated function export and simplified type exports properly reflect the removal of sandbox-related configuration options.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts (3)
11-12: Update outdated comment.The comment states "open with sandbox mode" but the code no longer explicitly passes a sandbox mode option since 'safe' is now the default. Consider updating the comment to reflect the current behavior or removing it.
Apply this diff:
- // Create first collection - open with sandbox mode + // Create first collection await createCollection(page, 'source-collection', await createTmpDir('source-collection'));
47-48: Update outdated comment.Similar to line 11, this comment references "open with sandbox mode" but no sandbox option is passed. Update or remove the comment.
Apply this diff:
- // Create second collection - open with sandbox mode + // Create second collection await createCollection(page, 'target-collection', await createTmpDir('target-collection'));
104-105: Update outdated comment.The comment references sandbox mode, but no explicit option is passed since 'safe' is now the default.
Apply this diff:
- // Create first collection (source) - use unique names for this test + // Create first collection (source) await createCollection(page, 'source-collection', await createTmpDir('source-collection'));tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)
11-12: Update outdated comment.The comment states "open with sandbox mode" but no explicit sandbox option is passed since 'safe' is now the default. Update or remove the comment.
Apply this diff:
- // Create first collection - open with sandbox mode + // Create first collection await createCollection(page, 'source-collection', await createTmpDir('source-collection'));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tests/collection/draft/draft-indicator.spec.ts(1 hunks)tests/collection/draft/draft-values-in-requests.spec.ts(1 hunks)tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts(4 hunks)tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts(4 hunks)tests/import/postman/import-postman-with-examples.spec.ts(2 hunks)tests/response/large-response-crash-prevention.spec.ts(1 hunks)tests/utils/page/actions.ts(5 hunks)tests/variable-tooltip/variable-tooltip.spec.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/variable-tooltip/variable-tooltip.spec.ts
- tests/collection/draft/draft-indicator.spec.ts
- tests/collection/draft/draft-values-in-requests.spec.ts
- tests/utils/page/actions.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/response/large-response-crash-prevention.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/response/large-response-crash-prevention.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.tstests/import/postman/import-postman-with-examples.spec.tstests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts
🧬 Code graph analysis (4)
tests/collection/moving-requests/cross-collection-drag-drop-folder.spec.ts (1)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
createCollection(2296-2316)createCollection(2296-2316)
tests/response/large-response-crash-prevention.spec.ts (1)
tests/utils/page/actions.ts (1)
createCollection(694-694)
tests/import/postman/import-postman-with-examples.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(693-693)
tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)
tests/utils/page/actions.ts (1)
createCollection(694-694)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (3)
tests/response/large-response-crash-prevention.spec.ts (1)
17-18: LGTM! Correct simplification aligned with default sandbox mode.The removal of the explicit
openWithSandboxMode: 'safe'option is correct now that 'safe' mode is the default. The test continues to function properly with the default behavior, and the updated comment accurately reflects the simplified API.tests/import/postman/import-postman-with-examples.spec.ts (1)
84-85: LGTM! API simplification correctly implemented.The test step and API call have been updated to use the new simplified
openCollectionfunction, which aligns with the PR objective of defaulting sandbox mode to 'safe'. The updated step label 'Open collection' is also more accurate than the previous description.tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)
25-25: LGTM! Collection creation correctly simplified.The removal of explicit sandbox mode options from
createCollectioncalls aligns with the PR objective. Since 'safe' mode is now the default, these changes correctly simplify the test code without affecting behavior.Also applies to: 66-66, 80-80
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/utils/page/actions.ts (1)
4-4: Remove the unusedSandboxModetype.The type is defined and exported but never imported anywhere in the codebase. The
setSandboxModefunction uses inline literal types instead of referencingSandboxMode.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/collection/moving-tabs/move-tabs.spec.ts(2 hunks)tests/utils/page/actions.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/collection/moving-tabs/move-tabs.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/utils/page/actions.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/utils/page/actions.ts
🧬 Code graph analysis (1)
tests/utils/page/actions.ts (2)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (4)
openCollection(2328-2345)openCollection(2328-2345)createCollection(2296-2316)createCollection(2296-2316)packages/bruno-electron/src/app/collections.js (1)
openCollection(100-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (3)
tests/utils/page/actions.ts (3)
38-42: LGTM! Simplified collection opening flow.The function correctly implements the new simplified flow without sandbox modal interaction, aligning with the PR objectives.
53-53: LGTM! Correctly simplified createCollection API.The function signature and implementation properly reflect the removal of sandbox mode options, and the call to
openCollectionensures the newly created collection is opened in the UI.Also applies to: 69-69
695-695: LGTM! Export correctly updated.The export statement properly reflects the renamed function.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/utils/page/actions.ts (1)
4-4: Remove theSandboxModetype definition and its export.The
SandboxModetype (line 4) is no longer used anywhere in the codebase. With the sandbox modal flows removed in this PR, this type definition and its export at line 725 are dead code and should be deleted.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/onboarding/sample-collection.spec.ts(0 hunks)tests/utils/page/actions.ts(6 hunks)
💤 Files with no reviewable changes (1)
- tests/onboarding/sample-collection.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/utils/page/actions.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/utils/page/actions.ts
🧬 Code graph analysis (1)
tests/utils/page/actions.ts (2)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (4)
openCollection(2328-2345)openCollection(2328-2345)createCollection(2296-2316)createCollection(2296-2316)packages/bruno-electron/src/app/collections.js (1)
openCollection(100-131)
🔇 Additional comments (4)
tests/utils/page/actions.ts (4)
38-42: LGTM!The function has been correctly simplified to remove sandbox modal handling. The step description accurately reflects the current behavior (opening the collection only), addressing the previous review feedback.
53-71: LGTM!The function signature has been correctly simplified by removing the
optionsparameter. The flow now consistently creates the collection and opens it using the simplifiedopenCollectionhelper, aligning with the PR's goal of removing sandbox modal flows.
267-269: LGTM!The conditional check properly guards against calling
openCollectionwith an undefined or empty collection name, addressing the previous review feedback. The simplified flow correctly opens the imported collection only whenexpectedCollectionNameis provided.
695-725: LGTM!The exports have been correctly updated:
openCollectionreplaces the previousopenCollectionAndAcceptSandboxCreateCollectionOptionshas been properly removed from type exports- All other exports remain consistent with the simplified API
sid-bruno
left a comment
There was a problem hiding this comment.
Some recommended additions were made offline, waiting on that
…default-for-new-collections
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/global-environments/non-string-values.spec.ts (1)
2-2: Remove unused importopenCollectionAndAcceptSandbox.The import statement includes
openCollectionAndAcceptSandboxwhich is no longer used in this file after the refactor. This is dead code that should be cleaned up.🔎 Apply this diff to remove the unused import:
-import { openCollectionAndAcceptSandbox, closeAllCollections, sendRequest, addEnvironmentVariables } from '../utils/page'; +import { openCollection, closeAllCollections, sendRequest, addEnvironmentVariables } from '../utils/page';
🧹 Nitpick comments (2)
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts (1)
5-38: Consider wrapping logical sections in test.step for better reporting.While the test is clear, wrapping logical sections in
test.stepwould improve readability of test reports, as per the coding guidelines.🔎 Example refactor with test.step:
await createCollection(page, collectionName, await createTmpDir()); - // Verify sandbox mode is set to safe by default - const sandboxModeSelector = page.getByTestId('sandbox-mode-selector'); - await expect(sandboxModeSelector).toBeVisible(); - await expect(sandboxModeSelector).toHaveAttribute('title', 'Safe Mode'); - - // Click on sandbox mode selector to open security settings - await sandboxModeSelector.click(); - - // Change to developer mode - const developerRadio = page.locator('input[id="developer"]'); - await developerRadio.click(); - - // Save - const saveButton = page.getByRole('button', { name: 'Save' }); - await saveButton.click(); - - // Verify mode changed to developer - await expect(sandboxModeSelector).toHaveAttribute('title', 'Developer Mode'); - - // Close all tabs - await page.keyboard.press('Meta+Shift+W'); - - // Reopen the collection - await openCollection(page, collectionName); - - // Verify mode is still developer (persisted) - await expect(sandboxModeSelector).toHaveAttribute('title', 'Developer Mode'); + await test.step('Verify sandbox mode is set to safe by default', async () => { + const sandboxModeSelector = page.getByTestId('sandbox-mode-selector'); + await expect(sandboxModeSelector).toBeVisible(); + await expect(sandboxModeSelector).toHaveAttribute('title', 'Safe Mode'); + }); + + await test.step('Change to developer mode and save', async () => { + const sandboxModeSelector = page.getByTestId('sandbox-mode-selector'); + await sandboxModeSelector.click(); + + const developerRadio = page.locator('input[id="developer"]'); + await developerRadio.click(); + + const saveButton = page.getByRole('button', { name: 'Save' }); + await saveButton.click(); + + await expect(sandboxModeSelector).toHaveAttribute('title', 'Developer Mode'); + }); + + await test.step('Verify mode persists after reopening collection', async () => { + await page.keyboard.press('Meta+Shift+W'); + await openCollection(page, collectionName); + + const sandboxModeSelector = page.getByTestId('sandbox-mode-selector'); + await expect(sandboxModeSelector).toHaveAttribute('title', 'Developer Mode'); + });tests/utils/page/actions.ts (1)
261-269: Redundant conditional check for expectedCollectionName.The check on line 267 is redundant because line 261 already verifies
options.expectedCollectionNameexists before waiting for it in the sidebar. If it reaches line 267,expectedCollectionNameis guaranteed to be defined.🔎 Apply this diff to remove redundant check:
if (options.expectedCollectionName) { await expect( page.locator('#sidebar-collection-name').filter({ hasText: options.expectedCollectionName }) ).toBeVisible(); + await openCollection(page, options.expectedCollectionName); } - - if (options.expectedCollectionName) { - await openCollection(page, options.expectedCollectionName); - }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js(1 hunks)packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js(2 hunks)tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts(1 hunks)tests/environments/create-environment/collection-env-create.spec.ts(1 hunks)tests/environments/create-environment/global-env-create.spec.ts(1 hunks)tests/environments/import-environment/collection-env-import.spec.ts(0 hunks)tests/environments/import-environment/global-env-import.spec.ts(0 hunks)tests/global-environments/non-string-values.spec.ts(1 hunks)tests/import/insomnia/import-insomnia-v4-environments.spec.ts(2 hunks)tests/import/insomnia/import-insomnia-v5-environments.spec.ts(2 hunks)tests/request/newlines/newlines-persistence.spec.ts(1 hunks)tests/utils/page/actions.ts(6 hunks)
💤 Files with no reviewable changes (2)
- tests/environments/import-environment/collection-env-import.spec.ts
- tests/environments/import-environment/global-env-import.spec.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/environments/create-environment/collection-env-create.spec.ts
- tests/request/newlines/newlines-persistence.spec.ts
- tests/environments/create-environment/global-env-create.spec.ts
- tests/import/insomnia/import-insomnia-v5-environments.spec.ts
- packages/bruno-app/src/components/SecuritySettings/JsSandboxMode/index.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/import/insomnia/import-insomnia-v4-environments.spec.tstests/collection/default-sandbox-mode/default-sandbox-mode.spec.tstests/global-environments/non-string-values.spec.tspackages/bruno-app/src/components/Sidebar/Collections/Collection/index.jstests/utils/page/actions.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/import/insomnia/import-insomnia-v4-environments.spec.tstests/collection/default-sandbox-mode/default-sandbox-mode.spec.tstests/global-environments/non-string-values.spec.tstests/utils/page/actions.ts
🧠 Learnings (3)
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
tests/import/insomnia/import-insomnia-v4-environments.spec.tstests/utils/page/actions.ts
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.
Applied to files:
packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
🧬 Code graph analysis (2)
tests/import/insomnia/import-insomnia-v4-environments.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(709-709)
packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js (1)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (2)
saveCollectionSecurityConfig(2417-2431)saveCollectionSecurityConfig(2417-2431)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Linux
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (8)
tests/import/insomnia/import-insomnia-v4-environments.spec.ts (2)
3-3: LGTM! Import statement updated correctly.The import has been properly updated to use
openCollectioninstead of the deprecatedopenCollectionAndAcceptSandbox, aligning with the new simplified API that defaults to safe mode.
45-45: LGTM! Function call updated correctly.The call to
openCollectioncorrectly removes the sandbox mode parameter, as collections now default to safe mode automatically. This simplifies the test flow.tests/global-environments/non-string-values.spec.ts (1)
14-14: LGTM! Function call updated correctly.The call to
openCollectioncorrectly removes the sandbox mode parameter, consistent with the new default behavior.packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js (1)
25-25: LGTM! Import added correctly.The
saveCollectionSecurityConfigaction is properly imported and will be used to persist the default sandbox mode.tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts (1)
1-38: Good test coverage for default sandbox mode behavior.The test effectively verifies:
- Default safe mode on collection creation
- Mode change persistence
- UI state after reopen
Well-structured with clear assertions.
tests/utils/page/actions.ts (3)
38-42: LGTM! Function simplified correctly.The
openCollectionfunction is properly simplified, removing the sandbox acceptance logic. The test.step description was updated correctly (as noted in past review).
53-71: LGTM! Collection creation flow updated correctly.The
createCollectionfunction correctly:
- Removes the
CreateCollectionOptionstype- Always calls
openCollectionafter creation- Maintains the wait periods for UI stability
707-737: LGTM! Exports updated correctly.The public API exports are properly updated:
openCollectionAndAcceptSandboxremovedopenCollectionaddedCreateCollectionOptionsremoved from type exports
packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
Show resolved
Hide resolved
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/request/newlines/newlines-persistence.spec.ts (2)
5-78: Consider usingtest.stepfor improved test reporting.The test has clear logical phases (create collection, add data, save, restart, verify) that would benefit from
test.stepblocks for better readability and reporting. This aligns with the coding guidelines to "promote the use oftest.stepas much as possible so the generated reports are easier to read."🔎 Example refactor with test.step:
test('should persist request with newlines across app restarts', async ({ createTmpDir, launchElectronApp }) => { const userDataPath = await createTmpDir('newlines-persistence-userdata'); const collectionPath = await createTmpDir('newlines-persistence-collection'); - // Create collection and request - const app1 = await launchElectronApp({ userDataPath }); - const page = await app1.firstWindow(); + await test.step('Create collection and request', async () => { + const app1 = await launchElectronApp({ userDataPath }); + const page = await app1.firstWindow(); + // ... collection and request creation code ... + }); - // Verify persistence after restart - const app2 = await launchElectronApp({ userDataPath }); - const page2 = await app2.firstWindow(); - // ... verification code ... + await test.step('Add request data with newlines', async () => { + // ... add params, headers, vars ... + }); + + await test.step('Verify persistence after restart', async () => { + const app2 = await launchElectronApp({ userDataPath }); + const page2 = await app2.firstWindow(); + // ... verification code ... + });
2-2: Import change is correct; openCollection call is necessary for UI state management.The import change from
openCollectionAndAcceptSandboxtoopenCollectioncorrectly implements the default safe sandbox mode per the PR objective. TheopenCollection(page, 'newlines-persistence')call on line 28 is necessary—after creating the collection and request through modal dialogs (lines 13-26), the collection tree needs to be explicitly loaded/expanded in the sidebar before opening the request item on line 30.Consider using
test.step()throughout the test to improve readability and report visibility. This test spans multiple logical phases (create collection, add request data, save, restart app, verify persistence) that would benefit from step-based organization for debugging and reporting purposes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts(1 hunks)tests/request/newlines/newlines-persistence.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.tstests/request/newlines/newlines-persistence.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.tstests/request/newlines/newlines-persistence.spec.ts
🧠 Learnings (2)
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
tests/request/newlines/newlines-persistence.spec.ts
🧬 Code graph analysis (1)
tests/request/newlines/newlines-persistence.spec.ts (1)
tests/utils/page/actions.ts (1)
openCollection(709-709)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (3)
tests/collection/default-sandbox-mode/default-sandbox-mode.spec.ts (3)
1-2: LGTM!Imports are appropriate and follow the project's conventions.
4-8: LGTM!Test suite structure and setup are well-organized. The test name clearly describes the expected behavior.
30-31: LGTM! Cross-platform keyboard shortcut correctly implemented.The keyboard shortcut now works across all platforms using
process.platformdetection. This addresses the previous review feedback about Mac-specific shortcuts.
Description
JIRA
Issue: #4779
Contribution Checklist:
Screen.Recording.2025-06-03.at.4.28.16.PM.mov
Summary by CodeRabbit
New Features
Bug Fixes
Removals
Tests
✏️ Tip: You can customize this high-level summary in your review settings.