diff --git a/extension/src/background.test.ts b/extension/src/background.test.ts index e6c830fd3..16728897a 100644 --- a/extension/src/background.test.ts +++ b/extension/src/background.test.ts @@ -339,6 +339,49 @@ describe('background tab isolation', () => { ]); }); + it('includes out-of-process iframe targets that are missing from the frame tree', async () => { + const { chrome } = createChromeMock(); + chrome.debugger.getTargets = vi.fn(async () => ([ + { id: 'page-1', tabId: 1, type: 'page', url: 'https://automation.example', title: 'automation' }, + { id: 'oopif-frame', tabId: 1, type: 'other', url: 'https://x.example/widget', title: 'oopif-x' }, + ])); + chrome.debugger.sendCommand = vi.fn(async (_target: unknown, method: string) => { + if (method === 'Runtime.enable') return {}; + if (method === 'Runtime.evaluate') return { result: { value: 1 } }; + if (method === 'Page.getFrameTree') { + return { + frameTree: { + frame: { id: 'root', url: 'https://main.example/' }, + childFrames: [ + { frame: { id: 'same-origin-parent', url: 'https://main.example/embed' } }, + ], + }, + }; + } + return {}; + }); + vi.stubGlobal('chrome', chrome); + + const mod = await import('./background'); + mod.__test__.setAutomationWindowId(adapterKey('twitter'), 1); + + const result = await mod.__test__.handleCommand({ id: 'frames', action: 'frames', session: 'twitter', surface: 'adapter' }); + const execResult = await mod.__test__.handleCommand({ + id: 'exec', + action: 'exec', + code: 'document.title', + frameIndex: 0, + session: 'twitter', + surface: 'adapter', + }); + + expect(result.ok).toBe(true); + expect(result.data).toEqual([ + { index: 0, frameId: 'oopif-frame', url: 'https://x.example/widget', name: 'oopif-x' }, + ]); + expect(execResult.ok).toBe(true); + }); + it('does not parse lease-key separators from command session fields', async () => { const { chrome } = createChromeMock(); vi.stubGlobal('chrome', chrome); @@ -441,6 +484,7 @@ describe('background tab isolation', () => { vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), registerFrameTracking: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => false), detach: vi.fn(async () => {}), ensureAttached: vi.fn(async () => {}), @@ -486,6 +530,7 @@ describe('background tab isolation', () => { vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), registerFrameTracking: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => false), detach: vi.fn(async () => {}), waitForDownload, @@ -522,6 +567,7 @@ describe('background tab isolation', () => { vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), registerFrameTracking: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => false), detach: vi.fn(async () => {}), evaluateAsync: vi.fn(async () => 'main-result'), @@ -548,6 +594,7 @@ describe('background tab isolation', () => { startNetworkCapture: vi.fn(), readNetworkCapture: vi.fn(async () => []), ensureAttached: vi.fn(), + getIframeTargets: vi.fn(async () => []), })); const mod = await import('./background'); @@ -581,6 +628,7 @@ describe('background tab isolation', () => { vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), registerFrameTracking: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => false), detach: vi.fn(async () => {}), evaluateAsync, @@ -592,6 +640,7 @@ describe('background tab isolation', () => { startNetworkCapture: vi.fn(), readNetworkCapture: vi.fn(async () => []), ensureAttached: vi.fn(), + getIframeTargets: vi.fn(async () => []), })); const mod = await import('./background'); @@ -800,6 +849,7 @@ describe('background tab isolation', () => { const detachMock = vi.fn(async () => {}); vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => true), detach: detachMock, })); @@ -1029,6 +1079,7 @@ describe('background tab isolation', () => { let maxInFlight = 0; vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), + registerFrameTracking: vi.fn(), evaluateAsync: vi.fn(async (tabId: number, code: string) => { inFlight++; maxInFlight = Math.max(maxInFlight, inFlight); @@ -1067,6 +1118,7 @@ describe('background tab isolation', () => { let maxInFlight = 0; vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), + registerFrameTracking: vi.fn(), evaluateAsync: vi.fn(async (tabId: number, code: string) => { inFlight++; maxInFlight = Math.max(maxInFlight, inFlight); @@ -1985,6 +2037,7 @@ describe('background tab isolation', () => { vi.doMock('./cdp', () => ({ registerListeners: vi.fn(), registerFrameTracking: vi.fn(), + registerFrameTracking: vi.fn(), hasActiveNetworkCapture: vi.fn(() => false), detach: vi.fn(async () => {}), })); diff --git a/extension/src/background.ts b/extension/src/background.ts index efcb9efa9..1cb7d358e 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -28,12 +28,23 @@ let connectInFlight: Promise | null = null; // initialize() replaces this with the real recovery promise; it always // resolves (never rejects) so gated handlers can never wedge permanently. let workerReady: Promise = Promise.resolve(); +let startupRecoveryError: Error | null = null; // Synchronous mirror of workerReady's settled state. Lets connect() skip the // `await workerReady` microtask hop once recovery is done, so the steady-state // (post-recovery) connect path is byte-for-byte the original — only the // pre-recovery wake is gated. let workerRecovered = true; +async function awaitWorkerReady(): Promise { + await workerReady; + if (startupRecoveryError) throw startupRecoveryError; +} + +function isMissingTabOrWindowError(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return /No tab with id|No window with id|No window with id/i.test(message); +} + async function getCurrentContextId(): Promise { if (contextIdPromise) return contextIdPromise; contextIdPromise = (async () => { @@ -51,7 +62,8 @@ async function getCurrentContextId(): Promise { currentContextId = generated; return currentContextId; } catch { - return currentContextId; + console.error('[opencli] Failed to load or persist the browser context id'); + throw new Error('Failed to load or persist the browser context id'); } })(); return contextIdPromise; @@ -503,42 +515,45 @@ function emptyRegistry(): StoredRegistry { } async function readRegistry(): Promise { - try { - const session = chrome.storage?.session; - if (!session) return emptyRegistry(); // no session storage — degrade to memory-only - const raw = await session.get(REGISTRY_KEY) as Record; - const stored = raw[REGISTRY_KEY] as Partial | undefined; - if (!stored || stored.version !== 2 || typeof stored.leases !== 'object') return emptyRegistry(); - const storedContainers = stored.ownedContainers && typeof stored.ownedContainers === 'object' - ? stored.ownedContainers - : emptyRegistry().ownedContainers; - return { - version: 2, - contextId: currentContextId, - ownedContainers: { - interactive: { - windowId: typeof storedContainers.interactive?.windowId === 'number' ? storedContainers.interactive.windowId : null, - groupIds: Array.isArray(storedContainers.interactive?.groupIds) - ? storedContainers.interactive.groupIds.filter((id): id is number => typeof id === 'number') - : [], - }, - automation: { - windowId: typeof storedContainers.automation?.windowId === 'number' ? storedContainers.automation.windowId : null, - }, - }, - leases: stored.leases as Record, - }; - } catch { + const session = chrome.storage?.session; + if (!session) { + console.warn('[opencli] Session storage unavailable while reading registry'); + return emptyRegistry(); + } + const raw = await session.get(REGISTRY_KEY) as Record; + const stored = raw[REGISTRY_KEY] as Partial | undefined; + if (!stored) { + console.warn('[opencli] No registry data found in session storage'); return emptyRegistry(); } + if (stored.version !== 2 || typeof stored.leases !== 'object') { + throw new Error('Invalid registry data while reading registry'); + } + const storedContainers = stored.ownedContainers && typeof stored.ownedContainers === 'object' + ? stored.ownedContainers + : emptyRegistry().ownedContainers; + return { + version: 2, + contextId: currentContextId, + ownedContainers: { + interactive: { + windowId: typeof storedContainers.interactive?.windowId === 'number' ? storedContainers.interactive.windowId : null, + groupIds: Array.isArray(storedContainers.interactive?.groupIds) + ? storedContainers.interactive.groupIds.filter((id): id is number => typeof id === 'number') + : [], + }, + automation: { + windowId: typeof storedContainers.automation?.windowId === 'number' ? storedContainers.automation.windowId : null, + }, + }, + leases: stored.leases as Record, + }; } async function writeRegistry(registry: StoredRegistry): Promise { - try { - await chrome.storage?.session?.set({ [REGISTRY_KEY]: registry }); - } catch { - // Registry persistence is a recovery aid; command execution should not fail on storage errors. - } + const session = chrome.storage?.session; + if (!session) throw new Error('Session storage unavailable while persisting registry'); + await session.set({ [REGISTRY_KEY]: registry }); } async function persistRuntimeState(): Promise { @@ -575,14 +590,12 @@ async function persistRuntimeState(): Promise { function scheduleIdleAlarm(leaseKey: string, timeout: number): void { const alarmName = makeAlarmName(leaseKey); - try { - if (timeout > 0) { - chrome.alarms?.create?.(alarmName, { when: Date.now() + timeout }); - } else { - chrome.alarms?.clear?.(alarmName); - } - } catch { - // setTimeout remains the in-process fast path; alarms are the MV3 restart recovery path. + const alarms = chrome.alarms; + if (!alarms) throw new Error('Alarms API unavailable while updating idle alarm'); + if (timeout > 0) { + alarms.create(alarmName, { when: Date.now() + timeout }); + } else { + alarms.clear(alarmName); } } @@ -1154,7 +1167,7 @@ async function getAutomationWindow(leaseKey: string, initialUrl?: string): Promi chrome.windows.onRemoved.addListener(async (windowId) => { // A window-close event can wake the worker before recovery; persisting the // empty pre-recovery snapshot here would wipe the registry. - await workerReady; + await awaitWorkerReady(); for (const container of Object.values(ownedContainers)) { if (container.windowId === windowId) { container.windowId = null; @@ -1176,7 +1189,7 @@ chrome.windows.onRemoved.addListener(async (windowId) => { // Evict identity mappings when tabs are closed chrome.tabs.onRemoved.addListener(async (tabId) => { // Same wake-before-recovery hazard as windows.onRemoved. - await workerReady; + await awaitWorkerReady(); identity.evictTab(tabId); for (const [leaseKey, session] of automationSessions.entries()) { if (session.preferredTabId === tabId) { @@ -1199,12 +1212,8 @@ function initialize(): void { initialized = true; chrome.alarms.create('keepalive', { periodInMinutes: 0.5 }); // Chrome production minimum: 30 seconds executor.registerListeners(); - try { - const registerFrameTracking = (executor as { registerFrameTracking?: () => void }).registerFrameTracking; - registerFrameTracking?.(); - } catch { - // Some focused tests mock only the cdp functions they exercise. - } + const registerFrameTracking = Reflect.get(executor as object, 'registerFrameTracking') as (() => void) | undefined; + if (typeof registerFrameTracking === 'function') registerFrameTracking(); // Migration cleanup: older versions persisted the registry in // chrome.storage.local, where its browser-session-scoped ids go stale after // a browser restart (see StoredRegistry). Remove that one legacy key — @@ -1224,13 +1233,15 @@ function initialize(): void { await getCurrentContextId(); await reconcileTargetLeaseRegistry(); })().catch((err) => { - // Never leave workerReady rejected/pending: a wedged gate would freeze - // every gated handler for the life of the worker. - console.warn(`[opencli] Startup recovery failed: ${err instanceof Error ? err.message : String(err)}`); - }).finally(() => { + startupRecoveryError = err instanceof Error ? err : new Error(String(err)); + console.warn(`[opencli] Startup recovery failed: ${startupRecoveryError.message}`); + }); + void workerReady.finally(() => { workerRecovered = true; }); - void workerReady.then(() => connect()); + void workerReady.then(() => { + if (!startupRecoveryError) connect(); + }); console.log('[opencli] OpenCLI extension initialized'); } @@ -1250,7 +1261,7 @@ initialize(); chrome.alarms.onAlarm.addListener(async (alarm) => { // Idle-lease alarms and keepalive can both fire in a freshly woken worker; // gate on recovery so releaseLease never persists an empty snapshot. - await workerReady; + await awaitWorkerReady(); if (alarm.name === 'keepalive') void connect(); const leaseKey = leaseKeyFromAlarmName(alarm.name); if (!leaseKey) return; @@ -1415,7 +1426,10 @@ function getUrlOrigin(url: string | undefined): string | null { } } -function enumerateCrossOriginFrames(tree: any): Array<{ index: number; frameId: string; url: string; name: string }> { +function enumerateCrossOriginFrames( + tree: any, + extraTargets: Array<{ targetId: string; url: string; name: string }> = [], +): Array<{ index: number; frameId: string; url: string; name: string }> { const frames: Array<{ index: number; frameId: string; url: string; name: string }> = []; function collect(node: any, accessibleOrigin: string | null) { @@ -1444,6 +1458,18 @@ function enumerateCrossOriginFrames(tree: any): Array<{ index: number; frameId: const rootFrame = tree?.frameTree?.frame; const rootUrl = rootFrame?.url || rootFrame?.unreachableUrl || ''; collect(tree.frameTree, getUrlOrigin(rootUrl)); + + for (const target of extraTargets) { + if (!target.targetId) continue; + if (frames.some((frame) => frame.frameId === target.targetId)) continue; + frames.push({ + index: frames.length, + frameId: target.targetId, + url: target.url, + name: target.name || '', + }); + } + return frames; } @@ -1682,7 +1708,7 @@ async function handleExec(cmd: Command, leaseKey: string): Promise { const aggressive = getSurfaceFromKey(leaseKey) === 'browser'; if (cmd.frameIndex != null) { const tree = await executor.getFrameTree(tabId); - const frames = enumerateCrossOriginFrames(tree); + const frames = enumerateCrossOriginFrames(tree, await executor.getIframeTargets(tabId)); if (cmd.frameIndex < 0 || cmd.frameIndex >= frames.length) { return { id: cmd.id, ok: false, error: `Frame index ${cmd.frameIndex} out of range (${frames.length} cross-origin frames available)` }; } @@ -1701,7 +1727,8 @@ async function handleFrames(cmd: Command, leaseKey: string): Promise { const tabId = await resolveTabId(cmdTabId, leaseKey); try { const tree = await executor.getFrameTree(tabId); - return { id: cmd.id, ok: true, data: enumerateCrossOriginFrames(tree) }; + const targets = await executor.getIframeTargets(tabId); + return { id: cmd.id, ok: true, data: enumerateCrossOriginFrames(tree, targets) }; } catch (err) { return errorResult(cmd.id, err); } @@ -2144,7 +2171,7 @@ async function releaseLease(leaseKey: string, reason: string = 'released'): Prom await safeDetach(tabId); identity.evictTab(tabId); if (hasOtherOwnedLease) { - await chrome.tabs.remove(tabId).catch(() => {}); + await chrome.tabs.remove(tabId); console.log(`[opencli] Released owned tab lease ${tabId} (session=${session.session}, surface=${session.surface}, ${reason})`); } else { try { @@ -2153,7 +2180,7 @@ async function releaseLease(leaseKey: string, reason: string = 'released'): Prom if (group) session.windowId = group.windowId; console.log(`[opencli] Released owned tab lease ${tabId} as reusable placeholder (session=${session.session}, surface=${session.surface}, ${reason})`); } catch { - await chrome.tabs.remove(tabId).catch(() => {}); + await chrome.tabs.remove(tabId); console.log(`[opencli] Released owned tab lease ${tabId} (session=${session.session}, surface=${session.surface}, ${reason})`); } } @@ -2186,7 +2213,8 @@ async function reconcileTargetLeaseRegistry(): Promise { if (windowId !== null) { try { await chrome.windows.get(windowId); - } catch { + } catch (err) { + if (!isMissingTabOrWindowError(err)) throw err; ownedContainers[role].windowId = null; } } @@ -2235,7 +2263,8 @@ async function reconcileTargetLeaseRegistry(): Promise { resetWindowIdleTimer(leaseKey, remaining); } } - } catch { + } catch (err) { + if (!isMissingTabOrWindowError(err)) throw err; // Registry is semantic state, not truth. If Chrome no longer has the tab, // drop the lease record and never close unrelated user resources. } diff --git a/extension/src/cdp.test.ts b/extension/src/cdp.test.ts index 3d96fd7ad..170b262e7 100644 --- a/extension/src/cdp.test.ts +++ b/extension/src/cdp.test.ts @@ -16,6 +16,7 @@ function createChromeMock() { const debuggerApi = { attach: vi.fn(async () => {}), detach: vi.fn(async () => {}), + getTargets: vi.fn(async () => []), sendCommand: vi.fn(async (_target: unknown, method: string) => { if (method === 'Runtime.evaluate') return { result: { value: 'ok' } }; return {}; @@ -95,10 +96,10 @@ describe('cdp attach recovery', () => { it('falls back to a frame target when no same-target execution context exists', async () => { const { chrome, debuggerApi, debuggerEventListeners } = createChromeMock(); + debuggerApi.getTargets = vi.fn(async () => ([ + { id: 'oopif-frame', tabId: 1, type: 'other', url: 'https://frame.test', title: 'oopif' }, + ])); debuggerApi.sendCommand = vi.fn(async (target: any, method: string, _params?: any) => { - if (method === 'Target.setDiscoverTargets') return {}; - if (method === 'Target.setAutoAttach') return {}; - if (method === 'Target.getTargets') return { targetInfos: [{ targetId: 'oopif-frame', type: 'iframe', url: 'https://frame.test' }] }; if (target?.targetId === 'oopif-frame' && method === 'Runtime.enable') return {}; if (target?.targetId === 'oopif-frame' && method === 'Runtime.evaluate') { return { result: { value: 'frame-ok' } }; @@ -122,6 +123,36 @@ describe('cdp attach recovery', () => { ); }); + it('resolves out-of-process frame targets from chrome.debugger.getTargets', async () => { + const { chrome, debuggerApi, debuggerEventListeners } = createChromeMock(); + debuggerApi.getTargets = vi.fn(async () => ([ + { id: 'oopif-frame', tabId: 1, type: 'other', url: 'https://frame.test', title: 'oopif' }, + ])); + debuggerApi.sendCommand = vi.fn(async (target: any, method: string, _params?: any) => { + if (method === 'Target.getTargets') throw new Error('Target.getTargets should not be called'); + if (target?.targetId === 'oopif-frame' && method === 'Runtime.enable') return {}; + if (target?.targetId === 'oopif-frame' && method === 'Runtime.evaluate') { + return { result: { value: 'frame-ok' } }; + } + return {}; + }); + vi.stubGlobal('chrome', chrome); + + const mod = await import('./cdp'); + mod.registerFrameTracking(); + + const result = await mod.evaluateInFrame(1, 'document.title', 'oopif-frame'); + + expect(result).toBe('frame-ok'); + expect(debuggerApi.getTargets).toHaveBeenCalled(); + expect(debuggerApi.sendCommand).toHaveBeenCalledWith( + { targetId: 'oopif-frame' }, + 'Runtime.evaluate', + expect.any(Object), + ); + expect(debuggerEventListeners.length).toBeGreaterThanOrEqual(1); + }); + }); function chromeMockForScreenshot(content: { width: number; height: number } = { width: 1024, height: 2048 }) { @@ -567,17 +598,15 @@ describe('cdp evaluateInFrame stale context fallback', () => { const debuggerApi = { attach: vi.fn(async () => {}), detach: vi.fn(async () => {}), + getTargets: vi.fn(async () => ([ + { id: 'stale-frame', tabId: 1, type: 'other', url: 'https://frame.test', title: 'stale' }, + ])), sendCommand: vi.fn(async (target, method, params) => { if (method === 'Runtime.enable') return {}; // The cached context id is stale after the frame navigated: CDP rejects. if (method === 'Runtime.evaluate' && params?.contextId === 99) { throw new Error('Cannot find context with specified id'); } - if (method === 'Target.setDiscoverTargets') return {}; - if (method === 'Target.setAutoAttach') return {}; - if (method === 'Target.getTargets') { - return { targetInfos: [{ targetId: 'stale-frame', type: 'iframe', url: 'https://frame.test' }] }; - } if (target?.targetId === 'stale-frame' && method === 'Runtime.evaluate') { return { result: { value: 'frame-ok' } }; } diff --git a/extension/src/cdp.ts b/extension/src/cdp.ts index c2ce05149..539926752 100644 --- a/extension/src/cdp.ts +++ b/extension/src/cdp.ts @@ -534,6 +534,18 @@ function clearFrameTarget(targetId: string): void { frameTargetKeys.delete(targetId); } +export async function getIframeTargets(tabId: number): Promise> { + const targets = await chrome.debugger.getTargets(); + return targets + .filter((candidate) => candidate.tabId === tabId && (candidate.type === 'iframe' || candidate.type === 'other')) + .map((candidate) => ({ + targetId: String(candidate.id || ''), + url: String(candidate.url || ''), + name: String(candidate.title || ''), + })) + .filter((candidate) => candidate.targetId); +} + async function ensureFrameTarget( tabId: number, frameId: string, @@ -545,14 +557,6 @@ async function ensureFrameTarget( const key = frameTargetKey(tabId, frameId); const existing = frameTargets.get(key); if (existing) return existing; - - await sendDebuggerCommand({ tabId }, 'Target.setDiscoverTargets', { discover: true }).catch(() => {}); - await sendDebuggerCommand({ tabId }, 'Target.setAutoAttach', { - autoAttach: true, - waitForDebuggerOnStart: false, - flatten: true, - filter: [{ type: 'iframe', exclude: false }], - }).catch(() => {}); const targetId = await resolveFrameTargetId(tabId, frameId, targetUrl); try { await chrome.debugger.attach({ targetId } as chrome.debugger.Debuggee, '1.3'); @@ -566,22 +570,13 @@ async function ensureFrameTarget( } async function resolveFrameTargetId(tabId: number, frameId: string, targetUrl?: string): Promise { - const result = await sendDebuggerCommand({ tabId }, 'Target.getTargets').catch(() => null) as - | { targetInfos?: Array<{ targetId?: string; id?: string; type?: string; url?: string }> } - | null; - const targets = result?.targetInfos ?? []; - const frameTarget = targets.find((candidate) => { - const candidateId = candidate.targetId || candidate.id; - return candidate.type === 'iframe' - && ( - candidateId === frameId - || (!!targetUrl && candidate.url === targetUrl) - ); - }); - const targetId = frameTarget?.targetId || frameTarget?.id; - if (targetId) return targetId; - const candidates = targets - .filter((target) => target.type === 'iframe') + const debuggerTargets = await getIframeTargets(tabId); + const targetFromDebugger = debuggerTargets.find((candidate) => ( + candidate.targetId === frameId + || (!!targetUrl && candidate.url === targetUrl) + )); + if (targetFromDebugger) return targetFromDebugger.targetId; + const candidates = debuggerTargets .map((target) => `${target.targetId || target.id || '?'} ${target.url || ''}`) .join('; '); throw new Error(`No iframe target found for frame ${frameId}${targetUrl ? ` (${targetUrl})` : ''}. Candidates: ${candidates || 'none'}`); @@ -659,7 +654,7 @@ export async function evaluateInFrame( ): Promise { await ensureAttached(tabId, aggressiveRetry); - await sendDebuggerCommand({ tabId }, 'Runtime.enable').catch(() => {}); + await sendDebuggerCommand({ tabId }, 'Runtime.enable'); const contexts = tabFrameContexts.get(tabId); const contextId = contexts?.get(frameId); @@ -698,7 +693,7 @@ export async function evaluateInFrame( } // No cached context, or the cached one went stale: resolve via the frame target. - await sendCommandInFrameTarget(tabId, frameId, 'Runtime.enable', {}, aggressiveRetry, timeoutMs).catch(() => undefined); + await sendCommandInFrameTarget(tabId, frameId, 'Runtime.enable', {}, aggressiveRetry, timeoutMs); const result = await sendCommandInFrameTarget(tabId, frameId, 'Runtime.evaluate', { expression, returnByValue: true,