mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(browser): keep headless tab order, active flag and env knobs honest
Compat-lens findings from the fourth review round (1 P1, 4 P2): P1 — the merged listing ordered resident tabs before parked ones, so a background park renumbered indices that pre-reclaim moved only on create/close: an index the agent read minutes ago could close a different tab, and a paired client's tab bar reordered whenever a tab parked or woke. Listings now sort by the page book's creation order, which parking cannot change. P2 — closing the page that carried the scope's only active claim (the parked-active holder, or the last live tab) left a worktree whose every open page reported inactive, breaking the one-selected-tab assumption paired clients render on. closeTab now promotes the most recently used parked page when nothing live claims active. P2 — claimParkedActive treated an undefined worktree as 'every worktree', so a worktree-less page (a remote 'tab create' without --worktree) parking while active wiped every other worktree's parked-active flag host-wide. The claim now scopes strictly, undefined matching only undefined. P2 — readPositiveIntEnv used parseInt, which reads '1e9' as 1: an operator following the docs' 'set the idle window very high' advice silently got a 1ms window. Number() consumes the whole string, and trailing garbage now falls back instead of truncating. P2 — the docs' closed enumeration of park vetoes omitted the download and in-flight-wake vetoes; the download one is the guarantee an operator most needs stated, since parking cancels a download. Renderer construction moved to offscreen-browser-renderer-attachment.ts — the backend crossed the max-lines budget, and building + wiring a renderer to its record was a coherent seam to split.
This commit is contained in:
@@ -292,8 +292,9 @@ park does _not_ preserve is in-page JavaScript state — cookies and local
|
||||
storage live in the profile partition and survive, but an unsubmitted form or
|
||||
an SPA's in-memory state does not. A page is never parked while a paired
|
||||
client is streaming it, while a command against it is in flight, while its
|
||||
initial load is still running, or while it is waiting on a certificate
|
||||
decision. A stream ending counts as use, so a viewer who closes the pane gets
|
||||
initial load is still running, while it is waiting on a certificate decision,
|
||||
while a wake is still rebuilding it, or while it is writing an active
|
||||
download — parking cancels a download, so an active one always vetoes it. A stream ending counts as use, so a viewer who closes the pane gets
|
||||
the full idle window before the page parks, even if they only watched. 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
|
||||
@@ -882,7 +883,7 @@ refuse to run there and print the command to run on the machine you want.
|
||||
- Clients cannot connect: make sure `--pairing-address` is an address reachable
|
||||
from the client, and make sure firewalls allow the selected `--port`.
|
||||
- Journal shows `Another Orca instance is already running for this userData
|
||||
profile` and the unit exits `3`: another process already owns the profile, so
|
||||
profile` and the unit exits `3`: another process already owns the profile, so
|
||||
`RestartPreventExitStatus=3` leaves the unit `failed` on purpose. Find the
|
||||
owner with `systemctl status orca-serve` and `pgrep -af orca`. Stop it (or
|
||||
keep it and leave the unit down), then run
|
||||
|
||||
@@ -45,6 +45,9 @@ export type BrowserBackend = {
|
||||
wakeTab?(browserPageId: string): Promise<boolean>
|
||||
/** Pages this backend still owns whose renderer is currently reclaimed. */
|
||||
listParkedPages?(worktreeId?: string): ParkedBrowserPage[]
|
||||
/** Open page ids in creation order, resident or parked — listings sort by
|
||||
* this so parking never renumbers a tab index the caller already read. */
|
||||
listOpenPageIds?(worktreeId?: string): string[]
|
||||
/** The parked page a page-less command should target: the one that was
|
||||
* active when it parked, else the most recently used. */
|
||||
getParkedPageIdForImplicitTarget?(worktreeId?: string): string | null
|
||||
|
||||
@@ -68,6 +68,33 @@ describe('mergeParkedBrowserTabs', () => {
|
||||
expect(merged.filter((tab) => tab.active).map((tab) => tab.browserPageId)).toEqual(['b'])
|
||||
})
|
||||
|
||||
it('orders by creation so parking never renumbers an index the caller read', () => {
|
||||
// Why: the bridge lists by registration order, which a park/wake cycle
|
||||
// mutates (a woken tab re-registers at the end). A tab created first must
|
||||
// stay at index 0 whether or not its renderer is currently reclaimed.
|
||||
const merged = mergeParkedBrowserTabs([live('b', 0, true)], [parked('a')], ['a', 'b'])
|
||||
expect(merged.map((tab) => [tab.browserPageId, tab.index, tab.parked === true])).toEqual([
|
||||
['a', 0, true],
|
||||
['b', 1, false]
|
||||
])
|
||||
})
|
||||
|
||||
it('reindexes an all-live listing when the creation order disagrees', () => {
|
||||
const merged = mergeParkedBrowserTabs([live('b', 0), live('a', 1, true)], [], ['a', 'b'])
|
||||
expect(merged.map((tab) => [tab.browserPageId, tab.index])).toEqual([
|
||||
['a', 0],
|
||||
['b', 1]
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps ids missing from the creation order at their merged position', () => {
|
||||
const merged = mergeParkedBrowserTabs([live('x', 0, true)], [parked('a')], ['a'])
|
||||
expect(merged.map((tab) => [tab.browserPageId, tab.index])).toEqual([
|
||||
['a', 0],
|
||||
['x', 1]
|
||||
])
|
||||
})
|
||||
|
||||
it('carries a parked page load failure into the listing', () => {
|
||||
const loadError = { code: -105, description: 'NAME_NOT_RESOLVED', validatedUrl: 'https://nope' }
|
||||
const [tab] = mergeParkedBrowserTabs([], [{ ...parked('b'), loadError }])
|
||||
|
||||
@@ -9,9 +9,14 @@ import type { ParkedBrowserPage } from './browser-backend'
|
||||
|
||||
export function mergeParkedBrowserTabs(
|
||||
live: readonly BrowserTabInfo[],
|
||||
parked: readonly ParkedBrowserPage[]
|
||||
parked: readonly ParkedBrowserPage[],
|
||||
// Why: the bridge lists by registration order, which parking and waking both
|
||||
// mutate (a woken tab re-registers at the end). Ordering by the page book's
|
||||
// creation order instead keeps every tab's index stable across a park — an
|
||||
// index the caller read must never be renumbered by a background timer.
|
||||
creationOrder?: readonly string[]
|
||||
): BrowserTabInfo[] {
|
||||
if (parked.length === 0) {
|
||||
if (parked.length === 0 && !creationOrder) {
|
||||
return [...live]
|
||||
}
|
||||
// Why: parking clears the bridge's active pointer, which then lands on a
|
||||
@@ -37,5 +42,16 @@ export function mergeParkedBrowserTabs(
|
||||
profileId: page.profileId ?? null
|
||||
})
|
||||
}
|
||||
if (!creationOrder) {
|
||||
return merged
|
||||
}
|
||||
const rank = new Map(creationOrder.map((browserPageId, index) => [browserPageId, index]))
|
||||
return merged
|
||||
.map((tab, index) => ({ tab, index }))
|
||||
.sort((left, right) => {
|
||||
const leftRank = rank.get(left.tab.browserPageId) ?? creationOrder.length + left.index
|
||||
const rightRank = rank.get(right.tab.browserPageId) ?? creationOrder.length + right.index
|
||||
return leftRank - rightRank
|
||||
})
|
||||
.map(({ tab }, index) => ({ ...tab, index }))
|
||||
}
|
||||
|
||||
@@ -160,7 +160,8 @@ function createHarness(
|
||||
onProcessSwap: vi.fn(async (browserPageId: string, webContentsId: number) => {
|
||||
order.push(`process-swap:${browserPageId}:${webContentsId}`)
|
||||
}),
|
||||
isActiveBrowserPage: (browserPageId: string) => state.activePageId === browserPageId
|
||||
isActiveBrowserPage: (browserPageId: string) => state.activePageId === browserPageId,
|
||||
getActivePageId: () => state.activePageId
|
||||
} as unknown as AgentBrowserBridge
|
||||
|
||||
const backend = new OffscreenBrowserBackend(manager, {
|
||||
@@ -729,6 +730,50 @@ describe('OffscreenBrowserBackend reclamation', () => {
|
||||
expect(claimed.map((page) => page.browserPageId)).toEqual(['b'])
|
||||
})
|
||||
|
||||
it('hands the active flag to a survivor when the parked holder closes', async () => {
|
||||
// Why: closing a parked page has no bridge teardown to promote a successor,
|
||||
// so without this the worktree's every remaining tab reports inactive and a
|
||||
// paired client loses its one-selected-tab assumption.
|
||||
const h = createHarness({ activePageId: 'b' })
|
||||
await h.backend.createTab({ url: 'https://a', browserPageId: 'a', worktreeId: 'wt-1' })
|
||||
h.clock.value += 1_000
|
||||
await h.backend.createTab({ url: 'https://b', browserPageId: 'b', worktreeId: 'wt-1' })
|
||||
h.clock.value += 120_000
|
||||
await h.backend.reclaimIdlePages()
|
||||
h.activePageId = undefined
|
||||
|
||||
await h.backend.closeTab('b')
|
||||
|
||||
expect(
|
||||
h.backend.listParkedPages('wt-1').map((page) => [page.browserPageId, page.active === true])
|
||||
).toEqual([['a', true]])
|
||||
})
|
||||
|
||||
it('scopes the parked active claim to its worktree, undefined included', async () => {
|
||||
// Why: a remote `tab create` without --worktree makes a worktree-less page;
|
||||
// parking it while active must not wipe another worktree's parked flag.
|
||||
const h = createHarness({ activePageId: 'w' })
|
||||
await h.backend.createTab({ url: 'https://w', browserPageId: 'w', worktreeId: 'wt-1' })
|
||||
await h.backend.createTab({ url: 'https://g', browserPageId: 'g' })
|
||||
h.clock.value += 120_000
|
||||
await h.backend.wakeTab('g')
|
||||
await h.backend.reclaimIdlePages()
|
||||
h.activePageId = 'g'
|
||||
h.clock.value += 120_000
|
||||
await h.backend.reclaimIdlePages()
|
||||
|
||||
expect(
|
||||
h.backend.listParkedPages('wt-1').map((page) => [page.browserPageId, page.active === true])
|
||||
).toEqual([['w', true]])
|
||||
expect(
|
||||
h.backend
|
||||
.listParkedPages()
|
||||
.filter((page) => page.active)
|
||||
.map((page) => page.browserPageId)
|
||||
.sort()
|
||||
).toEqual(['g', 'w'])
|
||||
})
|
||||
|
||||
it('targets the page that was active, not merely the most recently used', async () => {
|
||||
// Why: an explicit `--page b` command makes b the most recently used while
|
||||
// a is still the active tab, and a page-less command means "the active tab".
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import type { BrowserWindow } from 'electron'
|
||||
import { ORCA_BROWSER_PARTITION } from '../../shared/constants'
|
||||
import type { AgentBrowserBridge } from './agent-browser-bridge'
|
||||
import type { BrowserBackend, BrowserBackendCreateTab, ParkedBrowserPage } from './browser-backend'
|
||||
@@ -11,8 +10,8 @@ import {
|
||||
OffscreenBrowserOpenPages,
|
||||
type OffscreenBrowserPage
|
||||
} from './offscreen-browser-open-pages'
|
||||
import { materializeOffscreenBrowserRenderer } from './offscreen-browser-renderer-attachment'
|
||||
import {
|
||||
createOffscreenBrowserWindow,
|
||||
loadOffscreenBrowserUrl,
|
||||
OFFSCREEN_BROWSER_WAKE_LOAD_BUDGET_MS
|
||||
} from './offscreen-browser-window'
|
||||
@@ -117,6 +116,13 @@ export class OffscreenBrowserBackend implements BrowserBackend {
|
||||
const page = this.pages.delete(browserPageId) ?? null
|
||||
this.waking.delete(browserPageId)
|
||||
await this.releaseRenderer(page, browserPageId)
|
||||
// Why: the closed page may have carried the scope's only active claim —
|
||||
// either the parked-active flag, or a live pointer whose teardown found no
|
||||
// other live tab to promote. A worktree with open pages and no active tab
|
||||
// breaks the one-selected-tab assumption every paired client renders on.
|
||||
if (page && !this.options.getAgentBrowserBridge?.()?.getActivePageId(page.worktreeId)) {
|
||||
this.pages.promoteParkedActive(page.worktreeId)
|
||||
}
|
||||
// Why: closing changes both residency and rank, and with the last page gone
|
||||
// this arms nothing at all.
|
||||
this.reclaimer.reschedule()
|
||||
@@ -207,6 +213,11 @@ export class OffscreenBrowserBackend implements BrowserBackend {
|
||||
return this.pages.listParked(worktreeId)
|
||||
}
|
||||
|
||||
/** Open page ids in creation order — the stable order listings sort by. */
|
||||
listOpenPageIds(worktreeId?: string): string[] {
|
||||
return this.pages.openIds(worktreeId)
|
||||
}
|
||||
|
||||
/** The parked page a page-less command should target in this worktree. */
|
||||
getParkedPageIdForImplicitTarget(worktreeId?: string): string | null {
|
||||
return this.pages.parkedIdForImplicitTarget(worktreeId)
|
||||
@@ -315,70 +326,30 @@ export class OffscreenBrowserBackend implements BrowserBackend {
|
||||
}
|
||||
|
||||
private materialize(page: OffscreenBrowserPage): void {
|
||||
const win = createOffscreenBrowserWindow(page.partition)
|
||||
try {
|
||||
this.attachWindow(page, win)
|
||||
} catch (error) {
|
||||
// Why: the window exists before anything that can fail. Abandoning it
|
||||
// here would leak a hidden renderer nothing owns or can reach.
|
||||
page.window = null
|
||||
if (!win.isDestroyed()) {
|
||||
win.destroy()
|
||||
materializeOffscreenBrowserRenderer(page, {
|
||||
isDeliberateTeardown: (browserPageId) => this.releasing.has(browserPageId),
|
||||
onRendererLost: (webContentsId) => {
|
||||
this.pages.delete(page.browserPageId)
|
||||
// Why: an unprompted renderer loss must reclaim the helper session too,
|
||||
// or a crash loop leaks one session, CDP proxy and listening port per
|
||||
// page — the same leak the deliberate teardown path fixes.
|
||||
void this.options
|
||||
.getAgentBrowserBridge?.()
|
||||
?.onTabClosed(webContentsId)
|
||||
.catch(() => {})
|
||||
this.browserManager.unregisterGuest(page.browserPageId)
|
||||
this.reclaimer.reschedule()
|
||||
},
|
||||
registerGuest: (webContentsId) => {
|
||||
const profile = page.profileId ? browserSessionRegistry.getProfile(page.profileId) : null
|
||||
this.browserManager.registerOffscreenGuest({
|
||||
browserPageId: page.browserPageId,
|
||||
worktreeId: page.worktreeId,
|
||||
sessionProfileId: page.profileId ?? null,
|
||||
userAgentMode: profile?.userAgentMode,
|
||||
webContentsId
|
||||
})
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
private attachWindow(page: OffscreenBrowserPage, win: BrowserWindow): void {
|
||||
page.window = win
|
||||
// Why: reading win.webContents once the contents are destroyed throws, and
|
||||
// the throw would escape the 'destroyed' listener into the main process.
|
||||
// Capture the id while it is still safe to read.
|
||||
const webContentsId = win.webContents.id
|
||||
// Why: the record's address must follow the page, not the create call — an
|
||||
// agent that navigates with `goto` or in-page script has to be woken back
|
||||
// to where it actually is. A chrome-error address is the failure, not a
|
||||
// destination, so it never replaces the address that produced it.
|
||||
const recordAddress = (url: string): void => {
|
||||
if (page.window === win && url && !url.startsWith('chrome-error://')) {
|
||||
page.url = url
|
||||
}
|
||||
}
|
||||
// did-navigate is main-frame only; did-frame-navigate covers subframes.
|
||||
win.webContents.on('did-navigate', (_event, url) => recordAddress(url))
|
||||
// Why: an iframe changing its own hash also fires did-navigate-in-page. A
|
||||
// subframe navigation is not a navigation of the tab, and adopting its
|
||||
// address would wake the page onto the iframe's document.
|
||||
win.webContents.on('did-navigate-in-page', (_event, url, isMainFrame) => {
|
||||
if (isMainFrame) {
|
||||
recordAddress(url)
|
||||
}
|
||||
})
|
||||
// Why: if the window is destroyed out from under us (crash, app teardown),
|
||||
// drop the page so commands fail cleanly instead of resolving a dead
|
||||
// WebContents. Parking destroys it deliberately, so it opts out here.
|
||||
win.webContents.once('destroyed', () => {
|
||||
if (this.releasing.has(page.browserPageId) || page.window !== win) {
|
||||
return
|
||||
}
|
||||
this.pages.delete(page.browserPageId)
|
||||
// Why: an unprompted renderer loss must reclaim the helper session too, or
|
||||
// a crash loop leaks one session, CDP proxy and listening port per page —
|
||||
// the same leak the deliberate teardown path fixes.
|
||||
void this.options
|
||||
.getAgentBrowserBridge?.()
|
||||
?.onTabClosed(webContentsId)
|
||||
.catch(() => {})
|
||||
this.browserManager.unregisterGuest(page.browserPageId)
|
||||
this.reclaimer.reschedule()
|
||||
})
|
||||
const profile = page.profileId ? browserSessionRegistry.getProfile(page.profileId) : null
|
||||
this.browserManager.registerOffscreenGuest({
|
||||
browserPageId: page.browserPageId,
|
||||
worktreeId: page.worktreeId,
|
||||
sessionProfileId: page.profileId ?? null,
|
||||
userAgentMode: profile?.userAgentMode,
|
||||
webContentsId
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -73,6 +73,17 @@ export class OffscreenBrowserOpenPages {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Every open page's id in creation order, resident or parked. Listings order
|
||||
* by this so a tab's position never moves because its renderer was reclaimed
|
||||
* — an index read minutes ago must not be renumbered by a background timer.
|
||||
*/
|
||||
openIds(worktreeId?: string): string[] {
|
||||
return this.all()
|
||||
.filter((page) => !worktreeId || page.worktreeId === worktreeId)
|
||||
.map((page) => page.browserPageId)
|
||||
}
|
||||
|
||||
listParked(worktreeId?: string): ParkedBrowserPage[] {
|
||||
return this.parked(worktreeId).map((page) => ({
|
||||
browserPageId: page.browserPageId,
|
||||
@@ -105,12 +116,36 @@ export class OffscreenBrowserOpenPages {
|
||||
return best?.browserPageId ?? null
|
||||
}
|
||||
|
||||
/** Only one page per worktree may carry the active flag across a park. */
|
||||
/**
|
||||
* Only one page per worktree may carry the active flag across a park.
|
||||
* Strict scope: a worktree-less page clears only other worktree-less pages —
|
||||
* an undefined filter would wipe every worktree's flag host-wide.
|
||||
*/
|
||||
claimParkedActive(browserPageId: string, worktreeId?: string): void {
|
||||
for (const page of this.parked(worktreeId)) {
|
||||
if (page.browserPageId !== browserPageId) {
|
||||
for (const page of this.parked()) {
|
||||
if (page.browserPageId !== browserPageId && page.worktreeId === worktreeId) {
|
||||
page.activeWhenParked = false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Hand the active flag to the most recently used parked page when nothing
|
||||
* in the scope holds it. Closing the flag's holder (or the last live tab)
|
||||
* must not leave a worktree whose every page reports inactive — a paired
|
||||
* client assumes one selected browser tab per worktree.
|
||||
*/
|
||||
promoteParkedActive(worktreeId?: string): void {
|
||||
const scoped = this.parked().filter((page) => page.worktreeId === worktreeId)
|
||||
if (scoped.length === 0 || scoped.some((page) => page.activeWhenParked)) {
|
||||
return
|
||||
}
|
||||
let best = scoped[0]
|
||||
for (const page of scoped) {
|
||||
if (page.lastActivityAt > best.lastActivityAt) {
|
||||
best = page
|
||||
}
|
||||
}
|
||||
best.activeWhenParked = true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,6 +73,18 @@ describe('readOffscreenBrowserRetentionBudget', () => {
|
||||
process.env.ORCA_HEADLESS_BROWSER_RESIDENT_LIMIT = 'lots'
|
||||
expect(readOffscreenBrowserRetentionBudget().limit).toBe(OFFSCREEN_BROWSER_RESIDENT_LIMIT)
|
||||
})
|
||||
|
||||
it('reads scientific notation whole, not truncated to its mantissa', () => {
|
||||
// Why: parseInt('1e9') is 1 — an operator following the docs' "set the
|
||||
// idle window very high" advice would get a 1ms window instead.
|
||||
process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = '1e9'
|
||||
expect(readOffscreenBrowserRetentionBudget().idleMs).toBe(1_000_000_000)
|
||||
})
|
||||
|
||||
it('falls back on trailing garbage instead of truncating it', () => {
|
||||
process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = '1234abc'
|
||||
expect(readOffscreenBrowserRetentionBudget().idleMs).toBe(OFFSCREEN_BROWSER_IDLE_PARK_MS)
|
||||
})
|
||||
})
|
||||
|
||||
describe('nextOffscreenBrowserReclaimCheckAt', () => {
|
||||
|
||||
@@ -28,10 +28,14 @@ export const OFFSCREEN_BROWSER_PINNED_RECHECK_MS = OFFSCREEN_BROWSER_PARK_GRACE_
|
||||
|
||||
function readPositiveIntEnv(name: string, fallback: number): number {
|
||||
const raw = process.env[name]
|
||||
if (raw === undefined) {
|
||||
if (raw === undefined || raw.trim() === '') {
|
||||
return fallback
|
||||
}
|
||||
const parsed = Number.parseInt(raw, 10)
|
||||
// Why Number, not parseInt: parseInt('1e9') is 1, silently turning the
|
||||
// documented "set the idle window very high" advice into a 1ms window.
|
||||
// Number() consumes the whole string, so '1e9' parses fully and trailing
|
||||
// garbage falls back instead of truncating.
|
||||
const parsed = Number(raw)
|
||||
return Number.isInteger(parsed) && parsed >= 0 ? parsed : fallback
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import type { BrowserWindow } from 'electron'
|
||||
import type { OffscreenBrowserPage } from './offscreen-browser-open-pages'
|
||||
import { createOffscreenBrowserWindow } from './offscreen-browser-window'
|
||||
|
||||
// Why (STA-4341): building a page's renderer and wiring it to the page record
|
||||
// is one transaction — the window, the address tracking that keeps the record
|
||||
// wakeable, the loss handler that cleans up a crash, and the guest
|
||||
// registration. The backend owns what happens on those events; this module
|
||||
// owns that they are attached to every renderer identically, whether it came
|
||||
// from a create or a wake.
|
||||
|
||||
export type OffscreenBrowserRendererHooks = {
|
||||
/** A teardown the owner started on purpose — the loss handler stands down. */
|
||||
isDeliberateTeardown: (browserPageId: string) => boolean
|
||||
/** The renderer died out from under the owner (crash, app teardown). */
|
||||
onRendererLost: (webContentsId: number) => void
|
||||
/** Map the fresh WebContents into the guest registries. */
|
||||
registerGuest: (webContentsId: number) => void
|
||||
}
|
||||
|
||||
export function materializeOffscreenBrowserRenderer(
|
||||
page: OffscreenBrowserPage,
|
||||
hooks: OffscreenBrowserRendererHooks
|
||||
): void {
|
||||
const win = createOffscreenBrowserWindow(page.partition)
|
||||
try {
|
||||
attachWindow(page, win, hooks)
|
||||
} catch (error) {
|
||||
// Why: the window exists before anything that can fail. Abandoning it
|
||||
// here would leak a hidden renderer nothing owns or can reach.
|
||||
page.window = null
|
||||
if (!win.isDestroyed()) {
|
||||
win.destroy()
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
function attachWindow(
|
||||
page: OffscreenBrowserPage,
|
||||
win: BrowserWindow,
|
||||
hooks: OffscreenBrowserRendererHooks
|
||||
): void {
|
||||
page.window = win
|
||||
// Why: reading win.webContents once the contents are destroyed throws, and
|
||||
// the throw would escape the 'destroyed' listener into the main process.
|
||||
// Capture the id while it is still safe to read.
|
||||
const webContentsId = win.webContents.id
|
||||
// Why: the record's address must follow the page, not the create call — an
|
||||
// agent that navigates with `goto` or in-page script has to be woken back
|
||||
// to where it actually is. A chrome-error address is the failure, not a
|
||||
// destination, so it never replaces the address that produced it.
|
||||
const recordAddress = (url: string): void => {
|
||||
if (page.window === win && url && !url.startsWith('chrome-error://')) {
|
||||
page.url = url
|
||||
}
|
||||
}
|
||||
// did-navigate is main-frame only; did-frame-navigate covers subframes.
|
||||
win.webContents.on('did-navigate', (_event, url) => recordAddress(url))
|
||||
// Why: an iframe changing its own hash also fires did-navigate-in-page. A
|
||||
// subframe navigation is not a navigation of the tab, and adopting its
|
||||
// address would wake the page onto the iframe's document.
|
||||
win.webContents.on('did-navigate-in-page', (_event, url, isMainFrame) => {
|
||||
if (isMainFrame) {
|
||||
recordAddress(url)
|
||||
}
|
||||
})
|
||||
// Why: if the window is destroyed out from under us (crash, app teardown),
|
||||
// report the loss so commands fail cleanly instead of resolving a dead
|
||||
// WebContents. Parking destroys it deliberately, so it opts out here.
|
||||
win.webContents.once('destroyed', () => {
|
||||
if (hooks.isDeliberateTeardown(page.browserPageId) || page.window !== win) {
|
||||
return
|
||||
}
|
||||
hooks.onRendererLost(webContentsId)
|
||||
})
|
||||
hooks.registerGuest(webContentsId)
|
||||
}
|
||||
@@ -50,7 +50,13 @@ type FakeHeadlessHost = {
|
||||
closedPageIds: string[]
|
||||
}
|
||||
|
||||
function createFakeHeadlessHost(live: string[], parked: string[]): FakeHeadlessHost {
|
||||
function createFakeHeadlessHost(
|
||||
live: string[],
|
||||
parked: string[],
|
||||
// Why a separate order: creation order is what the real page book carries;
|
||||
// live-then-parked only coincides with it until something parks.
|
||||
creationOrder: string[] = [...live, ...parked]
|
||||
): FakeHeadlessHost {
|
||||
const state: FakeHeadlessHost = {
|
||||
host: null as unknown as RuntimeBrowserCommandHost,
|
||||
livePageIds: [...live],
|
||||
@@ -109,7 +115,11 @@ function createFakeHeadlessHost(live: string[], parked: string[]): FakeHeadlessH
|
||||
url: `https://example.test/${browserPageId}`,
|
||||
title: browserPageId
|
||||
})),
|
||||
getParkedPageIdForImplicitTarget: () => state.parkedPageIds.at(-1) ?? null
|
||||
getParkedPageIdForImplicitTarget: () => state.parkedPageIds.at(-1) ?? null,
|
||||
listOpenPageIds: () =>
|
||||
creationOrder.filter(
|
||||
(id) => state.livePageIds.includes(id) || state.parkedPageIds.includes(id)
|
||||
)
|
||||
} as unknown as BrowserBackend
|
||||
|
||||
state.host = {
|
||||
@@ -145,6 +155,22 @@ describe('headless parked-page targeting', () => {
|
||||
expect(fake.wakeCalls).toEqual([])
|
||||
})
|
||||
|
||||
it('keeps a tab index stable when an older tab parks', async () => {
|
||||
// Why: pre-reclaim, headless indices moved only on create/close. A listing
|
||||
// ordered by residency would let a background park renumber an index the
|
||||
// agent read minutes earlier, silently retargeting its next command.
|
||||
const { RuntimeBrowserCommands } = await import('./orca-runtime-browser')
|
||||
const fake = createFakeHeadlessHost(['live-b'], ['parked-a'], ['parked-a', 'live-b'])
|
||||
const commands = new RuntimeBrowserCommands(fake.host)
|
||||
|
||||
const listed = await commands.browserTabList({ worktree: 'id:wt-1' })
|
||||
|
||||
expect(listed.tabs.map((tab) => [tab.browserPageId, tab.index, tab.parked === true])).toEqual([
|
||||
['parked-a', 0, true],
|
||||
['live-b', 1, false]
|
||||
])
|
||||
})
|
||||
|
||||
it('switches to the page the listed index named, not one an implicit wake promoted', async () => {
|
||||
// Why: with every page parked, resolving the worktree first wakes the most
|
||||
// recently used one, which makes it live and moves it to listing index 0.
|
||||
|
||||
@@ -658,9 +658,11 @@ export class RuntimeBrowserCommands {
|
||||
private listBrowserTabsIncludingParked(
|
||||
worktreeId: string | undefined
|
||||
): BrowserTabListResult['tabs'] {
|
||||
const offscreen = this.host.getOffscreenBrowserBackend()
|
||||
const merged = mergeParkedBrowserTabs(
|
||||
this.requireAgentBrowserBridge().tabList(worktreeId).tabs,
|
||||
this.host.getOffscreenBrowserBackend()?.listParkedPages?.(worktreeId) ?? []
|
||||
offscreen?.listParkedPages?.(worktreeId) ?? [],
|
||||
offscreen?.listOpenPageIds?.(worktreeId)
|
||||
)
|
||||
return merged.map((tab) =>
|
||||
tab.parked
|
||||
|
||||
@@ -33491,7 +33491,8 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
return mergeParkedBrowserTabs(
|
||||
this.agentBrowserBridge.tabList(worktreeId).tabs,
|
||||
this.offscreenBrowserBackend?.listParkedPages?.(worktreeId) ?? []
|
||||
this.offscreenBrowserBackend?.listParkedPages?.(worktreeId) ?? [],
|
||||
this.offscreenBrowserBackend?.listOpenPageIds?.(worktreeId)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -238,11 +238,12 @@ test('resolves --index against the listing that includes parked tabs', async ({
|
||||
|
||||
const before = await listTabs(host, worktreeId)
|
||||
expect(before).toHaveLength(5)
|
||||
// Why: parked pages sit after the resident ones, so the last index is the
|
||||
// one the bridge's live-only index handling would never have reached.
|
||||
const targetIndex = before.length - 1
|
||||
// Why: the listing is creation-ordered, so parked and resident tabs are
|
||||
// interleaved. Target a parked one — an index the bridge's live-only index
|
||||
// handling would either miss or resolve to a different (live) tab.
|
||||
const targetIndex = before.findIndex((tab) => tab.parked === true)
|
||||
expect(targetIndex).toBeGreaterThanOrEqual(0)
|
||||
const targeted = before[targetIndex]
|
||||
expect(targeted.parked).toBe(true)
|
||||
|
||||
await host.client.call('browser.tabClose', {
|
||||
index: targetIndex,
|
||||
|
||||
Reference in New Issue
Block a user