fix: fence browser recovery to attach inventory placements (#18910)

* fix: fence browser recovery to attach inventory placements

* refactor: name the attach-inventory fence and make its test deterministic

Extract the placement check into isPlacedAsObservedAtAttach so the recovery
filter stays a flat list of named predicates, and document that omitting
pagePlacementsAtAttach recovers against unfenced live state.

Replace the 30-microtask drain in the post-attach regression with the handler's
own completion: attach only settles after recovery returns, so awaiting the
dispatch orders the assertions instead of guessing at a microtask count.

Verified by forcing the fence open: both regressions fail (the post-attach one
in 60ms on a retired placement) and the other 27 still pass.

* test: settle the attach handler even when the regression fails early

The barrier ran inline, so a waitFor timeout or the placement guard left the
attach handler parked on a promise nothing awaited. Hoist it into settleAttach
and call it from a finally as well; cleanup is guarded and the dispatch promise
is already settled, so the second call is a no-op.

src/main/runtime/rpc/methods/browser-client-host-attach-adoption.test.ts

@@ -183,6 +183,52 @@ describe('browser.clientHost.attach adoption', () => {
     await rig.dispatch
   })
 
+  it('does not recover a page created after the attach inventory was captured', async () => {
+    let releaseAdoption!: (value: BrowserExecutionHostKeyResolution) => void
+    const route = new Promise<BrowserExecutionHostKeyResolution>((resolve) => {
+      releaseAdoption = resolve
+    })
+    const resolveExecutionHostKey = vi.fn(() => route)
+    const rig = attachHost([orphanedPage()], { resolveExecutionHostKey })
+    const settleAttach = async (): Promise<void> => {
+      rig.cleanups.get(`browser-client-host:${HOST_CLIENT_ID}`)?.()
+      await rig.dispatch
+    }
+    try {
+      await vi.waitFor(() => expect(resolveExecutionHostKey).toHaveBeenCalled())
+      const authority = getBrowserHostLeaseRegistry(rig.hostRuntime)
+      const pages = getRuntimeBrowserPageRegistry(rig.hostRuntime)
+      const placement = authority.placeClientPage('page-created-after-attach', HOST_CLIENT_ID)
+      if (placement.kind !== 'client') {
+        throw new Error('expected client placement')
+      }
+      pages.publishClientPage({
+        browserPageId: 'page-created-after-attach',
+        workspaceId: WORKSPACE_ID,
+        browserProfileId: 'default',
+        executionHostKey: EXECUTION_HOST_KEY,
+        placement,
+        pairedDeviceId: 'device-a',
+        url: 'https://remote.internal/new',
+        loading: false,
+        active: true
+      })
+      releaseAdoption({ status: 'resolved', executionHostKey: EXECUTION_HOST_KEY })
+      await vi.waitFor(() => expect(rig.markClientHostedPagesReconciled).toHaveBeenCalled())
+      // Attach only settles once recovery has returned, so this is the barrier the assertions need:
+      // draining microtasks would let a regression slip through as a not-yet-issued command.
+      await settleAttach()
+
+      expect(authority.getPlacement('page-created-after-attach')).toEqual(placement)
+      expect(pages.getPage('page-created-after-attach')).toMatchObject({ placement, active: true })
+      expect(
+        rig.commands().filter((event) => event.browserPageId === 'page-created-after-attach')
+      ).toEqual([])
+    } finally {
+      await settleAttach()
+    }
+  })
+
   it('does not re-enter recovery for a page it just adopted', async () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
     const rig = attachHost([orphanedPage({ browserPageId: 'page-d' })], {

src/main/runtime/rpc/methods/browser-client-host.ts

@@ -39,6 +39,12 @@ export const BROWSER_CLIENT_HOST_METHODS: RpcAnyMethod[] = [
       }
 
       const registry = getBrowserHostLeaseRegistry(runtime)
+      // Attach inventory cannot describe pages created or replaced after readiness is published.
+      const pagePlacementsAtAttach = new Map(
+        getRuntimeBrowserPageRegistry(runtime)
+          .listPages()
+          .map((page) => [page.browserPageId, page.placement])
+      )
       const handle = registry.attach({
         browserHostClientId: params.browserHostClientId,
         connectionId,
@@ -127,6 +133,7 @@ export const BROWSER_CLIENT_HOST_METHODS: RpcAnyMethod[] = [
           lease: handle.lease,
           authority: registry,
           pages: getRuntimeBrowserPageRegistry(runtime),
+          pagePlacementsAtAttach,
           notifyWorkspace: (workspaceId) => runtime.notifyMobileSessionTabsChanged(workspaceId),
           releaseUnrecoverablePage: (page) =>
             releaseRuntimeBrowserClientPageRecord(runtime, page.browserPageId, page.placement),

src/main/runtime/runtime-browser-client-page-recovery.test.ts

@@ -43,6 +43,25 @@ describe('runtime browser client page recovery', () => {
     expect(notifyWorkspace).toHaveBeenCalledOnce()
   })
 
+  it('does not apply an attach inventory to a replacement placed after capture', async () => {
+    const { authority, commands, notifyWorkspace, pages, placements } = harness()
+    const pagePlacementsAtAttach = new Map([['page-a', oldPlacement]])
+    pages.replaceClientPagePlacement('page-a', oldPlacement, newPlacement)
+    placements.set('page-a', newPlacement)
+    await recoverUnavailableRuntimeBrowserClientPages({
+      lease: lease([]),
+      authority,
+      pages,
+      notifyWorkspace,
+      pagePlacementsAtAttach
+    })
+    expect(authority.getPlacement('page-a')).toEqual(newPlacement)
+    expect(pages.getPage('page-a')?.placement).toEqual(newPlacement)
+    expect(authority.createClientPage).not.toHaveBeenCalled()
+    expect(commands).toEqual([])
+    expect(notifyWorkspace).not.toHaveBeenCalled()
+  })
+
   it('retains an exact active generation without commands or metadata churn', async () => {
     const { authority, commands, notifyWorkspace, pages } = harness()
 

src/main/runtime/runtime-browser-client-page-recovery.ts

@@ -45,6 +45,8 @@ export async function recoverUnavailableRuntimeBrowserClientPages(options: {
   }
   authority: RecoveryAuthority
   pages: RuntimeBrowserPageRegistry
+  /** Placements as of the attach inventory. Omitting it recovers against unfenced live state. */
+  pagePlacementsAtAttach?: ReadonlyMap<string, RuntimeBrowserClientPage['placement']>
   notifyWorkspace(workspaceId: string): void
   /** Drops a page whose placement recovery destroyed without replacing it. */
   releaseUnrecoverablePage?: (page: RuntimeBrowserClientPage) => void
@@ -77,6 +79,7 @@ export async function recoverUnavailableRuntimeBrowserClientPages(options: {
     .listPages()
     .filter(
       (page) =>
+        isPlacedAsObservedAtAttach(page, options.pagePlacementsAtAttach) &&
         !options.adoptedPageIds?.has(page.browserPageId) &&
         isRecoverableByLease(page, options.lease) &&
         !isActiveExactPage(page, inventoryByPageId.get(page.browserPageId), options.lease)
@@ -101,6 +104,23 @@ export async function recoverUnavailableRuntimeBrowserClientPages(options: {
   )
 }
 
+/**
+ * Whether the attach inventory can still speak for this page.
+ *
+ * Readiness is published before recovery runs, so the client can place a page the inventory predates
+ * -- and absence from the inventory means "recreate". Those are left to the attach that can see them.
+ */
+function isPlacedAsObservedAtAttach(
+  page: RuntimeBrowserClientPage,
+  pagePlacementsAtAttach: ReadonlyMap<string, RuntimeBrowserClientPage['placement']> | undefined
+): boolean {
+  if (!pagePlacementsAtAttach) {
+    return true
+  }
+  const observed = pagePlacementsAtAttach.get(page.browserPageId)
+  return observed !== undefined && sameRuntimeBrowserPlacement(observed, page.placement)
+}
+
 /**
  * Whether this lease is the one allowed to take a page back.
  *