refactor(browser): match the desktop guest budget instead of inventing policy

Headless reclamation now has exactly the policy surface the desktop guest budget has, and nothing more. Dropped the retained-page limit and its outright close. Desktop destroys guests but never removes a tab, so closing a page past a count had no precedent here and was the one genuinely novel product decision in this change — it permanently destroyed a page id an agent still held. Parked records are ~200 bytes against the 220-480 MB renderers being reclaimed, so retaining them indefinitely is the right trade and matches what desktop already does. Dropped the download veto's progress window with it. Desktop treats an active download as work in progress for as long as it stays active, and headless now does the same. That accepts the same unbounded-veto shape desktop has: a download that never completes keeps its page resident. Deliberate, and the alternative was a threshold desktop does not have. What stays is the part with precedent: a resident cap of 4 (the desktop limit), LRU eviction, and the idle clock headless needs because it has no notion of a visible worktree.
2026-10-23 08:03:39 +00:00 · 2026-08-19 19:07:13 -04:00
parent 2b309dfb5a
commit 3d50a886c7
7 changed files with 29 additions and 210 deletions
@@ -294,18 +294,18 @@ initial load is still running, or while it is waiting on a certificate
 decision. A navigation that has still not finished by the load timeout is
 deliberately reclaimable — otherwise one stalled page per create could hold a
 renderer forever, which is the failure this exists to prevent; waking it simply
-retries the address.
+retries the address. Parking never closes a page: the page stays open until
+something closes it, exactly as on the desktop.

 Two evictors decide what stays resident, mirroring terminal pane parking. The
 cap is the primary one:

-| Variable                                   | Default  | Meaning                                                                |
-| ------------------------------------------ | -------- | ---------------------------------------------------------------------- |
-| `ORCA_HEADLESS_BROWSER_RESIDENT_LIMIT`     | `4`      | Renderers kept resident. Least-recently-used pages park past this.     |
-| `ORCA_HEADLESS_BROWSER_PARK_IDLE_MS`       | `300000` | A page parks after this long untouched, even under the cap.            |
-| `ORCA_HEADLESS_BROWSER_PARK_GRACE_MS`      | `30000`  | A page is never parked this soon after its last command.               |
-| `ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS`      | `15000`  | How often the server re-evaluates residency.                           |
-| `ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES` | `100`    | Open pages retained before the oldest parked ones are closed outright. |
+| Variable                               | Default  | Meaning                                                            |
+| -------------------------------------- | -------- | ------------------------------------------------------------------ |
+| `ORCA_HEADLESS_BROWSER_RESIDENT_LIMIT` | `4`      | Renderers kept resident. Least-recently-used pages park past this. |
+| `ORCA_HEADLESS_BROWSER_PARK_IDLE_MS`   | `300000` | A page parks after this long untouched, even under the cap.        |
+| `ORCA_HEADLESS_BROWSER_PARK_GRACE_MS`  | `30000`  | A page is never parked this soon after its last command.           |
+| `ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS`  | `15000`  | How often the server re-evaluates residency.                       |

 Raise the limit on a host with memory to spare, or set both the limit and the
 idle window very high to keep every page resident — at the cost this
