diff --git a/src/main/browser/agent-browser-bridge.test.ts b/src/main/browser/agent-browser-bridge.test.ts index ee55fb1dd3c..7c752588a4b 100644 --- a/src/main/browser/agent-browser-bridge.test.ts +++ b/src/main/browser/agent-browser-bridge.test.ts @@ -69,6 +69,7 @@ function mockBrowserManager( getWebContentsIdByTabId: () => tabs, getWorktreeIdForTab: (tabId: string) => worktrees.get(tabId), getGuestWebContentsId: vi.fn(() => null), + unregisterGuest: vi.fn(), ensureWebviewVisible: vi.fn(async () => () => {}), acquireAutomationVisibility: vi.fn(async () => () => {}), ...overrides @@ -377,6 +378,21 @@ describe('AgentBrowserBridge', () => { ]) expect(b.getActiveWebContentsId()).toBeNull() }) + + it('unregisters stale tab-list entries when their WebContents is gone', () => { + const tabs = new Map([ + ['tab-a', 1], + ['tab-b', 2] + ]) + const wc2 = mockWebContents(2, 'https://b.com', 'B') + webContentsFromIdMock.mockImplementation((id: number) => (id === 2 ? wc2 : null)) + const unregisterGuest = vi.fn() + + const b = new AgentBrowserBridge(mockBrowserManager(tabs, new Map(), { unregisterGuest })) + + expect(b.tabList().tabs).toMatchObject([{ browserPageId: 'tab-b', active: true }]) + expect(unregisterGuest).toHaveBeenCalledWith('tab-a') + }) }) // ── Tab switch ── diff --git a/src/main/browser/agent-browser-bridge.ts b/src/main/browser/agent-browser-bridge.ts index d234d7563e2..d209b54f366 100644 --- a/src/main/browser/agent-browser-bridge.ts +++ b/src/main/browser/agent-browser-bridge.ts @@ -584,6 +584,7 @@ export class AgentBrowserBridge { for (const [tabId, wcId] of tabs) { const wc = this.getWebContents(wcId) if (!wc) { + this.browserManager.unregisterGuest(tabId) continue } if (firstLiveWcId === null) { @@ -1849,6 +1850,7 @@ export class AgentBrowserBridge { } if (!this.getWebContents(webContentsId)) { + this.browserManager.unregisterGuest(browserPageId) throw new BrowserError( 'browser_tab_not_found', `Browser page ${browserPageId} is no longer available` @@ -1875,6 +1877,15 @@ export class AgentBrowserBridge { if (wcId === preferredWcId && this.getWebContents(wcId)) { return { browserPageId: tabId, webContentsId: wcId } } + if (wcId === preferredWcId) { + this.browserManager.unregisterGuest(tabId) + if (this.activeWebContentsId === wcId) { + this.activeWebContentsId = null + } + if (worktreeId && this.activeWebContentsPerWorktree.get(worktreeId) === wcId) { + this.activeWebContentsPerWorktree.delete(worktreeId) + } + } } } @@ -1890,6 +1901,7 @@ export class AgentBrowserBridge { } return { browserPageId: tabId, webContentsId: wcId } } + this.browserManager.unregisterGuest(tabId) } throw new BrowserError( @@ -2044,6 +2056,7 @@ export class AgentBrowserBridge { // could be destroyed. Check here to give a clear error instead of letting the // proxy fail with cryptic Electron debugger errors. if (!this.getWebContents(session.webContentsId)) { + await this.destroySession(sessionName) throw this.createPageUnavailableError(sessionName) } diff --git a/src/main/browser/browser-manager.test.ts b/src/main/browser/browser-manager.test.ts index 93f2c1efbed..44be73b694c 100644 --- a/src/main/browser/browser-manager.test.ts +++ b/src/main/browser/browser-manager.test.ts @@ -849,6 +849,61 @@ describe('browserManager', () => { expect(browserManager.getGuestWebContentsId('browser-destroyed-before-register')).toBeNull() }) + it('fully unregisters stale guests discovered during authorization', () => { + const guest = { + id: 305, + isDestroyed: vi.fn(() => false), + getType: vi.fn(() => 'webview'), + setBackgroundThrottling: guestSetBackgroundThrottlingMock, + setWindowOpenHandler: guestSetWindowOpenHandlerMock, + on: guestOnMock, + off: guestOffMock, + openDevTools: guestOpenDevToolsMock + } + webContentsFromIdMock.mockReturnValue(guest) + + browserManager.attachGuestPolicies(guest as never) + browserManager.registerGuest({ + browserPageId: 'browser-stale', + workspaceId: 'workspace-stale', + worktreeId: 'worktree-stale', + sessionProfileId: 'profile-stale', + webContentsId: guest.id, + rendererWebContentsId + }) + + const internals = browserManager as unknown as { + rendererWebContentsIdByTabId: Map + workspaceIdByPageId: Map + sessionProfileIdByPageId: Map + worktreeIdByTabId: Map + contextMenuCleanupByTabId: Map void> + grabShortcutCleanupByTabId: Map void> + shortcutForwardingCleanupByTabId: Map void> + } + expect(internals.rendererWebContentsIdByTabId.has('browser-stale')).toBe(true) + expect(internals.workspaceIdByPageId.has('browser-stale')).toBe(true) + expect(internals.sessionProfileIdByPageId.has('browser-stale')).toBe(true) + expect(internals.worktreeIdByTabId.has('browser-stale')).toBe(true) + expect(internals.contextMenuCleanupByTabId.has('browser-stale')).toBe(true) + expect(internals.grabShortcutCleanupByTabId.has('browser-stale')).toBe(true) + expect(internals.shortcutForwardingCleanupByTabId.has('browser-stale')).toBe(true) + + webContentsFromIdMock.mockReturnValue(null) + + expect(browserManager.getAuthorizedGuest('browser-stale', rendererWebContentsId)).toBeNull() + + expect(browserManager.getGuestWebContentsId('browser-stale')).toBeNull() + expect(internals.rendererWebContentsIdByTabId.has('browser-stale')).toBe(false) + expect(internals.workspaceIdByPageId.has('browser-stale')).toBe(false) + expect(internals.sessionProfileIdByPageId.has('browser-stale')).toBe(false) + expect(internals.worktreeIdByTabId.has('browser-stale')).toBe(false) + expect(internals.contextMenuCleanupByTabId.has('browser-stale')).toBe(false) + expect(internals.grabShortcutCleanupByTabId.has('browser-stale')).toBe(false) + expect(internals.shortcutForwardingCleanupByTabId.has('browser-stale')).toBe(false) + expect(guestOffMock).toHaveBeenCalled() + }) + it('replays a queued main-frame load failure after the guest registers', () => { const rendererSendMock = vi.fn() const guest = { diff --git a/src/main/browser/browser-manager.ts b/src/main/browser/browser-manager.ts index 6e3b392629e..edd2b3ccf3c 100644 --- a/src/main/browser/browser-manager.ts +++ b/src/main/browser/browser-manager.ts @@ -1044,8 +1044,9 @@ export class BrowserManager { } const guest = webContents.fromId(webContentsId) if (!guest || guest.isDestroyed()) { - this.webContentsIdByTabId.delete(browserTabId) - this.tabIdByWebContentsId.delete(webContentsId) + // Why: stale guest discovery must clear every per-tab registry entry, + // not just the forward/reverse WebContents maps. + this.unregisterGuest(browserTabId) return false } guest.openDevTools({ mode: 'detach' }) @@ -1111,8 +1112,9 @@ export class BrowserManager { } const guest = webContents.fromId(webContentsId) if (!guest || guest.isDestroyed()) { - this.webContentsIdByTabId.delete(browserTabId) - this.tabIdByWebContentsId.delete(webContentsId) + // Why: stale guest discovery must clear every per-tab registry entry, + // not just the forward/reverse WebContents maps. + this.unregisterGuest(browserTabId) return false } @@ -1140,8 +1142,9 @@ export class BrowserManager { } const guest = webContents.fromId(webContentsId) if (!guest || guest.isDestroyed()) { - this.webContentsIdByTabId.delete(browserTabId) - this.tabIdByWebContentsId.delete(webContentsId) + // Why: stale guest discovery must clear every per-tab registry entry, + // not just the forward/reverse WebContents maps. + this.unregisterGuest(browserTabId) return false } @@ -1247,8 +1250,9 @@ export class BrowserManager { } const guest = webContents.fromId(guestId) if (!guest || guest.isDestroyed()) { - this.webContentsIdByTabId.delete(browserTabId) - this.tabIdByWebContentsId.delete(guestId) + // Why: stale guest discovery must clear every per-tab registry entry, + // not just the forward/reverse WebContents maps. + this.unregisterGuest(browserTabId) return null } return guest