Merge remote-tracking branch 'origin/brennanb2025/native-chat-tab-rename' into brennanb2025/native-chat-conversation-name

src/renderer/src/app-shell/app-command-handlers-tab-rename.test.ts

@@ -0,0 +1,74 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import type { AppState } from '@/store/types'
+import type { AppShortcutState, ShortcutDispatchInput } from './app-command-handlers'
+
+const mocks = vi.hoisted(() => ({
+  requestTerminalTabRename: vi.fn(),
+  store: {} as AppState
+}))
+
+vi.mock('../store', () => ({
+  useAppStore: Object.assign(vi.fn(), { getState: () => mocks.store })
+}))
+
+vi.mock('../components/tab-bar/terminal-tab-rename-request', () => ({
+  requestTerminalTabRename: mocks.requestTerminalTabRename
+}))
+
+vi.mock('@/lib/floating-workspace-terminal-actions', () => ({
+  isFloatingWorkspacePanelFocused: () => false
+}))
+
+vi.mock('@/lib/terminal-shortcut-capture-notification', () => ({
+  showTerminalShortcutCaptureNotification: vi.fn()
+}))
+
+import { createAppCommandHandlers } from './app-command-handlers'
+
+function shortcutState(): AppShortcutState {
+  return {
+    activeView: 'terminal',
+    activeWorktreeId: 'repo::/feature',
+    actions: {} as AppShortcutState['actions'],
+    creationLayoutActive: false,
+    floatingTerminalEnabled: false,
+    floatingTerminalOpen: false,
+    floatingVisibleTabCount: 0,
+    keybindings: {},
+    openFloatingWorkspaceMaximized: vi.fn(),
+    pluginCommands: [],
+    setFloatingTerminalOpen: vi.fn(),
+    terminalShortcutPolicy: 'orca-first',
+    workspaceChromeActive: true
+  }
+}
+
+function shortcutInput(): ShortcutDispatchInput {
+  return { target: null, defaultPrevented: false, preventDefault: vi.fn() }
+}
+
+function runRename(activeTabType: string | null): boolean | undefined {
+  mocks.store = { activeTabType, activeTabId: 'tab-1' } as unknown as AppState
+  return createAppCommandHandlers(shortcutState(), shortcutInput(), 'terminal').get(
+    'tab.rename'
+  )?.()
+}
+
+describe('tab.rename shortcut', () => {
+  beforeEach(() => vi.clearAllMocks())
+
+  it('opens the rename editor on a structured chat tab', () => {
+    expect(runRename('agent-session')).toBe(true)
+    expect(mocks.requestTerminalTabRename).toHaveBeenCalledWith('tab-1')
+  })
+
+  it('still opens the rename editor on a terminal tab', () => {
+    expect(runRename('terminal')).toBe(true)
+    expect(mocks.requestTerminalTabRename).toHaveBeenCalledWith('tab-1')
+  })
+
+  it('does not claim the chord for a tab type that has no inline rename', () => {
+    expect(runRename('browser')).toBe(false)
+    expect(mocks.requestTerminalTabRename).not.toHaveBeenCalled()
+  })
+})

src/renderer/src/app-shell/app-command-handlers.ts

