fix: ignore v4 migration tab in snapshot restore

[prateek-bruno]

Description

Jira

v4-migration tab snapshot was introduced as a temporary migration notice for v3.5.x builds, warning them about affected APIs in their scripts.

In v4 that tab snapshot key becomes redundant, so this PR ignores it in snapshot restore path.

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes

  • Improved snapshot-based tab restoration by filtering out legacy migration (“v4-migration”) tabs from hydrated snapshot data, preventing them from reappearing or being selected as the active tab when reopening saved collections.

• Tests

  • Added coverage to ensure legacy migration tabs are removed during snapshot lookup construction, collection tabs hydration, and active-tab resolution (returning no active tab for legacy-only snapshots).

[coderabbitai]

Walkthrough

Adds a legacy tab-type filter to snapshot normalization, collection tab restoration, and active-tab resolution, with tests covering lookup hydration, restore payloads, and null active-tab handling for v4-migration.

Changes

Legacy Tab Filtering in Snapshot Hydration

Layer / File(s)	Summary
Filter legacy tabs in snapshot flow packages/bruno-app/src/utils/snapshot/index.js	Defines the ignored tab type set and sanitization helper, then applies filtering in normalizeCollectionSnapshotEntry, hydrateCollectionTabs, and getActiveTabFromSnapshot.
Test legacy tab filtering packages/bruno-app/src/utils/snapshot/index.spec.js	Adds tests covering hydrated lookup filtering, filtered restore payloads, and null active-tab resolution for v4-migration tabs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• usebruno/bruno#7794: Implements the snapshot utility functions that this PR extends with legacy tab filtering.

Suggested reviewers

• helloanoop
• lohit-bruno
• naman-bruno
• bijin-bruno

Poem

🗂️ Old migration tabs drift out of sight,
A tiny filter trims them right.
Snapshots restore with cleaner grace,
And active tabs keep their place.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: ignoring the v4 migration tab during snapshot restore.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/bruno-app/src/utils/snapshot/index.js (1)

630-649: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Filter or nullify activeTab when it references a legacy tab type.

Lines 634 and 649 pass tabsSnapshot.activeTab unfiltered. If the activeTab references a legacy type (e.g., accessor: 'type', value: 'v4-migration'), you create an inconsistency:

• Line 634: hasPersistedActiveTab becomes true
• Line 649: dispatch includes activeTab pointing to a legacy type
• But the tab itself is filtered out from snapshotTabs (line 631)

This means the condition on line 644 can pass when only a legacy activeTab exists, dispatching an empty tabs array with a non-null activeTab that references a tab that doesn't exist.

🛡️ Proposed fix to filter activeTab

 const snapshotTabs = Array.isArray(tabsSnapshot?.tabs)
   ? tabsSnapshot.tabs.filter((tab) => !isIgnoredLegacyTab(tab))
   : [];
 const hasPersistedTabs = snapshotTabs.length > 0;
-const hasPersistedActiveTab = Boolean(tabsSnapshot?.activeTab);
+const activeTabReferencesLegacyType = tabsSnapshot?.activeTab?.accessor === 'type' 
+  && IGNORED_LEGACY_TAB_TYPES.has(tabsSnapshot.activeTab.value);
+const filteredActiveTab = activeTabReferencesLegacyType ? null : tabsSnapshot?.activeTab ?? null;
+const hasPersistedActiveTab = Boolean(filteredActiveTab);
 const shouldRestoreEmptyWorkspaceScopedTabs = Boolean(workspacePathname) && (
   strictWorkspaceScope
   || Boolean(snapshotLookups?.hasWorkspaceScopedTabs)
   || isCollectionSharedAcrossWorkspaces(snapshotLookups, collection.pathname)
 );

 if (
   tabsSnapshot
   && Array.isArray(tabsSnapshot.tabs)
   && (hasPersistedTabs || hasPersistedActiveTab || shouldRestoreEmptyWorkspaceScopedTabs)
 ) {
   dispatch(restoreTabs({
     collection,
     tabs: snapshotTabs,
-    activeTab: tabsSnapshot.activeTab
+    activeTab: filteredActiveTab
   }));
 }

🤖 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 630 - 649, The
activeTab from tabsSnapshot is not checked for legacy tab types before being
used, creating an inconsistency where hasPersistedActiveTab could be true and
activeTab dispatched even when the referenced tab is filtered out by
isIgnoredLegacyTab. Add a check to filter or nullify the activeTab if it
references a legacy tab type (similar to how snapshotTabs filters legacy tabs),
then use this filtered activeTab value in both the hasPersistedActiveTab Boolean
check and the dispatch call to restoreTabs to ensure consistency between what
tabs are available and what activeTab is referenced.

🧹 Nitpick comments (1)

packages/bruno-app/src/utils/snapshot/index.spec.js (1)

826-863: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider merging duplicate describe blocks.

You now have two describe('getActiveTabFromSnapshot', ...) blocks (line 593 and line 826). While Jest allows this, it can be confusing for navigation and reporting. Consider merging the new test into the existing describe block at line 593.

🤖 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.spec.js` around lines 826 - 863,
There are two separate describe blocks for getActiveTabFromSnapshot - one at
line 593 and another at line 826. Remove the duplicate describe block and move
the test case (ignores a legacy v4 migration active tab snapshot) from the
second describe block into the existing describe block at line 593 to
consolidate all tests for the getActiveTabFromSnapshot function in a single
describe block.

🤖 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/utils/snapshot/index.spec.js`:
- Around line 790-823: The test for hydrateCollectionTabs mocks an activeTab
with value 'v4-migration' but only asserts that the tabs array is filtered. Add
an additional assertion after the existing expect.objectContaining call to
verify that the activeTab in the dispatched payload is either null or properly
updated to not reference the removed v4-migration type. This ensures the
implementation correctly handles the activeTab alongside filtering the tabs
array when legacy v4-migration tabs are removed.

---

Outside diff comments:
In `@packages/bruno-app/src/utils/snapshot/index.js`:
- Around line 630-649: The activeTab from tabsSnapshot is not checked for legacy
tab types before being used, creating an inconsistency where
hasPersistedActiveTab could be true and activeTab dispatched even when the
referenced tab is filtered out by isIgnoredLegacyTab. Add a check to filter or
nullify the activeTab if it references a legacy tab type (similar to how
snapshotTabs filters legacy tabs), then use this filtered activeTab value in
both the hasPersistedActiveTab Boolean check and the dispatch call to
restoreTabs to ensure consistency between what tabs are available and what
activeTab is referenced.

---

Nitpick comments:
In `@packages/bruno-app/src/utils/snapshot/index.spec.js`:
- Around line 826-863: There are two separate describe blocks for
getActiveTabFromSnapshot - one at line 593 and another at line 826. Remove the
duplicate describe block and move the test case (ignores a legacy v4 migration
active tab snapshot) from the second describe block into the existing describe
block at line 593 to consolidate all tests for the getActiveTabFromSnapshot
function in a single describe block.

🪄 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: eec8746a-fa8b-45fb-bfcf-a012c3b46bab

📥 Commits

Reviewing files that changed from the base of the PR and between 5777306 and 17a8282.

📒 Files selected for processing (2)

• packages/bruno-app/src/utils/snapshot/index.js
• packages/bruno-app/src/utils/snapshot/index.spec.js