fix: clear stale browser guest registrations (#4201)

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 ──

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)
     }
 

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<string, number>
+      workspaceIdByPageId: Map<string, string>
+      sessionProfileIdByPageId: Map<string, string | null>
+      worktreeIdByTabId: Map<string, string>
+      contextMenuCleanupByTabId: Map<string, () => void>
+      grabShortcutCleanupByTabId: Map<string, () => void>
+      shortcutForwardingCleanupByTabId: Map<string, () => 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 = {

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