diff --git a/src/renderer/src/app-shell/app-command-handlers-tab-rename.test.ts b/src/renderer/src/app-shell/app-command-handlers-tab-rename.test.ts new file mode 100644 index 00000000000..0f4e85092f5 --- /dev/null +++ b/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() + }) +}) diff --git a/src/renderer/src/app-shell/app-command-handlers.ts b/src/renderer/src/app-shell/app-command-handlers.ts index bb60f898130..9e3df4c39fd 100644 --- a/src/renderer/src/app-shell/app-command-handlers.ts +++ b/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 diff --git a/src/renderer/src/runtime/web-session-tabs-sync/mirrored-agent-tab-label.test.ts b/src/renderer/src/runtime/web-session-tabs-sync/mirrored-agent-tab-label.test.ts index 91e58f3cedf..7d5ef3c25f3 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/mirrored-agent-tab-label.test.ts +++ b/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') }) }) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts b/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts index 0534b4f6ec0..1eb9078ca1d 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts +++ b/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, diff --git a/src/renderer/src/store/terminals/renamable-unified-tab.ts b/src/renderer/src/store/terminals/renamable-unified-tab.ts new file mode 100644 index 00000000000..74d3eee0234 --- /dev/null +++ b/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, + 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) + ) +} diff --git a/src/renderer/src/store/terminals/structured-chat-tab-rename.test.ts b/src/renderer/src/store/terminals/structured-chat-tab-rename.test.ts index c632a9f4021..5d1e6343f74 100644 --- a/src/renderer/src/store/terminals/structured-chat-tab-rename.test.ts +++ b/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 { const store = createTestStore() seedStore(store, { @@ -43,6 +61,40 @@ function labelOf(store: ReturnType): string | null | und .unifiedTabsByWorktree[WORKTREE]?.find((tab) => tab.id === STRUCTURED_TAB_ID)?.customLabel } +function colorOf(store: ReturnType): 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() diff --git a/src/renderer/src/store/terminals/terminal-tab-attention.ts b/src/renderer/src/store/terminals/terminal-tab-attention.ts index 0d07319a926..3ba84961763 100644 --- a/src/renderer/src/store/terminals/terminal-tab-attention.ts +++ b/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. diff --git a/src/shared/agent-session-chat-label.ts b/src/shared/agent-session-chat-label.ts index aec5bc70cb3..131285b0626 100644 --- a/src/shared/agent-session-chat-label.ts +++ b/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` }