diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts new file mode 100644 index 00000000000..2f7712be669 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts @@ -0,0 +1,136 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { dispatchTerminalNotification } from './use-notification-dispatch' +import type { AgentStatusEntry } from '../../../../shared/agent-status-types' + +type MockState = { + activeWorktreeId: string | null + tabsByWorktree: Record + ptyIdsByTabId: Record + browserTabsByWorktree: Record + retainedAgentsByPaneKey: Record + agentStatusByPaneKey: Record + worktreesByRepo: Record< + string, + { + id: string + repoId: string + displayName?: string + branch?: string + workspaceStatus?: string + }[] + > + repos: { id: string; displayName?: string; connectionId?: string | null }[] + settings: { + notifications?: { + customSoundPath?: string | null + } + } +} + +const playDesktopNotificationSound = vi.hoisted(() => vi.fn()) +let mockState: MockState + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockState + } +})) + +vi.mock('@/lib/desktop-notification-sound', () => ({ + playDesktopNotificationSound +})) + +function makeAgentStatus(paneKey: string): AgentStatusEntry { + return { + state: 'done', + prompt: 'codex-hook-notify', + updatedAt: Date.now(), + stateStartedAt: Date.now(), + agentType: 'codex', + paneKey, + terminalTitle: 'codex', + stateHistory: [], + lastAssistantMessage: 'Done.' + } +} + +describe('dispatchTerminalNotification', () => { + const paneKey = 'tab-1:11111111-1111-4111-8111-111111111111' + + beforeEach(() => { + vi.clearAllMocks() + mockState = { + activeWorktreeId: 'wt-secondary', + tabsByWorktree: {}, + ptyIdsByTabId: { + 'tab-1': ['pty-1'] + }, + browserTabsByWorktree: {}, + retainedAgentsByPaneKey: {}, + agentStatusByPaneKey: { + [paneKey]: makeAgentStatus(paneKey) + }, + worktreesByRepo: { + repo1: [ + { + id: 'wt-primary', + repoId: 'repo1', + displayName: 'master', + branch: 'master' + }, + { + id: 'wt-secondary', + repoId: 'repo1', + displayName: 'e2e-secondary', + branch: 'e2e-secondary' + } + ] + }, + repos: [{ id: 'repo1', displayName: 'orca', connectionId: null }], + settings: { notifications: { customSoundPath: null } } + } + ;(globalThis as unknown as { window: unknown }).window = { + api: { + notifications: { + dispatch: vi.fn().mockResolvedValue({ delivered: true }) + } + } + } + }) + + it('uses a live pane key when inactive worktree tab membership is not hydrated', () => { + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'codex', + paneKey + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + source: 'agent-task-complete', + worktreeId: 'wt-primary', + paneKey, + repoLabel: 'orca', + worktreeLabel: 'master', + terminalTitle: 'codex', + isActiveWorktree: false, + agentType: 'codex', + agentState: 'done', + agentPrompt: 'codex-hook-notify', + agentLastAssistantMessage: 'Done.' + }) + ) + }) + + it('still drops stale notifications when neither worktree nor pane has a live pty', () => { + mockState.ptyIdsByTabId = {} + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'codex', + paneKey + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index 60796813d7e..44a52e1ef7b 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -22,6 +22,28 @@ function hasLivePtyForWorktree( return tabs.some((tab) => (state.ptyIdsByTabId[tab.id] ?? []).length > 0) } +function hasLivePtyForPaneKey( + state: ReturnType, + paneKey: string | undefined +): boolean { + if (!paneKey) { + return false + } + const tabId = getPaneKeyTabId(paneKey) + return tabId !== null && (state.ptyIdsByTabId[tabId] ?? []).length > 0 +} + +function hasLivePtyForNotification( + state: ReturnType, + worktreeId: string, + paneKey: string | undefined +): boolean { + // Why: inactive-worktree hook completions can arrive while the worktree tab + // list is between renderer hydration states; the pane-key PTY binding is the + // live terminal source in that path. + return hasLivePtyForWorktree(state, worktreeId) || hasLivePtyForPaneKey(state, paneKey) +} + function getPaneKeyTabId(paneKey: string): string | null { const parsed = parsePaneKey(paneKey) if (parsed) { @@ -132,7 +154,7 @@ export function dispatchTerminalNotification( // state. Checking for live PTYs at dispatch time catches ALL phantom // notification sources regardless of which timer or callback produced // them, rather than trying to cancel each one individually. - if (!hasLivePtyForWorktree(state, worktreeId)) { + if (!hasLivePtyForNotification(state, worktreeId, event.paneKey)) { return } diff --git a/tests/e2e/terminal-shortcuts.spec.ts b/tests/e2e/terminal-shortcuts.spec.ts index e861b0750a2..f31f8525415 100644 --- a/tests/e2e/terminal-shortcuts.spec.ts +++ b/tests/e2e/terminal-shortcuts.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- Terminal shortcut E2E keeps platform keyboard paths beside their shared PTY assertions. */ /** * E2E test for terminal keyboard shortcuts. * @@ -98,10 +99,24 @@ async function enableKittyKeyboardReporting(page: Page, flags: number): Promise< async function pressShiftedRussianLayoutKey(page: Page): Promise<{ keydownDefaultPrevented: boolean keypressSent: boolean + inputSent: boolean keyupSent: boolean }> { return page.evaluate(() => { - const textarea = document.querySelector('.xterm-helper-textarea') as HTMLTextAreaElement | null + const state = window.__store?.getState() + const worktreeId = state?.activeWorktreeId + const tabId = + state?.activeTabType === 'terminal' + ? state.activeTabId + : worktreeId + ? (state?.activeTabIdByWorktree?.[worktreeId] ?? null) + : null + const manager = tabId ? window.__paneManagers?.get(tabId) : null + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + pane?.terminal.focus() + const textarea = pane?.container.querySelector( + '.xterm-helper-textarea' + ) as HTMLTextAreaElement | null if (!textarea) { throw new Error('No xterm helper textarea to receive keyboard input') } @@ -119,7 +134,12 @@ async function pressShiftedRussianLayoutKey(page: Page): Promise<{ textarea.dispatchEvent(keydown) if (keydown.defaultPrevented) { - return { keydownDefaultPrevented: true, keypressSent: false, keyupSent: false } + return { + keydownDefaultPrevented: true, + keypressSent: false, + inputSent: false, + keyupSent: false + } } const keypress = new KeyboardEvent('keypress', { @@ -134,6 +154,17 @@ async function pressShiftedRussianLayoutKey(page: Page): Promise<{ Object.defineProperty(keypress, 'which', { get: () => 1060 }) textarea.dispatchEvent(keypress) + // Why: Chromium on Linux can surface layout text through the `input` event + // even when an untrusted synthetic keypress does not carry a usable charCode. + const input = new InputEvent('input', { + data: 'Ф', + inputType: 'insertText', + bubbles: true, + cancelable: false, + composed: false + }) + textarea.dispatchEvent(input) + const keyup = new KeyboardEvent('keyup', { key: 'Ф', code: 'KeyA', @@ -145,7 +176,7 @@ async function pressShiftedRussianLayoutKey(page: Page): Promise<{ Object.defineProperty(keyup, 'which', { get: () => 65 }) textarea.dispatchEvent(keyup) - return { keydownDefaultPrevented: false, keypressSent: true, keyupSent: true } + return { keydownDefaultPrevented: false, keypressSent: true, inputSent: true, keyupSent: true } }) } @@ -360,6 +391,7 @@ test.describe('Terminal Shortcuts', () => { expect(dispatch).toEqual({ keydownDefaultPrevented: false, keypressSent: true, + inputSent: true, keyupSent: true }) await expect