@@ -176,7 +176,9 @@ export function createAppCommandHandlers(
         if (
           !workspaceChromeActive ||
           floatingWorkspaceFocused ||
-          store.activeTabType !== 'terminal' ||
+          // Why: a structured chat tab is renamed through the same inline editor,
+          // so gating on 'terminal' alone left the shortcut a silent no-op there.
+          (store.activeTabType !== 'terminal' && store.activeTabType !== 'agent-session') ||
           !store.activeTabId
         ) {
           return false

src/renderer/src/runtime/web-session-tabs-sync/mirrored-agent-tab-label.test.ts

@@ -60,6 +60,26 @@ describe('buildMirroredAgentTabs', () => {
   })
 
   it('leaves customLabel null when the tab was never renamed', () => {
-    expect(build(snapshotWith('codex', 'Codex Chat')).customLabel).toBeNull()
+    // Guard: assert the row is actually built, so this cannot pass on an empty
+    // result the way a bare null-check would.
+    const tab = build(snapshotWith('codex', 'Codex Chat'))
+    expect(tab.label).toBe('Codex Chat')
+    expect(tab.customLabel).toBeNull()
+  })
+
+  it('degrades to the placeholder when the host violates the string contract', () => {
+    const snapshot = snapshotWith('claude', 'Named')
+    // The wire type says `string`, but a host clearing a name can send null.
+    ;(snapshot.tabs[0] as { title: unknown }).title = null
+    expect(() => build(snapshot)).not.toThrow()
+    expect(build(snapshot).label).toBe('Claude Chat')
+  })
+
+  it('names an agent this build does not know after itself, not Codex', () => {
+    const snapshot = snapshotWith('codex', '')
+    // Cast: the wire union is claude|codex today, but Tab.agentSessionAgent is
+    // the open AgentType, so a future agent can reach this label.
+    ;(snapshot.tabs[0] as { agent: string }).agent = 'gemini'
+    expect(build(snapshot).label).toBe('Gemini Chat')
   })
 })

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

@@ -74,7 +74,9 @@ export function buildMirroredAgentTabs(
         worktreeId: snapshot.worktree,
         contentType: 'agent-session',
         agentSessionAgent: tab.agent,
-        label: tab.title.trim() || defaultAgentChatLabel(tab.agent),
+        // Why: `title` is wire data typed `string`; a host that violates that must
+        // degrade to the placeholder, not throw inside the snapshot patch.
+        label: tab.title?.trim() || defaultAgentChatLabel(tab.agent),
         // Why: a manual rename lives only on the client; re-nulling it here made
         // every host snapshot silently discard the user's title.
         customLabel: existing?.customLabel ?? null,

src/renderer/src/store/terminals/renamable-unified-tab.ts

@@ -0,0 +1,15 @@
+import type { Tab } from '../../../../shared/tab-types'
+
+/** Resolves the unified tab a per-tab presentation action (rename, color) targets.
+ *  Terminal tabs are addressed by their backing terminal's entityId; a structured
+ *  chat has no TerminalTab record and is addressed by the unified tab id itself. */
+export function findRenamableUnifiedTab(
+  unifiedTabsByWorktree: Record<string, Tab[]>,
+  tabId: string
+): Tab | undefined {
+  const unified = Object.values(unifiedTabsByWorktree).flat()
+  return (
+    unified.find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId) ??
+    unified.find((entry) => entry.contentType === 'agent-session' && entry.id === tabId)
+  )
+}

src/renderer/src/store/terminals/structured-chat-tab-rename.test.ts

@@ -25,6 +25,24 @@ function structuredTab(): Tab {
   }
 }
 
