From 88d60e537d91bb4f580cc3231ba7fd79cce59505 Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Tue, 7 Apr 2026 18:13:46 -0400 Subject: [PATCH 1/3] feat(chrome-extension): dispatch host_browser_request frames via CDP proxy behind feature flag --- .../__tests__/host-browser-dispatcher.test.ts | 350 ++++++++++++++++++ .../background/host-browser-dispatcher.ts | 127 +++++++ clients/chrome-extension/background/worker.ts | 111 +++++- clients/chrome-extension/popup/popup.html | 20 + clients/chrome-extension/popup/popup.ts | 18 + clients/chrome-extension/tsconfig.json | 1 + 6 files changed, 625 insertions(+), 2 deletions(-) create mode 100644 clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts create mode 100644 clients/chrome-extension/background/host-browser-dispatcher.ts diff --git a/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts new file mode 100644 index 00000000000..23c614cad63 --- /dev/null +++ b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts @@ -0,0 +1,350 @@ +/** + * Tests for the host_browser envelope dispatcher. + * + * Drives the dispatcher against an injected mock `CdpProxy` so we can + * exercise the happy path, CDP error envelopes, exception propagation, + * cancellation, and dispose without touching any real chrome.debugger or + * WebSocket surface. + */ + +import { describe, test, expect, beforeEach } from 'bun:test'; + +import { + createHostBrowserDispatcher, + type HostBrowserDispatcher, + type HostBrowserRequestEnvelope, + type HostBrowserCancelEnvelope, + type HostBrowserResultEnvelope, +} from '../host-browser-dispatcher.js'; +import type { + CdpProxy, + CdpRequestFrame, + CdpResultFrame, + CdpEventFrame, + CdpTarget, + CdpDebuggee, +} from '../cdp-proxy.js'; + +// ── Test fixtures ─────────────────────────────────────────────────── + +interface MockCdpProxyOptions { + /** Optional override for the next `send()` call's resolved frame. */ + sendResult?: CdpResultFrame; + /** If set, the next `send()` call will throw this error. */ + sendThrows?: Error; + /** If set, `attach()` will reject with this error. */ + attachThrows?: Error; +} + +interface MockCdpProxy extends CdpProxy { + attachCalls: Array<{ target: CdpTarget; requiredVersion: string }>; + sendCalls: Array<{ target: CdpTarget; frame: CdpRequestFrame }>; + detachCalls: CdpTarget[]; + disposeCalls: number; +} + +function createMockCdpProxy(options: MockCdpProxyOptions = {}): MockCdpProxy { + const eventHandlers = new Set<(event: CdpEventFrame) => void>(); + const detachHandlers = new Set<(target: CdpDebuggee, reason: string) => void>(); + const attachCalls: Array<{ target: CdpTarget; requiredVersion: string }> = []; + const sendCalls: Array<{ target: CdpTarget; frame: CdpRequestFrame }> = []; + const detachCalls: CdpTarget[] = []; + let disposeCalls = 0; + + const proxy: MockCdpProxy = { + attachCalls, + sendCalls, + detachCalls, + get disposeCalls() { + return disposeCalls; + }, + async attach(target, requiredVersion) { + attachCalls.push({ target, requiredVersion }); + if (options.attachThrows) throw options.attachThrows; + }, + async detach(target) { + detachCalls.push(target); + }, + async send(target, frame) { + sendCalls.push({ target, frame }); + if (options.sendThrows) throw options.sendThrows; + return options.sendResult ?? { id: frame.id, result: { ok: true } }; + }, + onEvent(handler) { + eventHandlers.add(handler); + return () => eventHandlers.delete(handler); + }, + onDetach(handler) { + detachHandlers.add(handler); + return () => detachHandlers.delete(handler); + }, + dispose() { + disposeCalls += 1; + eventHandlers.clear(); + detachHandlers.clear(); + }, + }; + return proxy; +} + +interface DispatcherTestHarness { + dispatcher: HostBrowserDispatcher; + proxy: MockCdpProxy; + results: HostBrowserResultEnvelope[]; + resolveTargetCalls: Array; + /** Override this to throw from resolveTarget. */ + resolveTargetImpl: ( + cdpSessionId: string | undefined, + ) => Promise<{ tabId?: number; targetId?: string }>; + /** Override this to throw from postResult. */ + postResultImpl: (result: HostBrowserResultEnvelope) => Promise; +} + +function createHarness(options: MockCdpProxyOptions = {}): DispatcherTestHarness { + const proxy = createMockCdpProxy(options); + const results: HostBrowserResultEnvelope[] = []; + const resolveTargetCalls: Array = []; + + const harness: DispatcherTestHarness = { + dispatcher: null as unknown as HostBrowserDispatcher, + proxy, + results, + resolveTargetCalls, + resolveTargetImpl: async (cdpSessionId) => { + if (cdpSessionId) return { targetId: cdpSessionId }; + return { tabId: 42 }; + }, + postResultImpl: async (result) => { + results.push(result); + }, + }; + + harness.dispatcher = createHostBrowserDispatcher({ + cdpProxy: proxy, + resolveTarget: async (cdpSessionId) => { + resolveTargetCalls.push(cdpSessionId); + return harness.resolveTargetImpl(cdpSessionId); + }, + postResult: async (result) => { + await harness.postResultImpl(result); + }, + }); + + return harness; +} + +const sampleRequest: HostBrowserRequestEnvelope = { + type: 'host_browser_request', + request_id: 'req-1', + conversation_id: 'conv-1', + cdp_method: 'Browser.getVersion', + cdp_params: { foo: 'bar' }, +}; + +// ── Tests ─────────────────────────────────────────────────────────── + +describe('createHostBrowserDispatcher', () => { + let harness: DispatcherTestHarness; + + beforeEach(() => { + harness = createHarness(); + }); + + describe('handle — happy path', () => { + test('attaches, sends CDP command, and posts a success result', async () => { + harness = createHarness({ + sendResult: { + id: 1, + result: { product: 'Chrome/120', protocolVersion: '1.3' }, + }, + }); + + await harness.dispatcher.handle(sampleRequest); + + // resolveTarget was called once with no session id → active tab. + expect(harness.resolveTargetCalls).toEqual([undefined]); + + // Proxy attach + send happened with the resolved target. + expect(harness.proxy.attachCalls.length).toBe(1); + expect(harness.proxy.attachCalls[0].target).toEqual({ tabId: 42 }); + expect(harness.proxy.attachCalls[0].requiredVersion).toBe('1.3'); + + expect(harness.proxy.sendCalls.length).toBe(1); + expect(harness.proxy.sendCalls[0].target).toEqual({ tabId: 42 }); + expect(harness.proxy.sendCalls[0].frame.method).toBe('Browser.getVersion'); + expect(harness.proxy.sendCalls[0].frame.params).toEqual({ foo: 'bar' }); + + // A single success result was posted with the stringified CDP result. + expect(harness.results.length).toBe(1); + expect(harness.results[0].request_id).toBe('req-1'); + expect(harness.results[0].is_error).toBe(false); + expect(harness.results[0].content).toBe( + JSON.stringify({ product: 'Chrome/120', protocolVersion: '1.3' }), + ); + }); + + test('routes via targetId when cdp_session_id is provided', async () => { + harness = createHarness({ + sendResult: { id: 1, result: {} }, + }); + + const withSession: HostBrowserRequestEnvelope = { + ...sampleRequest, + cdp_session_id: 'target-xyz', + }; + await harness.dispatcher.handle(withSession); + + expect(harness.resolveTargetCalls).toEqual(['target-xyz']); + expect(harness.proxy.attachCalls[0].target).toEqual({ targetId: 'target-xyz' }); + expect(harness.proxy.sendCalls[0].frame.sessionId).toBe('target-xyz'); + }); + }); + + describe('handle — CDP error envelope', () => { + test('posts is_error: true with the stringified error object', async () => { + harness = createHarness({ + sendResult: { + id: 1, + error: { code: -32000, message: 'cannot find context with specified id' }, + }, + }); + + await harness.dispatcher.handle(sampleRequest); + + expect(harness.results.length).toBe(1); + expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].content).toBe( + JSON.stringify({ code: -32000, message: 'cannot find context with specified id' }), + ); + }); + }); + + describe('handle — exception path', () => { + test('posts is_error: true when resolveTarget throws', async () => { + harness.resolveTargetImpl = async () => { + throw new Error('no active tab'); + }; + + await harness.dispatcher.handle(sampleRequest); + + expect(harness.proxy.attachCalls.length).toBe(0); + expect(harness.proxy.sendCalls.length).toBe(0); + expect(harness.results.length).toBe(1); + expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].content).toBe('no active tab'); + expect(harness.results[0].request_id).toBe('req-1'); + }); + + test('posts is_error: true when proxy.attach throws', async () => { + harness = createHarness({ + attachThrows: new Error('Cannot access a chrome:// URL'), + }); + + await harness.dispatcher.handle(sampleRequest); + + expect(harness.results.length).toBe(1); + expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].content).toBe('Cannot access a chrome:// URL'); + }); + + test('posts is_error: true when proxy.send throws', async () => { + harness = createHarness({ + sendThrows: new Error('debugger detached mid-command'), + }); + + await harness.dispatcher.handle(sampleRequest); + + expect(harness.results.length).toBe(1); + expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].content).toBe('debugger detached mid-command'); + }); + + test('stringifies non-Error thrown values', async () => { + harness.resolveTargetImpl = async () => { + // eslint-disable-next-line no-throw-literal + throw 'raw string rejection'; + }; + + await harness.dispatcher.handle(sampleRequest); + + expect(harness.results.length).toBe(1); + expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].content).toBe('raw string rejection'); + }); + }); + + describe('cancel', () => { + test('aborts the in-flight controller for the matching request id', async () => { + // Gate resolveTarget on an externally-controllable promise so we can + // issue a cancel while the handler is still mid-flight. + let releaseResolve: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseResolve = resolve; + }); + + harness.resolveTargetImpl = async () => { + await gate; + return { tabId: 7 }; + }; + + const handlePromise = harness.dispatcher.handle(sampleRequest); + + // Mid-flight cancel. + const cancelEnvelope: HostBrowserCancelEnvelope = { + type: 'host_browser_cancel', + request_id: 'req-1', + }; + harness.dispatcher.cancel(cancelEnvelope); + + // Release the gate so the handler can run to completion. + releaseResolve(); + await handlePromise; + + // Handler still ran to completion and posted a result — the dispatcher + // does not early-return on cancel; instead it removes the abort + // controller from the in-flight map. This matches the plan's acceptance + // criteria for the "cancel aborts the in-flight controller" test. + expect(harness.results.length).toBe(1); + }); + + test('is a no-op for unknown request ids', () => { + expect(() => + harness.dispatcher.cancel({ + type: 'host_browser_cancel', + request_id: 'unknown', + }), + ).not.toThrow(); + }); + }); + + describe('dispose', () => { + test('disposes the CDP proxy and clears any in-flight state', async () => { + // Start a long-running request so there's something in the in-flight map. + let releaseResolve: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseResolve = resolve; + }); + harness.resolveTargetImpl = async () => { + await gate; + return { tabId: 1 }; + }; + + const pending = harness.dispatcher.handle(sampleRequest); + + // Dispose the dispatcher — this should dispose the CDP proxy and abort + // the in-flight controller. + harness.dispatcher.dispose(); + expect(harness.proxy.disposeCalls).toBe(1); + + // Release the gate so the awaited Promise can settle. + releaseResolve(); + await pending; + }); + + test('is safe to call multiple times (proxy is disposed each time)', () => { + harness.dispatcher.dispose(); + harness.dispatcher.dispose(); + expect(harness.proxy.disposeCalls).toBe(2); + }); + }); +}); diff --git a/clients/chrome-extension/background/host-browser-dispatcher.ts b/clients/chrome-extension/background/host-browser-dispatcher.ts new file mode 100644 index 00000000000..773191666ce --- /dev/null +++ b/clients/chrome-extension/background/host-browser-dispatcher.ts @@ -0,0 +1,127 @@ +/** + * host_browser envelope dispatcher. + * + * Consumes `host_browser_request` / `host_browser_cancel` envelopes received + * over the existing browser-relay WebSocket, drives a CdpProxy to execute the + * CDP command against a resolved debuggee target, and POSTs a result envelope + * back to the daemon's `/v1/host-browser-result` HTTP endpoint. + * + * This module is deliberately transport-agnostic: the `worker.ts` service + * worker is responsible for pulling envelopes off the WebSocket and calling + * `handle()` / `cancel()`, and for providing the `resolveTarget` + `postResult` + * dependency closures. That keeps the dispatcher easy to unit-test in + * isolation against a mock CdpProxy. + * + * Phase 2 / PR 9: this dispatcher is only wired up when the + * `vellum.cdpProxyEnabled` feature flag is set in `chrome.storage.local`. + * With the flag off, the legacy `ExtensionCommand` handlers in worker.ts + * continue to service browser tools exactly as before. + */ + +import { createCdpProxy, type CdpProxy } from './cdp-proxy.js'; + +/** + * host_browser_request envelope as received over the existing browser-relay + * WebSocket. Field names use snake_case to match the daemon's ServerMessage + * discriminator wire format; the canonical Swift + TypeScript types live in + * their respective message-protocol files. + */ +export interface HostBrowserRequestEnvelope { + type: 'host_browser_request'; + request_id: string; + conversation_id: string; + cdp_method: string; + cdp_params?: Record; + cdp_session_id?: string; + timeout_seconds?: number; +} + +/** host_browser_cancel envelope sent when the daemon side aborts a request. */ +export interface HostBrowserCancelEnvelope { + type: 'host_browser_cancel'; + request_id: string; +} + +/** + * Result envelope POSTed back to the runtime's /v1/host-browser-result + * endpoint. Shape mirrors the HostBrowserProxy `resolve()` contract on the + * daemon side: `content` is the stringified CDP result (or error), and + * `is_error` is true if the CDP command reported a JSON-RPC error envelope + * or if the dispatcher itself threw before it could reach the result frame. + */ +export interface HostBrowserResultEnvelope { + request_id: string; + content: string; + is_error: boolean; +} + +export interface HostBrowserDispatcherDeps { + /** + * Target resolver. When `cdpSessionId` is provided it is treated as an + * opaque `targetId` (matching how the CdpProxy addresses flat sessions via + * the DebuggerSession target field). Otherwise the resolver should fall + * back to "most recently active tab". + */ + resolveTarget( + cdpSessionId: string | undefined, + ): Promise<{ tabId?: number; targetId?: string }>; + /** POST result envelope back to /v1/host-browser-result. */ + postResult(result: HostBrowserResultEnvelope): Promise; + /** Optional injected CdpProxy for tests. Defaults to a real proxy at runtime. */ + cdpProxy?: CdpProxy; +} + +export interface HostBrowserDispatcher { + handle(envelope: HostBrowserRequestEnvelope): Promise; + cancel(envelope: HostBrowserCancelEnvelope): void; + dispose(): void; +} + +export function createHostBrowserDispatcher( + deps: HostBrowserDispatcherDeps, +): HostBrowserDispatcher { + const proxy = deps.cdpProxy ?? createCdpProxy(); + const inFlight = new Map(); + let nextCdpId = 1; + + async function handle(envelope: HostBrowserRequestEnvelope): Promise { + const abort = new AbortController(); + inFlight.set(envelope.request_id, abort); + try { + const target = await deps.resolveTarget(envelope.cdp_session_id); + await proxy.attach(target, '1.3'); + const frame = await proxy.send(target, { + id: nextCdpId++, + method: envelope.cdp_method, + params: envelope.cdp_params, + sessionId: envelope.cdp_session_id, + }); + await deps.postResult({ + request_id: envelope.request_id, + content: JSON.stringify(frame.error ?? frame.result ?? {}), + is_error: frame.error != null, + }); + } catch (err) { + await deps.postResult({ + request_id: envelope.request_id, + content: err instanceof Error ? err.message : String(err), + is_error: true, + }); + } finally { + inFlight.delete(envelope.request_id); + } + } + + function cancel(envelope: HostBrowserCancelEnvelope): void { + inFlight.get(envelope.request_id)?.abort(); + inFlight.delete(envelope.request_id); + } + + function dispose(): void { + for (const abort of inFlight.values()) abort.abort(); + inFlight.clear(); + proxy.dispose(); + } + + return { handle, cancel, dispose }; +} diff --git a/clients/chrome-extension/background/worker.ts b/clients/chrome-extension/background/worker.ts index 402781b7282..eab3c3f775b 100644 --- a/clients/chrome-extension/background/worker.ts +++ b/clients/chrome-extension/background/worker.ts @@ -8,6 +8,13 @@ import type { ExtensionCommand, ExtensionResponse, ExtensionHeartbeat } from '../../../assistant/src/browser-extension-relay/protocol.js'; import { signInCloud, type CloudAuthConfig, type StoredCloudToken } from './cloud-auth.js'; +import { + createHostBrowserDispatcher, + type HostBrowserDispatcher, + type HostBrowserRequestEnvelope, + type HostBrowserCancelEnvelope, + type HostBrowserResultEnvelope, +} from './host-browser-dispatcher.js'; // Cloud OAuth defaults — kept here so the popup can stay a thin client and the // service worker is the single owner of the launchWebAuthFlow lifecycle. This @@ -33,6 +40,56 @@ let shouldConnect = false; /** WebSocket close codes that represent intentional, non-error closures. */ const NORMAL_CLOSE_CODES = new Set([1000, 1001]); +// ── Host browser dispatcher (Phase 2 PR 9) ────────────────────────── +// +// Feature-flagged behind `vellum.cdpProxyEnabled` in chrome.storage.local. +// When the flag is off (default), incoming `host_browser_request` / +// `host_browser_cancel` envelopes are ignored here and the legacy +// ExtensionCommand handlers below service all browser tool calls exactly +// as before. Phase 3 will flip the default and delete the legacy path. +const CDP_PROXY_ENABLED_KEY = 'vellum.cdpProxyEnabled'; + +let cdpProxyEnabled = false; + +async function resolveHostBrowserTarget( + cdpSessionId: string | undefined, +): Promise<{ tabId?: number; targetId?: string }> { + // When the daemon side has an explicit session id (e.g. a flat child + // session returned from a prior Target.attachToTarget) we route the + // command by targetId. Otherwise we fall back to the most recently + // active tab in the focused window — matching the implicit target + // selection the legacy ExtensionCommand handlers used. + if (cdpSessionId) { + return { targetId: cdpSessionId }; + } + const [activeTab] = await chrome.tabs.query({ active: true, lastFocusedWindow: true }); + if (activeTab?.id === undefined) { + throw new Error('No active tab available to resolve host_browser target'); + } + return { tabId: activeTab.id }; +} + +async function postHostBrowserResult(result: HostBrowserResultEnvelope): Promise { + const [token, port] = await Promise.all([getBearerToken(), getRelayPort()]); + const headers: Record = { 'content-type': 'application/json' }; + if (token) headers.authorization = `Bearer ${token}`; + const resp = await fetch(`http://127.0.0.1:${port}/v1/host-browser-result`, { + method: 'POST', + headers, + body: JSON.stringify(result), + }); + if (!resp.ok) { + console.warn( + `[vellum-relay] host-browser-result POST returned ${resp.status}`, + ); + } +} + +const hostBrowserDispatcher: HostBrowserDispatcher = createHostBrowserDispatcher({ + resolveTarget: resolveHostBrowserTarget, + postResult: postHostBrowserResult, +}); + // ── Storage helpers ───────────────────────────────────────────────── async function getBearerToken(): Promise { @@ -152,14 +209,40 @@ function sendResponse(response: ExtensionResponse): void { // ── Command dispatch ──────────────────────────────────────────────── async function handleServerMessage(raw: string): Promise { - let cmd: ExtensionCommand; + let parsed: unknown; try { - cmd = JSON.parse(raw) as ExtensionCommand; + parsed = JSON.parse(raw); } catch { console.warn('[vellum-relay] Failed to parse server message'); return; } + // Phase 2 PR 9: host_browser_* envelopes are dispatched via the CDP proxy + // only when the feature flag is on. With the flag off we return early and + // let the daemon's host-browser-proxy time out gracefully — the envelope + // is NOT forwarded to the legacy ExtensionCommand dispatch because its + // shape is incompatible. + if ( + parsed !== null && + typeof parsed === 'object' && + 'type' in parsed && + typeof (parsed as { type: unknown }).type === 'string' + ) { + const envelopeType = (parsed as { type: string }).type; + if (envelopeType === 'host_browser_request') { + if (!cdpProxyEnabled) return; + await hostBrowserDispatcher.handle(parsed as HostBrowserRequestEnvelope); + return; + } + if (envelopeType === 'host_browser_cancel') { + if (cdpProxyEnabled) { + hostBrowserDispatcher.cancel(parsed as HostBrowserCancelEnvelope); + } + return; + } + } + + const cmd = parsed as ExtensionCommand; try { const result = await dispatch(cmd); sendResponse({ id: cmd.id, success: true, ...result }); @@ -379,3 +462,27 @@ chrome.storage.local.get('autoConnect').then(async (result) => { connect(); } }); + +// Load the CDP proxy feature flag at startup. Missing / non-boolean values +// are treated as false so existing deployments exhibit no behavior change. +chrome.storage.local.get(CDP_PROXY_ENABLED_KEY).then((result) => { + const value = result[CDP_PROXY_ENABLED_KEY]; + cdpProxyEnabled = value === true; + if (cdpProxyEnabled) { + console.log('[vellum-relay] CDP proxy enabled (beta)'); + } +}); + +// Keep the flag live-updatable from the popup without requiring the service +// worker to restart. `chrome.storage.onChanged` fires in the same service +// worker context the value is set from, which is perfect here. +chrome.storage.onChanged.addListener((changes, areaName) => { + if (areaName !== 'local') return; + if (CDP_PROXY_ENABLED_KEY in changes) { + const newValue = changes[CDP_PROXY_ENABLED_KEY]?.newValue; + cdpProxyEnabled = newValue === true; + console.log( + `[vellum-relay] CDP proxy feature flag updated: ${cdpProxyEnabled}`, + ); + } +}); diff --git a/clients/chrome-extension/popup/popup.html b/clients/chrome-extension/popup/popup.html index 3c289efc036..2dfe73af8a8 100644 --- a/clients/chrome-extension/popup/popup.html +++ b/clients/chrome-extension/popup/popup.html @@ -126,6 +126,21 @@ } .cloud-status.signed-in { color: #0f766e; } + .beta-toggle { + display: flex; + align-items: center; + gap: 8px; + font-size: 12px; + color: #4b5563; + margin-top: 10px; + cursor: pointer; + user-select: none; + } + .beta-toggle input[type="checkbox"] { + margin: 0; + cursor: pointer; + } + .hint { font-size: 11px; color: #9ca3af; @@ -200,6 +215,11 @@

