fix: correct the request type tabs in the snapshot#7994
Merged
Conversation
sid-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
WalkthroughAdds type-aware defaulting for request-pane tabs, persists ChangesRequest pane snapshot and tab restoration
Sequence Diagram(s)(No sequence diagrams generated: the core change is localized defaulting/deserialization and test additions; interactions are straightforward and captured in the layers.) Possibly related PRs
Suggested reviewers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/snapshots/request-pane-interactivity.spec.ts (1)
166-167: ⚡ Quick winReplace fixed sleeps with condition-based snapshot waits.
Line 166 and Line 222 use
page.waitForTimeout(2000). This is brittle under slow/fast environments; wait on the snapshot condition instead, then close the app.♻️ Suggested change
- await page.waitForTimeout(2000); + await expect.poll(() => { + const snapshot = readSnapshot(userDataPath); + const tab = findSnapshotRequestTab(snapshot, 'ReqGrpcSnapshot'); + return tab?.request?.tab; + }).toBe('body'); await closeElectronApp(app);- await page.waitForTimeout(2000); + await expect.poll(() => { + const snapshot = readSnapshot(userDataPath); + const tab = findSnapshotRequestTab(snapshot, 'ReqWsSnapshot'); + return tab?.request?.tab; + }).toBe('body'); await closeElectronApp(app);As per coding guidelines: "Replace magic timeouts with event-driven waits in E2E tests."
Also applies to: 222-223
🤖 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/snapshots/request-pane-interactivity.spec.ts` around lines 166 - 167, Replace the fixed 2s sleeps in request-pane-interactivity.spec.ts (the page.waitForTimeout(2000) calls) with condition-based waits that observe the snapshot-ready condition before calling closeElectronApp(app); for example, await a stable UI element or snapshot marker using page.locator('<snapshot-or-result-selector>').waitFor({ state: 'visible' }) or await page.waitForFunction(() => !!window.__SNAPSHOT_READY__), then proceed to await closeElectronApp(app) — update both occurrences that use page.waitForTimeout to use these event-driven waits referencing page.waitForFunction or page.locator(...).waitFor instead of page.waitForTimeout.packages/bruno-app/src/utils/snapshot/index.js (1)
352-362: ⚡ Quick winUse one shared default-tab resolver to prevent mapping drift.
This local mapping duplicates request-pane default logic already used in tab creation. Reusing a single helper for both write/restore paths will avoid future mismatches.
🤖 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-app/src/utils/snapshot/index.js` around lines 352 - 362, The local function getDefaultRequestPaneTabForType duplicates the request-pane default-tab logic used elsewhere; replace it by importing and calling the single shared default-tab resolver used by tab creation (remove getDefaultRequestPaneTabForType and any local mapping), update callers to use that shared helper, and ensure the import name matches the existing resolver in the request-pane/tab creation module so both write/restore paths use the same logic.
🤖 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.
Nitpick comments:
In `@packages/bruno-app/src/utils/snapshot/index.js`:
- Around line 352-362: The local function getDefaultRequestPaneTabForType
duplicates the request-pane default-tab logic used elsewhere; replace it by
importing and calling the single shared default-tab resolver used by tab
creation (remove getDefaultRequestPaneTabForType and any local mapping), update
callers to use that shared helper, and ensure the import name matches the
existing resolver in the request-pane/tab creation module so both write/restore
paths use the same logic.
In `@tests/snapshots/request-pane-interactivity.spec.ts`:
- Around line 166-167: Replace the fixed 2s sleeps in
request-pane-interactivity.spec.ts (the page.waitForTimeout(2000) calls) with
condition-based waits that observe the snapshot-ready condition before calling
closeElectronApp(app); for example, await a stable UI element or snapshot marker
using page.locator('<snapshot-or-result-selector>').waitFor({ state: 'visible'
}) or await page.waitForFunction(() => !!window.__SNAPSHOT_READY__), then
proceed to await closeElectronApp(app) — update both occurrences that use
page.waitForTimeout to use these event-driven waits referencing
page.waitForFunction or page.locator(...).waitFor instead of
page.waitForTimeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcff364d-d844-4fbe-a703-2892d35008b1
📒 Files selected for processing (4)
packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.jspackages/bruno-app/src/utils/snapshot/index.jspackages/bruno-app/src/utils/snapshot/index.spec.jstests/snapshots/request-pane-interactivity.spec.ts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates snapshot/tab restoration to correctly persist and restore request-pane tab defaults for gRPC and WebSocket requests (including ensuring concrete request types are stored in the snapshot), and adds regression tests to cover the behavior.
Changes:
- Added Playwright snapshot tests to assert snapshot persistence/restoration for gRPC and WebSocket request tabs.
- Updated snapshot deserialization to apply request-type-specific default request pane tabs and to resolve generic snapshot request types via pathname lookup.
- Updated the task middleware’s
addTabpayload to includetypeandpathnamewhen opening a newly-created request.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/snapshots/request-pane-interactivity.spec.ts | Adds e2e coverage ensuring gRPC/WS snapshot stores concrete type + correct request tab key and restores without 404. |
| packages/bruno-app/src/utils/snapshot/index.spec.js | Adds unit tests for request-pane defaulting and resolving generic snapshot request type via pathname. |
| packages/bruno-app/src/utils/snapshot/index.js | Implements request-pane default selection by request type and resolves generic request snapshot type using collection items. |
| packages/bruno-app/src/providers/ReduxStore/middlewares/tasks/middleware.js | Ensures newly opened request tabs include type and pathname so snapshots can serialize request metadata correctly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/snapshots/request-pane-interactivity.spec.ts`:
- Line 251: The test title "graphql snapshot stores concrete type and query tab
key" is inconsistent with the assertions which check the Headers pane; update
the test name string passed to the test(...) call (the title literal) to reflect
"headers tab key" (e.g., "graphql snapshot stores concrete type and headers tab
key") so the test name matches the selection step and assertion in the test
function.
🪄 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: 187efee0-37e1-446b-99c3-3fb1a68b2025
📒 Files selected for processing (4)
packages/bruno-app/src/components/OpenAPISyncTab/hooks/useOpenAPISync.jspackages/bruno-app/src/utils/snapshot/index.spec.jstests/snapshots/request-pane-interactivity.spec.tstests/utils/page/actions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/utils/snapshot/index.spec.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/utils/page/actions.ts (1)
819-819: ⚡ Quick winStrengthen tab-name matching to avoid false positives with similar label pairs.
All three branches use substring matching:
getByRole('tab', { name: tabName })(line 819),getByRole('menuitem', { name: new RegExp(..., 'i') })without anchors (lines 842–844), and.filter({ hasText: tabName })(line 856). For labels like'Body'vs'Body Auth'or'Query'vs'Query Params', the first substring match could be incorrect.Add
{ exact: true }to the role queries, anchor the RegExp with^...$, and scope the menuitem lookup under.tippy-boxto eliminate substring false matches.🤖 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/utils/page/actions.ts` at line 819, The tab lookups use loose substring matching causing false positives; update the locator calls: change the pane.locator('.tabs').getByRole('tab', { name: tabName }) call (visibleTab) to pass exact: true; scope the menuitem search to the tippy popup (use pane.locator('.tippy-box').getByRole('menuitem', { name: new RegExp(`^${escapeRegExp(tabName)}$`, 'i'), exact: true })) and anchor the RegExp with ^...$ (ensure tabName is escaped when building the RegExp); and replace .filter({ hasText: tabName }) with a stricter match (either use getByRole with exact: true or filter using an anchored/escaped RegExp) so only exact label matches like "Body" vs "Body Auth" are returned.
🤖 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 `@tests/utils/page/actions.ts`:
- Around line 836-868: The overflow dropdown can remain open when a subsequent
path (dropdownItem or fallbackDropdownItem) fails, causing flakes; in the catch
blocks inside the function handling overflowButton.click and the two dropdown
click/expect blocks (references: overflowButton.click, dropdownItem,
fallbackDropdownItem, visibleTab) dismiss the popover before returning false —
e.g. call a neutral dismiss action such as page.keyboard.press('Escape') or
click a neutral area (page.click on a non-interactive selector) and await it,
then return false so the next poll iteration starts with the menu closed.
- Line 824: The test uses Playwright's matcher toContainClass on visibleTab (see
await expect(visibleTab).toContainClass('active', { timeout: 500 });) which
requires `@playwright/test` >= v1.52.0; update the package.json dependency for
`@playwright/test` from ^1.51.1 to ^1.52.0 (or later) so the runtime resolver will
install a version that includes toContainClass, then run install and CI tests to
verify the assertions (also update any other occurrences referenced in the
review).
---
Nitpick comments:
In `@tests/utils/page/actions.ts`:
- Line 819: The tab lookups use loose substring matching causing false
positives; update the locator calls: change the
pane.locator('.tabs').getByRole('tab', { name: tabName }) call (visibleTab) to
pass exact: true; scope the menuitem search to the tippy popup (use
pane.locator('.tippy-box').getByRole('menuitem', { name: new
RegExp(`^${escapeRegExp(tabName)}$`, 'i'), exact: true })) and anchor the RegExp
with ^...$ (ensure tabName is escaped when building the RegExp); and replace
.filter({ hasText: tabName }) with a stricter match (either use getByRole with
exact: true or filter using an anchored/escaped RegExp) so only exact label
matches like "Body" vs "Body Auth" are returned.
🪄 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: 80d7c052-bb5a-4e1f-b461-c90402dd3951
📒 Files selected for processing (1)
tests/utils/page/actions.ts
bijin-bruno
approved these changes
May 13, 2026
lohit-bruno
pushed a commit
that referenced
this pull request
May 14, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
JIRA
Corrects how the snapshot refers to the different request types
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Bug Fixes
Tests