refactor: gate split group surface ownership in Terminal.tsx (#642)

docs/split-groups-rollout-pr5.md

@@ -0,0 +1,19 @@
+# Split Groups PR 5: Hook Group Surfaces Into Flagged Path
+
+This branch wires terminal, editor, and browser surfaces into the split-group
+ownership path inside `Terminal.tsx`, but holds that path behind a temporary
+local gate.
+
+Scope:
+- remove duplicate legacy ownership under the flagged path
+- route group-local surface creation and restore through the new model
+- preserve existing default behavior while the flag stays off
+
+What Is Actually Hooked Up In This PR:
+- `Terminal.tsx` now contains the real split-group surface path
+- the new path mounts `TabGroupSplitLayout` and avoids keeping duplicate legacy surfaces mounted underneath
+- the old legacy surface path is still present as the active runtime path in this branch
+
+What Is Not Hooked Up Yet:
+- the split-group path is still disabled by the temporary local rollout gate in `Terminal.tsx`
+- users should still get legacy behavior by default in this branch

src/renderer/src/components/Terminal.tsx

@@ -27,9 +27,14 @@ import { isUpdaterQuitAndInstallInProgress } from '@/lib/updater-beforeunload'
 import EditorAutosaveController from './editor/EditorAutosaveController'
 import BrowserPane, { destroyPersistentWebview } from './browser-pane/BrowserPane'
 import { reconcileTabOrder } from './tab-bar/reconcile-order'
+import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout'
 import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal'
 
 const EditorPanel = lazy(() => import('./editor/EditorPanel'))
+// Why: the split-group ownership path lands before the rollout switch so we
+// can exercise the full renderer/model integration in code review without
+// exposing partial behavior to users. PR6 flips this to true.
+const ENABLE_SPLIT_GROUPS = false
 
 function Terminal(): React.JSX.Element | null {
   const activeWorktreeId = useAppStore((s) => s.activeWorktreeId)
@@ -50,8 +55,6 @@ function Terminal(): React.JSX.Element | null {
   const clearCodexRestartNotice = useAppStore((s) => s.clearCodexRestartNotice)
   const expandedPaneByTabId = useAppStore((s) => s.expandedPaneByTabId)
   const workspaceSessionReady = useAppStore((s) => s.workspaceSessionReady)
-  const ensureWorktreeRootGroup = useAppStore((s) => s.ensureWorktreeRootGroup)
-  const reconcileWorktreeTabModel = useAppStore((s) => s.reconcileWorktreeTabModel)
   const openFiles = useAppStore((s) => s.openFiles)
   const activeFileId = useAppStore((s) => s.activeFileId)
   const activeBrowserTabId = useAppStore((s) => s.activeBrowserTabId)
@@ -66,6 +69,11 @@ function Terminal(): React.JSX.Element | null {
   const createBrowserTab = useAppStore((s) => s.createBrowserTab)
   const closeBrowserTab = useAppStore((s) => s.closeBrowserTab)
   const setActiveBrowserTab = useAppStore((s) => s.setActiveBrowserTab)
+  const groupsByWorktree = useAppStore((s) => s.groupsByWorktree)
+  const layoutByWorktree = useAppStore((s) => s.layoutByWorktree)
+  const activeGroupIdByWorktree = useAppStore((s) => s.activeGroupIdByWorktree)
+  const ensureWorktreeRootGroup = useAppStore((s) => s.ensureWorktreeRootGroup)
+  const reconcileWorktreeTabModel = useAppStore((s) => s.reconcileWorktreeTabModel)
 
   const markFileDirty = useAppStore((s) => s.markFileDirty)
   const setTabBarOrder = useAppStore((s) => s.setTabBarOrder)
@@ -100,6 +108,90 @@ function Terminal(): React.JSX.Element | null {
   const worktreeBrowserTabs = activeWorktreeId
     ? (browserTabsByWorktree[activeWorktreeId] ?? [])
     : []
+  const getEffectiveLayoutForWorktree = useCallback(
+    (worktreeId: string) => {
+      const layout = layoutByWorktree[worktreeId]
+      if (layout) {
+        return layout
+      }
+      const groups = groupsByWorktree[worktreeId] ?? []
+      const fallbackGroupId = activeGroupIdByWorktree[worktreeId] ?? groups[0]?.id ?? null
+      if (!fallbackGroupId) {
+        return undefined
+      }
+      return { type: 'leaf', groupId: fallbackGroupId } as const
+    },
+    [activeGroupIdByWorktree, groupsByWorktree, layoutByWorktree]
+  )
+  const effectiveActiveLayout = activeWorktreeId
+    ? ENABLE_SPLIT_GROUPS
+      ? getEffectiveLayoutForWorktree(activeWorktreeId)
+      : undefined
+    : undefined
+  const activeWorktree = activeWorktreeId
+    ? (allWorktrees.find((worktree) => worktree.id === activeWorktreeId) ?? null)
+    : null
+  const activeTerminalTab = tabs.find((tab) => tab.id === activeTabId) ?? null
+  const activeEditorFile = worktreeFiles.find((file) => file.id === activeFileId) ?? null
+  const activeBrowserTab = worktreeBrowserTabs.find((tab) => tab.id === activeBrowserTabId) ?? null
+  const activeSurfaceLabel =
+    activeTabType === 'browser'
+      ? (activeBrowserTab?.title ?? activeBrowserTab?.url ?? 'Browser')
+      : activeTabType === 'editor'
+        ? (activeEditorFile?.relativePath ?? activeEditorFile?.filePath ?? 'Editor')
+        : (activeTerminalTab?.customTitle ?? activeTerminalTab?.title ?? 'Terminal')
+  const renderStaleCodexRestartChip = useCallback(
+    (worktreeId: string) => {
+      const worktreeTabs = tabsByWorktree[worktreeId] ?? []
+      const staleWorktreePtyIds = worktreeTabs.flatMap((tab) =>
+        (ptyIdsByTabId[tab.id] ?? []).filter((ptyId) => Boolean(codexRestartNoticeByPtyId[ptyId]))
+      )
+      if (staleWorktreePtyIds.length === 0) {
+        return null
+      }
+      // Why: split-group and legacy workspace rendering both represent the
+      // same worktree-level Codex session state. Keeping one shared chip here
+      // preserves the single-prompt UX across rollout paths instead of letting
+      // one branch silently lose the restart/dismiss affordance.
+      return (
+        <div className="pointer-events-none absolute right-3 top-3 z-20">
+          <div className="pointer-events-auto flex items-center gap-2 rounded-lg border border-border/80 bg-popover/95 px-2 py-1.5 shadow-lg backdrop-blur-sm">
+            <span className="text-[11px] text-muted-foreground">
+              Codex is using the previous account
+            </span>
+            <div className="flex items-center gap-1.5">
+              <button
+                type="button"
+                onClick={() => queueCodexPaneRestarts(staleWorktreePtyIds)}
+                className="inline-flex items-center gap-1.5 rounded-md bg-foreground px-2 py-1 text-[11px] font-medium text-background transition-colors hover:opacity-90"
+              >
+                <RefreshCw className="size-3" />
+                Restart
+              </button>
+              <button
+                type="button"
+                onClick={() => {
+                  for (const ptyId of staleWorktreePtyIds) {
+                    clearCodexRestartNotice(ptyId)
+                  }
+                }}
+                className="rounded-md px-1.5 py-1 text-[11px] text-muted-foreground transition-colors hover:bg-accent/60 hover:text-foreground"
+              >
+                Dismiss
+              </button>
+            </div>
+          </div>
+        </div>
+      )
+    },
+    [
+      clearCodexRestartNotice,
+      codexRestartNoticeByPtyId,
+      ptyIdsByTabId,
+      queueCodexPaneRestarts,
+      tabsByWorktree
+    ]
+  )
   const activeWorktreeBrowserTabIdsKey = activeWorktreeId
     ? (browserTabsByWorktree[activeWorktreeId] ?? []).map((tab) => tab.id).join(',')
     : ''
@@ -197,13 +289,7 @@ function Terminal(): React.JSX.Element | null {
       return
     }
     createTab(activeWorktreeId)
-  }, [
-    workspaceSessionReady,
-    activeWorktreeId,
-    tabs.length,
-    createTab,
-    reconcileWorktreeTabModel
-  ])
+  }, [workspaceSessionReady, activeWorktreeId, tabs.length, createTab, reconcileWorktreeTabModel])
 
   const handleNewTab = useCallback(() => {
     if (!activeWorktreeId) {
@@ -771,10 +857,11 @@ function Terminal(): React.JSX.Element | null {
     >
       <EditorAutosaveController />
 
-      {/* Why: the tab bar is rendered into the titlebar via a portal so it
-          shares the same visual row as the "Orca" title. The portal target
-          (#titlebar-tabs) lives in App.tsx's titlebar. */}
+      {/* Why: once split groups are enabled, each group owns its own tab strip
+          inline like VS Code. The old titlebar portal stays only as a fallback
+          before the root-group layout has been established. */}
       {activeWorktreeId &&
+        !effectiveActiveLayout &&
         titlebarTabsTarget &&
         createPortal(
           <TabBar
@@ -812,146 +899,164 @@ function Terminal(): React.JSX.Element | null {
           titlebarTabsTarget
         )}
 
-      {/* Terminal panes container - hidden when editor tab active */}
-      <div
-        className={`relative flex-1 min-h-0 overflow-hidden ${
-          // Why: only hide the terminal container when another tab type has
-          // content to display. Hiding unconditionally for non-terminal types
-          // causes a blank screen when activeTabType is stale (e.g. 'editor'
-          // with no files after session restore). The terminal stays visible
-          // as a fallback until another surface is ready.
-          (activeTabType === 'editor' && worktreeFiles.length > 0) ||
-          (activeTabType === 'browser' && worktreeBrowserTabs.length > 0)
-            ? 'hidden'
-            : ''
-        }`}
-      >
-        {allWorktrees
-          .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id))
-          .map((worktree) => {
-            const worktreeTabs = tabsByWorktree[worktree.id] ?? []
-            const isVisible = activeView !== 'settings' && worktree.id === activeWorktreeId
+      {activeWorktreeId &&
+        effectiveActiveLayout &&
+        titlebarTabsTarget &&
+        createPortal(
+          <div className="flex h-full min-w-0 items-center px-3 text-xs text-muted-foreground">
+            {/* Why: split layouts can show several independent tab rows, so the
+                titlebar cannot host the real tabs without collapsing multiple
+                groups into one shared surface. A lightweight summary still uses
+                that otherwise empty strip and keeps the window chrome balanced. */}
+            <span className="truncate font-medium text-foreground/80">
+              {activeWorktree?.displayName ?? 'Workspace'}
+            </span>
+            <span className="px-2 text-border">/</span>
+            <span className="truncate">{activeSurfaceLabel}</span>
+          </div>,
+          titlebarTabsTarget
+        )}
 
-            return (
-              <div
-                key={worktree.id}
-                className={isVisible ? 'absolute inset-0' : 'absolute inset-0 hidden'}
-                aria-hidden={!isVisible}
-              >
-                {(() => {
-                  const staleWorktreePtyIds = worktreeTabs.flatMap((tab) =>
-                    (ptyIdsByTabId[tab.id] ?? []).filter((ptyId) =>
-                      Boolean(codexRestartNoticeByPtyId[ptyId])
-                    )
-                  )
-                  if (staleWorktreePtyIds.length === 0) {
-                    return null
-                  }
-                  // Why: account switching is global, but repeating the same
-                  // stale-session prompt in every affected Codex pane quickly
-                  // turns into noise. Keep one worktree-scoped chip in the
-                  // same visual corner so users get the same prompt style
-                  // without having to dismiss it in every pane.
-                  return (
-                    <div className="pointer-events-none absolute right-3 top-3 z-20">
-                      <div className="pointer-events-auto flex items-center gap-2 rounded-lg border border-border/80 bg-popover/95 px-2 py-1.5 shadow-lg backdrop-blur-sm">
-                        <span className="text-[11px] text-muted-foreground">
-                          Codex is using the previous account
-                        </span>
-                        <div className="flex items-center gap-1.5">
-                          <button
-                            type="button"
-                            onClick={() => queueCodexPaneRestarts(staleWorktreePtyIds)}
-                            className="inline-flex items-center gap-1.5 rounded-md bg-foreground px-2 py-1 text-[11px] font-medium text-background transition-colors hover:opacity-90"
-                          >
-                            <RefreshCw className="size-3" />
-                            Restart
-                          </button>
-                          <button
-                            type="button"
-                            onClick={() => {
-                              for (const ptyId of staleWorktreePtyIds) {
-                                clearCodexRestartNotice(ptyId)
-                              }
-                            }}
-                            className="rounded-md px-1.5 py-1 text-[11px] text-muted-foreground transition-colors hover:bg-accent/60 hover:text-foreground"
-                          >
-                            Dismiss
-                          </button>
-                        </div>
-                      </div>
-                    </div>
-                  )
-                })()}
-                {worktreeTabs.map((tab) => (
-                  <TerminalPane
-                    key={`${tab.id}-${tab.generation ?? 0}`}
-                    tabId={tab.id}
+      {effectiveActiveLayout ? (
+        <div className="relative flex flex-1 min-w-0 min-h-0 overflow-hidden">
+          {allWorktrees
+            .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id))
+            .map((worktree) => {
+              const layout = getEffectiveLayoutForWorktree(worktree.id)
+              if (!layout) {
+                return null
+              }
+              const isVisible = activeView !== 'settings' && worktree.id === activeWorktreeId
+              return (
+                <div
+                  key={`tab-groups-${worktree.id}`}
+                  className={isVisible ? 'absolute inset-0 flex' : 'absolute inset-0 hidden'}
+                  aria-hidden={!isVisible}
+                >
+                  {renderStaleCodexRestartChip(worktree.id)}
+                  <TabGroupSplitLayout
+                    layout={layout}
                     worktreeId={worktree.id}
-                    cwd={worktree.path}
-                    isActive={isVisible && tab.id === activeTabId && activeTabType === 'terminal'}
-                    // Why: the legacy single-surface terminal host still expects
-                    // tab visibility to follow active-tab selection. Split-group
-                    // surfaces pass a broader visibility signal separately, but
-                    // this path must preserve the old one-pane-at-a-time stacking
-                    // behavior until it is migrated off the legacy host.
-                    isVisible={isVisible && tab.id === activeTabId && activeTabType === 'terminal'}
-                    onPtyExit={(ptyId) => handlePtyExit(tab.id, ptyId)}
-                    onCloseTab={() => handleCloseTab(tab.id)}
+                    focusedGroupId={activeGroupIdByWorktree[worktree.id]}
                   />
-                ))}
-              </div>
-            )
-          })}
-      </div>
+                </div>
+              )
+            })}
+        </div>
+      ) : null}
+
+      {!effectiveActiveLayout && (
+        <>
+          {/* Why: split-group layouts render their own terminal/browser/editor
+              surfaces inside TabGroupPanel. Keeping the legacy workspace-level
+              panes mounted underneath as hidden DOM creates duplicate
+              TerminalPane/BrowserPane instances for the same tab, which lets
+              two React trees race over one PTY or webview. Render only one
+              surface model at a time. */}
+          {/* Terminal panes container - hidden when editor tab active */}
+          <div
+            className={`relative flex-1 min-h-0 overflow-hidden ${
+              // Why: only hide the terminal container when another tab type has
+              // content to display. Hiding unconditionally for non-terminal types
+              // causes a blank screen when activeTabType is stale (e.g. 'editor'
+              // with no files after session restore). The terminal stays visible
+              // as a fallback until another surface is ready.
+              (activeTabType === 'editor' && worktreeFiles.length > 0) ||
+              (activeTabType === 'browser' && worktreeBrowserTabs.length > 0)
+                ? 'hidden'
+                : ''
+            }`}
+          >
+            {allWorktrees
+              .filter((wt) => mountedWorktreeIdsRef.current.has(wt.id))
+              .map((worktree) => {
+                const worktreeTabs = tabsByWorktree[worktree.id] ?? []
+                const isVisible = activeView !== 'settings' && worktree.id === activeWorktreeId
 
-      {/* Browser panes container — all browser panes for the active worktree
-          stay mounted so webview DOM state (scroll position, form inputs, etc.)
-          survives tab switches. BrowserPagePane uses isActive + CSS to show/hide. */}
-      <div
-        className={`relative flex-1 min-h-0 overflow-hidden ${activeTabType !== 'browser' ? 'hidden' : ''}`}
-      >
-        {allWorktrees.map((worktree) => {
-          const browserTabs = browserTabsByWorktree[worktree.id] ?? []
-          const isVisibleWorktree = activeView !== 'settings' && worktree.id === activeWorktreeId
-          if (browserTabs.length === 0) {
-            return null
-          }
-          return (
-            <div
-              key={`browser-${worktree.id}`}
-              className={isVisibleWorktree ? 'absolute inset-0' : 'absolute inset-0 hidden'}
-              aria-hidden={!isVisibleWorktree}
-            >
-              {browserTabs.map((browserTab) => {
-                const isBrowserActive =
-                  isVisibleWorktree &&
-                  activeTabType === 'browser' &&
-                  browserTab.id === activeBrowserTabId
                 return (
                   <div
-                    key={browserTab.id}
-                    className={`absolute inset-0${isBrowserActive ? '' : ' pointer-events-none hidden'}`}
+                    key={worktree.id}
+                    className={isVisible ? 'absolute inset-0' : 'absolute inset-0 hidden'}
+                    aria-hidden={!isVisible}
                   >
-                    <BrowserPane browserTab={browserTab} isActive={isBrowserActive} />
+                    {renderStaleCodexRestartChip(worktree.id)}
+                    {worktreeTabs.map((tab) => (
+                      <TerminalPane
+                        key={`${tab.id}-${tab.generation ?? 0}`}
+                        tabId={tab.id}
+                        worktreeId={worktree.id}
+                        cwd={worktree.path}
+                        isActive={
+                          isVisible && tab.id === activeTabId && activeTabType === 'terminal'
+                        }
+                        // Why: the legacy single-surface terminal host still expects
+                        // tab visibility to follow active-tab selection. Without this,
+                        // background terminals never get suspendRendering() and PTY
+                        // data bypasses the buffer, wasting CPU on hidden xterm renders.
+                        isVisible={
+                          isVisible && tab.id === activeTabId && activeTabType === 'terminal'
+                        }
+                        onPtyExit={(ptyId) => handlePtyExit(tab.id, ptyId)}
+                        onCloseTab={() => handleCloseTab(tab.id)}
+                      />
+                    ))}
                   </div>
                 )
               })}
-            </div>
-          )
-        })}
-      </div>
+          </div>
 
-      {activeWorktreeId && activeTabType === 'editor' && worktreeFiles.length > 0 && (
-        <Suspense
-          fallback={
-            <div className="flex-1 flex items-center justify-center text-muted-foreground text-sm">
-              Loading editor...
-            </div>
-          }
-        >
-          <EditorPanel />
-        </Suspense>
+          {/* Browser panes container — all browser panes for the active worktree
+              stay mounted so webview DOM state (scroll position, form inputs, etc.)
+              survives tab switches. BrowserPagePane uses isActive + CSS to show/hide. */}
+          <div
+            className={`relative flex-1 min-h-0 overflow-hidden ${
+              activeTabType !== 'browser' ? 'hidden' : ''
+            }`}
+          >
+            {allWorktrees.map((worktree) => {
+              const browserTabs = browserTabsByWorktree[worktree.id] ?? []
+              const isVisibleWorktree =
+                activeView !== 'settings' && worktree.id === activeWorktreeId
+              if (browserTabs.length === 0) {
+                return null
+              }
+              return (
+                <div
+                  key={`browser-${worktree.id}`}
+                  className={isVisibleWorktree ? 'absolute inset-0' : 'absolute inset-0 hidden'}
+                  aria-hidden={!isVisibleWorktree}
+                >
+                  {browserTabs.map((browserTab) => {
+                    const isBrowserActive =
+                      isVisibleWorktree &&
+                      activeTabType === 'browser' &&
+                      browserTab.id === activeBrowserTabId
+                    return (
+                      <div
+                        key={browserTab.id}
+                        className={`absolute inset-0${isBrowserActive ? '' : ' pointer-events-none hidden'}`}
+                      >
+                        <BrowserPane browserTab={browserTab} isActive={isBrowserActive} />
+                      </div>
+                    )
+                  })}
+                </div>
+              )
+            })}
+          </div>
+
+          {activeWorktreeId && activeTabType === 'editor' && worktreeFiles.length > 0 && (
+            <Suspense
+              fallback={
+                <div className="flex-1 flex items-center justify-center text-muted-foreground text-sm">
+                  Loading editor...
+                </div>
+              }
+            >
+              <EditorPanel />
+            </Suspense>
+          )}
+        </>
       )}
 
       {/* Save confirmation dialog */}