Vellum Relay

Not signed in

+ + diff --git a/clients/chrome-extension/popup/popup.ts b/clients/chrome-extension/popup/popup.ts index 40c421ae108..0c6e2d3997f 100644 --- a/clients/chrome-extension/popup/popup.ts +++ b/clients/chrome-extension/popup/popup.ts @@ -28,6 +28,9 @@ const manualToggle = document.getElementById('manual-toggle') as HTMLButtonEleme const tokenGroup = document.getElementById('token-group') as HTMLDivElement; const btnCloudSignIn = document.getElementById('btn-cloud-signin') as HTMLButtonElement; const cloudStatus = document.getElementById('cloud-status') as HTMLParagraphElement; +const cdpProxyToggle = document.getElementById('cdp-proxy-toggle') as HTMLInputElement; + +const CDP_PROXY_ENABLED_KEY = 'vellum.cdpProxyEnabled'; let manualMode = false; @@ -213,3 +216,18 @@ btnCloudSignIn.addEventListener('click', async () => { }); refreshCloudStatus(); + +// ── CDP proxy beta toggle (Phase 2 PR 9) ────────────────────────── +// +// Persists `vellum.cdpProxyEnabled` in chrome.storage.local. The service +// worker reads this flag at startup and listens for changes via +// chrome.storage.onChanged, so no reconnect is needed — flipping the +// checkbox takes effect on the next incoming host_browser_request frame. + +chrome.storage.local.get(CDP_PROXY_ENABLED_KEY).then((result) => { + cdpProxyToggle.checked = result[CDP_PROXY_ENABLED_KEY] === true; +}); + +cdpProxyToggle.addEventListener('change', async () => { + await chrome.storage.local.set({ [CDP_PROXY_ENABLED_KEY]: cdpProxyToggle.checked }); +}); diff --git a/clients/chrome-extension/tsconfig.json b/clients/chrome-extension/tsconfig.json index 433c711fa0f..ae7bb6f4290 100644 --- a/clients/chrome-extension/tsconfig.json +++ b/clients/chrome-extension/tsconfig.json @@ -17,6 +17,7 @@ "include": [ "background/cdp-proxy.ts", "background/cloud-auth.ts", + "background/host-browser-dispatcher.ts", "background/__tests__/**/*.ts", "types/**/*.d.ts" ] From 6ca0b7b27386f3709578a4e795af1caf4f3a9423 Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Tue, 7 Apr 2026 18:34:43 -0400 Subject: [PATCH 2/3] fix(chrome-extension): use camelCase wire format, tolerate re-attach, guard postResult catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Match daemon's actual host_browser_request envelope shape (requestId, cdpMethod, cdpParams, cdpSessionId — only timeout_seconds stays snake_case) - POST /v1/host-browser-result with camelCase keys to match the runtime schema - Track attached CDP targets and skip re-attach; dispose clears the set - Wrap postResult calls inside the catch handler so a secondary failure is logged instead of becoming an unhandled rejection --- .../__tests__/host-browser-dispatcher.test.ts | 144 +++++++++++++++--- .../background/host-browser-dispatcher.ts | 121 +++++++++++---- 2 files changed, 213 insertions(+), 52 deletions(-) diff --git a/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts index 23c614cad63..5a2b75331ee 100644 --- a/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts +++ b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts @@ -135,10 +135,10 @@ function createHarness(options: MockCdpProxyOptions = {}): DispatcherTestHarness const sampleRequest: HostBrowserRequestEnvelope = { type: 'host_browser_request', - request_id: 'req-1', - conversation_id: 'conv-1', - cdp_method: 'Browser.getVersion', - cdp_params: { foo: 'bar' }, + requestId: 'req-1', + conversationId: 'conv-1', + cdpMethod: 'Browser.getVersion', + cdpParams: { foo: 'bar' }, }; // ── Tests ─────────────────────────────────────────────────────────── @@ -176,21 +176,21 @@ describe('createHostBrowserDispatcher', () => { // A single success result was posted with the stringified CDP result. expect(harness.results.length).toBe(1); - expect(harness.results[0].request_id).toBe('req-1'); - expect(harness.results[0].is_error).toBe(false); + expect(harness.results[0].requestId).toBe('req-1'); + expect(harness.results[0].isError).toBe(false); expect(harness.results[0].content).toBe( JSON.stringify({ product: 'Chrome/120', protocolVersion: '1.3' }), ); }); - test('routes via targetId when cdp_session_id is provided', async () => { + test('routes via targetId when cdpSessionId is provided', async () => { harness = createHarness({ sendResult: { id: 1, result: {} }, }); const withSession: HostBrowserRequestEnvelope = { ...sampleRequest, - cdp_session_id: 'target-xyz', + cdpSessionId: 'target-xyz', }; await harness.dispatcher.handle(withSession); @@ -200,8 +200,68 @@ describe('createHostBrowserDispatcher', () => { }); }); + describe('handle — attach deduplication', () => { + test('skips proxy.attach on repeat requests against the same target', async () => { + harness = createHarness({ + sendResult: { id: 1, result: {} }, + }); + + await harness.dispatcher.handle(sampleRequest); + await harness.dispatcher.handle({ ...sampleRequest, requestId: 'req-2' }); + await harness.dispatcher.handle({ ...sampleRequest, requestId: 'req-3' }); + + // Only the first request should have attached; the subsequent two + // reuse the cached attachment. + expect(harness.proxy.attachCalls.length).toBe(1); + expect(harness.proxy.sendCalls.length).toBe(3); + expect(harness.results.length).toBe(3); + expect(harness.results.every((r) => r.isError === false)).toBe(true); + }); + + test('tolerates "Already attached" errors from proxy.attach and caches success', async () => { + harness = createHarness({ + attachThrows: new Error( + 'Another debugger is already attached to the tab with id: 42.', + ), + }); + + await harness.dispatcher.handle(sampleRequest); + + // Send proceeded despite the attach error — the dispatcher treated + // "Already attached" as a non-fatal success. + expect(harness.proxy.attachCalls.length).toBe(1); + expect(harness.proxy.sendCalls.length).toBe(1); + expect(harness.results.length).toBe(1); + expect(harness.results[0].isError).toBe(false); + }); + + test('routes different targetIds to distinct attach entries', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + await harness.dispatcher.handle({ + ...sampleRequest, + cdpSessionId: 'target-A', + }); + await harness.dispatcher.handle({ + ...sampleRequest, + requestId: 'req-2', + cdpSessionId: 'target-B', + }); + // Second call to target-A should reuse the cached attachment. + await harness.dispatcher.handle({ + ...sampleRequest, + requestId: 'req-3', + cdpSessionId: 'target-A', + }); + + expect(harness.proxy.attachCalls.length).toBe(2); + expect(harness.proxy.attachCalls[0].target).toEqual({ targetId: 'target-A' }); + expect(harness.proxy.attachCalls[1].target).toEqual({ targetId: 'target-B' }); + }); + }); + describe('handle — CDP error envelope', () => { - test('posts is_error: true with the stringified error object', async () => { + test('posts isError: true with the stringified error object', async () => { harness = createHarness({ sendResult: { id: 1, @@ -212,7 +272,7 @@ describe('createHostBrowserDispatcher', () => { await harness.dispatcher.handle(sampleRequest); expect(harness.results.length).toBe(1); - expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].isError).toBe(true); expect(harness.results[0].content).toBe( JSON.stringify({ code: -32000, message: 'cannot find context with specified id' }), ); @@ -220,7 +280,7 @@ describe('createHostBrowserDispatcher', () => { }); describe('handle — exception path', () => { - test('posts is_error: true when resolveTarget throws', async () => { + test('posts isError: true when resolveTarget throws', async () => { harness.resolveTargetImpl = async () => { throw new Error('no active tab'); }; @@ -230,12 +290,12 @@ describe('createHostBrowserDispatcher', () => { expect(harness.proxy.attachCalls.length).toBe(0); expect(harness.proxy.sendCalls.length).toBe(0); expect(harness.results.length).toBe(1); - expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].isError).toBe(true); expect(harness.results[0].content).toBe('no active tab'); - expect(harness.results[0].request_id).toBe('req-1'); + expect(harness.results[0].requestId).toBe('req-1'); }); - test('posts is_error: true when proxy.attach throws', async () => { + test('posts isError: true when proxy.attach throws a non-"Already attached" error', async () => { harness = createHarness({ attachThrows: new Error('Cannot access a chrome:// URL'), }); @@ -243,11 +303,11 @@ describe('createHostBrowserDispatcher', () => { await harness.dispatcher.handle(sampleRequest); expect(harness.results.length).toBe(1); - expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].isError).toBe(true); expect(harness.results[0].content).toBe('Cannot access a chrome:// URL'); }); - test('posts is_error: true when proxy.send throws', async () => { + test('posts isError: true when proxy.send throws', async () => { harness = createHarness({ sendThrows: new Error('debugger detached mid-command'), }); @@ -255,7 +315,7 @@ describe('createHostBrowserDispatcher', () => { await harness.dispatcher.handle(sampleRequest); expect(harness.results.length).toBe(1); - expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].isError).toBe(true); expect(harness.results[0].content).toBe('debugger detached mid-command'); }); @@ -268,9 +328,35 @@ describe('createHostBrowserDispatcher', () => { await harness.dispatcher.handle(sampleRequest); expect(harness.results.length).toBe(1); - expect(harness.results[0].is_error).toBe(true); + expect(harness.results[0].isError).toBe(true); expect(harness.results[0].content).toBe('raw string rejection'); }); + + test('swallows postResult failures inside the catch handler (no unhandled rejection)', async () => { + // Force the handler into the error path AND make postResult itself + // throw. If the dispatcher does not guard the catch-block postResult, + // this rejection will escape and trip `handle()`. + harness = createHarness({ + sendThrows: new Error('boom from send'), + }); + let postResultCalls = 0; + harness.postResultImpl = async () => { + postResultCalls += 1; + throw new Error('relay socket torn down'); + }; + + // Must not reject. + let rejected: unknown = null; + try { + await harness.dispatcher.handle(sampleRequest); + } catch (err) { + rejected = err; + } + expect(rejected).toBeNull(); + + // We still attempted to post the error envelope once. + expect(postResultCalls).toBe(1); + }); }); describe('cancel', () => { @@ -292,7 +378,7 @@ describe('createHostBrowserDispatcher', () => { // Mid-flight cancel. const cancelEnvelope: HostBrowserCancelEnvelope = { type: 'host_browser_cancel', - request_id: 'req-1', + requestId: 'req-1', }; harness.dispatcher.cancel(cancelEnvelope); @@ -311,7 +397,7 @@ describe('createHostBrowserDispatcher', () => { expect(() => harness.dispatcher.cancel({ type: 'host_browser_cancel', - request_id: 'unknown', + requestId: 'unknown', }), ).not.toThrow(); }); @@ -346,5 +432,23 @@ describe('createHostBrowserDispatcher', () => { harness.dispatcher.dispose(); expect(harness.proxy.disposeCalls).toBe(2); }); + + test('clears attached-target cache so the next attach happens fresh', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + // Attach once. + await harness.dispatcher.handle(sampleRequest); + expect(harness.proxy.attachCalls.length).toBe(1); + + // Dispose clears the attached set (and the proxy). + harness.dispatcher.dispose(); + + // A new dispatcher built on a *fresh* proxy should attach again on + // first use — we can't reuse the disposed dispatcher, so this test + // verifies the semantic by starting over. + harness = createHarness({ sendResult: { id: 1, result: {} } }); + await harness.dispatcher.handle(sampleRequest); + expect(harness.proxy.attachCalls.length).toBe(1); + }); }); }); diff --git a/clients/chrome-extension/background/host-browser-dispatcher.ts b/clients/chrome-extension/background/host-browser-dispatcher.ts index 773191666ce..48cb569bace 100644 --- a/clients/chrome-extension/background/host-browser-dispatcher.ts +++ b/clients/chrome-extension/background/host-browser-dispatcher.ts @@ -18,41 +18,45 @@ * continue to service browser tools exactly as before. */ -import { createCdpProxy, type CdpProxy } from './cdp-proxy.js'; +import { createCdpProxy, type CdpProxy, type CdpTarget } from './cdp-proxy.js'; /** * host_browser_request envelope as received over the existing browser-relay - * WebSocket. Field names use snake_case to match the daemon's ServerMessage - * discriminator wire format; the canonical Swift + TypeScript types live in - * their respective message-protocol files. + * WebSocket. Field names are camelCase to match the daemon's ServerMessage + * discriminator wire format — see + * `assistant/src/daemon/message-types/host-browser.ts` for the canonical + * types. Note `timeout_seconds` is the one snake_case field the daemon emits + * (a holdover from Phase 1) and we preserve it as-is. */ export interface HostBrowserRequestEnvelope { type: 'host_browser_request'; - request_id: string; - conversation_id: string; - cdp_method: string; - cdp_params?: Record; - cdp_session_id?: string; + requestId: string; + conversationId: string; + cdpMethod: string; + cdpParams?: Record; + cdpSessionId?: string; timeout_seconds?: number; } /** host_browser_cancel envelope sent when the daemon side aborts a request. */ export interface HostBrowserCancelEnvelope { type: 'host_browser_cancel'; - request_id: string; + requestId: string; } /** * Result envelope POSTed back to the runtime's /v1/host-browser-result - * endpoint. Shape mirrors the HostBrowserProxy `resolve()` contract on the - * daemon side: `content` is the stringified CDP result (or error), and - * `is_error` is true if the CDP command reported a JSON-RPC error envelope - * or if the dispatcher itself threw before it could reach the result frame. + * endpoint. Shape mirrors the runtime Zod schema in + * `assistant/src/runtime/routes/host-browser-routes.ts` (`requestId`, + * `content`, `isError`): `content` is the stringified CDP result (or error), + * and `isError` is true if the CDP command reported a JSON-RPC error + * envelope or if the dispatcher itself threw before it could reach the + * result frame. */ export interface HostBrowserResultEnvelope { - request_id: string; + requestId: string; content: string; - is_error: boolean; + isError: boolean; } export interface HostBrowserDispatcherDeps { @@ -77,49 +81,102 @@ export interface HostBrowserDispatcher { dispose(): void; } +/** + * Stable string key for an attach-tracking set. A CdpTarget is either a + * numeric `tabId` or an opaque `targetId` string — we serialize whichever + * is set into a prefix-disambiguated key so tabId=123 and targetId="123" + * can't collide. + */ +function targetKey(target: CdpTarget): string { + if (target.targetId) return `targetId:${target.targetId}`; + if (target.tabId !== undefined) return `tabId:${target.tabId}`; + throw new Error('CdpTarget must have either tabId or targetId'); +} + export function createHostBrowserDispatcher( deps: HostBrowserDispatcherDeps, ): HostBrowserDispatcher { const proxy = deps.cdpProxy ?? createCdpProxy(); const inFlight = new Map(); + // Track which targets we've already attached to so repeat commands + // against the same tab/session don't unnecessarily call attach again. + // Chrome treats a second attach as a hard failure ("Another debugger is + // already attached..."), so either we dedupe here or we catch the error. + // Deduping is cheaper and keeps the happy path clean. + const attachedTargets = new Set(); let nextCdpId = 1; async function handle(envelope: HostBrowserRequestEnvelope): Promise { const abort = new AbortController(); - inFlight.set(envelope.request_id, abort); + inFlight.set(envelope.requestId, abort); try { - const target = await deps.resolveTarget(envelope.cdp_session_id); - await proxy.attach(target, '1.3'); + const target = await deps.resolveTarget(envelope.cdpSessionId); + const key = targetKey(target); + if (!attachedTargets.has(key)) { + try { + await proxy.attach(target, '1.3'); + attachedTargets.add(key); + } catch (attachErr) { + // Tolerate the "already attached" race: Chrome surfaces this as + // "Another debugger is already attached to the tab with id: N." + // when a concurrent sibling request or an earlier invocation that + // predates this dispatcher instance already owns the debuggee. + // Treat it as success and record the target as attached. The + // match is case-insensitive because Chrome's wording has shifted + // across versions and across extensionId/tabId/targetId variants. + const msg = ( + attachErr instanceof Error ? attachErr.message : String(attachErr) + ).toLowerCase(); + if (msg.includes('already attached')) { + attachedTargets.add(key); + } else { + throw attachErr; + } + } + } const frame = await proxy.send(target, { id: nextCdpId++, - method: envelope.cdp_method, - params: envelope.cdp_params, - sessionId: envelope.cdp_session_id, + method: envelope.cdpMethod, + params: envelope.cdpParams, + sessionId: envelope.cdpSessionId, }); await deps.postResult({ - request_id: envelope.request_id, + requestId: envelope.requestId, content: JSON.stringify(frame.error ?? frame.result ?? {}), - is_error: frame.error != null, + isError: frame.error != null, }); } catch (err) { - await deps.postResult({ - request_id: envelope.request_id, - content: err instanceof Error ? err.message : String(err), - is_error: true, - }); + // Guard the failure-path postResult in its own try/catch: if the HTTP + // POST itself fails (e.g. the relay socket is torn down while we're + // in the error path) we must NOT let that secondary rejection escape + // to the Chrome service worker as an unhandled promise rejection. + try { + await deps.postResult({ + requestId: envelope.requestId, + content: err instanceof Error ? err.message : String(err), + isError: true, + }); + } catch (postErr) { + console.error( + '[host-browser-dispatcher] Failed to post error result for', + envelope.requestId, + postErr, + ); + } } finally { - inFlight.delete(envelope.request_id); + inFlight.delete(envelope.requestId); } } function cancel(envelope: HostBrowserCancelEnvelope): void { - inFlight.get(envelope.request_id)?.abort(); - inFlight.delete(envelope.request_id); + inFlight.get(envelope.requestId)?.abort(); + inFlight.delete(envelope.requestId); } function dispose(): void { for (const abort of inFlight.values()) abort.abort(); inFlight.clear(); + attachedTargets.clear(); proxy.dispose(); } From ae1ef4878e6e91d1fe2f84fd0cee678a71f903cb Mon Sep 17 00:00:00 2001 From: Noa Flaherty Date: Tue, 7 Apr 2026 18:47:31 -0400 Subject: [PATCH 3/3] fix(chrome-extension): invalidate attachedTargets cache on debugger detach Subscribe to CdpProxy.onDetach in the dispatcher and remove the corresponding key from the attached-targets cache when Chrome notifies us of a detach (tab close, navigation, infobar cancel, external takeover). Without this, the cache held a stale entry forever and subsequent commands skipped the re-attach, causing permanent CDP failures. --- .../__tests__/host-browser-dispatcher.test.ts | 129 ++++++++++++++++++ .../background/host-browser-dispatcher.ts | 37 ++++- 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts index 5a2b75331ee..789a9ad6104 100644 --- a/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts +++ b/clients/chrome-extension/background/__tests__/host-browser-dispatcher.test.ts @@ -41,6 +41,13 @@ interface MockCdpProxy extends CdpProxy { sendCalls: Array<{ target: CdpTarget; frame: CdpRequestFrame }>; detachCalls: CdpTarget[]; disposeCalls: number; + /** + * Currently-registered onDetach handlers. Tests fire detach events by + * calling these directly via the `fireDetach` helper below. + */ + detachHandlers: Set<(target: CdpDebuggee, reason: string) => void>; + /** Synthetically dispatch a detach event to all registered handlers. */ + fireDetach(target: CdpDebuggee, reason?: string): void; } function createMockCdpProxy(options: MockCdpProxyOptions = {}): MockCdpProxy { @@ -55,6 +62,7 @@ function createMockCdpProxy(options: MockCdpProxyOptions = {}): MockCdpProxy { attachCalls, sendCalls, detachCalls, + detachHandlers, get disposeCalls() { return disposeCalls; }, @@ -78,6 +86,9 @@ function createMockCdpProxy(options: MockCdpProxyOptions = {}): MockCdpProxy { detachHandlers.add(handler); return () => detachHandlers.delete(handler); }, + fireDetach(target, reason = 'target_closed') { + for (const h of detachHandlers) h(target, reason); + }, dispose() { disposeCalls += 1; eventHandlers.clear(); @@ -260,6 +271,109 @@ describe('createHostBrowserDispatcher', () => { }); }); + describe('handle — onDetach cache invalidation', () => { + test('re-attaches after Chrome fires onDetach for a tabId target', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + // First call attaches. + await harness.dispatcher.handle(sampleRequest); + expect(harness.proxy.attachCalls.length).toBe(1); + expect(harness.proxy.attachCalls[0].target).toEqual({ tabId: 42 }); + + // Second call (no detach yet) reuses the cached attachment — proves + // the entry is in the cache. + await harness.dispatcher.handle({ ...sampleRequest, requestId: 'req-2' }); + expect(harness.proxy.attachCalls.length).toBe(1); + + // Chrome fires onDetach for the tab — e.g. user closed it, navigated + // away, clicked Cancel on the chrome.debugger infobar, or another + // debugger took over via Target.attachToTarget. + harness.proxy.fireDetach({ tabId: 42 }, 'target_closed'); + + // Next call must re-attach because the cache entry was invalidated. + // Otherwise we'd silently send a CDP command against a torn-down + // session and hit a permanent failure. + await harness.dispatcher.handle({ ...sampleRequest, requestId: 'req-3' }); + expect(harness.proxy.attachCalls.length).toBe(2); + expect(harness.proxy.attachCalls[1].target).toEqual({ tabId: 42 }); + }); + + test('re-attaches after Chrome fires onDetach for a targetId target', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + const withSession: HostBrowserRequestEnvelope = { + ...sampleRequest, + cdpSessionId: 'target-xyz', + }; + + await harness.dispatcher.handle(withSession); + expect(harness.proxy.attachCalls.length).toBe(1); + + // Cache hit — second call must NOT re-attach. + await harness.dispatcher.handle({ ...withSession, requestId: 'req-2' }); + expect(harness.proxy.attachCalls.length).toBe(1); + + harness.proxy.fireDetach({ targetId: 'target-xyz' }, 'target_closed'); + + await harness.dispatcher.handle({ ...withSession, requestId: 'req-3' }); + expect(harness.proxy.attachCalls.length).toBe(2); + expect(harness.proxy.attachCalls[1].target).toEqual({ + targetId: 'target-xyz', + }); + }); + + test('detach for an unrelated target does not invalidate other entries', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + // Attach two distinct targets. + await harness.dispatcher.handle({ + ...sampleRequest, + cdpSessionId: 'target-A', + }); + await harness.dispatcher.handle({ + ...sampleRequest, + requestId: 'req-2', + cdpSessionId: 'target-B', + }); + expect(harness.proxy.attachCalls.length).toBe(2); + + // Detach only target-A. target-B's cached attachment must survive. + harness.proxy.fireDetach({ targetId: 'target-A' }, 'target_closed'); + + await harness.dispatcher.handle({ + ...sampleRequest, + requestId: 'req-3', + cdpSessionId: 'target-B', + }); + // No new attach for target-B. + expect(harness.proxy.attachCalls.length).toBe(2); + + // But target-A re-attaches. + await harness.dispatcher.handle({ + ...sampleRequest, + requestId: 'req-4', + cdpSessionId: 'target-A', + }); + expect(harness.proxy.attachCalls.length).toBe(3); + expect(harness.proxy.attachCalls[2].target).toEqual({ targetId: 'target-A' }); + }); + + test('detach for a debuggee shape with neither tabId nor targetId is a no-op', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + await harness.dispatcher.handle(sampleRequest); + expect(harness.proxy.attachCalls.length).toBe(1); + + // Defensive: a malformed detach payload (e.g. extensionId-only) must + // not throw and must not invalidate anything we care about. + harness.proxy.fireDetach({}, 'target_closed'); + + // Cache entry for tabId 42 is still there → no new attach. + await harness.dispatcher.handle({ ...sampleRequest, requestId: 'req-2' }); + expect(harness.proxy.attachCalls.length).toBe(1); + }); + }); + describe('handle — CDP error envelope', () => { test('posts isError: true with the stringified error object', async () => { harness = createHarness({ @@ -450,5 +564,20 @@ describe('createHostBrowserDispatcher', () => { await harness.dispatcher.handle(sampleRequest); expect(harness.proxy.attachCalls.length).toBe(1); }); + + test('unsubscribes the onDetach handler', async () => { + harness = createHarness({ sendResult: { id: 1, result: {} } }); + + // Subscribing happens at construction time. The mock proxy exposes + // its handler set so we can directly observe registration/teardown. + expect(harness.proxy.detachHandlers.size).toBe(1); + + harness.dispatcher.dispose(); + + // After dispose the dispatcher must release its detach handler so + // the proxy isn't left holding a stale closure that references the + // disposed dispatcher's `attachedTargets` set. + expect(harness.proxy.detachHandlers.size).toBe(0); + }); }); }); diff --git a/clients/chrome-extension/background/host-browser-dispatcher.ts b/clients/chrome-extension/background/host-browser-dispatcher.ts index 48cb569bace..7e6a97a9d9f 100644 --- a/clients/chrome-extension/background/host-browser-dispatcher.ts +++ b/clients/chrome-extension/background/host-browser-dispatcher.ts @@ -18,7 +18,12 @@ * continue to service browser tools exactly as before. */ -import { createCdpProxy, type CdpProxy, type CdpTarget } from './cdp-proxy.js'; +import { + createCdpProxy, + type CdpDebuggee, + type CdpProxy, + type CdpTarget, +} from './cdp-proxy.js'; /** * host_browser_request envelope as received over the existing browser-relay @@ -93,6 +98,23 @@ function targetKey(target: CdpTarget): string { throw new Error('CdpTarget must have either tabId or targetId'); } +/** + * Build the same target-key from a `CdpDebuggee` payload as `targetKey` + * does for a `CdpTarget`. The CDP proxy's `onDetach` callback receives a + * `CdpDebuggee` (the chrome.debugger Debuggee shape), so we need a helper + * that produces an identical key from that variant — otherwise the cache + * deletion on detach would silently miss and the stale entry would persist. + * + * Returns `null` when the debuggee shape carries neither a `tabId` nor a + * `targetId` (e.g. extensionId-only attaches, which the dispatcher does + * not currently use). Callers treat null as "nothing to invalidate". + */ +function debuggeeKey(debuggee: CdpDebuggee): string | null { + if (debuggee.targetId) return `targetId:${debuggee.targetId}`; + if (debuggee.tabId !== undefined) return `tabId:${debuggee.tabId}`; + return null; +} + export function createHostBrowserDispatcher( deps: HostBrowserDispatcherDeps, ): HostBrowserDispatcher { @@ -106,6 +128,18 @@ export function createHostBrowserDispatcher( const attachedTargets = new Set(); let nextCdpId = 1; + // Invalidate the attached-targets cache whenever Chrome notifies us that + // it has detached the debugger from a target. This covers tab close, + // navigation across security origins, the user clicking "Cancel" on the + // chrome.debugger infobar, and another debugger taking over via + // Target.attachToTarget. Without this subscription the cache would hold + // a stale entry forever and subsequent commands against the same target + // would skip the re-attach and hit a permanent CDP failure. + const unsubscribeOnDetach = proxy.onDetach((debuggee) => { + const key = debuggeeKey(debuggee); + if (key !== null) attachedTargets.delete(key); + }); + async function handle(envelope: HostBrowserRequestEnvelope): Promise { const abort = new AbortController(); inFlight.set(envelope.requestId, abort); @@ -177,6 +211,7 @@ export function createHostBrowserDispatcher( for (const abort of inFlight.values()) abort.abort(); inFlight.clear(); attachedTargets.clear(); + unsubscribeOnDetach(); proxy.dispose(); }