mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
Fix release E2E notification and keyboard flakes
Merged after local lint/typecheck/unit/E2E verification. The PR check remained pending with no failure attached.
This commit is contained in:
@@ -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<string, { id: string }[]>
|
||||
ptyIdsByTabId: Record<string, string[]>
|
||||
browserTabsByWorktree: Record<string, unknown[]>
|
||||
retainedAgentsByPaneKey: Record<string, { worktreeId: string }>
|
||||
agentStatusByPaneKey: Record<string, AgentStatusEntry>
|
||||
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()
|
||||
})
|
||||
})
|
||||
@@ -22,6 +22,28 @@ function hasLivePtyForWorktree(
|
||||
return tabs.some((tab) => (state.ptyIdsByTabId[tab.id] ?? []).length > 0)
|
||||
}
|
||||
|
||||
function hasLivePtyForPaneKey(
|
||||
state: ReturnType<typeof useAppStore.getState>,
|
||||
paneKey: string | undefined
|
||||
): boolean {
|
||||
if (!paneKey) {
|
||||
return false
|
||||
}
|
||||
const tabId = getPaneKeyTabId(paneKey)
|
||||
return tabId !== null && (state.ptyIdsByTabId[tabId] ?? []).length > 0
|
||||
}
|
||||
|
||||
function hasLivePtyForNotification(
|
||||
state: ReturnType<typeof useAppStore.getState>,
|
||||
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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user