[andreasasprou]

Links

• ExecPlan: apps/desktop/plans/done/20260114-1739-fix-file-pane-cross-workspace.md
• Follow-up ExecPlan: apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md

Demo of bug and fix: https://www.loom.com/share/26e180759f1143219a47950ffb99b654

Summary

• Prevent cross-workspace tab rendering so workspace switching can’t show a stale file viewer pane.
• Resolve file viewer worktreePath from tab.workspaceId and sanitize persisted tab pointers on hydration.
• Add unit tests for the active-tab resolver and guard against cross-workspace pane moves.

Why / Context

Switching workspaces while a file viewer pane was open could render an empty/blank file pane in the target workspace. The root cause was an invariant violation: the route workspaceId and the rendered tab’s workspaceId could drift (stale activeTabIds / history), so the UI rendered a tab from another workspace.

How It Works

• Introduces resolveActiveTabIdForWorkspace (active → MRU → first tab fallback).
• Uses the resolver in the store + UI render path, and sanitizes persisted pointers (activeTabIds, tabHistoryStacks, focusedPaneIds) at hydration.
• Makes TabView fetch the workspace by tab.workspaceId for file viewer panes.
• Disallows moving panes across workspaces (movePaneToTab guard).

Manual QA Checklist

• Open a file viewer in workspace A, switch to workspace B with a terminal open → workspace B shows its own panes (no blank file viewer)
• Switch back to A → file viewer still present
• Repeat switching a few times (no blank panes)
• Restart app and repeat

Testing

• bun run typecheck --filter=@superset/desktop
• bun run lint
• bun test apps/desktop/src/renderer/stores/tabs/utils.test.ts
• Note: bun test (repo-wide) currently fails due to apps/cli/src/lib/storage/lowdb-adapter.test.ts (unrelated).

Risks / Rollback

• Risk: on startup, corrupted persisted tab pointers may be auto-corrected to a different tab than before; this is intended to avoid cross-workspace panes.
• Rollback: revert commits 570a819f and b800faf4.

Summary by CodeRabbit

• Bug Fixes

  • Fixed blank file viewer panes that appeared when switching between workspaces
  • Improved tab consistency and recovery when navigating across workspaces
  • Prevented invalid pane transfers between different workspaces

• Tests

  • Added regression tests for workspace tab resolution and workspace-switching scenarios

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This change introduces workspace-scoped active tab resolution to prevent blank file viewer panes when switching workspaces. It adds a new resolveActiveTabIdForWorkspace utility, updates the tabs store with sanitization logic to enforce workspace consistency, and modifies UI components to use the resolver when determining the active tab, preventing cross-workspace tab references.

Changes

Cohort / File(s)	Summary
Planning Documents apps/desktop/plans/done/20260114-1739-fix-file-pane-cross-workspace.md, apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md	Adds ExecPlan markdown documents outlining the fix for cross-workspace tab pane issues and a comprehensive refactor roadmap to workspace-scoped tabs state with multi-milestone approach.
Tab Resolution Utility apps/desktop/src/renderer/stores/tabs/utils.ts, apps/desktop/src/renderer/stores/tabs/utils.test.ts	Introduces new resolveActiveTabIdForWorkspace function that determines the active tab for a workspace by validating active IDs, consulting history stacks, and falling back to the first workspace tab. Adds comprehensive unit tests for valid IDs, MRU history fallback, cross-workspace history filtering, and null cases.
Tabs Store apps/desktop/src/renderer/stores/tabs/store.ts	Replaces direct activeTabIds lookups with resolver calls; introduces comprehensive sanitization/normalization during persistence merge that validates tab pointers are workspace-scoped, recomputes activeTabIds, prunes tabHistoryStacks, and cleans focusedPaneIds.
Pane Actions apps/desktop/src/renderer/stores/tabs/actions/move-pane.ts	Adds validation to reject pane moves when source and target tabs belong to different workspaces.
UI Components apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx, apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx, apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx, apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx	Update active tab computation to use resolveActiveTabIdForWorkspace with memoization; TabView now derives workspace from tab.workspaceId instead of route params; TabsContent adds workspace consistency validation before rendering tabs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

• fix(desktop): fetch latest file content when clicking already-open file #660: Modifies addFileViewerPane and tabs store logic with pane reuse and worktreePath behavior changes.
• fix(desktop): allow creating multiple terminal tabs per workspace #452: Modifies tabs store and active-tab/tab-creation behavior.
• refactor window to tabs lmao  #284: Refactors tabs store and utils functionality that this PR builds upon.

Poem

🐰 Workspace tabs hop no more astray,
The resolver ensures each bounds its way,
Cross-workspace whispers now fall silent and still,
Each rabbit pane finds its rightful windowsill! 🪟✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: preventing blank file viewer panes when switching workspaces, which is the core objective of this PR.
Description check	✅ Passed	The description provides comprehensive context including links to ExecPlans, a summary of changes, explanation of the bug and fix, implementation details, manual QA results, testing commands, and rollback strategy.

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

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc9a360 and 29bee5e.

