From 2f1638f20532dec58ccbf66245795981fc958e4f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 12 Aug 2026 03:51:18 -0400 Subject: [PATCH] fix(remote): reconcile browser create ordering --- .../src/runtime/web-runtime-session.test.ts | 65 ++++++++- .../src/runtime/web-runtime-session.ts | 24 +++- .../src/runtime/web-session-tabs-sync.test.ts | 129 ++++++++++++++++++ 3 files changed, 216 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/runtime/web-runtime-session.test.ts b/src/renderer/src/runtime/web-runtime-session.test.ts index eb20243e1ff..576515e74ed 100644 --- a/src/renderer/src/runtime/web-runtime-session.test.ts +++ b/src/renderer/src/runtime/web-runtime-session.test.ts @@ -42,6 +42,7 @@ import { const mocks = vi.hoisted(() => ({ getState: vi.fn(), setState: vi.fn(), + subscribe: vi.fn(), setActiveWorktree: vi.fn(), createBrowserTab: vi.fn(), closeEmptyGroup: vi.fn(), @@ -60,7 +61,8 @@ const mocks = vi.hoisted(() => ({ vi.mock('../store', () => ({ useAppStore: { getState: mocks.getState, - setState: mocks.setState + setState: mocks.setState, + subscribe: mocks.subscribe } })) @@ -283,6 +285,7 @@ describe('createWebRuntimeSessionBrowserTab', () => { activeWorktreeId: WORKTREE_ID }) }) + mocks.subscribe.mockReturnValue(vi.fn()) mocks.createBrowserTab.mockReturnValue({ id: 'local-browser-workspace-1', activePageId: 'local-page-1', @@ -362,6 +365,13 @@ describe('createWebRuntimeSessionBrowserTab', () => { snapshot, ENVIRONMENT_ID ) + expect(mocks.acceptReplayedWebSessionTabsSnapshot).toHaveBeenCalledWith( + ENVIRONMENT_ID, + WORKTREE_ID + ) + expect(mocks.acceptReplayedWebSessionTabsSnapshot.mock.invocationCallOrder[0]).toBeLessThan( + mocks.applyFreshWebSessionTabsSnapshot.mock.invocationCallOrder[0]! + ) expect(peekWebSessionFocusIntent({ environmentId: ENVIRONMENT_ID }, WORKTREE_ID)).toEqual({ hostTabId: 'remote-browser-page-1', expectedCurrentLocalTabId: 'local-editor-tab' @@ -408,6 +418,59 @@ describe('createWebRuntimeSessionBrowserTab', () => { }) }) + it('does not focus a slow browser create after an editor A-B-A selection', async () => { + let resolveCreate!: (response: unknown) => void + const createResponse = new Promise((resolve) => { + resolveCreate = resolve + }) + let state = mocks.getState() + let listener: ((next: typeof state, previous: typeof state) => void) | null = null + mocks.getState.mockImplementation(() => state) + mocks.subscribe.mockImplementation((nextListener) => { + listener = nextListener + return vi.fn() + }) + const runtimeCall = vi.fn(async (request: { method: string }) => { + if (request.method === 'browser.tabCreate') { + return createResponse + } + return { id: 'list', ok: true, result: makeSnapshot() } + }) + vi.stubGlobal('window', { api: { runtimeEnvironments: { call: runtimeCall } } }) + + const pendingCreate = createWebRuntimeSessionBrowserTab({ worktreeId: WORKTREE_ID }) + await vi.waitFor(() => expect(listener).not.toBeNull()) + const editorBState = { + ...state, + activeFileIdByWorktree: { [WORKTREE_ID]: '/worktree/other.html' }, + unifiedTabsByWorktree: { + [WORKTREE_ID]: [ + ...state.unifiedTabsByWorktree[WORKTREE_ID], + { + id: 'other-editor-tab', + entityId: '/worktree/other.html', + contentType: 'editor' + } + ] + } + } + listener!(editorBState, state) + const editorAState = { + ...editorBState, + activeFileIdByWorktree: { [WORKTREE_ID]: '/worktree/index.html' } + } + listener!(editorAState, editorBState) + state = editorAState + resolveCreate({ + id: 'create', + ok: true, + result: { browserPageId: 'created-host-browser' } + }) + await expect(pendingCreate).resolves.toBe(true) + + expect(peekWebSessionFocusIntent({ environmentId: ENVIRONMENT_ID }, WORKTREE_ID)).toBeNull() + }) + it('does not retry a failed browser create', async () => { const runtimeCall = vi.fn().mockResolvedValue({ id: 'create-lost', diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index ee04e2ff7d7..12f5e062c9c 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -467,6 +467,7 @@ export async function createWebRuntimeSessionBrowserTab(args: { if (shouldSelectWorktree) { selectWebRuntimeSessionBrowserWorktree(args.worktreeId, environmentId) } + let unsubscribeFocusGuard = (): void => {} try { if (matchesWebSessionIntentOwner(intentOwner)) { recordWebSessionFocusIntent( @@ -476,6 +477,24 @@ export async function createWebRuntimeSessionBrowserTab(args: { undefined, expectedCurrentLocalTabId ) + unsubscribeFocusGuard = useAppStore.subscribe((state, previousState) => { + if ( + state.activeBrowserTabIdByWorktree === previousState.activeBrowserTabIdByWorktree && + state.activeFileIdByWorktree === previousState.activeFileIdByWorktree && + state.activeTabIdByWorktree === previousState.activeTabIdByWorktree && + state.activeTabType === previousState.activeTabType && + state.activeTabTypeByWorktree === previousState.activeTabTypeByWorktree && + state.activeWorktreeId === previousState.activeWorktreeId && + state.unifiedTabsByWorktree === previousState.unifiedTabsByWorktree + ) { + return + } + if (resolveWebSessionVisibleTabId(state, args.worktreeId) === expectedCurrentLocalTabId) { + return + } + clearWebSessionFocusIntentIfMatches(intentOwner, args.worktreeId, provisionalPageId) + unsubscribeFocusGuard() + }) } const created = unwrapRuntimeRpcResult( (await callEnvironment({ @@ -494,6 +513,7 @@ export async function createWebRuntimeSessionBrowserTab(args: { timeoutMs: 15_000 })) as RuntimeRpcResponse ) + unsubscribeFocusGuard() if (created.browserPageId !== provisionalPageId) { moveWebSessionBrowserPlacement({ environmentId, @@ -514,7 +534,8 @@ export async function createWebRuntimeSessionBrowserTab(args: { } try { await refreshWebRuntimeSessionTabsSnapshot(environmentId, args.worktreeId, { - expectedEnvironmentPairingRevision: intentOwner.pairingRevision + expectedEnvironmentPairingRevision: intentOwner.pairingRevision, + acceptCurrentSnapshot: true }) } catch (error) { console.warn( @@ -524,6 +545,7 @@ export async function createWebRuntimeSessionBrowserTab(args: { } return true } catch (error) { + unsubscribeFocusGuard() forgetWebSessionBrowserPlacement({ environmentId, worktreeId: args.worktreeId, diff --git a/src/renderer/src/runtime/web-session-tabs-sync.test.ts b/src/renderer/src/runtime/web-session-tabs-sync.test.ts index ba8e00758f8..e830e027208 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.test.ts @@ -34,6 +34,7 @@ import { resetWebAgentSessionHandoffsForTests } from './web-agent-session-handoff' import { + moveWebSessionBrowserPlacement, recordWebSessionBrowserPlacement, takeWebSessionBrowserPlacementGroup } from './web-session-browser-placement' @@ -4206,6 +4207,134 @@ describe('applyWebSessionTabsSnapshot', () => { expect(patch.browserTabsByWorktree?.[WT]).toHaveLength(1) }) + it('replays a pre-response browser snapshot after canonical placement and focus are known', () => { + const editorGroupId = 'client-editor-group' + const previewGroupId = 'client-preview-group' + const editorFileId = '/srv/repo/example.html' + const editorTab: Tab = { + id: 'host-editor', + worktreeId: WT, + groupId: editorGroupId, + contentType: 'editor', + entityId: editorFileId, + label: 'example.html', + sortOrder: 0, + createdAt: NOW, + isPreview: false, + isPinned: false, + customLabel: null, + color: null + } + const state = makeState({ + activeFileId: editorFileId, + activeFileIdByWorktree: { [WT]: editorFileId }, + activeTabType: 'editor', + activeTabTypeByWorktree: { [WT]: 'editor' }, + activeGroupIdByWorktree: { [WT]: editorGroupId }, + groupsByWorktree: { + [WT]: [ + { + id: editorGroupId, + worktreeId: WT, + activeTabId: editorTab.id, + tabOrder: [editorTab.id] + }, + { + id: previewGroupId, + worktreeId: WT, + activeTabId: null, + tabOrder: [] + } + ] + }, + layoutByWorktree: { + [WT]: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: editorGroupId }, + second: { type: 'leaf', groupId: previewGroupId }, + ratio: 0.5 + } + }, + unifiedTabsByWorktree: { [WT]: [editorTab] } + }) + const snapshot = makeSnapshot( + [ + { + type: 'file', + id: 'host-editor', + title: 'example.html', + filePath: editorFileId, + relativePath: 'example.html', + language: 'html', + isDirty: false, + isActive: false + }, + { + type: 'browser', + id: 'host-browser-tab', + title: 'example.html', + browserWorkspaceId: 'host-browser-workspace', + browserPageId: 'host-browser-page', + url: 'file:///srv/repo/example.html', + loading: false, + canGoBack: false, + canGoForward: false, + isActive: true + } + ], + { activeTabId: 'host-browser-tab', activeTabType: 'browser' } + ) + recordWebSessionBrowserPlacement({ + environmentId: ENV, + worktreeId: WT, + remotePageId: 'provisional-browser-page', + groupId: previewGroupId + }) + + const subscriptionPatch = applyFreshWebSessionTabsSnapshot(state, snapshot, ENV, NOW) + const afterSubscription = { + ...state, + ...(subscriptionPatch as Partial) + } + expect(afterSubscription.activeTabType).toBe('editor') + expect( + afterSubscription.unifiedTabsByWorktree[WT]?.find((tab) => tab.contentType === 'browser') + ?.groupId + ).not.toBe(previewGroupId) + + moveWebSessionBrowserPlacement({ + environmentId: ENV, + worktreeId: WT, + fromRemotePageId: 'provisional-browser-page', + toRemotePageId: 'host-browser-page' + }) + recordWebSessionFocusIntent( + { environmentId: ENV }, + WT, + 'host-browser-page', + undefined, + editorTab.id + ) + acceptReplayedWebSessionTabsSnapshot(ENV, WT) + const replayPatch = applyFreshWebSessionTabsSnapshot( + afterSubscription, + snapshot, + ENV, + NOW + 1 + ) as Partial + const afterReplay = { ...afterSubscription, ...replayPatch } + + expect( + afterReplay.unifiedTabsByWorktree[WT]?.find((tab) => tab.contentType === 'browser') + ).toMatchObject({ id: 'host-browser-tab', groupId: previewGroupId }) + expect( + afterReplay.groupsByWorktree[WT]?.find((group) => group.id === previewGroupId) + ).toMatchObject({ activeTabId: 'host-browser-tab', tabOrder: ['host-browser-tab'] }) + expect(afterReplay.activeBrowserTabIdByWorktree[WT]).toBe('host-browser-workspace') + expect(afterReplay.activeTabTypeByWorktree[WT]).toBe('browser') + }) + it('keeps a reserved side-preview split across a pre-publication snapshot', () => { const editorGroupId = 'client-editor-group' const previewGroupId = 'client-preview-group'