@@ -201,8 +201,6 @@ type ActiveDownload = {
  savePath: string
  reservationKey: string | null
  receivedBytes: number
-  /** Last time Electron reported progress; the liveness signal a reclaimer needs. */
-  lastProgressAt: number
  transientState: BrowserDownloadProgressEvent['state']
  terminalEvent: BrowserDownloadFinishedEvent | null
  startedSent: boolean
@@ -1474,23 +1472,18 @@ export class BrowserManager {
      : (this.loadErrorsByGuestId.get(webContentsId) ?? null)
  }

-  /**
-   * When this page's in-flight downloads last made progress, or null when it has
-   * none. Why (STA-4341): unregisterGuest cancels a tab's in-flight downloads, so
-   * a reclaimer must not take a page that is still writing one — the desktop guest
-   * budget vetoes eviction for the same reason. A timestamp rather than a boolean
-   * because a download that stalls forever must not pin a renderer forever; the
-   * caller owns that bound.
-   */
-  getBrowserPageActiveDownloadProgressAt(browserPageId: string): number | null {
-    let latest: number | null = null
+  // Why (STA-4341): unregisterGuest cancels a tab's in-flight downloads, so a
+  // reclaimer must not take a page that is still writing one. Mirrors the
+  // desktop guest budget's veto (browser-page-download-activity.ts) exactly,
+  // including its unbounded nature: a download is treated as work in progress
+  // for as long as it stays active.
+  hasActiveBrowserPageDownload(browserPageId: string): boolean {
    for (const download of this.downloadsById.values()) {
      if (download.browserTabId === browserPageId && !download.terminalEvent) {
-        latest =
-          latest === null ? download.lastProgressAt : Math.max(latest, download.lastProgressAt)
+        return true
      }
    }
-    return latest
+    return false
  }

  getBrowserPageCertificateFailure(browserPageId: string): BrowserCertificateFailure | null {
@@ -1638,7 +1631,6 @@ export class BrowserManager {
      savePath: fallbackSavePath,
      reservationKey: destination?.reservationKey ?? null,
      receivedBytes: 0,
-      lastProgressAt: Date.now(),
      transientState: null,
      terminalEvent: null,
      startedSent: false,
@@ -1680,7 +1672,6 @@ export class BrowserManager {

    const updatedHandler = (_event: Electron.Event, state: 'progressing' | 'interrupted'): void => {
      download.receivedBytes = this.getDownloadReceivedBytes(download.item)
-      download.lastProgressAt = Date.now()
      download.transientState = state
      this.sendDownloadProgress(download.browserTabId, {
        browserPageId: download.browserTabId ?? undefined,
@@ -96,7 +96,6 @@ type Harness = {
  pagesChanged: (string | undefined)[]
  certificateFailurePageIds: Set<string>
  downloadingPageIds: Set<string>
-  downloadProgressAt: number
 }

 function createHarness(
@@ -112,7 +111,6 @@ function createHarness(
    activePageId: overrides.activePageId,
    certificateFailurePageIds: new Set<string>(overrides.certificateFailurePageIds ?? []),
    downloadingPageIds: new Set<string>(overrides.downloadingPageIds ?? []),
-    downloadProgressAt: 1_000_000,
    pagesChanged: [] as (string | undefined)[]
  }
  const order: string[] = []
@@ -145,8 +143,8 @@ function createHarness(
    getBrowserPageLoadError: () => overrides.loadError ?? null,
    getBrowserPageCertificateFailure: (browserPageId: string) =>
      state.certificateFailurePageIds.has(browserPageId) ? { challengeId: 'c' } : null,
-    getBrowserPageActiveDownloadProgressAt: (browserPageId: string) =>
-      state.downloadingPageIds.has(browserPageId) ? state.downloadProgressAt : null
+    hasActiveBrowserPageDownload: (browserPageId: string) =>
+      state.downloadingPageIds.has(browserPageId)
  } as unknown as BrowserManager

  const bridge = {
@@ -181,10 +179,7 @@ function createHarness(
      return state.pagesChanged
    },
    certificateFailurePageIds: state.certificateFailurePageIds,
-    downloadingPageIds: state.downloadingPageIds,
-    set downloadProgressAt(value: number) {
-      state.downloadProgressAt = value
-    }
+    downloadingPageIds: state.downloadingPageIds
  }
 }

@@ -195,7 +190,6 @@ beforeEach(() => {
  process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = '60000'
  process.env.ORCA_HEADLESS_BROWSER_PARK_GRACE_MS = '5000'
  process.env.ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS = '100000'
-  process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '100'
 })

 describe('OffscreenBrowserBackend reclamation', () => {
@@ -330,43 +324,6 @@ describe('OffscreenBrowserBackend reclamation', () => {
    expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['a'])
  })

-  it('does not close a parked page that was woken while an earlier close was in flight', async () => {
-    let releaseFirstTeardown = (): void => {}
-    process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '2'
-    // Why: pinning the two newer pages keeps the park loop empty, so the sweep
-    // that blocks is the retention close — the path under test.
-    const h = createHarness({ pinned: new Set(['c', 'd']) })
-    const bridge = h.bridge as unknown as { onTabClosed: ReturnType<typeof vi.fn> }
-
-    for (const id of ['a', 'b']) {
-      await h.backend.createTab({ url: `https://${id}`, browserPageId: id })
-      h.clock.value += 1_000
-    }
-    h.clock.value += 120_000
-    await h.backend.reclaimIdlePages()
-    expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['a', 'b'])
-
-    // Two more push the retention budget over, dooming the two oldest parked.
-    for (const id of ['c', 'd']) {
-      await h.backend.createTab({ url: `https://${id}`, browserPageId: id })
-      h.clock.value += 1_000
-    }
-    h.clock.value += 120_000
-    bridge.onTabClosed.mockImplementationOnce(
-      async () => new Promise<void>((resolve) => (releaseFirstTeardown = resolve))
-    )
-
-    const sweep = h.backend.reclaimIdlePages()
-    await Promise.resolve()
-    await h.backend.wakeTab('b')
-    releaseFirstTeardown()
-    await sweep
-
-    // a was closed; b survived the stale close list and is resident again.
-    expect(await h.backend.wakeTab('a')).toBe(false)
-    expect(await h.backend.wakeTab('b')).toBe(true)
-  })
-
  it('does not park a page whose wake is still rebuilding it', async () => {
    // Why: a wake is resident but not yet loading while it awaits the process
    // swap, so without an explicit pin the sweep can destroy it mid-rebuild.
@@ -428,14 +385,13 @@ describe('OffscreenBrowserBackend reclamation', () => {
    expect(h.pagesChanged).toEqual(['wt-1'])
  })

-  it('does not park a page whose download is still advancing', async () => {
+  it('does not park a page that is still writing a download', async () => {
    // Why: releasing a renderer unregisters its guest, and browser-manager
    // cancels that page's in-flight downloads on unregister. The desktop guest
    // budget vetoes eviction for the same reason.
    const h = createHarness({ downloadingPageIds: ['a'] })
    await h.backend.createTab({ url: 'https://a', browserPageId: 'a' })
    h.clock.value += 120_000
-    h.downloadProgressAt = h.clock.value

    expect(await h.backend.reclaimIdlePages()).toEqual([])

@@ -443,18 +399,6 @@ describe('OffscreenBrowserBackend reclamation', () => {
    expect(await h.backend.reclaimIdlePages()).toEqual(['a'])
  })

-  it('parks a page whose download has stalled so the cap cannot be defeated', async () => {
-    // Why: a download that never progresses again would otherwise pin its
-    // renderer forever, and one stalled download per page would defeat the
-    // resident cap outright — the same shape as the load-pin bug before it.
-    const h = createHarness({ downloadingPageIds: ['a'] })
-    await h.backend.createTab({ url: 'https://a', browserPageId: 'a' })
-    h.downloadProgressAt = h.clock.value
-    h.clock.value += 120_000
-
-    expect(await h.backend.reclaimIdlePages()).toEqual(['a'])
-  })
-
  it('does not park a page waiting on a certificate decision', async () => {
    // Why: the challenge id dies with the renderer, so parking would discard
    // both the warning and the ability to approve it.
@@ -824,21 +768,6 @@ describe('OffscreenBrowserBackend reclamation', () => {
    expect(await h.backend.wakeTab('a')).toBe(false)
  })

-  it('closes the oldest parked pages once too many records are retained', async () => {
-    // Why: parking bounds renderer processes, not the page records behind them.
-    process.env.ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES = '2'
-    const h = createHarness()
-    for (const id of ['a', 'b', 'c', 'd']) {
-      await h.backend.createTab({ url: `https://example.test/${id}`, browserPageId: id })
-      h.clock.value += 1_000
-    }
-    h.clock.value += 120_000
-    await h.backend.reclaimIdlePages()
-
-    expect(h.backend.listParkedPages().map((page) => page.browserPageId)).toEqual(['c', 'd'])
-    expect(h.windows.every((win) => win.isDestroyed())).toBe(true)
-  })
-
  it('scopes parked listings and implicit targeting to a worktree', async () => {
    const h = createHarness()
    await h.backend.createTab({ url: 'https://a', browserPageId: 'a', worktreeId: 'wt-1' })
@@ -48,8 +48,8 @@ export class OffscreenBrowserBackend implements BrowserBackend {
      isWaking: (browserPageId) => this.waking.has(browserPageId),
      hasCertificateChallenge: (browserPageId) =>
        this.browserManager.getBrowserPageCertificateFailure(browserPageId) !== null,
-      activeDownloadProgressAt: (browserPageId) =>
-        this.browserManager.getBrowserPageActiveDownloadProgressAt(browserPageId),
+      hasActiveDownload: (browserPageId) =>
+        this.browserManager.hasActiveBrowserPageDownload(browserPageId),
      isHostPinned: (browserPageId) => this.options.isPagePinned?.(browserPageId) === true,
      park: (browserPageId) => this.parkPage(browserPageId),
      close: (browserPageId) => this.closeTab(browserPageId),
@@ -1,6 +1,5 @@
 import { describe, expect, it } from 'vitest'
 import {
-  selectOffscreenBrowserPagesToClose,
  selectOffscreenBrowserPagesToPark,
  type OffscreenBrowserReclaimCandidate,
  type OffscreenBrowserReclaimPolicy
@@ -12,8 +11,7 @@ const POLICY: OffscreenBrowserReclaimPolicy = {
  residentLimit: 2,
  idleParkMs: 60_000,
  parkGraceMs: 5_000,
-  sweepIntervalMs: 1_000,
-  retainedPageLimit: 100
+  sweepIntervalMs: 1_000
 }

 function page(
@@ -98,24 +96,3 @@ describe('selectOffscreenBrowserPagesToPark', () => {
    ).toEqual([])
  })
 })
-
-describe('selectOffscreenBrowserPagesToClose', () => {
-  it('retains every parked page under the limit', () => {
-    expect(selectOffscreenBrowserPagesToClose([page('a', 1), page('b', 2)], 2, POLICY)).toEqual([])
-  })
-
-  it('closes the least recently used parked pages down to the limit', () => {
-    // Why: parking bounds renderer processes, not the records behind them.
-    const parked = [page('new', 1_000), page('old', 90_000), page('older', 99_000)]
-    expect(
-      selectOffscreenBrowserPagesToClose(parked, 5, { ...POLICY, retainedPageLimit: 3 })
-    ).toEqual(['older', 'old'])
-  })
-
-  it('never closes a pinned page', () => {
-    const parked = [page('pinned', 99_000, true), page('idle', 90_000)]
-    expect(
-      selectOffscreenBrowserPagesToClose(parked, 4, { ...POLICY, retainedPageLimit: 2 })
-    ).toEqual(['idle'])
-  })
-})
@@ -16,22 +16,12 @@ export const OFFSCREEN_BROWSER_PARK_GRACE_MS = 30_000

 /** How often the backend re-evaluates which pages should be resident. */
 export const OFFSCREEN_BROWSER_SWEEP_INTERVAL_MS = 15_000
-/** Parked page records retained before the oldest are closed outright. */
-export const OFFSCREEN_BROWSER_RETAINED_PAGE_LIMIT = 100
-/**
- * How long a download keeps its page resident after its last progress. Parking
- * cancels a page's in-flight downloads, so an advancing one must veto — but a
- * download that stalls forever must not, or one stalled download per page would
- * defeat the resident cap outright.
- */
-export const OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS = 60_000

 export type OffscreenBrowserReclaimPolicy = {
  residentLimit: number
  idleParkMs: number
  parkGraceMs: number
  sweepIntervalMs: number
-  retainedPageLimit: number
 }

 export type OffscreenBrowserReclaimCandidate = {
@@ -67,13 +57,6 @@ export function readOffscreenBrowserReclaimPolicy(): OffscreenBrowserReclaimPoli
    sweepIntervalMs: Math.max(
      100,
      readPositiveIntEnv('ORCA_HEADLESS_BROWSER_PARK_SWEEP_MS', OFFSCREEN_BROWSER_SWEEP_INTERVAL_MS)
-    ),
-    retainedPageLimit: Math.max(
-      1,
-      readPositiveIntEnv(
-        'ORCA_HEADLESS_BROWSER_MAX_RETAINED_PAGES',
-        OFFSCREEN_BROWSER_RETAINED_PAGE_LIMIT
-      )
    )
  }
 }
@@ -123,30 +106,3 @@ export function selectOffscreenBrowserPagesToPark(

  return parkable.map((page) => page.browserPageId).filter((id) => parked.has(id))
 }
-
-/**
- * Pick parked pages to close outright. Parking bounds renderer processes but
- * not the records behind them, so an agent that opens pages forever and closes
- * none would still grow the backend without limit. Only parked pages are
- * eligible and only the least recently used go, so this can never take a page
- * an agent is still working with.
- */
-export function selectOffscreenBrowserPagesToClose(
-  parkedPages: readonly OffscreenBrowserReclaimCandidate[],
-  totalPageCount: number,
-  policy: OffscreenBrowserReclaimPolicy
-): string[] {
-  const excess = totalPageCount - policy.retainedPageLimit
-  if (excess <= 0) {
-    return []
-  }
-  return parkedPages
-    .filter((page) => !page.pinned)
-    .sort(
-      (left, right) =>
-        left.lastActivityAt - right.lastActivityAt ||
-        left.browserPageId.localeCompare(right.browserPageId)
-    )
-    .slice(0, excess)
-    .map((page) => page.browserPageId)
-}
@@ -3,8 +3,6 @@ import type {
  OffscreenBrowserPage
 } from './offscreen-browser-open-pages'
 import {
-  OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS,
-  selectOffscreenBrowserPagesToClose,
  selectOffscreenBrowserPagesToPark,
  type OffscreenBrowserReclaimPolicy
 } from './offscreen-browser-page-reclaim'
@@ -26,8 +24,8 @@ export type OffscreenBrowserReclaimerDeps = {
  isWaking: (browserPageId: string) => boolean
  /** A certificate challenge the page is blocked on. */
  hasCertificateChallenge: (browserPageId: string) => boolean
-  /** When the page's in-flight downloads last advanced, null when it has none. */
-  activeDownloadProgressAt: (browserPageId: string) => number | null
+  /** A download the page is still writing. */
+  hasActiveDownload: (browserPageId: string) => boolean
  /** A client streaming the page, or a command in flight against it. */
  isHostPinned: (browserPageId: string) => boolean
  park: (browserPageId: string) => Promise<void>
@@ -63,7 +61,6 @@ export class OffscreenBrowserPageReclaimer {
      await this.deps.park(browserPageId)
      parked.push(browserPageId)
    }
-    await this.closeOverRetainedPages()
    return parked
  }

@@ -100,30 +97,6 @@ export class OffscreenBrowserPageReclaimer {
    this.sweepInFlight = null
  }

-  // Why: parking bounds renderer processes, not the records behind them. An
-  // agent that opens pages forever and closes none would still grow the backend
-  // without limit, so the oldest parked pages are eventually closed outright.
-  private async closeOverRetainedPages(): Promise<void> {
-    const doomed = selectOffscreenBrowserPagesToClose(
-      this.deps.pages.parked().map((page) => this.toCandidate(page)),
-      this.deps.pages.size,
-      this.deps.policy
-    )
-    for (const browserPageId of doomed) {
-      // Why: closing is destructive and awaits teardown, so a page selected
-      // here can be woken before its turn comes. Only close one that is still
-      // parked and still unwanted.
-      const page = this.deps.pages.get(browserPageId)
-      if (!page || page.window || !this.isSafeToReclaim(browserPageId)) {
-        continue
-      }
-      console.warn(
-        `[offscreen-browser] closing parked page ${browserPageId}: more than ${this.deps.policy.retainedPageLimit} pages are open`
-      )
-      await this.deps.close(browserPageId)
-    }
-  }
-
  // Why: a page is off limits while anything depends on its renderer — a client
  // streaming it, a command in flight, its first navigation still committing, a
  // wake still rebuilding it, a certificate decision it is blocked on, or a
@@ -137,21 +110,14 @@ export class OffscreenBrowserPageReclaimer {
      // Why: a challenge id dies with the renderer, so parking would discard
      // both the warning and the ability to approve it.
      this.deps.hasCertificateChallenge(page.browserPageId) ||
-      this.hasAdvancingDownload(page.browserPageId) ||
+      // Why: releasing a renderer unregisters its guest, and that cancels the
+      // page's in-flight downloads. Mirrors the desktop guest budget's veto
+      // (browser-guest-worktree-retention.ts).
+      this.deps.hasActiveDownload(page.browserPageId) ||
      this.deps.isHostPinned(page.browserPageId)
    )
  }

-  // Why: releasing a renderer unregisters its guest, and that cancels the page's
-  // in-flight downloads — the desktop guest budget vetoes eviction for the same
-  // reason (browser-guest-worktree-retention.ts). Bounded by progress, because a
-  // download that stalls forever would otherwise pin its renderer forever, and
-  // one stalled download per page would defeat the resident cap outright.
-  private hasAdvancingDownload(browserPageId: string): boolean {
-    const progressAt = this.deps.activeDownloadProgressAt(browserPageId)
-    return progressAt !== null && this.deps.now() - progressAt < OFFSCREEN_BROWSER_DOWNLOAD_VETO_MS
-  }
-
  private toCandidate(page: OffscreenBrowserPage): {
    browserPageId: string
    lastActivityAt: number