From 747a22ee57f35eb1970ac83ab6ff7f3f1d4e17b6 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 2 Sep 2026 18:09:53 -0700 Subject: [PATCH] fix(native-chat): guard duplicate launches and bound sync recovery --- .../TabBarCreateEntry.keyboard.test.tsx | 30 +++++++++++ .../components/tab-bar/TabBarCreateEntry.tsx | 3 ++ ...launch-agent-structured-chat-guard.test.ts | 29 ++++++++--- ...local-structured-session-tabs-sync.test.ts | 29 +++++++++++ .../local-structured-session-tabs-sync.ts | 52 ++++++++++++++++--- 5 files changed, 128 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx index 035cba0c0c1..88dd5ed32aa 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.keyboard.test.tsx @@ -18,6 +18,9 @@ import type { AppState } from '@/store/types' // Why: the real entry-action module pulls in runtime IPC + the app store; the // keyboard behavior under test only needs a controllable option list. const entryOptionsMock = vi.hoisted(() => ({ options: [] as TabEntryOption[] })) +const structuredLaunchMock = vi.hoisted(() => ({ + status: 'idle' as 'idle' | 'pending' | 'unknown' +})) vi.mock('./tab-create-entry-action', () => ({ getTabEntryOptions: () => entryOptionsMock.options, createTabEntryAllowAbsolutePathsSelector: () => () => true, @@ -35,6 +38,9 @@ vi.mock('@/lib/agent-catalog', () => ({ getAgentCatalog: () => [], AgentIcon: () => null })) +vi.mock('@/lib/structured-agent-session-launch', () => ({ + useStructuredCodexLaunchStatus: () => structuredLaunchMock.status +})) import TabBarCreateEntry from './TabBarCreateEntry' @@ -170,6 +176,7 @@ afterEach(() => { act(() => root.unmount()) container.remove() vi.clearAllMocks() + structuredLaunchMock.status = 'idle' }) describe('TabBarCreateEntry keyboard navigation', () => { @@ -260,6 +267,29 @@ describe('TabBarCreateEntry keyboard navigation', () => { expect(onLaunchAgent).toHaveBeenCalledWith('gemini') }) + it('does not relaunch Codex when a structured launch is already pending', () => { + structuredLaunchMock.status = 'pending' + const agentOptions: TabAgentLaunchOption[] = [ + { agent: 'codex', aliases: ['codex'], label: 'Codex' } + ] + const onLaunchAgent = vi.fn() + mount( + + ) + + setQuery('cod') + submitForm() + + expect(onLaunchAgent).not.toHaveBeenCalled() + }) + it('exposes the highlighted row to assistive tech via aria-activedescendant', () => { entryOptionsMock.options = [fileOption('a.ts'), fileOption('b.ts'), fileOption('c.ts')] mount( diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx index 744c48b3520..3aa1755de51 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx @@ -228,6 +228,9 @@ function TabBarCreateEntrySession({ return } if (selectedOption.kind === 'agent') { + if (selectedOption.option.agent === 'codex' && structuredCodexLaunchStatus === 'pending') { + return + } onLaunchAgent?.(selectedOption.option.agent) onDidOpenEntry?.() return diff --git a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts index 5e6df50e903..df042e6d324 100644 --- a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts +++ b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts @@ -49,9 +49,10 @@ const store = { detectedWorktreesByRepo: {}, allWorktrees: vi.fn(() => store.worktreesByRepo['repo-1']), tabsByWorktree: { 'wt-1': [{ id: 'tab-1' }] }, - unifiedTabsByWorktree: { - 'wt-1': [{ contentType: 'agent-session', entityId: 'codex-session-1', worktreeId: 'wt-1' }] - }, + unifiedTabsByWorktree: {} as Record< + string, + { contentType: string; entityId: string; worktreeId: string }[] + >, openFiles: [] as { id: string; worktreeId: string }[], browserTabsByWorktree: {} as Record, tabBarOrderByWorktree: {} as Record, @@ -105,6 +106,9 @@ vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({ describe('structured chat adoption guard on the launch path', () => { beforeEach(() => { vi.clearAllMocks() + store.unifiedTabsByWorktree = { + 'wt-1': [{ contentType: 'agent-session', entityId: 'codex-session-1', worktreeId: 'wt-1' }] + } store.repos = [{ id: 'repo-1', connectionId: null, path: '/repo' }] store.projects = [{ id: 'repo-1', localWindowsRuntimePreference: { kind: 'inherit-global' } }] mockCreateTab.mockReturnValue({ id: 'tab-1' }) @@ -199,6 +203,7 @@ describe('structured chat adoption guard on the launch path', () => { }) it('keeps the single-flight reservation until the published tab inventory is refreshed', async () => { + store.unifiedTabsByWorktree = {} let resolveRefresh!: (snapshots: unknown[]) => void mockRefreshLocalStructuredSessionTabs.mockImplementationOnce( () => new Promise((resolve) => (resolveRefresh = resolve)) @@ -212,6 +217,9 @@ describe('structured chat adoption guard on the launch path', () => { launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) expect(mockLaunchStructuredCodexSession).toHaveBeenCalledTimes(1) + store.unifiedTabsByWorktree['wt-1'] = [ + { contentType: 'agent-session', entityId: 'codex-session-1', worktreeId: 'wt-1' } + ] resolveRefresh([ { worktree: 'wt-1', tabs: [{ type: 'agent-session', sessionId: 'codex-session-1' }] } ]) @@ -219,6 +227,7 @@ describe('structured chat adoption guard on the launch path', () => { }) it('does not create a sibling when post-create visibility proof is unknown', async () => { + store.unifiedTabsByWorktree = {} const firstIntent = structuredLaunchIntent('wt-1', 'codex-session-1') const secondIntent = structuredLaunchIntent('wt-1', 'codex-session-2') mockCreateStructuredCodexSessionLaunchIntent @@ -231,9 +240,15 @@ describe('structured chat adoption guard on the launch path', () => { mockRefreshLocalStructuredSessionTabs .mockRejectedValueOnce(new Error('inventory unavailable')) .mockResolvedValueOnce([]) - .mockResolvedValueOnce([ - { worktree: 'wt-1', tabs: [{ type: 'agent-session', sessionId: 'codex-session-1' }] } - ]) + .mockImplementationOnce(() => { + // The inventory refresh also publishes the host snapshot into the renderer projection. + store.unifiedTabsByWorktree['wt-1'] = [ + { contentType: 'agent-session', entityId: firstIntent.sessionId, worktreeId: 'wt-1' } + ] + return Promise.resolve([ + { worktree: 'wt-1', tabs: [{ type: 'agent-session', sessionId: firstIntent.sessionId }] } + ]) + }) .mockResolvedValueOnce([ { worktree: 'wt-1', tabs: [{ type: 'agent-session', sessionId: 'codex-session-2' }] } ]) @@ -256,7 +271,7 @@ describe('structured chat adoption guard on the launch path', () => { { contentType: 'agent-session', entityId: secondIntent.sessionId, worktreeId: 'wt-1' } ] launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) - await vi.waitFor(() => expect(mockRefreshLocalStructuredSessionTabs).toHaveBeenCalledTimes(4)) + await vi.waitFor(() => expect(mockLaunchStructuredCodexSession).toHaveBeenCalledTimes(3)) expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledTimes(2) expect(mockLaunchStructuredCodexSession).toHaveBeenCalledTimes(3) expect(mockLaunchStructuredCodexSession.mock.calls[2]?.[0]).toBe(secondIntent) diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync.test.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync.test.ts index 1dc640e092f..909a00a5cc8 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it } from 'vitest' import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' import type { Tab } from '../../../shared/tab-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { WorktreeRuntimeOwnerState } from '../lib/worktree-runtime-owner' import { buildPersistedUnifiedTabSessionData } from '../lib/workspace-session-unified-tabs' import { buildHydratedTabState } from '../store/slices/tabs-hydration' import { @@ -212,6 +213,34 @@ describe('local structured session tab projection', () => { } }) + it('forgets publisher versions when a worktree is removed', () => { + type OwnerState = WebSessionTabsSyncState & WorktreeRuntimeOwnerState + const owner = { + id: WORKTREE_ID, + repoId: 'repo-1', + hostId: null, + runtimeOwnerEnvironmentId: null + } + let state = { + ...createSnapshot(), + worktreesByRepo: { 'repo-1': [owner] } + } as OwnerState + state = applyLocalStructuredSessionTabSnapshots(state, [ + structuredInventory('epoch-1', 10, 'session-old') + ]) + state = applyLocalStructuredSessionTabSnapshots( + { ...state, worktreesByRepo: {}, unifiedTabsByWorktree: {} }, + [] + ) + state = applyLocalStructuredSessionTabSnapshots( + { ...state, worktreesByRepo: { 'repo-1': [owner] } }, + [structuredInventory('epoch-1', 1, 'session-new')] + ) + expect(state.unifiedTabsByWorktree[WORKTREE_ID]).toEqual( + expect.arrayContaining([expect.objectContaining({ entityId: 'session-new' })]) + ) + }) + it('drops terminal topology while retaining structured tabs', () => { const snapshot = { worktree: 'workspace-1', diff --git a/src/renderer/src/runtime/local-structured-session-tabs-sync.ts b/src/renderer/src/runtime/local-structured-session-tabs-sync.ts index 50527ba51c7..75be14c847d 100644 --- a/src/renderer/src/runtime/local-structured-session-tabs-sync.ts +++ b/src/renderer/src/runtime/local-structured-session-tabs-sync.ts @@ -105,6 +105,24 @@ export function applyLocalStructuredSessionTabSnapshots< snapshotVersion: snapshot.snapshotVersion }) } + // Drop publisher cursors for worktrees that no longer exist. Without this, + // every deleted worktree leaves an entry for the lifetime of the renderer. + const knownWorktreeIds = new Set(Object.keys(next.unifiedTabsByWorktree)) + for (const worktrees of Object.values(next.worktreesByRepo ?? {})) { + for (const worktree of worktrees) { + knownWorktreeIds.add(worktree.id) + } + } + for (const detected of Object.values(next.detectedWorktreesByRepo ?? {})) { + for (const worktree of detected.worktrees) { + knownWorktreeIds.add(worktree.id) + } + } + for (const worktreeId of localStructuredSessionVersionByWorktree.keys()) { + if (!knownWorktreeIds.has(worktreeId)) { + localStructuredSessionVersionByWorktree.delete(worktreeId) + } + } return next } @@ -150,6 +168,7 @@ async function startLocalStructuredSessionTabsSync(args: { return } let subscriptionGeneration = 0 + let reconnectTimer: ReturnType | null = null const subscribeCurrent = async (): Promise => { if (args.isDisposed()) { return @@ -171,20 +190,37 @@ async function startLocalStructuredSessionTabsSync(args: { // Reattach with one refresh so a runtime-restart boundary cannot strand stale tabs. subscriptionGeneration += 1 handle?.unsubscribe() - void refreshLocalStructuredSessionTabs() - .catch((error) => console.warn('[structured-session-tabs] resync failed', error)) - .finally(() => { - if (!args.isDisposed()) { - void subscribeCurrent() - } - }) + if (reconnectTimer !== null) { + clearTimeout(reconnectTimer) + } + reconnectTimer = setTimeout(() => { + reconnectTimer = null + void refreshLocalStructuredSessionTabs() + .catch((error) => console.warn('[structured-session-tabs] resync failed', error)) + .finally(() => { + if (!args.isDisposed()) { + reconnectTimer = setTimeout(() => { + reconnectTimer = null + void subscribeCurrent().catch((error) => + console.warn('[structured-session-tabs] resubscribe failed', error) + ) + }, 250) + } + }) + }, 250) } } ) if (args.isDisposed() || generation !== subscriptionGeneration) { handle.unsubscribe() } else { - args.setUnsubscribe(handle.unsubscribe) + args.setUnsubscribe(() => { + if (reconnectTimer !== null) { + clearTimeout(reconnectTimer) + reconnectTimer = null + } + handle?.unsubscribe() + }) } } await subscribeCurrent()