Address CodeRabbit follow-ups from #4825 (#4831)

Co-authored-by: Cursor <cursoragent@cursor.com>

src/renderer/src/components/GitHubItemDialog.tsx

@@ -3502,6 +3502,7 @@ function ChecksTab({
         toast.error('Could not build the agent launch command.')
         return
       }
+      // Why: host-backed web launches can succeed without a local tab id.
       if (result.tabId) {
         focusTerminalTabSurface(result.tabId)
       }

src/renderer/src/components/terminal/terminal-tab-actions.test.ts

@@ -126,7 +126,7 @@ describe('closeTerminalTab', () => {
     expect(closeTab).toHaveBeenCalledWith('local-tab-1')
     expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({
       worktreeId: 'wt-1',
-      tabId: 'local-tab-1',
+      tabId: 'host-tab-1',
       environmentId: 'web-runtime'
     })
   })
@@ -169,6 +169,60 @@ describe('closeTerminalTab', () => {
     expect(closeUnifiedTab).toHaveBeenCalledWith('unified-tab-1')
   })
 
+  it('activates the next unified terminal tab when closing the active unified-only tab', () => {
+    const closeUnifiedTab = vi.fn()
+    const setActiveTab = vi.fn()
+    getStateMock.mockReturnValue({
+      settings: { activeRuntimeEnvironmentId: null },
+      tabsByWorktree: {},
+      unifiedTabsByWorktree: {
+        'wt-1': [
+          {
+            id: 'unified-tab-1',
+            entityId: 'terminal-entity-1',
+            contentType: 'terminal',
+            groupId: 'group-1',
+            worktreeId: 'wt-1',
+            label: 'Claude',
+            customLabel: null,
+            color: null,
+            sortOrder: 0,
+            createdAt: 0,
+            isPreview: false,
+            isPinned: false
+          },
+          {
+            id: 'unified-tab-2',
+            entityId: 'terminal-entity-2',
+            contentType: 'terminal',
+            groupId: 'group-1',
+            worktreeId: 'wt-1',
+            label: 'Terminal',
+            customLabel: null,
+            color: null,
+            sortOrder: 1,
+            createdAt: 0,
+            isPreview: false,
+            isPinned: false
+          }
+        ]
+      },
+      activeWorktreeId: 'wt-1',
+      activeTabId: 'terminal-entity-1',
+      openFiles: [],
+      browserTabsByWorktree: {},
+      closeTab: vi.fn(),
+      closeUnifiedTab,
+      setActiveTab,
+      setActiveWorktree: vi.fn()
+    })
+
+    closeTerminalTab('terminal-entity-1')
+
+    expect(setActiveTab).toHaveBeenCalledWith('terminal-entity-2')
+    expect(closeUnifiedTab).toHaveBeenCalledWith('unified-tab-1')
+  })
+
   it('closes local-only agent tabs locally when they have no host session binding', () => {
     const closeTab = vi.fn()
     isWebRuntimeSessionActiveMock.mockReturnValue(true)

src/renderer/src/components/terminal/terminal-tab-actions.ts

@@ -42,6 +42,22 @@ function resolveCloseTerminalTabTarget(
   return null
 }
 
+// Why: host-backed terminals may only exist in unifiedTabsByWorktree as
+// terminal entities, so close/sibling selection must merge tabsByWorktree and
+// unified terminal entityIds into one deduped list per worktree.
+function getWorktreeTerminalTabIds(state: TerminalTabActionState, worktreeId: string): string[] {
+  const ids = new Set<string>()
+  for (const tab of state.tabsByWorktree[worktreeId] ?? []) {
+    ids.add(tab.id)
+  }
+  for (const tab of state.unifiedTabsByWorktree?.[worktreeId] ?? []) {
+    if (tab.contentType === 'terminal') {
+      ids.add(tab.entityId)
+    }
+  }
+  return [...ids]
+}
+
 function closeLocalTerminalTabState(terminalTabId: string): void {
   const state = useAppStore.getState()
   if (
@@ -144,7 +160,7 @@ export function closeTerminalTab(tabId: string): void {
       closeLocalTerminalTabState(terminalTabId)
       void closeWebRuntimeSessionTab({
         worktreeId: owningWorktreeId,
-        tabId: terminalTabId,
+        tabId: hostBackedTabId,
         environmentId: runtimeEnvironmentId
       })
       return
@@ -153,8 +169,8 @@ export function closeTerminalTab(tabId: string): void {
     // have no host session binding and must still close locally.
   }
 
-  const currentTabs = state.tabsByWorktree[owningWorktreeId] ?? []
-  if (currentTabs.length <= 1) {
+  const currentTerminalTabIds = getWorktreeTerminalTabIds(state, owningWorktreeId)
+  if (currentTerminalTabIds.length <= 1) {
     closeLocalTerminalTabState(terminalTabId)
     if (state.activeWorktreeId === owningWorktreeId) {
       // Why: only deactivate the worktree when no tabs of any kind remain.
@@ -178,10 +194,11 @@ export function closeTerminalTab(tabId: string): void {
   }
 
   if (state.activeWorktreeId === owningWorktreeId && terminalTabId === state.activeTabId) {
-    const currentIndex = currentTabs.findIndex((tab) => tab.id === terminalTabId)
-    const nextTab = currentTabs[currentIndex + 1] ?? currentTabs[currentIndex - 1]
-    if (nextTab) {
-      state.setActiveTab(nextTab.id)
+    const currentIndex = currentTerminalTabIds.indexOf(terminalTabId)
+    const nextTabId =
+      currentTerminalTabIds[currentIndex + 1] ?? currentTerminalTabIds[currentIndex - 1]
+    if (nextTabId) {
+      state.setActiveTab(nextTabId)
     }
   }
 

src/renderer/src/hooks/useAutomationDispatchEvents.ts

@@ -378,11 +378,13 @@ export function useAutomationDispatchEvents(): void {
         if (!result) {
           throw new Error('Unable to build an agent launch plan.')
         }
-        if (!result.tabId) {
-          throw new Error('Host-backed agent tabs are not yet trackable for automation dispatch.')
-        }
         const launchedTabId = result.tabId
-        observeAgentStatus(launchedTabId, dispatchStartedAt)
+        // Why: host-backed automation terminals may lack a local tab id; skip
+        // pane-key status observation while background session output still
+        // tracks completion.
+        if (launchedTabId) {
+          observeAgentStatus(launchedTabId, dispatchStartedAt)
+        }
         try {
           await markDispatchResult({
             runId: run.id,

src/renderer/src/lib/launch-agent-in-new-tab.test.ts

@@ -33,8 +33,10 @@ vi.mock('@/store', () => ({
   }
 }))
 
+const mockToastError = vi.fn()
+
 vi.mock('sonner', () => ({
-  toast: { message: vi.fn() }
+  toast: { message: vi.fn(), error: mockToastError }
 }))
 
 vi.mock('@/components/tab-bar/reconcile-order', () => ({
@@ -125,10 +127,27 @@ describe('launchAgentInNewTab', () => {
     })
     expect(mockCreateTab).not.toHaveBeenCalled()
     expect(mockQueueTabStartupCommand).not.toHaveBeenCalled()
+    await Promise.resolve()
     expect(mockSetActiveTabType).toHaveBeenCalledWith('terminal')
     expect(store.closeTab).toHaveBeenCalledWith('stale-agent-tab')
   })
 
+  it('surfaces a toast when host agent launch fails in paired web clients', async () => {
+    mockIsWebRuntimeSessionActive.mockReturnValue(true)
+    mockCreateWebRuntimeSessionTerminal.mockResolvedValue(false)
+    store.settings = { agentCmdOverrides: {}, activeRuntimeEnvironmentId: 'web-runtime' }
+    const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
+
+    launchAgentInNewTab({
+      agent: 'claude',
+      worktreeId: 'wt-1'
+    })
+
+    await Promise.resolve()
+    expect(mockToastError).toHaveBeenCalledWith('Could not launch claude in a new terminal.')
+    expect(mockSetActiveTabType).not.toHaveBeenCalled()
+  })
+
   it('queues initial working status for Command Code argv prompt launches', async () => {
     const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
 

src/renderer/src/lib/launch-agent-in-new-tab.ts

@@ -205,8 +205,9 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI
 
   const runtimeEnvironmentId = store.settings?.activeRuntimeEnvironmentId?.trim()
   if (isWebRuntimeSessionActive(runtimeEnvironmentId) && pasteDraftAfterLaunch === null) {
-    // Why: paired web tabs are host-owned. Local-only agent tabs cannot be
-    // closed because close routes through session.tabs.close on the host.
+    // Why: paired web tabs are host-owned and return tabId: null on success.
+    // Local-only agent tabs cannot be closed because close routes through
+    // session.tabs.close on the host, so prune them before the host snapshot.
     removeStaleLocalAgentTabsForWebHostLaunch(worktreeId)
     void createWebRuntimeSessionTerminal({
       worktreeId,
@@ -214,13 +215,19 @@ export function launchAgentInNewTab(args: LaunchAgentInNewTabArgs): LaunchAgentI
       targetGroupId: groupId,
       activate: true,
       ...(hasPrompt ? { command: startupPlan.launchCommand } : { agent })
-    }).then(() => {
+    }).then((created) => {
+      // Why: created means the host accepted the launch, not that a local tab
+      // exists; keep pruning stale local rows until the snapshot mirrors.
       removeStaleLocalAgentTabsForWebHostLaunch(worktreeId)
+      if (!created) {
+        toast.error(`Could not launch ${agent} in a new terminal.`)
+        return
+      }
+      store.setActiveTabType('terminal')
+      if (hasPrompt) {
+        onPromptDelivered?.()
+      }
     })
-    store.setActiveTabType('terminal')
-    if (hasPrompt) {
-      onPromptDelivered?.()
-    }
     return { tabId: null, startupPlan, pasteDraftAfterLaunch: false }
   }
 

src/renderer/src/runtime/web-runtime-wake-terminal-respawn.test.ts

@@ -1,6 +1,7 @@
 import { beforeEach, describe, expect, it } from 'vitest'
 import {
   beginWebRuntimeWakeTerminalRespawn,
+  clearWebRuntimeWakeTerminalRespawnForWorktree,
   endWebRuntimeWakeTerminalRespawn,
   resetWebRuntimeWakeTerminalRespawnForTests,
   shouldSkipWebRuntimeWakeTerminalRespawn
@@ -11,11 +12,19 @@ describe('web-runtime-wake-terminal-respawn', () => {
     resetWebRuntimeWakeTerminalRespawnForTests()
   })
 
-  it('dedupes wake respawn requests for the same worktree', () => {
+  it('dedupes concurrent wake respawn requests for the same worktree', () => {
     expect(beginWebRuntimeWakeTerminalRespawn('wt-1')).toBe(true)
     expect(shouldSkipWebRuntimeWakeTerminalRespawn('wt-1')).toBe(true)
     expect(beginWebRuntimeWakeTerminalRespawn('wt-1')).toBe(false)
     endWebRuntimeWakeTerminalRespawn('wt-1')
-    expect(shouldSkipWebRuntimeWakeTerminalRespawn('wt-1')).toBe(true)
+    expect(shouldSkipWebRuntimeWakeTerminalRespawn('wt-1')).toBe(false)
+    expect(beginWebRuntimeWakeTerminalRespawn('wt-1')).toBe(true)
+  })
+
+  it('clears wake respawn tracking for a removed worktree', () => {
+    beginWebRuntimeWakeTerminalRespawn('wt-1')
+    clearWebRuntimeWakeTerminalRespawnForWorktree('wt-1')
+    expect(shouldSkipWebRuntimeWakeTerminalRespawn('wt-1')).toBe(false)
+    expect(beginWebRuntimeWakeTerminalRespawn('wt-1')).toBe(true)
   })
 })

src/renderer/src/runtime/web-runtime-wake-terminal-respawn.ts

@@ -1,18 +1,13 @@
-const wakeTerminalRespawnRequestedByWorktree = new Set<string>()
 const wakeTerminalRespawnInFlightByWorktree = new Set<string>()
 
 export function shouldSkipWebRuntimeWakeTerminalRespawn(worktreeId: string): boolean {
-  return (
-    wakeTerminalRespawnRequestedByWorktree.has(worktreeId) ||
-    wakeTerminalRespawnInFlightByWorktree.has(worktreeId)
-  )
+  return wakeTerminalRespawnInFlightByWorktree.has(worktreeId)
 }
 
 export function beginWebRuntimeWakeTerminalRespawn(worktreeId: string): boolean {
-  if (shouldSkipWebRuntimeWakeTerminalRespawn(worktreeId)) {
+  if (wakeTerminalRespawnInFlightByWorktree.has(worktreeId)) {
     return false
   }
-  wakeTerminalRespawnRequestedByWorktree.add(worktreeId)
   wakeTerminalRespawnInFlightByWorktree.add(worktreeId)
   return true
 }
@@ -21,7 +16,14 @@ export function endWebRuntimeWakeTerminalRespawn(worktreeId: string): void {
   wakeTerminalRespawnInFlightByWorktree.delete(worktreeId)
 }
 
-export function resetWebRuntimeWakeTerminalRespawnForTests(): void {
-  wakeTerminalRespawnRequestedByWorktree.clear()
+export function clearWebRuntimeWakeTerminalRespawnForWorktree(worktreeId: string): void {
+  wakeTerminalRespawnInFlightByWorktree.delete(worktreeId)
+}
+
+export function clearAllWebRuntimeWakeTerminalRespawn(): void {
   wakeTerminalRespawnInFlightByWorktree.clear()
 }
+
+export function resetWebRuntimeWakeTerminalRespawnForTests(): void {
+  clearAllWebRuntimeWakeTerminalRespawn()
+}

src/renderer/src/runtime/web-session-tabs-sync.test.ts

@@ -439,6 +439,45 @@ describe('applyWebSessionTabsSnapshot', () => {
     expect(patch.groupsByWorktree?.[WT]?.[0]?.tabOrder).toEqual([mirroredId])
   })
 
+  it('keeps stale local agent tabs when the host mirror is for a different agent', () => {
+    const staleLocalClaudeTab: TerminalTab = {
+      id: 'local-claude-tab',
+      ptyId: null,
+      worktreeId: WT,
+      title: 'Claude',
+      defaultTitle: 'Claude',
+      customTitle: null,
+      color: null,
+      sortOrder: 0,
+      createdAt: NOW,
+      launchAgent: 'claude'
+    }
+
+    const patch = applyWebSessionTabsSnapshot(
+      makeState({
+        tabsByWorktree: { [WT]: [staleLocalClaudeTab] }
+      }),
+      makeSnapshot([
+        {
+          type: 'terminal',
+          id: HOST_SURFACE_ID,
+          title: 'Codex',
+          parentTabId: 'host-tab-1',
+          leafId: LEAF_ID,
+          isActive: true,
+          launchAgent: 'codex',
+          status: 'ready',
+          terminal: 'terminal-1'
+        }
+      ]),
+      ENV,
+      NOW
+    ) as Partial<WebSessionTabsSyncState>
+
+    expect(patch.tabsByWorktree?.[WT]).toHaveLength(2)
+    expect(patch.tabsByWorktree?.[WT]?.some((tab) => tab.id === 'local-claude-tab')).toBe(true)
+  })
+
   it('hydrates ready host terminal surfaces as remote runtime terminal tabs', () => {
     const patch = applyWebSessionTabsSnapshot(
       makeState(),

src/renderer/src/runtime/web-session-tabs-sync.ts

@@ -25,7 +25,8 @@ import type {
   TabGroupLayoutNode,
   TerminalLayoutSnapshot,
   TerminalPaneLayoutNode,
-  TerminalTab
+  TerminalTab,
+  TuiAgent
 } from '../../../shared/types'
 import type { OpenFile } from '../store/slices/editor'
 import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id'
@@ -41,6 +42,8 @@ import {
 import { toRuntimeWorktreeSelector } from './runtime-worktree-selector'
 import {
   beginWebRuntimeWakeTerminalRespawn,
+  clearAllWebRuntimeWakeTerminalRespawn,
+  clearWebRuntimeWakeTerminalRespawnForWorktree,
   endWebRuntimeWakeTerminalRespawn,
   shouldSkipWebRuntimeWakeTerminalRespawn
 } from './web-runtime-wake-terminal-respawn'
@@ -245,6 +248,7 @@ function clearWebSessionTabsTrackingForWorktree(environmentId: string, worktreeI
   const key = sessionTabsFreshnessKey(environmentId, worktreeId)
   latestSessionTabsSnapshotByWorktree.delete(key)
   lastHostTerminalTabCountByWorktree.delete(key)
+  clearWebRuntimeWakeTerminalRespawnForWorktree(worktreeId)
   const keyPrefix = `${environmentId}:${worktreeId}:`
   for (const key of hostSessionTabIdByLocalKey.keys()) {
     if (key.startsWith(keyPrefix)) {
@@ -274,6 +278,7 @@ export function clearWebSessionTabsTrackingForEnvironment(environmentId: string)
       hostSessionTabIdByLocalKey.delete(key)
     }
   }
+  clearAllWebRuntimeWakeTerminalRespawn()
 }
 
 function hostSessionTabMappingKey(args: {
@@ -419,11 +424,16 @@ function shouldReplaceTerminalTab(
   tab: TerminalTab,
   environmentId: string,
   nextRemotePtyIds: ReadonlySet<string>,
-  nextMirroredTerminalIds: ReadonlySet<string>
+  nextMirroredTerminalIds: ReadonlySet<string>,
+  nextMirroredLaunchAgents: ReadonlySet<TuiAgent>
 ): boolean {
-  if (tab.launchAgent && !isMirroredTerminalSurfaceId(tab.id) && nextMirroredTerminalIds.size > 0) {
+  if (
+    tab.launchAgent &&
+    !isMirroredTerminalSurfaceId(tab.id) &&
+    nextMirroredLaunchAgents.has(tab.launchAgent)
+  ) {
     // Why: paired web agent quick-launch used to create local-only tabs before
-    // the host snapshot landed. Once host mirrors exist, retire the stale row.
+    // the host snapshot landed. Retire only the matching agent's stale row.
     return true
   }
   if (isMirroredTerminalSurfaceId(tab.id)) {
@@ -1438,9 +1448,20 @@ export function applyWebSessionTabsSnapshot(
   const nextMirroredTerminalIds = new Set(
     terminalSurfaceTabs.map((tab) => toWebTerminalSurfaceTabId(tab.parentTabId))
   )
+  const nextMirroredLaunchAgents = new Set(
+    terminalSurfaceTabs
+      .map((tab) => tab.launchAgent)
+      .filter((agent): agent is TuiAgent => Boolean(agent))
+  )
   const retainedTerminalTabs = currentTerminalTabs.filter(
     (tab) =>
-      !shouldReplaceTerminalTab(tab, environmentId, nextRemotePtyIds, nextMirroredTerminalIds)
+      !shouldReplaceTerminalTab(
+        tab,
+        environmentId,
+        nextRemotePtyIds,
+        nextMirroredTerminalIds,
+        nextMirroredLaunchAgents
+      )
   )
   const mirroredTerminalTabs = buildMirroredTerminalTabs(
     snapshot,
@@ -2300,6 +2321,8 @@ export function useWebSessionTabsSync(): void {
               beginWebRuntimeWakeTerminalRespawn(activeWorktreeId)
             ) {
               requestedRespawnAfterWake = true
+              // Why: wake recovery must recreate the terminal without changing
+              // selected worktree to avoid re-triggering activation churn.
               void createWebRuntimeSessionTerminal({
                 worktreeId: activeWorktreeId,
                 environmentId,