📒 Files selected for processing (2)

• apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
• apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx

🚧 Files skipped from review as they are similar to previous changes (2)

• apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
• apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx

⏰ 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: Build

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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 and usage tips.}

[saddlepaddle]

Thanks for grabbing this! This definitely needs to be fixed so i'm fine with merging to make sure the bug is gone, but if you have a moment it may be time to organize the workspace state under a workspace instead of it being global and having things like all the tabs in all workspaces at the top-level.i.e.:

// tabs state is now isolated per workspace
  {
    tabsState: Record<workspaceId, {
      tabs: Tab[],
      panes: Record<paneId, Pane>,
      activeTabId: string | null,
      focusedPaneIds: Record<tabId, paneId>,
      tabHistoryStack: string[]
    }>
  }

If you have the time it'd be nice to solve the root issue, otherwise ye we can merge this in 👍🏼

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md`:
- Around line 79-103: Handle tabs with missing/invalid workspaceId during the
migration by adding logic in the normalizer that checks tab.workspaceId and
either (a) places such tabs into a special "orphaned" workspace bucket under
byWorkspace["__orphaned__"] or (b) drops them while emitting a warning log;
update the normalizer acceptance criteria in Milestone 2 to specify the chosen
behavior (orphan bucket vs drop+warn) and implement a unit test in
tabs-state-normalize.test.ts that covers tab entries with
null/undefined/non-existent workspaceId to assert they are routed/removed and
that a warning is emitted when appropriate.
- Around line 260-271: Add a backward-compatibility test and document the chosen
downgrade behavior: update Milestone 7 validation to include a test that
simulates a version rollback by feeding the legacy tab-state shape and asserting
behavior for your chosen approach (either accept both shapes via the DL-1 union
type, require a new schemaVersion flag and assert old clients detect
incompatibility, or mark as breaking and assert state is lost); update the
Milestone 7 text to state which approach was selected and where the test lives,
and ensure the test exercises the migration code/path that handles tab state so
regressions are caught.
- Around line 106-125: Add a workspace-deletion invariant: ensure that when a
workspace is deleted its entire byWorkspace[workspaceId] slice is removed
atomically and UI code tolerates a missing active workspace. Update the
invariants list to include this rule and then update store actions that mutate
workspaces (e.g., the reducer/action that calls removeWorkspace or
deleteWorkspace, and any functions that read activeWorkspaceId/activeTabId,
focusedPaneIds, tabHistoryStack) to: (1) perform an atomic removal of
byWorkspace[workspaceId] (the whole slice) in the workspace deletion action, (2)
clear or re-evaluate activeWorkspaceId and any workspace-scoped pointers
(activeTabId, focusedPaneIds[tabId], tabHistoryStack) so they don’t point to
removed tabs/panes, and (3) make UI selectors that use byWorkspace[workspaceId]
resilient to missing slices (guard reads of byWorkspace[activeWorkspaceId],
activeTabIds, focusedPaneIds, tabHistoryStack) so the app renders gracefully
when the active workspace no longer exists; add tests that delete a workspace
and assert the slice is gone and selectors return safe defaults.
- Around line 139-144: Update normalizeTabsState to document and implement a
deterministic sanitization order: first, if input is legacy shape, group tabs by
tab.workspaceId into byWorkspace and create workspace slices; second, remove
orphan panes whose pane.tabId no longer exists in those slices; third, sanitize
focusedPaneIds per workspace (only referencing panes that remain); and finally,
sanitize activeTabId and tabHistoryStack using the Milestone 1 invariants (only
referencing tabs that remain). Ensure these steps are executed in that order
inside normalizeTabsState and add a brief comment describing the sequence to
prevent cascading failures.

🧹 Nitpick comments (2)

apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md (2)

29-30: Consider keeping defensive selectors even after migration.

While the new workspace-scoped state shape makes cross-workspace references structurally harder, keeping resolveActiveTabIdForWorkspace as a defensive fallback could protect against:

• Race conditions during workspace deletion
• Corrupted state from third-party tools or manual edits to ~/.superset/app-state.json
• Future bugs in normalization logic

If kept, document it explicitly as "corruption recovery" rather than a primary selection path, and add a warning log when it triggers to aid debugging.

---

273-287: Consider automatic backup before migration.

To provide a safety net if migration corrupts state, the normalizer (called during app startup) could automatically create a timestamped backup before performing the first migration:

~/.superset/app-state.json.pre-migration-20260114

Implementation:

• Check if backup already exists before creating (to avoid duplicates on restart)
• Store backup creation timestamp in the migrated state to skip backup on subsequent runs
• Document the backup location in logs and user-facing docs

This gives users a manual recovery path without requiring them to understand lowdb internals.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b800faf and 4056895.

📒 Files selected for processing (1)

• apps/desktop/plans/20260114-2050-workspace-scoped-tabs-state.md

⏰ 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: Build

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch
• ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