+const TERMINAL_TAB_ID = 'terminal-1'
+const TERMINAL_UNIFIED_ID = 'unified-terminal-1'
+
+function terminalTab(): Tab {
+  return {
+    id: TERMINAL_UNIFIED_ID,
+    entityId: TERMINAL_TAB_ID,
+    groupId: 'group-1',
+    worktreeId: WORKTREE,
+    contentType: 'terminal',
+    label: 'Terminal',
+    customLabel: null,
+    color: null,
+    sortOrder: 1,
+    createdAt: 2
+  }
+}
+
 function storeWithStructuredTab(): ReturnType<typeof createTestStore> {
   const store = createTestStore()
   seedStore(store, {
@@ -43,6 +61,40 @@ function labelOf(store: ReturnType<typeof createTestStore>): string | null | und
     .unifiedTabsByWorktree[WORKTREE]?.find((tab) => tab.id === STRUCTURED_TAB_ID)?.customLabel
 }
 
+function colorOf(store: ReturnType<typeof createTestStore>): string | null | undefined {
+  return store
+    .getState()
+    .unifiedTabsByWorktree[WORKTREE]?.find((tab) => tab.id === STRUCTURED_TAB_ID)?.color
+}
+
+describe('renaming a terminal tab still resolves', () => {
+  it('routes a terminal rename through its entityId, not the unified id', () => {
+    const store = createTestStore()
+    seedStore(store, {
+      repos: [{ id: 'local-repo', path: '/tmp/app', name: 'app' }] as never,
+      worktreesByRepo: {
+        'local-repo': [makeWorktree({ id: WORKTREE, repoId: 'local-repo', path: '/tmp/app' })]
+      },
+      unifiedTabsByWorktree: { [WORKTREE]: [terminalTab(), structuredTab()] }
+    })
+
+    // Keyed by the TERMINAL's entityId — the structured tab must not absorb it.
+    store.getState().setTabCustomTitle(TERMINAL_TAB_ID, 'Build logs')
+
+    const tabs = store.getState().unifiedTabsByWorktree[WORKTREE] ?? []
+    expect(tabs.find((t) => t.id === TERMINAL_UNIFIED_ID)?.customLabel).toBe('Build logs')
+    expect(tabs.find((t) => t.id === STRUCTURED_TAB_ID)?.customLabel).toBeNull()
+  })
+})
+
+describe('recoloring a structured chat tab', () => {
+  it('writes the color onto the agent-session tab', () => {
+    const store = storeWithStructuredTab()
+    store.getState().setTabColor(STRUCTURED_TAB_ID, 'red')
+    expect(colorOf(store)).toBe('red')
+  })
+})
+
 describe('renaming a structured chat tab', () => {
   it('writes the custom label onto the agent-session tab', () => {
     const store = storeWithStructuredTab()

src/renderer/src/store/terminals/terminal-tab-attention.ts

@@ -1,6 +1,7 @@
 import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph'
 import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route'
 import type { TerminalSlice, TerminalStoreGet, TerminalStoreSet } from './terminal-state'
+import { findRenamableUnifiedTab } from './renamable-unified-tab'
 
 export function createTerminalTabAttentionActions(
   set: TerminalStoreSet,
@@ -87,12 +88,7 @@ export function createTerminalTabAttentionActions(
         scheduleRuntimeGraphSync()
         return { tabsByWorktree: next }
       })
-      const unified = Object.values(get().unifiedTabsByWorktree).flat()
-      // Why: a structured chat tab has no TerminalTab record, and its rename
-      // arrives keyed by the unified tab id rather than a terminal entityId.
-      const item =
-        unified.find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId) ??
-        unified.find((entry) => entry.contentType === 'agent-session' && entry.id === tabId)
+      const item = findRenamableUnifiedTab(get().unifiedTabsByWorktree, tabId)
       if (item) {
         get().setTabCustomLabel(item.id, title, opts)
       }
@@ -105,9 +101,7 @@ export function createTerminalTabAttentionActions(
         }
         return { tabsByWorktree: next }
       })
-      const item = Object.values(get().unifiedTabsByWorktree)
-        .flat()
-        .find((entry) => entry.contentType === 'terminal' && entry.entityId === tabId)
+      const item = findRenamableUnifiedTab(get().unifiedTabsByWorktree, tabId)
       if (item) {
         get().setUnifiedTabColor(item.id, color)
         // Why: tab color is host-authoritative for remote-server tabs; mirror it so it persists instead of reverting on the next snapshot.

src/shared/agent-session-chat-label.ts

@@ -1,4 +1,9 @@
-/** Placeholder tab label for a structured chat that has no conversation name yet. */
-export function defaultAgentChatLabel(agent: 'claude' | 'codex' | null | undefined): string {
-  return agent === 'claude' ? 'Claude Chat' : 'Codex Chat'
+import type { AgentType } from './agent-status-types'
+import { formatAgentTypeLabel } from './agent-type-label'
+
+/** Placeholder tab label for a structured chat that has no conversation name yet.
+ *  Routed through the shared agent-name table so an agent this build does not
+ *  know reads as itself rather than silently as Codex. */
+export function defaultAgentChatLabel(agent: AgentType | null | undefined): string {
+  return `${formatAgentTypeLabel(agent)} Chat`
 }