fixes

Author: Sg312

apps/desktop/src/main/browser-agent/panel.test.ts

@@ -119,14 +119,157 @@ describe('panel chat scope', () => {
     })
   })
 
-  it('does not hide the native page behind a replacement captured at stale bounds', async () => {
-    const { win } = showPanel(panel)
+  it('force-hides the native page when its replacement was captured at stale bounds', async () => {
+    const { win, view } = showPanel(panel)
     const scopeId = panel.getActivePanelScopeId()
 
     await expect(panel.capturePanelSnapshot(win, scopeId)).resolves.not.toBeNull()
     panel.setPanelBounds({ x: 399, y: 64, width: 601, height: 786 }, win)
+    vi.mocked(view.setVisible).mockClear()
+
+    expect(panel.setPanelOccluded(true, win, scopeId)).toBe(false)
+    expect(view.setVisible).not.toHaveBeenCalled()
+
+    expect(panel.setPanelOccluded(true, win, scopeId, true)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+  })
+
+  it('force-hides the native page without a captured replacement frame', () => {
+    const { win, view } = showPanel(panel)
+    const scopeId = panel.getActivePanelScopeId()
+    vi.mocked(view.setVisible).mockClear()
 
     expect(panel.setPanelOccluded(true, win, scopeId)).toBe(false)
+    expect(view.setVisible).not.toHaveBeenCalled()
+
+    expect(panel.setPanelOccluded(true, win, scopeId, true)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+  })
+
+  it('applies a forced hide before the panel reports its first bounds', () => {
+    const win = new BrowserWindow()
+    const view = new WebContentsView()
+    const active = { id: 'tab-1', scopeId: 'chat-test', view, pinned: false }
+    panel.initPanel({
+      getMainWindow: () => win,
+      activeTab: () => active,
+      backgroundColor: () => '#0c0c0c',
+      ensureInitialTab: () => {},
+      onViewDetached: () => {},
+    })
+    panel.activatePanelScope('chat-test')
+
+    expect(panel.setPanelOccluded(true, win, 'chat-test')).toBe(false)
+    expect(panel.setPanelOccluded(true, win, 'chat-test', true)).toBe(true)
+
+    panel.setPanelBounds(PANEL_RECT, win)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+  })
+
+  it('transfers a hidden lease to a focused window before its bounds arrive', () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    const scopeId = panel.getActivePanelScopeId()
+    vi.mocked(other.isFocused).mockReturnValue(true)
+    vi.mocked(view.setVisible).mockClear()
+
+    expect(panel.setPanelOccluded(true, other, scopeId)).toBe(false)
+    expect(panel.setPanelOccluded(true, other, scopeId, true)).toBe(true)
+    expect(win.contentView.removeChildView).toHaveBeenCalledWith(view)
+
+    panel.setPanelBounds(PANEL_RECT, other)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+    expect(panel.setPanelOccluded(false, other, scopeId)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(true)
+    expect(panel.setPanelOccluded(false, win, scopeId)).toBe(true)
+  })
+
+  it('acknowledges forced occlusion in an unfocused window that has no local native view', () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    const scopeId = panel.getActivePanelScopeId()
+    vi.mocked(other.isFocused).mockReturnValue(false)
+    vi.mocked(view.setVisible).mockClear()
+
+    expect(panel.setPanelOccluded(true, other, scopeId, true)).toBe(true)
+    expect(view.setVisible).not.toHaveBeenCalled()
+    expect(panel.setPanelOccluded(false, other, scopeId)).toBe(true)
+    expect(view.setVisible).not.toHaveBeenCalled()
+    expect(panel.setPanelOccluded(false, win, scopeId)).toBe(true)
+  })
+
+  it('acknowledges cross-scope modal leases only for a window without the singleton view', () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    vi.mocked(other.isFocused).mockReturnValue(false)
+    vi.mocked(view.setVisible).mockClear()
+
+    expect(panel.setPanelOccluded(true, other, 'chat-in-other-window', true)).toBe(true)
+    expect(panel.setPanelOccluded(false, other, 'chat-in-other-window')).toBe(true)
+    expect(view.setVisible).not.toHaveBeenCalled()
+
+    vi.mocked(other.isFocused).mockReturnValue(true)
+    expect(panel.setPanelOccluded(true, other, 'focused-other-chat', true)).toBe(false)
+
+    // A stale scope from the real owner must never mutate or falsely
+    // acknowledge the currently hosted native surface.
+    expect(panel.setPanelOccluded(true, win, 'stale-owner-chat', true)).toBe(false)
+    expect(panel.setPanelOccluded(false, win, 'stale-owner-chat')).toBe(false)
+    expect(view.setVisible).not.toHaveBeenCalled()
+  })
+
+  it('releases the old window occlusion lease when panel ownership moves', async () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    const scopeId = panel.getActivePanelScopeId()
+
+    await expect(panel.capturePanelSnapshot(win, scopeId)).resolves.not.toBeNull()
+    expect(panel.setPanelOccluded(true, win, scopeId)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+
+    panel.setPanelBounds(PANEL_RECT, other)
+    expect(view.setVisible).toHaveBeenLastCalledWith(true)
+    // The displaced renderer can retire its stale local snapshot without
+    // changing the new owner's already-visible native view.
+    expect(panel.setPanelOccluded(false, win, scopeId)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(true)
+  })
+
+  it('lets a displaced window retire its snapshot without revealing the current modal lease', async () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    const scopeId = panel.getActivePanelScopeId()
+
+    await expect(panel.capturePanelSnapshot(win, scopeId)).resolves.not.toBeNull()
+    expect(panel.setPanelOccluded(true, win, scopeId)).toBe(true)
+
+    vi.mocked(other.isFocused).mockReturnValue(true)
+    expect(panel.setPanelOccluded(true, other, scopeId, true)).toBe(true)
+    panel.setPanelBounds(PANEL_RECT, other)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+
+    expect(panel.setPanelOccluded(false, win, scopeId)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+    expect(panel.setPanelOccluded(false, other, scopeId)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(true)
+  })
+
+  it('lets the prior scope retire after a focused window establishes the next hidden lease', () => {
+    const { win, view } = showPanel(panel)
+    const other = new BrowserWindow()
+    const previousScope = panel.getActivePanelScopeId()
+    const nextScope = 'chat-in-focused-window'
+    vi.mocked(other.isFocused).mockReturnValue(true)
+
+    panel.activatePanelScope(nextScope)
+    expect(panel.setPanelOccluded(true, other, nextScope, true)).toBe(true)
+    panel.setPanelBounds(PANEL_RECT, other, undefined, nextScope)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+
+    expect(panel.setPanelOccluded(false, win, previousScope)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(false)
+    expect(panel.setPanelOccluded(false, other, nextScope)).toBe(true)
+    expect(view.setVisible).toHaveBeenLastCalledWith(true)
   })
 
   it('treats an unpainted blank tab as a valid backdrop snapshot', async () => {
@@ -152,6 +295,7 @@ describe('panel chat scope', () => {
     vi.mocked(view.setVisible).mockClear()
 
     expect(panel.setPanelOccluded(true, win, 'some-other-chat')).toBe(false)
+    expect(panel.setPanelOccluded(true, win, 'some-other-chat', true)).toBe(false)
     expect(view.setVisible).not.toHaveBeenCalled()
   })
 })

apps/desktop/src/main/browser-agent/panel.ts

@@ -62,6 +62,8 @@ let panelBounds: BrowserPanelBounds | null = null
 let panelAnchor: BrowserPanelAnchor | null = null
 /** True only after a replacement frame has painted in Sim's renderer. */
 let panelOccluded = false
+/** Window whose renderer currently owns the native-surface replacement lease. */
+let occlusionOwnerWindow: BrowserWindow | null = null
 /** Invalidates captures when ownership, scope, or panel visibility changes. */
 let panelCaptureGeneration = 0
 let panelLeaseAt = 0
@@ -293,6 +295,7 @@ function detachAttachedView(): void {
 /** Reveals the native view and invalidates every frame captured for its old state. */
 function resetOcclusion(): void {
   panelOccluded = false
+  occlusionOwnerWindow = null
   occludableFrame = null
   panelCaptureGeneration++
 }
@@ -553,16 +556,108 @@ export async function capturePanelSnapshot(
 export function setPanelOccluded(
   occluded: boolean,
   ownerWindow?: BrowserWindow,
-  scopeId = activePanelScopeId
+  scopeId = activePanelScopeId,
+  force = false
 ): boolean {
-  if (!panelUpdateAllowed(ownerWindow, scopeId)) return false
-  if (occluded && (panelBounds === null || host.activeTab() === null)) return false
-  if (panelOccluded === occluded) return true
-  if (occluded) {
+  if (!scopeId) return false
+
+  if (scopeId !== activePanelScopeId) {
+    const currentOwner = occlusionOwnerWindow ?? panelOwner() ?? host.getMainWindow()
+    const requesterOwnsNativeSurface =
+      !ownerWindow || currentOwner === null || ownerWindow === currentOwner
+
+    // Every app window has its own renderer modal state, but the Browser is a
+    // singleton native surface hosted by only one of them. A background
+    // renderer on another chat therefore has nothing local to hide or reveal.
+    // Acknowledge its forced modal lease as a scoped no-op so its strict
+    // pre-paint gate can proceed, while still rejecting stale requests from
+    // the window that actually owns the native surface.
+    if (!requesterOwnsNativeSurface && !occluded) return true
+    if (
+      !requesterOwnsNativeSurface &&
+      force &&
+      ownerWindow &&
+      !ownerWindow.isDestroyed() &&
+      !ownerWindow.isFocused()
+    ) {
+      return true
+    }
+    return false
+  }
+
+  // Once ownership has transferred, an old renderer still needs to retire its
+  // local replacement when its modal closes. The old lease was released by
+  // the transfer, so revealing an already-visible panel is a scoped no-op.
+  if (!occluded && !panelOccluded) return true
+  // Another focused window may have replaced this renderer's lease with its
+  // own modal lease. Retiring the displaced renderer's local snapshot is also
+  // a no-op: it must not reveal the CURRENT owner's still-occluded view.
+  if (!occluded && ownerWindow && occlusionOwnerWindow && ownerWindow !== occlusionOwnerWindow) {
+    return true
+  }
+
+  const panelAllowed = panelUpdateAllowed(ownerWindow, scopeId)
+  const focusedForceTransfer =
+    occluded &&
+    force &&
+    !panelAllowed &&
+    Boolean(ownerWindow && !ownerWindow.isDestroyed() && ownerWindow.isFocused())
+  const forceWithoutLocalSurface =
+    occluded &&
+    force &&
+    !panelAllowed &&
+    Boolean(ownerWindow && !ownerWindow.isDestroyed() && !ownerWindow.isFocused())
+  // An unfocused non-owner window has no native view in its compositor. It can
+  // safely open its own renderer modal without mutating the focused/owning
+  // window's lease. If it later gains focus while the marker remains, the
+  // bounds-report guard establishes a real hidden lease before transfer.
+  if (forceWithoutLocalSurface) return true
+  if (!panelAllowed && !focusedForceTransfer) return false
+
+  // A focused second window can open a modal before its next rAF reports new
+  // panel bounds. Transfer a HIDDEN, bounds-less lease atomically: the old
+  // window stops painting now and the new window's first bounds attach the
+  // singleton already hidden. Clearing the old rect also prevents a reveal at
+  // another window's geometry if the modal closes unusually quickly.
+  if (focusedForceTransfer && ownerWindow) {
+    panelOwnerWindow = ownerWindow
+    panelBounds = null
+    panelAnchor = null
+    panelLeaseAt = 0
+    panelOccluded = true
+    occlusionOwnerWindow = ownerWindow
+    occludableFrame = null
+    panelCaptureGeneration++
+    layout()
+    return true
+  }
+
+  if (!occluded) {
+    if (ownerWindow && occlusionOwnerWindow && ownerWindow !== occlusionOwnerWindow) return false
+    panelOccluded = false
+    occlusionOwnerWindow = null
+    occludableFrame = null
     layout()
-    if (!occludableFrame || !frameGeometryIsCurrent(occludableFrame)) return false
+    return true
+  }
+
+  // A pre-paint modal handshake can arrive one React commit before the panel
+  // reports its first bounds. A forced lease must still stick in that state so
+  // a view attached later in the same frame starts hidden, rather than briefly
+  // punching through the already-visible renderer effect.
+  if (occluded && (panelBounds === null || host.activeTab() === null) && !force) return false
+  if (panelOccluded) {
+    return !ownerWindow || !occlusionOwnerWindow || ownerWindow === occlusionOwnerWindow
   }
-  panelOccluded = occluded
+  layout()
+  // The lossless frame is the normal path. A full-screen renderer effect
+  // may explicitly force the final fallback after capture/geometry retries:
+  // a temporarily blank/blurred host is preferable to a native rectangle
+  // punching above a modal or global takeover. Ordinary popovers never force
+  // this path because they must remain pixel-neutral.
+  if ((!occludableFrame || !frameGeometryIsCurrent(occludableFrame)) && !force) return false
+  panelOccluded = true
+  occlusionOwnerWindow = ownerWindow ?? panelWindow()
   occludableFrame = null
   layout()
   return true
@@ -590,7 +685,12 @@ export function setPanelBounds(
   // must not pull the browser out from under the window displaying it.
   if (bounds === null && !panelUpdateAllowed(ownerWindow)) return
   if (bounds !== null) {
-    panelOwnerWindow = ownerWindow ?? host.getMainWindow()
+    const nextOwner = ownerWindow ?? host.getMainWindow()
+    // Occlusion belongs to a renderer window, not to the mutable singleton.
+    // Moving the native view to another window releases the previous window's
+    // lease; the new owner must establish its own if it also has a modal.
+    if (occlusionOwnerWindow && nextOwner !== occlusionOwnerWindow) resetOcclusion()
+    panelOwnerWindow = nextOwner
   } else {
     panelOwnerWindow = null
   }

apps/desktop/src/main/browser-agent/session.test.ts

@@ -565,6 +565,23 @@ describe('browser-agent session', () => {
     expect(restoredTab?.pendingRestoreUrl).toBe('https://retained.example/')
   })
 
+  it('closes live pages even when the suspend descriptor cannot be saved', () => {
+    const { persistence } = memoryBrowserPersistence()
+    vi.mocked(persistence.save).mockReturnValue(false)
+    session = freshSession(win, {}, persistence)
+    const tab = session.withBrowserScope('chat-deleted', () => session.ensureTab())
+
+    // Suspension accompanies chat deletion: a failed descriptor save must
+    // never leave the deleted chat's pages loaded invisibly.
+    expect(session.suspendBrowserScope('chat-deleted')).toBe(true)
+
+    expect((tab.view as unknown as MockView).webContents.close).toHaveBeenCalledOnce()
+    expect(session.withBrowserScope('chat-deleted', () => session.peekTabsState())).toMatchObject({
+      tabs: [],
+      activeTabId: null,
+    })
+  })
+
   it('normalizes browser shortcuts to Command on macOS and Control elsewhere', () => {
     const input = {
       type: 'keyDown',

apps/desktop/src/main/browser-agent/session.ts

@@ -666,6 +666,11 @@ export function disposeBrowserScope(scopeId: string): void {
  * No empty-strip/session-closed events are published: soft deletion removes
  * the resource's UI separately, and those events would overwrite its retained
  * renderer descriptor before the chat can be restored.
+ *
+ * The persist is best-effort: suspension accompanies chat deletion, and a
+ * descriptor that could not be saved must never leave the deleted chat's
+ * pages loaded invisibly. A restore after a failed save falls back to the
+ * last successfully saved descriptor.
  */
 export function suspendBrowserScope(scopeId: string): boolean {
   const resolved = resolveBrowserScopeId(scopeId)
@@ -675,12 +680,10 @@ export function suspendBrowserScope(scopeId: string): boolean {
     return true
   }
 
-  let persisted = true
   withBrowserScope(resolved, () => {
-    if (hasSession()) persisted = persistBrowserSession()
-    if (persisted) closeLiveTabs()
+    if (hasSession()) persistBrowserSession()
+    closeLiveTabs()
   })
-  if (!persisted) return false
 
   suspendedBrowserScopes.add(resolved)
   browserScopeStates.delete(resolved)

apps/desktop/src/main/index.ts

@@ -629,9 +629,23 @@ function main(): void {
           const win = windowForContents(sender)
           return win ? captureBrowserAgentPanelSnapshot(win, scopeId) : Promise.resolve(null)
         },
-        setOccluded: (sender, occluded, scopeId) => {
+        setOccluded: (sender, occluded, scopeId, force) => {
           const win = windowForContents(sender)
-          return win ? setBrowserAgentPanelOccluded(occluded, win, scopeId) : false
+          if (!win) return false
+          // A modal in a stale renderer must not resurrect a soft-deleted
+          // Browser scope. There is no local native surface for that scope;
+          // acknowledge only its forced hide/any reveal as scoped no-ops.
+          if (isBrowserScopeSuspended(scopeId)) return !occluded || force === true
+          // The focused window may open a modal before its next bounds frame
+          // has transferred the singleton Browser from another app window.
+          // Move the session scope first so the forced hide establishes the
+          // new owner's hidden lease, rather than acknowledging a background
+          // no-op and then attaching the view visibly on the bounds report.
+          const resolvedScopeId =
+            occluded && force && focusedAppWindow() === win
+              ? activateAgentBrowserScope(scopeId)
+              : scopeId
+          return setBrowserAgentPanelOccluded(occluded, win, resolvedScopeId, force)
         },
       },
       beginOAuthConnect: (providerId, scope) => connectFlow.beginConnectHandoff(providerId, scope),