Fix split-pane console regressions and terminal context menu dismiss (#743)

src/renderer/src/components/tab-bar/reconcile-order.ts

@@ -10,8 +10,20 @@ export function reconcileTabOrder(
   browserIds: string[] = []
 ): string[] {
   const validIds = new Set([...terminalIds, ...editorIds, ...browserIds])
-  const result: string[] = (storedOrder ?? []).filter((id) => validIds.has(id))
-  const inResult = new Set(result)
+  // Why: storedOrder is persisted group tab order and is mutated by many
+  // codepaths (drop/move/reorder/hydrate). A stale or racey write can leave
+  // the same tab id twice in the list, which surfaces as React's "two
+  // children with the same key" warning when TabBar maps items to
+  // SortableTab/EditorFileTab/BrowserTab. Dedupe at the render boundary so
+  // the UI never produces duplicate keys regardless of store-side bugs.
+  const result: string[] = []
+  const inResult = new Set<string>()
+  for (const id of storedOrder ?? []) {
+    if (validIds.has(id) && !inResult.has(id)) {
+      result.push(id)
+      inResult.add(id)
+    }
+  }
   for (const id of [...terminalIds, ...editorIds, ...browserIds]) {
     if (!inResult.has(id)) {
       result.push(id)

src/renderer/src/components/terminal-pane/TerminalContextMenu.tsx

@@ -17,6 +17,7 @@ import {
   DropdownMenuShortcut,
   DropdownMenuTrigger
 } from '@/components/ui/dropdown-menu'
+import { shouldIgnoreTerminalMenuPointerDownOutside } from './terminal-context-menu-dismiss'
 
 type TerminalContextMenuProps = {
   open: boolean
@@ -90,6 +91,16 @@ export default function TerminalContextMenu({
           // Radix treat that as a dismiss signal.
           e.preventDefault()
         }}
+        onPointerDownOutside={(e) => {
+          if (
+            shouldIgnoreTerminalMenuPointerDownOutside({
+              openedAtMs: menuOpenedAtRef.current,
+              nowMs: Date.now()
+            })
+          ) {
+            e.preventDefault()
+          }
+        }}
       >
         <DropdownMenuItem onSelect={onCopy}>
           <Copy />

src/renderer/src/components/terminal-pane/terminal-context-menu-dismiss.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from 'vitest'
+import { shouldIgnoreTerminalMenuPointerDownOutside } from './terminal-context-menu-dismiss'
+
+describe('shouldIgnoreTerminalMenuPointerDownOutside', () => {
+  it('ignores the opening gesture immediately after the menu opens', () => {
+    expect(
+      shouldIgnoreTerminalMenuPointerDownOutside({
+        openedAtMs: 1_000,
+        nowMs: 1_050
+      })
+    ).toBe(true)
+  })
+
+  it('allows secondary-button pointerdowns after the menu is open', () => {
+    expect(
+      shouldIgnoreTerminalMenuPointerDownOutside({
+        openedAtMs: 1_000,
+        nowMs: 1_250
+      })
+    ).toBe(false)
+  })
+
+  it('allows macOS control-click after the opening-gesture window', () => {
+    expect(
+      shouldIgnoreTerminalMenuPointerDownOutside({
+        openedAtMs: 1_000,
+        nowMs: 1_250
+      })
+    ).toBe(false)
+  })
+
+  it('allows ordinary outside left-click dismissals', () => {
+    expect(
+      shouldIgnoreTerminalMenuPointerDownOutside({
+        openedAtMs: 1_000,
+        nowMs: 1_250
+      })
+    ).toBe(false)
+  })
+})

src/renderer/src/components/terminal-pane/terminal-context-menu-dismiss.ts

@@ -0,0 +1,10 @@
+export function shouldIgnoreTerminalMenuPointerDownOutside(args: {
+  openedAtMs: number
+  nowMs: number
+}): boolean {
+  const { openedAtMs, nowMs } = args
+  // Why: only the opening gesture should be ignored. After that brief window,
+  // outside clicks including right-click and macOS control-click must dismiss
+  // normally so the terminal menu behaves like the app's other context menus.
+  return nowMs - openedAtMs < 100
+}

src/renderer/src/lib/pane-manager/pane-manager.ts

@@ -139,8 +139,22 @@ export class PaneManager {
     // final rAF runs after fitPanes (FIFO ordering) and unconditionally
     // restores the scroll-to-bottom state.
     if (wasAtBottom) {
+      const existingPaneId = existing.id
       requestAnimationFrame(() => {
-        existing.terminal.scrollToBottom()
+        // Why: replayTerminalLayout can create a PaneManager, split panes,
+        // then tear the whole manager down (e.g. when the owning worktree
+        // deactivates) before this rAF fires. Touching xterm's renderer
+        // after disposePane throws "Cannot read properties of undefined
+        // (reading 'dimensions')". Resolve the pane from the live map so
+        // we no-op instead of crashing.
+        if (this.destroyed) {
+          return
+        }
+        const live = this.panes.get(existingPaneId)
+        if (!live) {
+          return
+        }
+        live.terminal.scrollToBottom()
       })
     }
 

src/renderer/src/store/slices/store-session-cascades.test.ts

@@ -615,6 +615,32 @@ describe('terminal slice behaviors', () => {
     expect(tab.ptyId).toBe('pty-1')
     expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-1'])
   })
+
+  it('keeps the original tab-level PTY when a split pane adds another PTY', () => {
+    const store = createTestStore()
+    const worktreeId = 'repo1::/path/wt1'
+
+    store.setState({
+      repos: [
+        { id: 'repo1', path: '/repo1', displayName: 'Repo 1', badgeColor: '#000', addedAt: 0 }
+      ],
+      worktreesByRepo: {
+        repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1', path: '/path/wt1' })]
+      },
+      tabsByWorktree: {
+        [worktreeId]: [makeTab({ id: 'tab-1', worktreeId, ptyId: 'pty-1' })]
+      },
+      ptyIdsByTabId: {
+        'tab-1': ['pty-1']
+      }
+    })
+
+    store.getState().updateTabPtyId('tab-1', 'pty-2')
+
+    const tab = store.getState().tabsByWorktree[worktreeId][0]
+    expect(tab.ptyId).toBe('pty-1')
+    expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-1', 'pty-2'])
+  })
 })
 
 // ─── Reconnect persisted terminals ──────────────────────────────────

src/renderer/src/store/slices/tabs-helpers.ts

@@ -124,6 +124,19 @@ export function selectHydratedActiveGroupId(
   return candidates[0]?.id
 }
 
+export function dedupeTabOrder(tabIds: string[]): string[] {
+  const seen = new Set<string>()
+  const deduped: string[] = []
+  for (const tabId of tabIds) {
+    if (seen.has(tabId)) {
+      continue
+    }
+    seen.add(tabId)
+    deduped.push(tabId)
+  }
+  return deduped
+}
+
 /**
  * Apply a partial update to a single tab, returning the new `unifiedTabsByWorktree`
  * map. Returns `null` if the tab is not found (callers should return `{}` to the

src/renderer/src/store/slices/tabs-hydration.ts

@@ -5,6 +5,7 @@ import type {
   WorkspaceSessionState
 } from '../../../../shared/types'
 import {
+  dedupeTabOrder,
   getPersistedEditFileIdsByWorktree,
   isTransientEditorContentType,
   selectHydratedActiveGroupId
@@ -83,7 +84,10 @@ function hydrateUnifiedFormat(
 
     const validTabIds = new Set((tabsByWorktree[worktreeId] ?? []).map((t) => t.id))
     const validatedGroups = groups.map((g) => {
-      const tabOrder = g.tabOrder.filter((tid) => validTabIds.has(tid))
+      // Why: persisted tabOrder can contain duplicates from older buggy
+      // writes. Deduping during hydration restores the store invariant before
+      // later group operations branch on tab counts or neighbors.
+      const tabOrder = dedupeTabOrder(g.tabOrder.filter((tid) => validTabIds.has(tid)))
       return {
         ...g,
         tabOrder,

src/renderer/src/store/slices/tabs.test.ts

@@ -787,6 +787,81 @@ describe('TabsSlice', () => {
       expect(state.groupsByWorktree[WT][0].activeTabId).toBe('/file.ts')
     })
 
+    it('deduplicates persisted tab order during unified hydration', () => {
+      store.setState({
+        worktreesByRepo: {
+          repo1: [
+            {
+              id: WT,
+              repoId: 'repo1',
+              path: '/tmp/feature',
+              head: 'abc',
+              branch: 'feature',
+              isBare: false,
+              isMainWorktree: false,
+              displayName: 'feature',
+              comment: '',
+              linkedIssue: null,
+              linkedPR: null,
+              isArchived: false,
+              isUnread: false,
+              isPinned: false,
+              sortOrder: 0,
+              lastActivityAt: 0
+            }
+          ]
+        }
+      })
+
+      const groupId = 'g-1'
+      const tabs: Tab[] = [
+        {
+          id: 't-1',
+          entityId: 't-1',
+          groupId,
+          worktreeId: WT,
+          contentType: 'terminal',
+          label: 'zsh',
+          customLabel: null,
+          color: null,
+          sortOrder: 0,
+          createdAt: 1000
+        },
+        {
+          id: '/file.ts',
+          entityId: '/file.ts',
+          groupId,
+          worktreeId: WT,
+          contentType: 'editor',
+          label: 'file.ts',
+          customLabel: null,
+          color: null,
+          sortOrder: 1,
+          createdAt: 2000
+        }
+      ]
+      const groups: TabGroup[] = [
+        {
+          id: groupId,
+          worktreeId: WT,
+          activeTabId: '/file.ts',
+          tabOrder: ['t-1', 't-1', '/file.ts', '/file.ts']
+        }
+      ]
+
+      store.getState().hydrateTabsSession({
+        activeRepoId: 'repo1',
+        activeWorktreeId: WT,
+        activeTabId: 't-1',
+        tabsByWorktree: {},
+        terminalLayoutsByTabId: {},
+        unifiedTabs: { [WT]: tabs },
+        tabGroups: { [WT]: groups }
+      })
+
+      expect(store.getState().groupsByWorktree[WT][0].tabOrder).toEqual(['t-1', '/file.ts'])
+    })
+
     it('filters out invalid worktree IDs during hydration', () => {
       store.setState({ worktreesByRepo: {} })
 
@@ -846,6 +921,18 @@ describe('TabsSlice', () => {
     })
   })
 
+  describe('tabOrder dedupe', () => {
+    it('deduplicates drag reorder payloads before persisting group order', () => {
+      const first = store.getState().createUnifiedTab(WT, 'terminal')
+      const second = store.getState().createUnifiedTab(WT, 'terminal')
+
+      const groupId = store.getState().groupsByWorktree[WT][0].id
+      store.getState().reorderUnifiedTabs(groupId, [second.id, first.id, second.id, first.id])
+
+      expect(store.getState().groupsByWorktree[WT][0].tabOrder).toEqual([second.id, first.id])
+    })
+  })
+
   describe('reconcileWorktreeTabModel', () => {
     it('drops unified tabs whose backing content no longer exists', () => {
       const groupId = 'g-1'

src/renderer/src/store/slices/tabs.ts

@@ -13,6 +13,7 @@ import type {
   WorkspaceVisibleTabType
 } from '../../../../shared/types'
 import {
+  dedupeTabOrder,
   ensureGroup,
   findGroupAndWorktree,
   findGroupForTab,
@@ -403,7 +404,7 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
       const existingTabs = state.unifiedTabsByWorktree[worktreeId] ?? []
 
       let nextTabs = existingTabs
-      let nextOrder = [...group.tabOrder]
+      let nextOrder = dedupeTabOrder(group.tabOrder)
       if (init?.isPreview) {
         const existingPreview = existingTabs.find(
           (tab) => tab.groupId === group.id && tab.isPreview && tab.contentType === contentType
@@ -430,7 +431,7 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
         isPinned: init?.isPinned
       }
 
-      nextOrder.push(created.id)
+      nextOrder = dedupeTabOrder([...nextOrder, created.id])
       return {
         unifiedTabsByWorktree: {
           ...state.unifiedTabsByWorktree,
@@ -514,13 +515,14 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
       return null
     }
 
-    const remainingOrder = group.tabOrder.filter((id) => id !== tabId)
+    const dedupedGroupOrder = dedupeTabOrder(group.tabOrder)
+    const remainingOrder = dedupeTabOrder(dedupedGroupOrder.filter((id) => id !== tabId))
     const wasLastTab = remainingOrder.length === 0
     const nextActiveTabId =
       group.activeTabId === tabId
         ? wasLastTab
           ? null
-          : pickNeighbor(group.tabOrder, tabId)
+          : pickNeighbor(dedupedGroupOrder, tabId)
         : group.activeTabId
 
     set((current) => {
@@ -623,11 +625,15 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
         if (!group) {
           continue
         }
-        const orderMap = new Map(tabIds.map((id, index) => [id, index]))
+        // Why: drag-and-drop should preserve a single canonical position for
+        // each tab. Sanitizing here restores the invariant at the store
+        // boundary so later group operations do not branch on duplicate ids.
+        const nextTabOrder = dedupeTabOrder(tabIds)
+        const orderMap = new Map(nextTabOrder.map((id, index) => [id, index]))
         return {
           groupsByWorktree: {
             ...state.groupsByWorktree,
-            [worktreeId]: updateGroup(groups, { ...group, tabOrder: tabIds })
+            [worktreeId]: updateGroup(groups, { ...group, tabOrder: nextTabOrder })
           },
           unifiedTabsByWorktree: {
             ...state.unifiedTabsByWorktree,
@@ -851,8 +857,12 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
       }
       moved = true
 
-      const sourceOrder = sourceGroup.tabOrder.filter((id) => id !== tabId)
-      const targetOrder = [...targetGroup.tabOrder]
+      const dedupedSourceGroupOrder = dedupeTabOrder(sourceGroup.tabOrder)
+      const sourceOrder = dedupeTabOrder(dedupedSourceGroupOrder.filter((id) => id !== tabId))
+      // Why: defensive filter so target order can't grow a duplicate if the
+      // tab id somehow already exists there (stale state, prior bug). See
+      // dropUnifiedTab for the same guard.
+      const targetOrder = dedupeTabOrder(targetGroup.tabOrder.filter((id) => id !== tabId))
       const targetIndex = Math.max(
         0,
         Math.min(opts?.index ?? targetOrder.length, targetOrder.length)
@@ -867,7 +877,9 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
           return {
             ...group,
             activeTabId:
-              group.activeTabId === tabId ? pickNeighbor(group.tabOrder, tabId) : group.activeTabId,
+              group.activeTabId === tabId
+                ? pickNeighbor(dedupedSourceGroupOrder, tabId)
+                : group.activeTabId,
             tabOrder: sourceOrder
           }
         }
@@ -963,10 +975,16 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
         }
       }
 
-      const sourceOrder = sourceGroup.tabOrder.filter((id) => id !== tabId)
+      const dedupedSourceGroupOrder = dedupeTabOrder(sourceGroup.tabOrder)
+      const sourceOrder = dedupeTabOrder(dedupedSourceGroupOrder.filter((id) => id !== tabId))
       const destinationGroup =
         nextGroups.find((group) => group.id === resolvedTargetGroupId) ?? targetGroup
-      const targetOrder = [...destinationGroup.tabOrder]
+      // Why: the target group's stored order can already contain this tab id
+      // from a prior racey write or a same-group split where the source and
+      // destination transiently share it. Splicing without filtering first
+      // would leave the same id in the order twice, which React surfaces as
+      // a duplicate-key warning in TabBar and can mis-reconcile xterm panes.
+      const targetOrder = dedupeTabOrder(destinationGroup.tabOrder.filter((id) => id !== tabId))
       const targetIndex = Math.max(
         0,
         Math.min(target.index ?? targetOrder.length, targetOrder.length)
@@ -978,7 +996,9 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
           return {
             ...group,
             activeTabId:
-              group.activeTabId === tabId ? pickNeighbor(group.tabOrder, tabId) : group.activeTabId,
+              group.activeTabId === tabId
+                ? pickNeighbor(dedupedSourceGroupOrder, tabId)
+                : group.activeTabId,
             tabOrder: sourceOrder
           }
         }
@@ -1164,12 +1184,10 @@ export const createTabsSlice: StateCreator<AppState, [], [], TabsSlice> = (set,
             // into the active/root group keeps existing live PTYs reachable
             // instead of making activation spawn a duplicate "Terminal 2".
             activeTabId: reconciliationGroup.activeTabId ?? restoredLegacyTabs[0]?.id ?? null,
-            tabOrder: [
+            tabOrder: dedupeTabOrder([
               ...reconciliationGroup.tabOrder,
-              ...restoredLegacyTabs
-                .map((tab) => tab.id)
-                .filter((tabId) => !reconciliationGroup.tabOrder.includes(tabId))
-            ]
+              ...restoredLegacyTabs.map((tab) => tab.id)
+            ])
           })
         : groups
     const liveTerminalIds = new Set(

src/renderer/src/store/slices/terminals.ts

@@ -552,7 +552,23 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
         if (found) {
           worktreeId = wId
         }
-        next[wId] = next[wId].map((t) => (t.id === tabId ? { ...t, ptyId } : t))
+        next[wId] = next[wId].map((t) => {
+          if (t.id !== tabId) {
+            return t
+          }
+          const existingPtyIds = s.ptyIdsByTabId[tabId] ?? []
+          const nextPtyIds = existingPtyIds.includes(ptyId)
+            ? existingPtyIds
+            : [...existingPtyIds, ptyId]
+          return {
+            ...t,
+            // Why: tab.ptyId is the single-pane fallback used by legacy attach
+            // paths. In split panes, later pane spawns must not steal that
+            // primary binding from the original pane or remount/close flows can
+            // reattach the tab to the wrong PTY and appear to "reset" panes.
+            ptyId: t.ptyId ?? nextPtyIds[0] ?? null
+          }
+        })
       }
       const existingPtyIds = s.ptyIdsByTabId[tabId] ?? []
       // Why: when a brand-new tab in the active worktree receives its first