From 0322083c883f50cdc8da9df81fdf5450f158506a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 21 May 2026 14:16:01 -0700 Subject: [PATCH] Fix release E2E flakes and gate publish Gate release publishing on tag-scoped E2E, harden Droid agent-status routing against renderer layout races, and stabilize the affected E2E setup helpers. --- .github/workflows/release-cut.yml | 8 +- src/renderer/src/hooks/useIpcEvents.test.ts | 108 +++++++++++++++ src/renderer/src/hooks/useIpcEvents.ts | 86 +++++++++++- tests/e2e/droid-notification.spec.ts | 30 +---- tests/e2e/helpers/store.ts | 140 ++++++++++---------- tests/e2e/helpers/terminal.ts | 81 +++++++++++ tests/e2e/setup-script-import.spec.ts | 6 +- 7 files changed, 346 insertions(+), 113 deletions(-) diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index aa72a96e8..ddee7dde8 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -441,10 +441,9 @@ jobs: --generate-notes \ --prerelease="$is_rc" - # Why: E2E runs alongside the release for visibility (failures surface as a - # red check on the tag), but is NOT in `publish-release`'s needs list. - # Releases already take a while and the suite is already a required check - # on PRs, so gating here would mostly delay shipping without adding signal. + # Why: release-cut is the last gate before a tag becomes public. PR checks + # catch most regressions, but tag-scoped E2E must pass before publish-release + # flips the draft visible. e2e: needs: cut if: needs.cut.outputs.should_release == 'true' @@ -695,6 +694,7 @@ jobs: needs: - cut - build + - e2e runs-on: ubuntu-latest permissions: contents: write diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index 353f115f1..0ef243ae2 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -2181,6 +2181,114 @@ describe('useIpcEvents agent status snapshot integration', () => { ) }) + it('buffers ready push events until the pane leaf resolves in renderer layout', async () => { + const setAgentStatus = vi.fn() + const track = vi.fn() + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } + + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + workspaceSessionReady: true, + settings: { terminalFontSize: 13, notifications: { enabled: false } }, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] + }, + terminalLayoutsByTabId: {} + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn((listener: StoreSubscribeListener) => { + subscribeListenerRef.current = listener + return () => { + subscribeListenerRef.current = null + } + }), + getState: () => storeState + } + })) + stubAuxiliaryModules() + vi.doMock('@/lib/telemetry', () => ({ track })) + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + onSetListenerRef.current = cb + return () => {} + } + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + + useIpcEvents() + await Promise.resolve() + + if (typeof onSetListenerRef.current !== 'function') { + throw new Error('Expected agentStatus.onSet listener to be registered') + } + + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + state: 'working', + prompt: 'queued prompt', + agentType: 'codex', + receivedAt: 1_700_000_000_100, + stateStartedAt: 1_699_999_999_100 + }) + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + state: 'done', + prompt: 'queued prompt', + agentType: 'codex', + lastAssistantMessage: 'queued completion', + receivedAt: 1_700_000_000_200, + stateStartedAt: 1_699_999_999_100 + }) + + expect(setAgentStatus).not.toHaveBeenCalled() + expect(track).toHaveBeenCalledWith('agent_hook_unattributed', { + reason: 'unknown_tab_id' + }) + + storeState.terminalLayoutsByTabId = { + 'tab-future': { + root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, + activeLeafId: FUTURE_LEAF_ID, + expandedLeafId: null + } + } + if (typeof subscribeListenerRef.current !== 'function') { + throw new Error('Expected useAppStore.subscribe listener to be registered') + } + subscribeListenerRef.current(storeState) + + expect(setAgentStatus).toHaveBeenCalledTimes(2) + expect(setAgentStatus).toHaveBeenNthCalledWith( + 1, + FUTURE_PANE_KEY, + expect.objectContaining({ state: 'working', prompt: 'queued prompt', agentType: 'codex' }), + 'Future Tab', + { updatedAt: 1_700_000_000_100, stateStartedAt: 1_699_999_999_100 } + ) + expect(setAgentStatus).toHaveBeenNthCalledWith( + 2, + FUTURE_PANE_KEY, + expect.objectContaining({ + state: 'done', + prompt: 'queued prompt', + agentType: 'codex', + lastAssistantMessage: 'queued completion' + }), + 'Future Tab', + { updatedAt: 1_700_000_000_200, stateStartedAt: 1_699_999_999_100 } + ) + }) + it('applies remote status snapshots while repo ownership is still hydrating', async () => { const setAgentStatus = vi.fn() const getSnapshot = vi.fn(() => diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 38a188fe2..4c913e529 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -81,6 +81,9 @@ import { export { resolveZoomTarget } from './resolve-zoom-target' const ZOOM_STEP = 0.5 +const PENDING_AGENT_STATUS_RETRY_MS = 100 +const PENDING_AGENT_STATUS_TTL_MS = 15_000 +const MAX_PENDING_AGENT_STATUS_EVENTS = 100 let remoteWorkspaceSnapshotApplyDepth = 0 let remoteWorkspaceSnapshotWriteSuppressUntil = 0 const REMOTE_WORKSPACE_SNAPSHOT_WRITE_SUPPRESS_MS = 1000 @@ -488,6 +491,13 @@ function getActiveRuntimeEnvironmentId(): string | null { export function useIpcEvents(): void { useEffect(() => { const unsubs: (() => void)[] = [] + type PendingAgentStatusEvent = { + data: AgentStatusIpcPayload + firstSeenAt: number + } + type AgentStatusApplyResult = 'applied' | 'pending' | 'dropped' + const pendingAgentStatusEvents: PendingAgentStatusEvent[] = [] + let pendingAgentStatusRetryTimer: ReturnType | null = null unsubs.push(attachMobileMarkdownBridge()) @@ -1801,13 +1811,55 @@ export function useIpcEvents(): void { // hook callback or an OSC fallback path. Startup pushes are ignored until // workspace session hydration finishes; the snapshot pull below replays the // main-process cache after tab identity is available. + function schedulePendingAgentStatusFlush(): void { + if (pendingAgentStatusRetryTimer !== null || pendingAgentStatusEvents.length === 0) { + return + } + pendingAgentStatusRetryTimer = globalThis.setTimeout(() => { + pendingAgentStatusRetryTimer = null + flushPendingAgentStatuses() + }, PENDING_AGENT_STATUS_RETRY_MS) + } + + function enqueuePendingAgentStatus(data: AgentStatusIpcPayload): void { + pendingAgentStatusEvents.push({ data, firstSeenAt: Date.now() }) + while (pendingAgentStatusEvents.length > MAX_PENDING_AGENT_STATUS_EVENTS) { + pendingAgentStatusEvents.shift() + } + schedulePendingAgentStatusFlush() + } + + function flushPendingAgentStatuses(): void { + if (pendingAgentStatusEvents.length === 0) { + return + } + const now = Date.now() + const remaining: PendingAgentStatusEvent[] = [] + for (const event of pendingAgentStatusEvents) { + if (now - event.firstSeenAt > PENDING_AGENT_STATUS_TTL_MS) { + continue + } + const result = applyAgentStatus(event.data, { retry: true }) + if (result === 'pending') { + remaining.push(event) + } + } + pendingAgentStatusEvents.length = 0 + pendingAgentStatusEvents.push(...remaining) + if (pendingAgentStatusEvents.length === 0 && pendingAgentStatusRetryTimer !== null) { + globalThis.clearTimeout(pendingAgentStatusRetryTimer) + pendingAgentStatusRetryTimer = null + } + schedulePendingAgentStatusFlush() + } + const applyAgentStatus = ( data: AgentStatusIpcPayload, - options?: { replay?: boolean } - ): void => { + options?: { replay?: boolean; retry?: boolean } + ): AgentStatusApplyResult => { const store = useAppStore.getState() if (!store.workspaceSessionReady) { - return + return 'dropped' } const payload = normalizeAgentStatusPayload({ state: data.state, @@ -1819,7 +1871,7 @@ export function useIpcEvents(): void { interrupted: data.interrupted }) if (!payload) { - return + return 'dropped' } const { exists, title, repoConnectionId, repoConnectionResolved, owningWorktreeId } = resolvePaneKey(store, data.paneKey) @@ -1831,9 +1883,23 @@ export function useIpcEvents(): void { // include entries whose tabs were closed before this session — that // reconciliation miss is not a regression signal. if (options?.replay !== true) { - track('agent_hook_unattributed', { reason: 'unknown_tab_id' }) + if (options?.retry !== true) { + track('agent_hook_unattributed', { reason: 'unknown_tab_id' }) + // Why: live hook IPC can beat the renderer's tab/layout hydration. + // Main already cached the event; retry locally so a transient + // pane-key miss does not drop Droid/Codex completion state. + enqueuePendingAgentStatus(data) + } + return 'pending' + } + return 'dropped' + } + if (options?.replay !== true && options?.retry !== true) { + for (let index = pendingAgentStatusEvents.length - 1; index >= 0; index -= 1) { + if (pendingAgentStatusEvents[index].data.paneKey === data.paneKey) { + pendingAgentStatusEvents.splice(index, 1) + } } - return } // Why: drop in-flight events from a connection that no longer owns // this pane. After an SSH disconnect (or tab destroy/recreate during @@ -1860,7 +1926,7 @@ export function useIpcEvents(): void { data.connectionId !== repoConnectionId && !canAcceptPendingRemoteOwnership ) { - return + return 'dropped' } store.setAgentStatus(data.paneKey, payload, title, { updatedAt: data.receivedAt, @@ -1877,6 +1943,7 @@ export function useIpcEvents(): void { payload }) } + return 'applied' } let snapshotRequestedForReadyWindow = false @@ -1972,6 +2039,7 @@ export function useIpcEvents(): void { unsubs.push( useAppStore.subscribe(() => { requestAgentStatusSnapshotIfReady() + flushPendingAgentStatuses() syncAgentHookCompletionNotificationSettings() }) ) @@ -2084,6 +2152,10 @@ export function useIpcEvents(): void { } return () => { + if (pendingAgentStatusRetryTimer !== null) { + globalThis.clearTimeout(pendingAgentStatusRetryTimer) + } + pendingAgentStatusEvents.length = 0 unsubs.forEach((fn) => fn()) resetAgentHookCompletionNotificationCoordinators() } diff --git a/tests/e2e/droid-notification.spec.ts b/tests/e2e/droid-notification.spec.ts index c9d1b6935..a0300ad2d 100644 --- a/tests/e2e/droid-notification.spec.ts +++ b/tests/e2e/droid-notification.spec.ts @@ -3,6 +3,7 @@ import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { getRendererTitleLog, installRendererTitleLog } from './helpers/terminal-title-log' import { sendToTerminal, + waitForActivePaneHookDescriptor, waitForActivePanePtyId, waitForActiveTerminalManager, waitForTerminalOutput @@ -116,33 +117,6 @@ async function getAgentStatuses(page: Page): Promise< }) } -async function getActivePaneDescriptor( - page: Page -): Promise<{ paneKey: string; worktreeId: string }> { - return page.evaluate(() => { - const store = window.__store - if (!store) { - throw new Error('Store unavailable') - } - const state = store.getState() - const worktreeId = state.activeWorktreeId - if (!worktreeId) { - throw new Error('No active worktree') - } - const tabId = state.activeTabIdByWorktree[worktreeId] ?? state.activeTabId - if (!tabId) { - throw new Error('No active tab') - } - const manager = window.__paneManagers?.get(tabId) - const activePane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] - const leafId = activePane ? manager?.getLeafIdMap?.().get(activePane.id) : null - if (!leafId) { - throw new Error('No active pane leaf id') - } - return { paneKey: `${tabId}:${leafId}`, worktreeId } - }) -} - test.describe('Droid notifications', () => { test('Codex hook completion dispatches while its worktree is inactive', async ({ orcaPage, @@ -162,7 +136,7 @@ test.describe('Droid notifications', () => { await sendToTerminal(orcaPage, ptyId, `printf '${readyMarker}\\n'\r`) await waitForTerminalOutput(orcaPage, readyMarker) - const { paneKey, worktreeId } = await getActivePaneDescriptor(orcaPage) + const { paneKey, worktreeId } = await waitForActivePaneHookDescriptor(orcaPage) const prompt = `codex-hook-notify-${Date.now()}` await emitCodexHookStatus(endpoint, { paneKey, diff --git a/tests/e2e/helpers/store.ts b/tests/e2e/helpers/store.ts index 2bec3590d..e79444083 100644 --- a/tests/e2e/helpers/store.ts +++ b/tests/e2e/helpers/store.ts @@ -166,58 +166,43 @@ export async function waitForSessionReady(page: Page, timeoutMs = 30_000): Promi /** Wait until a worktree is active and return its ID. */ export async function waitForActiveWorktree(page: Page, timeoutMs = 30_000): Promise { - const existingId = await getActiveWorktreeId(page) - if (existingId) { - return existingId - } - - const activatedFromStore = await page.evaluate(() => { - const store = window.__store - if (!store) { - return false - } - - const state = store.getState() - if (state.activeWorktreeId) { - return true - } - - const firstWorktree = Object.values(state.worktreesByRepo).flat()[0] - if (!firstWorktree) { - return false - } - - // Why: the sidebar no longer guarantees a role="option" worktree row - // during hydration, so DOM-click fallback can miss the only selectable - // worktree and leave fresh E2E sessions stuck with activeWorktreeId=null. - // Activating the first loaded worktree through the store matches the app's - // real selection path and keeps setup independent from sidebar markup. - state.setActiveWorktree(firstWorktree.id) - return true - }) - - if (!activatedFromStore) { - const primaryWorktreeOption = page.getByRole('option', { name: /primary/i }).first() - const anyWorktreeOption = page.getByRole('option').first() - const optionToClick = - (await primaryWorktreeOption.count()) > 0 ? primaryWorktreeOption : anyWorktreeOption - - if ((await optionToClick.count()) > 0) { - // Why: isolated E2E sessions can finish hydrating with worktrees loaded but - // no selection restored. Clicking the sidebar option matches the real user - // path and drives the same activation logic the app relies on in production. - await optionToClick.click() - } - } - + let activeWorktreeId: string | null = null await expect - .poll(async () => getActiveWorktreeId(page), { - timeout: timeoutMs, - message: 'activeWorktreeId did not become available' - }) + .poll( + async () => { + activeWorktreeId = await page.evaluate(() => { + const store = window.__store + if (!store) { + return null + } + + let state = store.getState() + if (state.activeWorktreeId) { + return state.activeWorktreeId + } + + const firstWorktree = Object.values(state.worktreesByRepo).flat()[0] + if (!firstWorktree) { + return null + } + + // Why: isolated E2E sessions can hydrate worktree rows without + // restoring a selection. Re-try store activation as worktrees load + // instead of relying on sidebar option click hit targets. + state.setActiveWorktree(firstWorktree.id) + state = store.getState() + return state.activeWorktreeId + }) + return activeWorktreeId + }, + { + timeout: timeoutMs, + message: 'activeWorktreeId did not become available' + } + ) .not.toBeNull() - return (await getActiveWorktreeId(page))! + return activeWorktreeId! } /** Get all worktree IDs across all repos. */ @@ -277,26 +262,6 @@ export async function switchToWorktree(page: Page, worktreeId: string): Promise< * hidden-window mode and avoids racing that initial auto-create step. */ export async function ensureTerminalVisible(page: Page, timeoutMs = 10_000): Promise { - await page.evaluate(() => { - const store = window.__store - if (!store) { - return - } - - const state = store.getState() - if (state.activeWorktreeId) { - const tabs = state.tabsByWorktree[state.activeWorktreeId] ?? [] - if (tabs.length === 0) { - // Why: fresh isolated E2E profiles may not have finished the UI-driven - // auto-create effect yet. Use the same store action to create the first - // terminal tab so terminal-focused specs start from a stable baseline. - state.createTab(state.activeWorktreeId) - } - } - if (state.activeTabType !== 'terminal') { - state.setActiveTabType('terminal') - } - }) await expect .poll( async () => @@ -305,12 +270,41 @@ export async function ensureTerminalVisible(page: Page, timeoutMs = 10_000): Pro if (!store) { return false } - const state = store.getState() - if (state.activeTabType !== 'terminal' || !state.activeWorktreeId) { + let state = store.getState() + let worktreeId = state.activeWorktreeId + if (!worktreeId) { + const firstWorktree = Object.values(state.worktreesByRepo).flat()[0] + if (!firstWorktree) { + return false + } + // Why: reload-based specs can briefly clear the active worktree + // after session readiness while worktrees are already loaded. + state.setActiveWorktree(firstWorktree.id) + state = store.getState() + worktreeId = state.activeWorktreeId ?? firstWorktree.id + } + + const tabs = state.tabsByWorktree[worktreeId] ?? [] + const activeTab = + tabs.find((tab) => tab.id === state.activeTabIdByWorktree[worktreeId]) ?? + tabs.find((tab) => tab.id === state.activeTabId) ?? + tabs[0] ?? + // Why: fresh isolated E2E profiles may not have finished the UI-driven + // auto-create effect yet. Use the same store action to create the first + // terminal tab so terminal-focused specs start from a stable baseline. + state.createTab(worktreeId) + state.setActiveTab(activeTab.id) + if (state.activeTabType !== 'terminal') { + state.setActiveTabType('terminal') + } + + state = store.getState() + if (state.activeTabType !== 'terminal' || state.activeWorktreeId !== worktreeId) { return false } - const tabs = state.tabsByWorktree[state.activeWorktreeId] ?? [] - return tabs.some((tab) => tab.id === state.activeTabId) + return (state.tabsByWorktree[worktreeId] ?? []).some( + (tab) => tab.id === state.activeTabId + ) }), { timeout: timeoutMs, message: 'No active terminal tab found for current worktree' } ) diff --git a/tests/e2e/helpers/terminal.ts b/tests/e2e/helpers/terminal.ts index 6b075f674..c0444e64c 100644 --- a/tests/e2e/helpers/terminal.ts +++ b/tests/e2e/helpers/terminal.ts @@ -17,6 +17,11 @@ export type PaneIdentitySnapshot = { ptyIdsByLeafId: Record } +export type ActivePaneHookDescriptor = { + paneKey: string + worktreeId: string +} + // Why: worktree restoration can render the terminal surface before the legacy // global activeTabId settles. Prefer the active worktree's saved terminal tab // pointer, then fall back to the first terminal tab. @@ -121,6 +126,82 @@ export async function waitForActivePanePtyId(page: Page, timeoutMs = 15_000): Pr return ptyId } +export async function waitForActivePaneHookDescriptor( + page: Page, + timeoutMs = 15_000 +): Promise { + let descriptor: ActivePaneHookDescriptor | null = null + await expect + .poll( + async () => { + const tabId = await resolveActiveTabId(page) + if (!tabId) { + descriptor = null + return false + } + descriptor = await page.evaluate((tabId) => { + const layoutHasLeaf = (node: unknown, targetLeafId: string): boolean => { + if (!node || typeof node !== 'object') { + return false + } + const record = node as { + type?: unknown + leafId?: unknown + first?: unknown + second?: unknown + } + if (record.type === 'leaf') { + return record.leafId === targetLeafId + } + return ( + layoutHasLeaf(record.first, targetLeafId) || + layoutHasLeaf(record.second, targetLeafId) + ) + } + + const store = window.__store + const manager = window.__paneManagers?.get(tabId) + if (!store || !manager) { + return null + } + const state = store.getState() + const worktreeId = state.activeWorktreeId + if ( + !worktreeId || + !(state.tabsByWorktree[worktreeId] ?? []).some((tab) => tab.id === tabId) + ) { + return null + } + + const activePane = manager.getActivePane?.() ?? manager.getPanes?.()[0] + const leafId = activePane?.leafId ?? null + const layout = state.terminalLayoutsByTabId[tabId] + if ( + !leafId || + !layoutHasLeaf(layout?.root, leafId) || + layout?.ptyIdsByLeafId?.[leafId] !== activePane?.container?.dataset?.ptyId + ) { + return null + } + return { paneKey: `${tabId}:${leafId}`, worktreeId } + }, tabId) + return descriptor !== null + }, + { + timeout: timeoutMs, + // Why: hook IPC routing drops statuses for pane keys before the store + // layout knows that leaf, even if the terminal DOM already has a PTY. + message: 'Active terminal pane did not become routable for hook status IPC' + } + ) + .toBe(true) + + if (!descriptor) { + throw new Error('Active terminal pane descriptor disappeared after routing wait') + } + return descriptor +} + // Why: PTY IDs are opaque integers not exposed in the DOM. Probe each // candidate with a unique marker and read back via SerializeAddon. export async function discoverActivePtyId(page: Page): Promise { diff --git a/tests/e2e/setup-script-import.spec.ts b/tests/e2e/setup-script-import.spec.ts index b57ae59d8..0da990f6f 100644 --- a/tests/e2e/setup-script-import.spec.ts +++ b/tests/e2e/setup-script-import.spec.ts @@ -126,7 +126,11 @@ async function openRepoSettings(page: Page, repoId: string): Promise { } async function openImportedSetupSettingsFromToast(page: Page, repoId: string): Promise { - await page.getByRole('button', { name: 'View in Settings' }).click() + const viewInSettings = page.getByRole('button', { name: 'View in Settings' }) + await expect(viewInSettings).toBeAttached({ timeout: 10_000 }) + // Why: in hidden Electron CI windows, the Sonner action can be laid out just + // outside Playwright's viewport even though the action is mounted and wired. + await viewInSettings.evaluate((button) => (button as HTMLButtonElement).click()) const localCommands = page.locator(`[id="repo-${repoId}-local-commands"]`) await expect(localCommands).toBeVisible({ timeout: 10_000 }) await expect(localCommands.getByText('Local Settings Commands').first()).toBeVisible()