diff --git a/docs/guides/web/browser-observation.md b/docs/guides/web/browser-observation.md index 9b737c65c5..6f1afb6068 100644 --- a/docs/guides/web/browser-observation.md +++ b/docs/guides/web/browser-observation.md @@ -239,10 +239,21 @@ file for you. `observe_web_session`, `discover_webmcp_tools`, and `capture_web_screenshot` accept a browser socket. Website runtime attribution, Electron inspection, and scenario attach mode still require HTTP discovery. -- Each call opens one WebSocket and uses it for discovery - (`Browser.getVersion`, `Target.getTargets`) and for the selected page. - Discovery must finish within the same 10-second deadline as HTTP discovery. - Chrome may ask you to approve each connection. +- An admitted browser socket is retained by the active browser provider across + calls to the same normalized browser WebSocket URL, and concurrent cold-start + calls share one handshake so MCP sessions do not re-open the browser socket on + every step. Scheme case, uncompressed IPv6 loopback spellings, and leading + port zeros normalize to one reuse key, while `127.0.0.1`, `[::1]`, and + distinct `/devtools/browser/` paths remain separate sockets. Each target + operation detaches its flattened target session when finished: unconfirmed + target attach or detach cleanup closes the shared browser socket for session + containment, whereas a caller's own discovery deadline or cancellation + releases only its lease when transport state remains clean. Provider shutdown + closes the retained browser socket. One-shot CLI invocations close their + provider on exit, so Chrome may still ask you to approve each separate CLI + connection. Discovery (`Browser.getVersion`, `Target.getTargets`) defaults to + a 10-second deadline and can be adjusted per call with `discovery_timeout_ms` + (`--discovery-timeout-ms` on the CLI). - The browser identifier controls access to the socket. Evidence records the endpoint as `ws://127.0.0.1:9222/devtools/browser/[REDACTED]`. - `browser.revision` is the socket's `Browser.getVersion` value, such as @@ -598,13 +609,13 @@ an unknown size, comparison reports unknown rather than treating the observation as a resource addition or removal; complete inventory coverage still establishes when an entire group is absent. -CDP discovery has a 10-second response deadline. HTTP discovery documents -use an 8 MiB budget. Browser-socket discovery (`Browser.getVersion`, -`Target.getTargets`) uses the 64 MiB protocol message budget. Neither budget -nor the discovery deadline has a runtime override. `list_browser_targets` accepts no -`command_timeout_ms`: HTTP discovery and browser-socket discovery -(`Browser.getVersion`, `Target.getTargets`) stay on that fixed deadline, and -the socket closes without a later command. Other CDP tools use +CDP discovery defaults to a 10-second response deadline, configurable per call +on passive browser tools with `discovery_timeout_ms` (default 10000). HTTP +discovery documents use an 8 MiB budget. Browser-socket discovery +(`Browser.getVersion`, `Target.getTargets`) uses the 64 MiB protocol message +budget. Neither byte budget has a runtime override. `list_browser_targets` +accepts `discovery_timeout_ms` and no `command_timeout_ms`, because it performs +only discovery without a post-discovery target command. Other CDP tools use `command_timeout_ms` (default 20000) for each WebSocket handshake and command response, separate from the caller-selected observation window, discovery, and the 5000 ms cleanup grace. A browser-level socket's handshake and the following diff --git a/src/application/BrowserEvidence.ts b/src/application/BrowserEvidence.ts index 3c8f46f2e3..d664dd7aa3 100644 --- a/src/application/BrowserEvidence.ts +++ b/src/application/BrowserEvidence.ts @@ -164,6 +164,10 @@ const browserParameters = ( : "target_id" in input && observedOrigin !== undefined ? [observedOrigin] : [], + ...("discovery_timeout_ms" in input && + input.discovery_timeout_ms !== undefined + ? { discovery_timeout_ms: input.discovery_timeout_ms } + : {}), ...("command_timeout_ms" in input && input.command_timeout_ms !== undefined ? { command_timeout_ms: input.command_timeout_ms } : {}), diff --git a/src/browser/CdpBrowserProvider.ts b/src/browser/CdpBrowserProvider.ts index 2fe78ceb5e..b925063861 100644 --- a/src/browser/CdpBrowserProvider.ts +++ b/src/browser/CdpBrowserProvider.ts @@ -75,7 +75,10 @@ import { type CdpTargetSession, } from "./CdpTargetSession.js"; import { capturePage, type CapturedPage } from "./CdpPageCapture.js"; -import { selectedCdpCommandTimeoutMs } from "../domain/browserDeadline.js"; +import { + selectedCdpCommandTimeoutMs, + selectedCdpDiscoveryTimeoutMs, +} from "../domain/browserDeadline.js"; import { fetchWebSourceMaps } from "./WebSourceMapFetcher.js"; import { authorizeCdpTarget } from "./CdpAuthorizedTarget.js"; import { observeCdpSession } from "./CdpObservationSession.js"; @@ -144,11 +147,16 @@ export class CdpBrowserProvider implements BrowserObservationPort { input.cdp_endpoint, "list_browser_targets", options.signal, - undefined, + selectedCdpDiscoveryTimeoutMs(input.discovery_timeout_ms), this.#owner, + undefined, + { reuseBrowserSocket: true }, ); - if (endpoint.connection !== undefined) - await this.#owner.finish(endpoint.connection); + if (endpoint.connection !== undefined) { + if (endpoint.reusableBrowserSocket === true) + await this.#owner.releaseBrowserSocket(endpoint.connection); + else await this.#owner.finish(endpoint.connection); + } const { discovery } = endpoint; const selected = selectTargets( discovery, @@ -391,11 +399,15 @@ interface CdpTargetOperationSession { readonly progress?: NonNullable; } -/** Authorize one discovered target, run work in its session, and close REA's socket. */ +/** Authorize one discovered target, run work in its session, and release or close REA's socket. */ const withTargetSession = async < Input extends Pick< InspectWebPageInput, - "cdp_endpoint" | "target_id" | "allowed_origins" | "command_timeout_ms" + | "cdp_endpoint" + | "target_id" + | "allowed_origins" + | "discovery_timeout_ms" + | "command_timeout_ms" >, Output, >( @@ -406,6 +418,9 @@ const withTargetSession = async < owner: CdpTargetSessionOwner, partialObservation?: (output: Output) => AnalysisPartialObservation, ): Promise => { + const discoveryTimeoutMs = selectedCdpDiscoveryTimeoutMs( + input.discovery_timeout_ms, + ); const commandTimeoutMs = selectedCdpCommandTimeoutMs( input.command_timeout_ms, ); @@ -413,11 +428,13 @@ const withTargetSession = async < input.cdp_endpoint, operation, options.signal, - undefined, + discoveryTimeoutMs, owner, commandTimeoutMs, + { reuseBrowserSocket: true }, ); let targetSession: CdpTargetSession | undefined; + let attachAttempted = false; let outcome: Result; let cleanupFailure: AnalysisError | undefined; try { @@ -431,23 +448,25 @@ const withTargetSession = async < input.allowed_origins.length > 0 ? input : { ...input, allowed_origins: [...allowedOrigins] }; - targetSession = - endpoint.connection === undefined - ? await openCdpTargetSession( - endpoint.discovery, - target, - operation, - options.signal, - { commandTimeoutMs }, - owner, - ) - : await attachCdpTargetSession( - endpoint.connection, - target, - operation, - options.signal, - owner, - ); + if (endpoint.connection === undefined) { + targetSession = await openCdpTargetSession( + endpoint.discovery, + target, + operation, + options.signal, + { commandTimeoutMs }, + owner, + ); + } else { + attachAttempted = true; + targetSession = await attachCdpTargetSession( + endpoint.connection, + target, + operation, + options.signal, + owner, + ); + } outcome = ok( await work({ connection: targetSession.connection, @@ -471,12 +490,20 @@ const withTargetSession = async < CLEANUP_DOMAINS, options.signal, owner, + endpoint.reusableBrowserSocket === true, ); if ( endpoint.connection !== undefined && endpoint.connection !== targetSession?.connection - ) - await owner.finish(endpoint.connection); + ) { + if (endpoint.reusableBrowserSocket === true) { + if (attachAttempted) + await owner.evictBrowserSocket(endpoint.connection); + else await owner.releaseBrowserSocket(endpoint.connection); + } else { + await owner.finish(endpoint.connection); + } + } } catch (cause: unknown) { cleanupFailure = cause instanceof AnalysisError diff --git a/src/browser/CdpConnection.ts b/src/browser/CdpConnection.ts index 2a15e41798..3dadb952ef 100644 --- a/src/browser/CdpConnection.ts +++ b/src/browser/CdpConnection.ts @@ -11,6 +11,7 @@ import { cdpCommandTimeoutIsSelectable, cdpCommandTimeoutUnrelatedControl, DEFAULT_CDP_COMMAND_TIMEOUT_MS, + MAX_BROWSER_DEADLINE_MS, noRuntimeOverrideSentence, } from "../domain/browserDeadline.js"; import { safeParseJson } from "../domain/safeJson.js"; @@ -33,6 +34,7 @@ export interface CdpTransportEvent extends CdpEvent { interface PendingCommand { readonly method: string; + readonly operation: BrowserObservationOperation; readonly sessionId?: string; readonly resolve: (value: unknown) => void; readonly reject: (error: AnalysisError) => void; @@ -40,31 +42,24 @@ interface PendingCommand { readonly reschedule: (timeoutMs: number) => void; } -/** Correlated JSON-RPC transport for one CDP target operation. */ -export class CdpConnection { +class CdpTransportCore { readonly #pending = new Map(); readonly #listeners = new Set<(event: CdpTransportEvent) => void>(); readonly #disconnectListeners = new Set< - (error: BrowserObservationError) => void - >(); - readonly #protocolFailureListeners = new Set< - (error: BrowserObservationError) => void + (reason: "disconnected" | "payload_limit") => void >(); + readonly #protocolFailureListeners = new Set<() => void>(); #nextId = 1; #closed = false; #closePromise: Promise | undefined; #protocolFailed = false; #transportFailure: "disconnected" | "payload_limit" = "disconnected"; - #commandTimeoutMs: number; - #deadlineKind: "command" | "cleanup" = "command"; + root!: CdpConnection; - private constructor( + constructor( private readonly socket: WebSocket, - private readonly operation: BrowserObservationOperation, private readonly maxPayloadBytes: number, - commandTimeoutMs: number, ) { - this.#commandTimeoutMs = commandTimeoutMs; socket.on("message", (data) => this.#receive(data)); socket.on("close", () => this.#disconnect()); socket.on("error", (cause) => @@ -76,129 +71,73 @@ export class CdpConnection { ); } - /** Connect to one already-validated loopback CDP WebSocket. */ - static async connect( - url: string, - operation: BrowserObservationOperation, - signal?: AbortSignal, - limits?: { - readonly maxPayloadBytes?: number; - readonly commandTimeoutMs?: number; - /** Handshake budget. Discovery passes its own deadline so a short command budget cannot start first. */ - readonly handshakeTimeoutMs?: number; - }, - onConnected?: (connection: CdpConnection) => void, - ): Promise { - if (signal?.aborted === true) - throw new AnalysisCancelledError(operation, { cause: signal.reason }); - const maxPayloadBytes = - limits?.maxPayloadBytes ?? WEB_RUNTIME_LIMITS.protocolBytes; - const commandTimeoutMs = - limits?.commandTimeoutMs ?? DEFAULT_CDP_COMMAND_TIMEOUT_MS; - const handshakeTimeoutMs = limits?.handshakeTimeoutMs ?? commandTimeoutMs; - if ( - !Number.isSafeInteger(maxPayloadBytes) || - maxPayloadBytes <= 0 || - maxPayloadBytes > MAX_WEBSOCKET_PAYLOAD_BYTES - ) - throw new RangeError( - `CDP maxPayloadBytes must be a positive safe integer no greater than ${String(MAX_WEBSOCKET_PAYLOAD_BYTES)}.`, - ); - const socket = new WebSocket(url, { - // waitForOpen owns the deadline and typed handshake failures; the - // connection owner registers this socket before waiting for open. - handshakeTimeout: 0, - maxPayload: maxPayloadBytes, - perMessageDeflate: false, - }); - const connection = new CdpConnection( - socket, - operation, - maxPayloadBytes, - commandTimeoutMs, + isReusable(): boolean { + return ( + !this.#closed && + this.#closePromise === undefined && + !this.#protocolFailed && + this.socket.readyState === WebSocket.OPEN ); - try { - onConnected?.(connection); - } catch (cause: unknown) { - await connection.close(); - throw cause; - } - try { - await waitForOpen(socket, operation, signal, handshakeTimeoutMs); - } catch (cause: unknown) { - if (onConnected === undefined) { - try { - await connection.close(); - } catch (cleanupCause: unknown) { - throw new AggregateError( - [cause, cleanupCause], - "CDP WebSocket handshake failed and its transport cleanup is incomplete.", - { cause }, - ); - } - } - throw cause; - } - return connection; } - /** Subscribe to validated CDP event envelopes. */ onEvent(listener: (event: CdpTransportEvent) => void): () => void { this.#listeners.add(listener); return () => this.#listeners.delete(listener); } - /** Subscribe to an unexpected transport loss while an operation is active. */ - onDisconnect(listener: (error: BrowserObservationError) => void): () => void { + onDisconnect( + operation: BrowserObservationOperation, + listener: (error: BrowserObservationError) => void, + ): () => void { if (this.#closed) { - listener(this.#transportError(this.#transportFailure)); + listener(this.transportError(this.#transportFailure, operation)); return () => undefined; } - this.#disconnectListeners.add(listener); - return () => this.#disconnectListeners.delete(listener); + const wrapped = (reason: "disconnected" | "payload_limit"): void => { + listener(this.transportError(reason, operation)); + }; + this.#disconnectListeners.add(wrapped); + return () => this.#disconnectListeners.delete(wrapped); } - /** Notify fatal wire parsing failure even when no command is outstanding. */ onProtocolFailure( + operation: BrowserObservationOperation, listener: (error: BrowserObservationError) => void, ): () => void { if (this.#protocolFailed) { - listener(this.#transportError("protocol_error")); + listener(this.transportError("protocol_error", operation)); return () => undefined; } - this.#protocolFailureListeners.add(listener); - return () => this.#protocolFailureListeners.delete(listener); + const wrapped = (): void => { + listener(this.transportError("protocol_error", operation)); + }; + this.#protocolFailureListeners.add(wrapped); + return () => this.#protocolFailureListeners.delete(wrapped); } - /** Apply the post-discovery command deadline to commands that have not started. */ - selectCommandTimeout(timeoutMs: number): void { - if (this.#deadlineKind === "cleanup") return; - this.#commandTimeoutMs = timeoutMs; + reschedulePending(ids: ReadonlySet, timeoutMs: number): void { + for (const id of ids) this.#pending.get(id)?.reschedule(timeoutMs); } - /** - * Restoration uses the fixed cleanup grace. Commands already waiting are - * rescheduled from now so a raised command deadline cannot extend them. - */ - beginCleanupGrace(timeoutMs: number): void { - this.#deadlineKind = "cleanup"; - this.#commandTimeoutMs = timeoutMs; - for (const pending of this.#pending.values()) pending.reschedule(timeoutMs); - } - - /** Bound each response separately from the caller-selected observation window. */ async send( method: string, - params: Readonly> = {}, - sessionId?: string, - signal?: AbortSignal, + params: Readonly>, + sessionId: string | undefined, + signal: AbortSignal | undefined, + context: { + readonly operation: BrowserObservationOperation; + readonly commandTimeoutMs: () => number; + readonly deadlineKind: () => "command" | "cleanup"; + readonly scopedIds: Set; + }, ): Promise { + const { operation } = context; if (this.#protocolFailed) - throw new BrowserObservationError(this.operation, "protocol_error"); + throw new BrowserObservationError(operation, "protocol_error"); if (this.#closed || this.socket.readyState !== WebSocket.OPEN) - throw this.#transportError(this.#transportFailure); + throw this.transportError(this.#transportFailure, operation); if (signal?.aborted === true) - throw new AnalysisCancelledError(this.operation, { + throw new AnalysisCancelledError(operation, { cause: signal.reason, }); const id = this.#nextId; @@ -215,7 +154,7 @@ export class CdpConnection { if (pending === undefined) return; this.#complete(id, pending); reject( - new AnalysisCancelledError(this.operation, { + new AnalysisCancelledError(operation, { cause: signal?.reason, }), ); @@ -224,33 +163,36 @@ export class CdpConnection { const pending = this.#pending.get(id); if (pending === undefined) return; this.#complete(id, pending); - if (this.#deadlineKind === "cleanup") { + if (context.deadlineKind() === "cleanup") { const detail = cdpCleanupGraceExpired(); reject( - new BrowserObservationError(this.operation, "timeout", { + new BrowserObservationError(operation, "timeout", { detail, remediationAction: detail, }), ); return; } + const timeoutMs = context.commandTimeoutMs(); const recovery = cdpCommandDeadlineRecovery( - this.#commandTimeoutMs, + timeoutMs, "command", method, - cdpCommandTimeoutIsSelectable(this.operation), + cdpCommandTimeoutIsSelectable(operation), ); reject( - new BrowserObservationError(this.operation, "timeout", { + new BrowserObservationError(operation, "timeout", { detail: recovery.userMessage, remediationAction: recovery.remediationAction, }), ); }; - let timer = setTimeout(failTimeout, this.#commandTimeoutMs); + let timer = setTimeout(failTimeout, context.commandTimeoutMs()); signal?.addEventListener("abort", onAbort, { once: true }); + context.scopedIds.add(id); this.#pending.set(id, { method, + operation, ...(sessionId === undefined ? {} : { sessionId }), resolve, reject, @@ -259,6 +201,7 @@ export class CdpConnection { timer = setTimeout(failTimeout, timeoutMs); }, dispose: () => { + context.scopedIds.delete(id); clearTimeout(timer); signal?.removeEventListener("abort", onAbort); }, @@ -269,7 +212,7 @@ export class CdpConnection { if (pending === undefined) return; this.#complete(id, pending); reject( - new BrowserObservationError(this.operation, "disconnected", { + new BrowserObservationError(operation, "disconnected", { cause: error, }), ); @@ -277,18 +220,18 @@ export class CdpConnection { }); } - /** Close only REA's socket; never close the browser or selected page. */ - close(): Promise { - this.#closePromise ??= this.#closeTransport().catch((cause: unknown) => { - this.#closePromise = undefined; - throw cause; - }); + close(operation: BrowserObservationOperation): Promise { + this.#closePromise ??= this.#closeTransport(operation).catch( + (cause: unknown) => { + this.#closePromise = undefined; + throw cause; + }, + ); return this.#closePromise; } - async #closeTransport(): Promise { - this.#closed = true; - this.#failPending("disconnected"); + async #closeTransport(operation: BrowserObservationOperation): Promise { + this.#disconnect("disconnected"); if (socketIsClosed(this.socket)) return; await new Promise((resolve, reject) => { let settled = false; @@ -333,7 +276,7 @@ export class CdpConnection { } finish( gracefulFailure ?? - new BrowserObservationError(this.operation, "cleanup_failed", { + new BrowserObservationError(operation, "cleanup_failed", { detail: "REA could not confirm release of its CDP WebSocket.", }), ); @@ -441,7 +384,7 @@ export class CdpConnection { this.#complete(id, pending); pending.reject( new CdpCommandRejection( - this.operation, + pending.operation, pending.method, reported.code, reported.message, @@ -462,11 +405,10 @@ export class CdpConnection { else this.#protocolFailed = true; for (const [id, pending] of this.#pending) { this.#complete(id, pending); - pending.reject(this.#transportError(reason)); + pending.reject(this.transportError(reason, pending.operation)); } if (reason === "protocol_error" && !wasProtocolFailed) { - const error = this.#transportError(reason); - for (const listener of this.#protocolFailureListeners) listener(error); + for (const listener of this.#protocolFailureListeners) listener(); this.#protocolFailureListeners.clear(); } } @@ -476,22 +418,22 @@ export class CdpConnection { if (!wasClosed) this.#transportFailure = reason; this.#failPending(reason); if (wasClosed) return; - const error = this.#transportError(reason); - for (const listener of this.#disconnectListeners) listener(error); + for (const listener of this.#disconnectListeners) listener(reason); this.#disconnectListeners.clear(); } - #transportError( + transportError( reason: "disconnected" | "protocol_error" | "payload_limit", + operation: BrowserObservationOperation, ): BrowserObservationError { return new BrowserObservationError( - this.operation, + operation, reason, reason === "payload_limit" ? { detail: noRuntimeOverrideSentence( `CDP message exceeded the ${String(this.maxPayloadBytes)} byte protocol budget.`, - cdpCommandTimeoutUnrelatedControl(this.operation), + cdpCommandTimeoutUnrelatedControl(operation), ), } : undefined, @@ -499,6 +441,155 @@ export class CdpConnection { } } +/** Correlated JSON-RPC transport for one CDP target operation. */ +export class CdpConnection { + readonly #scopedPendingIds = new Set(); + #commandTimeoutMs: number; + #deadlineKind: "command" | "cleanup" = "command"; + + private constructor( + private readonly core: CdpTransportCore, + private readonly operation: BrowserObservationOperation, + commandTimeoutMs: number, + ) { + this.#commandTimeoutMs = commandTimeoutMs; + } + + /** Canonical connection identity registered with `CdpTargetSessionOwner`. */ + get transportRoot(): CdpConnection { + return this.core.root; + } + + /** True while the underlying WebSocket remains open, unpoisoned, and outside cleanup grace. */ + isReusable(): boolean { + return this.#deadlineKind === "command" && this.core.isReusable(); + } + + /** Bind an isolated operation and command-deadline view over this shared WebSocket. */ + withOperation( + operation: BrowserObservationOperation, + commandTimeoutMs: number, + ): CdpConnection { + return new CdpConnection(this.core, operation, commandTimeoutMs); + } + + /** Connect to one already-validated loopback CDP WebSocket. */ + static async connect( + url: string, + operation: BrowserObservationOperation, + signal?: AbortSignal, + limits?: { + readonly maxPayloadBytes?: number; + readonly commandTimeoutMs?: number; + /** Handshake budget. Discovery passes its own deadline so a short command budget cannot start first. */ + readonly handshakeTimeoutMs?: number; + }, + onConnected?: (connection: CdpConnection) => void, + ): Promise { + if (signal?.aborted === true) + throw new AnalysisCancelledError(operation, { cause: signal.reason }); + const maxPayloadBytes = + limits?.maxPayloadBytes ?? WEB_RUNTIME_LIMITS.protocolBytes; + const commandTimeoutMs = + limits?.commandTimeoutMs ?? DEFAULT_CDP_COMMAND_TIMEOUT_MS; + const handshakeTimeoutMs = limits?.handshakeTimeoutMs ?? commandTimeoutMs; + if ( + !Number.isSafeInteger(maxPayloadBytes) || + maxPayloadBytes <= 0 || + maxPayloadBytes > MAX_WEBSOCKET_PAYLOAD_BYTES + ) + throw new RangeError( + `CDP maxPayloadBytes must be a positive safe integer no greater than ${String(MAX_WEBSOCKET_PAYLOAD_BYTES)}.`, + ); + const socket = new WebSocket(url, { + // waitForOpen owns the deadline and typed handshake failures; the + // connection owner registers this socket before waiting for open. + handshakeTimeout: 0, + maxPayload: maxPayloadBytes, + perMessageDeflate: false, + }); + const core = new CdpTransportCore(socket, maxPayloadBytes); + const connection = new CdpConnection(core, operation, commandTimeoutMs); + core.root = connection; + try { + onConnected?.(connection); + } catch (cause: unknown) { + await connection.close(); + throw cause; + } + try { + await waitForOpen(socket, operation, signal, handshakeTimeoutMs); + } catch (cause: unknown) { + if (onConnected === undefined) { + try { + await connection.close(); + } catch (cleanupCause: unknown) { + throw new AggregateError( + [cause, cleanupCause], + "CDP WebSocket handshake failed and its transport cleanup is incomplete.", + { cause }, + ); + } + } + throw cause; + } + return connection; + } + + /** Subscribe to validated CDP event envelopes. */ + onEvent(listener: (event: CdpTransportEvent) => void): () => void { + return this.core.onEvent(listener); + } + + /** Subscribe to an unexpected transport loss while an operation is active. */ + onDisconnect(listener: (error: BrowserObservationError) => void): () => void { + return this.core.onDisconnect(this.operation, listener); + } + + /** Notify fatal wire parsing failure even when no command is outstanding. */ + onProtocolFailure( + listener: (error: BrowserObservationError) => void, + ): () => void { + return this.core.onProtocolFailure(this.operation, listener); + } + + /** Apply the post-discovery command deadline to commands that have not started. */ + selectCommandTimeout(timeoutMs: number): void { + if (this.#deadlineKind === "cleanup") return; + this.#commandTimeoutMs = timeoutMs; + } + + /** + * Restoration uses the fixed cleanup grace. Commands already waiting are + * rescheduled from now so a raised command deadline cannot extend them. + */ + beginCleanupGrace(timeoutMs: number): void { + this.#deadlineKind = "cleanup"; + this.#commandTimeoutMs = timeoutMs; + this.core.reschedulePending(this.#scopedPendingIds, timeoutMs); + } + + /** Bound each response separately from the caller-selected observation window. */ + async send( + method: string, + params: Readonly> = {}, + sessionId?: string, + signal?: AbortSignal, + ): Promise { + return await this.core.send(method, params, sessionId, signal, { + operation: this.operation, + commandTimeoutMs: () => this.#commandTimeoutMs, + deadlineKind: () => this.#deadlineKind, + scopedIds: this.#scopedPendingIds, + }); + } + + /** Close only REA's socket; never close the browser or selected page. */ + close(): Promise { + return this.core.close(this.operation); + } +} + const socketIsClosed = (socket: WebSocket): boolean => socket.readyState === WebSocket.CLOSED; @@ -555,11 +646,15 @@ const waitForOpen = async ( socket.off("close", onClose); signal?.removeEventListener("abort", onAbort); }; + const boundedTimeoutMs = Math.min( + MAX_BROWSER_DEADLINE_MS, + commandTimeoutMs, + ); const timer = setTimeout(() => { terminateSocket(socket); cleanup(); const recovery = cdpCommandDeadlineRecovery( - commandTimeoutMs, + boundedTimeoutMs, "handshake", undefined, cdpCommandTimeoutIsSelectable(operation), @@ -570,7 +665,7 @@ const waitForOpen = async ( remediationAction: recovery.remediationAction, }), ); - }, commandTimeoutMs); + }, boundedTimeoutMs); socket.once("open", onOpen); socket.once("error", onFailure); socket.once("close", onClose); diff --git a/src/browser/CdpEndpoint.ts b/src/browser/CdpEndpoint.ts index 20d5804606..fab0ab7461 100644 --- a/src/browser/CdpEndpoint.ts +++ b/src/browser/CdpEndpoint.ts @@ -6,6 +6,7 @@ import { z } from "zod"; import { CDP_DISCOVERY_RESPONSE_TIMEOUT_MS, cdpCommandTimeoutUnrelatedControl, + cdpDiscoveryTimeoutRecovery, noRuntimeOverrideSentence, } from "../domain/browserDeadline.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; @@ -32,7 +33,7 @@ const endpointVersionSchema = z.object({ webSocketDebuggerUrl: z.string().min(1), }); -/** Per-response bounds for loopback CDP discovery documents. There is no runtime override. */ +/** Default per-response bounds for loopback CDP discovery documents. The byte budget has no runtime override. */ const CDP_DISCOVERY_RESPONSE_LIMITS = { bytes: 8 * 1024 * 1024, timeoutMs: CDP_DISCOVERY_RESPONSE_TIMEOUT_MS, @@ -48,17 +49,15 @@ const discoveryTimeout = ( timeoutMs: number, phase: "response" | "browser-socket", ): BrowserObservationError => { - const subject = - phase === "response" - ? `CDP discovery response exceeded its ${String(timeoutMs)} ms deadline.` - : `CDP browser-socket discovery exceeded its ${String(timeoutMs)} ms deadline.`; - const fixed = noRuntimeOverrideSentence( - `${discoveryBudget(phase)} and the ${String(timeoutMs)} ms discovery deadline stay fixed.`, - cdpCommandTimeoutUnrelatedControl(operation), + const recovery = cdpDiscoveryTimeoutRecovery( + operation, + timeoutMs, + phase, + discoveryBudget(phase), ); return new BrowserObservationError(operation, "timeout", { - detail: `${subject} ${fixed}`, - remediationAction: fixed, + detail: recovery.userMessage, + remediationAction: recovery.remediationAction, }); }; @@ -116,16 +115,23 @@ export const discoverCdpEndpoint = async ( endpoint: string, operation: BrowserObservationOperation, signal?: AbortSignal, + timeoutMs: number = CDP_DISCOVERY_RESPONSE_LIMITS.timeoutMs, ): Promise => { + const limits: CdpDiscoveryResponseLimits = { + bytes: CDP_DISCOVERY_RESPONSE_LIMITS.bytes, + timeoutMs, + }; const versionInput = await readCdpJson( new URL("/json/version", endpoint), operation, signal, + limits, ); const targetsInput = await readCdpJson( new URL("/json/list", endpoint), operation, signal, + limits, ); const version = parseCdpEndpointValue( endpointVersionSchema, diff --git a/src/browser/CdpTargetSession.ts b/src/browser/CdpTargetSession.ts index 2af3df282b..65072faf7e 100644 --- a/src/browser/CdpTargetSession.ts +++ b/src/browser/CdpTargetSession.ts @@ -1,12 +1,15 @@ import { CDP_DISCOVERY_RESPONSE_TIMEOUT_MS, DEFAULT_CDP_COMMAND_TIMEOUT_MS, + MAX_BROWSER_DEADLINE_MS, } from "../domain/browserDeadline.js"; import type { BrowserObservationEndpoint } from "../domain/browserObservation.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; +import { CdpCommandRejection } from "./CdpCommandRejection.js"; import { CdpConnection } from "./CdpConnection.js"; import { CdpTargetSessionOwner, @@ -35,6 +38,7 @@ export interface CdpOpenedEndpoint { * the user to approve each socket. HTTP discovery opens no socket. */ readonly connection: CdpConnection | undefined; + readonly reusableBrowserSocket?: boolean; } /** @@ -48,60 +52,104 @@ export const openCdpEndpoint = async ( timeoutMs?: number, owner?: CdpTargetSessionOwner, commandTimeoutMs?: number, + options?: { readonly reuseBrowserSocket?: boolean }, ): Promise => { + const discoveryTimeoutMs = timeoutMs ?? CDP_DISCOVERY_RESPONSE_TIMEOUT_MS; if (endpoint.transport === "http_discovery") return { - discovery: await discoverCdpEndpoint(endpoint.url, operation, signal), + discovery: await discoverCdpEndpoint( + endpoint.url, + operation, + signal, + discoveryTimeoutMs, + ), connection: undefined, }; - const discoveryTimeoutMs = timeoutMs ?? CDP_DISCOVERY_RESPONSE_TIMEOUT_MS; + const reuseBrowserSocket = + owner !== undefined && options?.reuseBrowserSocket === true; // The browser-socket handshake and discovery commands stay on the discovery // deadline. The selected command deadline starts only after discovery returns. + const discoveryGuardTimeoutMs = Math.min( + MAX_BROWSER_DEADLINE_MS, + discoveryTimeoutMs + 1_000, + ); const discoveryLimits = { - commandTimeoutMs: discoveryTimeoutMs, - // Stay past the discovery abort so a stalled open is labeled as discovery, - // not as a command_timeout_ms handshake. - handshakeTimeoutMs: discoveryTimeoutMs + 1_000, + // Stay past the discovery abort so a stalled open or discovery command is + // labeled as discovery, not as command_timeout_ms, without overflowing + // 32-bit timers. + commandTimeoutMs: discoveryGuardTimeoutMs, + handshakeTimeoutMs: discoveryGuardTimeoutMs, }; return await withCdpDiscoveryDeadline( operation, signal, async (deadlineSignal) => { - const connection = - owner === undefined - ? await CdpConnection.connect( - endpoint.url, - operation, - deadlineSignal, - discoveryLimits, - ) - : await owner.connect( - endpoint.url, - operation, - { signal: deadlineSignal }, - "browser-endpoint", - discoveryLimits, - ); + const acquired = reuseBrowserSocket + ? await owner.acquireBrowserSocket( + endpoint.url, + operation, + { signal: deadlineSignal }, + discoveryLimits, + ) + : { + connection: + owner === undefined + ? await CdpConnection.connect( + endpoint.url, + operation, + deadlineSignal, + discoveryLimits, + ) + : await owner.connect( + endpoint.url, + operation, + { signal: deadlineSignal }, + "browser-endpoint", + discoveryLimits, + ), + reused: false, + }; try { const discovery = await discoverCdpBrowserSocket( - connection, + acquired.connection, endpoint.url, operation, deadlineSignal, ); - connection.selectCommandTimeout( - commandTimeoutMs ?? DEFAULT_CDP_COMMAND_TIMEOUT_MS, - ); + const selectedTimeout = + commandTimeoutMs ?? DEFAULT_CDP_COMMAND_TIMEOUT_MS; + if (reuseBrowserSocket) { + owner.admitBrowserSocket(endpoint.url, acquired.connection); + return { + discovery, + connection: acquired.connection.withOperation( + operation, + selectedTimeout, + ), + reusableBrowserSocket: true, + }; + } + acquired.connection.selectCommandTimeout(selectedTimeout); return { discovery, - connection, + connection: acquired.connection, }; } catch (cause: unknown) { + const releaseLeaseOnly = + reuseBrowserSocket && + deadlineSignal.aborted && + cause instanceof AnalysisCancelledError && + acquired.connection.isReusable(); return await failCdpOpenAfterCleanup( - connection, + acquired.connection, cause, operation, owner, + reuseBrowserSocket + ? releaseLeaseOnly + ? "release" + : "evict" + : "finish", ); } }, @@ -176,9 +224,14 @@ export const failCdpOpenAfterCleanup = async ( primary: unknown, operation: BrowserObservationOperation, owner?: CdpTargetSessionOwner, + ownerDisposition: "finish" | "evict" | "release" = "finish", ): Promise => { try { if (owner === undefined) await connection.close(); + else if (ownerDisposition === "release") + await owner.releaseBrowserSocket(connection); + else if (ownerDisposition === "evict") + await owner.evictBrowserSocket(connection); else await owner.finish(connection); } catch (cleanup: unknown) { throw combineOpenFailure(primary, cleanup, operation); @@ -202,17 +255,24 @@ const combineOpenFailure = ( { cause: primary }, ); -/** Disable enabled domains, detach browser sessions, and close REA's socket. */ +/** Disable enabled domains, detach browser sessions, and release or close REA's socket. */ export const closeCdpTargetSession = async ( targetSession: CdpTargetSession, enabledDomains: readonly string[], signal?: AbortSignal, owner?: CdpTargetSessionOwner, + reuseBrowserSocket = false, ): Promise => { if (owner !== undefined) { - await owner.finish(targetSession.connection, () => - cleanupCdpTargetCommands(targetSession, enabledDomains, signal), - ); + if (reuseBrowserSocket) { + await owner.releaseBrowserSocket(targetSession.connection, () => + cleanupCdpTargetCommands(targetSession, enabledDomains, signal), + ); + return; + } + await owner.finish(targetSession.connection, async () => { + await cleanupCdpTargetCommands(targetSession, enabledDomains, signal); + }); return; } await cleanupCdpTargetCommands(targetSession, enabledDomains, signal); @@ -223,7 +283,7 @@ const cleanupCdpTargetCommands = async ( targetSession: CdpTargetSession, enabledDomains: readonly string[], signal?: AbortSignal, -): Promise => { +): Promise => { const { connection, sessionId } = targetSession; const cleanup = new AbortController(); const domainSignal = @@ -244,29 +304,41 @@ const cleanupCdpTargetCommands = async ( domainSignal, ); } catch { - // Best-effort domain cleanup; transport close is the definitive boundary. + // Best-effort domain cleanup; session detachment or transport close follows. } } } + if (sessionId === undefined) return true; // Cancellation skips optional domain shutdown but still releases the // attached session using its independent, bounded cleanup signal. - if (!cleanup.signal.aborted && sessionId !== undefined) - try { - await connection.send( - "Target.detachFromTarget", - { sessionId }, - undefined, - cleanup.signal, - ); - } catch { - // Best-effort detach; transport close is the definitive boundary. - } + if (cleanup.signal.aborted) return false; + try { + await connection.send( + "Target.detachFromTarget", + { sessionId }, + undefined, + cleanup.signal, + ); + return true; + } catch (cause: unknown) { + if (isAlreadyDetachedSessionRejection(cause)) return true; + // If detachment is unconfirmed, transport close becomes the definitive boundary. + return false; + } } finally { clearTimeout(timeout); cleanup.abort(); } }; +const isAlreadyDetachedSessionRejection = (cause: unknown): boolean => + cause instanceof CdpCommandRejection && + (cause.code === -32_001 || + (cause.reportedMessage !== null && + /no session with given id|session with given id not found/iu.test( + cause.reportedMessage, + ))); + const attachedSessionId = ( value: unknown, operation: BrowserObservationOperation, diff --git a/src/browser/CdpTargetSessionOwner.ts b/src/browser/CdpTargetSessionOwner.ts index 948646b5da..50bb08653f 100644 --- a/src/browser/CdpTargetSessionOwner.ts +++ b/src/browser/CdpTargetSessionOwner.ts @@ -3,7 +3,9 @@ import { AnalysisCleanupLeaseGroup, } from "../artifacts/AnalysisCleanupLease.js"; import type { JsonValue } from "../domain/jsonValue.js"; +import { MAX_BROWSER_DEADLINE_MS } from "../domain/browserDeadline.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { BrowserObservationError } from "../domain/browserObservationError.js"; import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; @@ -13,6 +15,8 @@ import type { ExecutionOptions } from "../application/AnalysisProvider.js"; /** Typed ownership facts shared by transport errors and their adapter projections. */ class CdpTransportCleanupFailure extends ProviderCleanupError { + readonly transportCause: unknown; + constructor( readonly cleanupLeases: readonly AnalysisCleanupLease[], failure: { @@ -26,10 +30,21 @@ class CdpTransportCleanupFailure extends ProviderCleanupError { operation: failure.operation, cause: failure.cause, }); + this.transportCause = failure.cause; new AnalysisCleanupLeaseGroup(this, cleanupLeases); } } +interface ComposedConnectCleanupFailure { + readonly primary: AnalysisError; + readonly cleanupFailure: AnalysisError; +} + +const composedConnectCleanupFailures = new WeakMap< + AnalysisError, + ComposedConnectCleanupFailure +>(); + /** Bind a translated error without interpreting diagnostic cause chains. */ export const bindCdpTransportCleanup = ( cause: unknown, @@ -42,25 +57,364 @@ export const bindCdpTransportCleanup = ( interface OwnedConnection { readonly connection: CdpConnection; - readonly operation: BrowserObservationOperation; + operation: BrowserObservationOperation; readonly resource: string; readonly cleanupLease: AnalysisCleanupLease; active: boolean; - readonly settled: Promise; + activeLeases: number; + settled: Promise; settle: () => void; + reusableUrl: string | undefined; + evicted: boolean; + closingTransport: boolean; + statusUnsubscribe: (() => void) | undefined; cleanup: (() => Promise) | undefined; cleanupAttempted: boolean; failure: ProviderCleanupError | undefined; } +interface BrowserSocketLimits { + readonly maxPayloadBytes?: number; + readonly commandTimeoutMs: number; + readonly handshakeTimeoutMs: number; +} + +interface BrowserSocketWaiter { + readonly operation: BrowserObservationOperation; + readonly commandTimeoutMs: number; + readonly signal: AbortSignal | undefined; + readonly resolve: (acquired: { + readonly connection: CdpConnection; + readonly reused: boolean; + }) => void; + readonly reject: (cause: unknown) => void; + detachAbort: () => void; +} + +interface PendingBrowserSocketOutcome { + readonly cleanupFailure: unknown; +} + +interface PendingBrowserSocket { + readonly controller: AbortController; + readonly waiters: Set; + done: Promise; +} + /** Retains exact REA-owned CDP transports until their close is observed. */ export class CdpTargetSessionOwner { readonly #connections = new Map(); + readonly #browserSockets = new Map(); + readonly #candidateBrowserSockets = new Map(); + readonly #pendingBrowserSockets = new Map(); #closing = false; #admissions = 0; #closePromise: Promise | undefined; readonly #admissionWaiters = new Set<() => void>(); + /** Reuse an admitted healthy browser socket for `url`, or coalesce a cold connect. */ + async acquireBrowserSocket( + url: string, + operation: BrowserObservationOperation, + options: ExecutionOptions, + limits: BrowserSocketLimits, + ): Promise<{ readonly connection: CdpConnection; readonly reused: boolean }> { + this.#assertOpen(operation, "browser-endpoint"); + await this.retryPending(); + const leased = await this.#leaseExistingBrowserSocket( + url, + operation, + options, + limits.commandTimeoutMs, + ); + if (leased !== undefined) return leased; + return this.#joinPendingBrowserSocket(url, operation, options, limits); + } + + async #leaseExistingBrowserSocket( + url: string, + operation: BrowserObservationOperation, + options: ExecutionOptions, + commandTimeoutMs: number, + ): Promise< + { readonly connection: CdpConnection; readonly reused: boolean } | undefined + > { + while (true) { + this.#assertOpen(operation, "browser-endpoint"); + if (options.signal?.aborted === true) + throw new AnalysisCancelledError(operation, { + cause: options.signal.reason, + }); + const admitted = this.#browserSockets.get(url); + const socket = admitted ?? this.#candidateBrowserSockets.get(url); + if (socket === undefined) return undefined; + const record = this.#connections.get(socket); + if ( + record !== undefined && + record.failure === undefined && + !record.evicted && + socket.isReusable() + ) { + if (record.activeLeases === 0) { + let settle = (): void => undefined; + record.settled = new Promise((resolve) => { + settle = resolve; + }); + record.settle = settle; + record.active = true; + } + record.activeLeases += 1; + record.operation = operation; + return { + connection: socket.withOperation(operation, commandTimeoutMs), + reused: admitted !== undefined, + }; + } + if (admitted === socket) this.#browserSockets.delete(url); + if (this.#candidateBrowserSockets.get(url) === socket) + this.#candidateBrowserSockets.delete(url); + if (record !== undefined) { + this.#evictRecord(record); + if (record.activeLeases === 0) + await this.#releaseTransportRecord(record); + } + } + } + + #joinPendingBrowserSocket( + url: string, + operation: BrowserObservationOperation, + options: ExecutionOptions, + limits: BrowserSocketLimits, + ): Promise<{ readonly connection: CdpConnection; readonly reused: boolean }> { + let pending = this.#pendingBrowserSockets.get(url); + let startPending: (() => void) | undefined; + if (pending === undefined) { + const created: PendingBrowserSocket = { + controller: new AbortController(), + waiters: new Set(), + done: Promise.resolve(undefined), + }; + this.#pendingBrowserSockets.set(url, created); + startPending = () => { + created.done = this.#runPendingBrowserSocket({ + url, + pending: created, + initialOperation: operation, + limits, + }); + }; + pending = created; + } + const activePending = pending; + return new Promise((resolve, reject) => { + const waiter: BrowserSocketWaiter = { + operation, + commandTimeoutMs: limits.commandTimeoutMs, + signal: options.signal, + resolve, + reject, + detachAbort: () => undefined, + }; + const onAbort = (): void => { + waiter.detachAbort(); + if (!activePending.waiters.delete(waiter)) return; + const cancelError = new AnalysisCancelledError(operation, { + cause: options.signal?.reason, + }); + if (activePending.waiters.size > 0) { + reject(cancelError); + return; + } + if (this.#pendingBrowserSockets.get(url) === activePending) + this.#pendingBrowserSockets.delete(url); + activePending.controller.abort(options.signal?.reason); + void activePending.done.then((outcome) => { + if (outcome !== undefined) + reject(rebindPendingConnectFailure(outcome.cleanupFailure, waiter)); + else reject(cancelError); + }, reject); + }; + if (options.signal !== undefined) { + options.signal.addEventListener("abort", onAbort, { once: true }); + waiter.detachAbort = () => + options.signal?.removeEventListener("abort", onAbort); + } + activePending.waiters.add(waiter); + if (options.signal?.aborted === true) { + onAbort(); + return; + } + startPending?.(); + }); + } + + async #runPendingBrowserSocket(context: { + readonly url: string; + readonly pending: PendingBrowserSocket; + readonly initialOperation: BrowserObservationOperation; + readonly limits: BrowserSocketLimits; + }): Promise { + const { url, pending, initialOperation, limits } = context; + try { + const root = await this.connect( + url, + initialOperation, + { signal: pending.controller.signal }, + "browser-endpoint", + { + ...(limits.maxPayloadBytes === undefined + ? {} + : { maxPayloadBytes: limits.maxPayloadBytes }), + commandTimeoutMs: limits.commandTimeoutMs, + handshakeTimeoutMs: MAX_BROWSER_DEADLINE_MS, + }, + ); + if (this.#pendingBrowserSockets.get(url) === pending) + this.#pendingBrowserSockets.delete(url); + if (pending.waiters.size === 0) { + await this.finish(root); + return undefined; + } + const record = this.#connections.get(root); + if (record !== undefined) { + record.reusableUrl = url; + record.activeLeases = pending.waiters.size; + if (!this.#closing && !record.evicted && root.isReusable()) + this.#candidateBrowserSockets.set(url, root); + } + const snapshot = [...pending.waiters]; + pending.waiters.clear(); + for (const waiter of snapshot) { + waiter.detachAbort(); + waiter.resolve({ + connection: root.withOperation( + waiter.operation, + waiter.commandTimeoutMs, + ), + reused: false, + }); + } + return undefined; + } catch (cause: unknown) { + if (this.#pendingBrowserSockets.get(url) === pending) + this.#pendingBrowserSockets.delete(url); + const snapshot = [...pending.waiters]; + pending.waiters.clear(); + for (const waiter of snapshot) { + waiter.detachAbort(); + waiter.reject(rebindPendingConnectFailure(cause, waiter)); + } + if ( + snapshot.length === 0 && + ((cause instanceof AnalysisError && cause.cleanupIncomplete) || + cause instanceof AggregateError) + ) + return { cleanupFailure: cause }; + return undefined; + } + } + + /** Publish a verified browser socket for reuse across subsequent operations. */ + admitBrowserSocket(url: string, connection: CdpConnection): void { + const root = transportRoot(connection); + const record = this.#connections.get(root); + if (record === undefined) return; + record.reusableUrl = url; + if (this.#candidateBrowserSockets.get(url) === root) + this.#candidateBrowserSockets.delete(url); + if (this.#closing || record.evicted || !root.isReusable()) { + record.evicted = true; + return; + } + const current = this.#browserSockets.get(url); + if (current === root) return; + if (current !== undefined) { + const currentRecord = this.#connections.get(current); + if ( + current.isReusable() && + currentRecord !== undefined && + currentRecord.failure === undefined && + !currentRecord.evicted + ) { + record.evicted = true; + return; + } + this.#browserSockets.delete(url); + if (currentRecord !== undefined) this.#evictRecord(currentRecord); + } + this.#browserSockets.set(url, root); + let offDisconnect: (() => void) | undefined; + let offProtocolFailure: (() => void) | undefined; + const onUnusable = (): void => { + if (this.#browserSockets.get(url) === root) + this.#browserSockets.delete(url); + record.evicted = true; + record.statusUnsubscribe?.(); + record.statusUnsubscribe = undefined; + if (record.activeLeases === 0 && !this.#closing) { + void this.#releaseTransportRecord(record).catch(() => undefined); + } + }; + record.statusUnsubscribe = () => { + offDisconnect?.(); + offProtocolFailure?.(); + }; + offDisconnect = root.onDisconnect(onUnusable); + offProtocolFailure = root.onProtocolFailure(onUnusable); + } + + /** Release one operation's lease on a browser socket, closing it when tainted or closing. */ + async releaseBrowserSocket( + connection: CdpConnection, + cleanup?: () => Promise, + ): Promise { + const root = transportRoot(connection); + const record = this.#connections.get(root); + if (record === undefined) return; + let keepSocket = !record.cleanupLease.released; + let cleanupFailure: unknown; + if (cleanup !== undefined && !record.cleanupLease.released) { + try { + keepSocket = await cleanup(); + } catch (cause: unknown) { + keepSocket = false; + cleanupFailure = cause; + } + } + const mustCloseImmediately = + !keepSocket || !root.isReusable() || record.failure !== undefined; + if (record.activeLeases > 0) record.activeLeases -= 1; + const canStayOpen = + !mustCloseImmediately && !this.#closing && !record.evicted; + const admitted = + record.reusableUrl !== undefined && + this.#browserSockets.get(record.reusableUrl) === root; + const retain = canStayOpen && (admitted || record.activeLeases > 0); + if (!retain) this.#evictRecord(record); + try { + if (mustCloseImmediately || (!retain && record.activeLeases === 0)) + await this.#releaseTransportRecord(record, cleanupFailure); + if (cleanupFailure !== undefined) { + const failure = + cleanupFailure instanceof CdpTransportCleanupFailure + ? cleanupFailure + : cleanupError(record, cleanupFailure); + throw failure; + } + } finally { + if (record.activeLeases === 0) { + record.active = false; + record.settle(); + } + } + } + + /** Evict a browser socket after failed discovery or unconfirmed attachment state. */ + async evictBrowserSocket(connection: CdpConnection): Promise { + await this.releaseBrowserSocket(connection, async () => false); + } + /** Admit and own one WebSocket from handshake completion onward. */ async connect( url: string, @@ -73,21 +427,9 @@ export class CdpTargetSessionOwner { readonly handshakeTimeoutMs?: number; }, ): Promise { - if (this.#closing) - throw new ProviderCleanupError( - "rea-cdp-browser", - [resource], - { reason: "CDP provider is closing and cannot admit another session." }, - { operation }, - ); + this.#assertOpen(operation, resource); await this.retryPending(); - if (this.#closing) - throw new ProviderCleanupError( - "rea-cdp-browser", - [resource], - { reason: "CDP provider is closing and cannot admit another session." }, - { operation }, - ); + this.#assertOpen(operation, resource); if (options.signal?.aborted === true) throw new AnalysisCancelledError(operation, { cause: options.signal.reason, @@ -113,11 +455,17 @@ export class CdpTargetSessionOwner { if ( cause instanceof AnalysisError && cleanupCause instanceof AnalysisError - ) - throw bindCdpTransportCleanup( + ) { + const combined = bindCdpTransportCleanup( cleanupCause, analysisErrorWithCleanupFailure(cause, cleanupCause, operation), ); + composedConnectCleanupFailures.set(combined, { + primary: cause, + cleanupFailure: cleanupCause, + }); + throw combined; + } throw new AggregateError( [cause, cleanupCause], `CDP operation ${operation} failed during handshake and cleanup remains incomplete.`, @@ -140,21 +488,24 @@ export class CdpTargetSessionOwner { operation: BrowserObservationOperation, resource: string, ): void { + const root = transportRoot(connection); let settle = (): void => undefined; const settled = new Promise((resolve) => { settle = resolve; }); - this.#connections.set(connection, { - connection, + this.#connections.set(root, { + connection: root, operation, resource, - cleanupLease: new AnalysisCleanupLease( - () => connection.close(), - [resource], - ), + cleanupLease: new AnalysisCleanupLease(() => root.close(), [resource]), active: true, + activeLeases: 1, settled, settle, + reusableUrl: undefined, + evicted: false, + closingTransport: false, + statusUnsubscribe: undefined, cleanup: undefined, cleanupAttempted: false, failure: undefined, @@ -166,19 +517,24 @@ export class CdpTargetSessionOwner { connection: CdpConnection, cleanup?: () => Promise, ): Promise { - const record = this.#connections.get(connection); + const root = transportRoot(connection); + const record = this.#connections.get(root); if (record === undefined) return; if (record.failure !== undefined) throw record.failure; + this.#evictRecord(record); record.cleanup ??= cleanup; + record.activeLeases = 0; try { if (!record.cleanupAttempted && record.cleanup !== undefined) { record.cleanupAttempted = true; await record.cleanup(); } - await record.cleanupLease.release(); - this.#connections.delete(connection); + await this.#releaseTransportRecord(record); } catch (cause: unknown) { - const failure = cleanupError(record, cause); + const failure = + cause instanceof CdpTransportCleanupFailure + ? cause + : cleanupError(record, cause); record.failure = failure; throw failure; } finally { @@ -187,34 +543,92 @@ export class CdpTargetSessionOwner { } } - /** Retry only retained failed transports; active peers finish their own cleanup. */ + /** Close retained browser sockets and retry failed transports after active peers finish. */ async close(): Promise { this.#closing = true; + this.#browserSockets.clear(); + this.#candidateBrowserSockets.clear(); this.#closePromise ??= this.#closeAll().catch((cause: unknown) => { this.#closePromise = undefined; throw cause; }); - return await this.#closePromise; + return this.#closePromise; } - /** Retry retained failures without touching concurrently active sessions. */ + /** Retry retained failures, including failed shared-socket closes with active peers. */ async retryPending(): Promise { const failures: CdpTransportCleanupFailure[] = []; for (const record of this.#connections.values()) { - if (record.active || record.failure === undefined) continue; + const needsRelease = + !record.cleanupLease.released && + (record.failure !== undefined || + record.closingTransport || + (!record.active && record.evicted)); + if (!needsRelease) continue; try { - await record.cleanupLease.release(); - this.#connections.delete(record.connection); + await this.#releaseTransportRecord(record); } catch (cause: unknown) { - const failure = cleanupError(record, cause); - record.failure = failure; - failures.push(failure); + if (cause instanceof CdpTransportCleanupFailure) failures.push(cause); + else throw cause; } } if (failures.length === 1) throw failures[0]; if (failures.length > 1) throw aggregateCleanupFailures(failures); } + #assertOpen(operation: BrowserObservationOperation, resource: string): void { + if (!this.#closing) return; + throw new ProviderCleanupError( + "rea-cdp-browser", + [resource], + { reason: "CDP provider is closing and cannot admit another session." }, + { operation }, + ); + } + + #evictRecord(record: OwnedConnection): void { + record.evicted = true; + if (record.reusableUrl !== undefined) { + if (this.#browserSockets.get(record.reusableUrl) === record.connection) + this.#browserSockets.delete(record.reusableUrl); + if ( + this.#candidateBrowserSockets.get(record.reusableUrl) === + record.connection + ) + this.#candidateBrowserSockets.delete(record.reusableUrl); + } + record.statusUnsubscribe?.(); + record.statusUnsubscribe = undefined; + } + + async #releaseTransportRecord( + record: OwnedConnection, + primaryCleanupCause?: unknown, + ): Promise { + record.closingTransport = true; + record.statusUnsubscribe?.(); + record.statusUnsubscribe = undefined; + try { + await record.cleanupLease.release(); + record.closingTransport = false; + record.failure = undefined; + if (record.activeLeases === 0) + this.#connections.delete(record.connection); + } catch (cause: unknown) { + const combinedCause = + primaryCleanupCause === undefined + ? cause + : new AggregateError( + [primaryCleanupCause, cause], + "Target cleanup and CDP transport close both failed.", + { cause: primaryCleanupCause }, + ); + const failure = cleanupError(record, combinedCause); + record.failure = failure; + throw failure; + } + } + async #closeAll(): Promise { while (this.#admissions > 0) await new Promise((resolve) => this.#admissionWaiters.add(resolve)); @@ -227,13 +641,12 @@ export class CdpTargetSessionOwner { } const failures: CdpTransportCleanupFailure[] = []; for (const record of this.#connections.values()) { + this.#evictRecord(record); try { - await record.cleanupLease.release(); - this.#connections.delete(record.connection); + await this.#releaseTransportRecord(record); } catch (cause: unknown) { - const failure = cleanupError(record, cause); - record.failure = failure; - failures.push(failure); + if (cause instanceof CdpTransportCleanupFailure) failures.push(cause); + else throw cause; } } if (failures.length === 1) throw failures[0]; @@ -241,6 +654,71 @@ export class CdpTargetSessionOwner { } } +const transportRoot = (connection: CdpConnection): CdpConnection => + (connection as Partial>).transportRoot ?? + connection; + +const rebindPendingConnectFailure = ( + cause: unknown, + waiter: Pick, +): unknown => { + if (cause instanceof BrowserObservationError) { + const rebound = cause.forOperation(waiter.operation); + return cause.cleanupIncomplete + ? rebound.bindCleanupDependencies([cause]) + : rebound; + } + if (cause instanceof AnalysisCancelledError) { + const rebound = new AnalysisCancelledError(waiter.operation, { + cause: waiter.signal?.reason ?? cause.cause, + cleanup: cause.cleanup, + }); + return cause.cleanupIncomplete + ? rebound.bindCleanupDependencies([cause]) + : rebound; + } + if (cause instanceof AnalysisError) { + const composed = composedConnectCleanupFailures.get(cause); + if (composed !== undefined) { + const reboundPrimary = rebindPendingConnectFailure( + composed.primary, + waiter, + ); + if ( + reboundPrimary instanceof AnalysisError && + reboundPrimary !== composed.primary + ) { + const reboundComposed = bindCdpTransportCleanup( + composed.cleanupFailure, + analysisErrorWithCleanupFailure( + reboundPrimary, + composed.cleanupFailure, + waiter.operation, + ), + ); + composedConnectCleanupFailures.set(reboundComposed, { + primary: reboundPrimary, + cleanupFailure: composed.cleanupFailure, + }); + return reboundComposed; + } + return cause; + } + } + if ( + cause instanceof CdpTransportCleanupFailure && + cause.operation !== waiter.operation + ) { + return new CdpTransportCleanupFailure(cause.cleanupLeases, { + resources: cause.cleanupResources, + operation: waiter.operation, + diagnostics: cause.diagnostics ?? {}, + cause: cause.transportCause, + }); + } + return cause; +}; + const cleanupError = ( record: OwnedConnection, cause: unknown, diff --git a/src/cli/browserAdvancedCommands.ts b/src/cli/browserAdvancedCommands.ts index 230de44fcd..33649c1fc5 100644 --- a/src/cli/browserAdvancedCommands.ts +++ b/src/cli/browserAdvancedCommands.ts @@ -4,8 +4,9 @@ import { Cli, z } from "incur"; import { browserCliError } from "./browserCliError.js"; import { createBrowserObservationProvider } from "../composition/browserObservation.js"; import { - browserScopeOptions, + passiveBrowserScopeOptions, selectedCommandTimeout, + selectedDiscoveryTimeout, } from "../cliObservationOptions.js"; import { captureWebScreenshot, @@ -66,7 +67,7 @@ const registerWebMcp = ( ), }), options: z.object({ - ...browserScopeOptions, + ...passiveBrowserScopeOptions, observationMs: cliDecimalField( discoverWebMcpToolsInputSchema.shape.observation_ms, ).describe("Observation duration in milliseconds"), @@ -80,6 +81,7 @@ const registerWebMcp = ( allowed_origins: options.allowedOrigins, target_id: args.targetId, observation_ms: options.observationMs, + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), ...selectedCommandTimeout(options.commandTimeoutMs), }; const parsed = discoverWebMcpToolsInputSchema.safeParse(input); @@ -183,7 +185,7 @@ const registerScreenshot = ( ), }), options: z.object({ - ...browserScopeOptions, + ...passiveBrowserScopeOptions, }), run: ({ args, options }) => withCommandCancellation((signal) => @@ -193,6 +195,7 @@ const registerScreenshot = ( cdp_endpoint: args.endpoint, allowed_origins: options.allowedOrigins, target_id: args.targetId, + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), ...selectedCommandTimeout(options.commandTimeoutMs), }; const parsed = captureWebScreenshotInputSchema.safeParse(input); diff --git a/src/cli/browserCommands.ts b/src/cli/browserCommands.ts index 610c1484dc..93e5c966b9 100644 --- a/src/cli/browserCommands.ts +++ b/src/cli/browserCommands.ts @@ -30,9 +30,10 @@ import { cliDecimalField } from "./options.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { browserPageInspectionOptions, - browserScopeOptions, observationDuration, + passiveBrowserScopeOptions, selectedCommandTimeout, + selectedDiscoveryTimeout, } from "../cliObservationOptions.js"; import { withCommandCancellation } from "./commandCancellation.js"; import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; @@ -61,7 +62,8 @@ const registerTargetList = ( ), }), options: z.object({ - allowedOrigins: browserScopeOptions.allowedOrigins, + allowedOrigins: passiveBrowserScopeOptions.allowedOrigins, + discoveryTimeoutMs: passiveBrowserScopeOptions.discoveryTimeoutMs, }), run: ({ args, options }) => withCommandCancellation((signal) => @@ -70,6 +72,7 @@ const registerTargetList = ( const input = { cdp_endpoint: args.endpoint, allowed_origins: options.allowedOrigins, + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), }; const parsed = listBrowserTargetsInputSchema.safeParse(input); if (!parsed.success) @@ -128,6 +131,7 @@ const registerPageInspection = ( include_script_sources: options.includeScriptSources, include_storage_keys: options.includeStorageKeys, include_storage_fingerprints: options.includeStorageFingerprints, + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), ...selectedCommandTimeout(options.commandTimeoutMs), }; const parsed = inspectWebPageInputSchema.safeParse(input); @@ -171,7 +175,7 @@ const registerBundleAnalysis = ( ), }), options: z.object({ - ...browserScopeOptions, + ...passiveBrowserScopeOptions, ...webBundleDeliveryOptions, observationMs: observationDuration(500), fetchSourceMaps: z @@ -220,6 +224,7 @@ const registerBundleAnalysis = ( : { source_map_fetch_timeout_ms: options.sourceMapFetchTimeoutMs, }), + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), ...selectedCommandTimeout(options.commandTimeoutMs), }; const parsed = analyzeWebBundleInputSchema.safeParse(input); @@ -263,7 +268,7 @@ const registerObservationSession = ( ), }), options: z.object({ - ...browserScopeOptions, + ...passiveBrowserScopeOptions, observationMs: observationDuration(10_000, 1), }), run: ({ args, options }) => @@ -275,6 +280,7 @@ const registerObservationSession = ( allowed_origins: options.allowedOrigins, target_id: args.targetId, observation_ms: options.observationMs, + ...selectedDiscoveryTimeout(options.discoveryTimeoutMs), ...selectedCommandTimeout(options.commandTimeoutMs), }; const parsed = observeWebSessionInputSchema.safeParse(input); diff --git a/src/cliObservationOptions.ts b/src/cliObservationOptions.ts index 8fce3a8f0f..f1ace24baa 100644 --- a/src/cliObservationOptions.ts +++ b/src/cliObservationOptions.ts @@ -1,7 +1,10 @@ import { javascriptRouteDeliveryOptions } from "./cli/javascriptRouteDelivery.js"; import { z } from "incur"; import { cliDecimalField } from "./cli/options.js"; -import { webRuntimeCommandTimeoutSchema } from "./domain/browserDeadline.js"; +import { + browserDiscoveryTimeoutSchema, + webRuntimeCommandTimeoutSchema, +} from "./domain/browserDeadline.js"; import { javaScriptAnalysisResourceControlsSchema } from "./domain/javascript/javascriptAnalysisResourceControls.js"; import { javaScriptSourcePathsSchema } from "./domain/javascript/javascriptSourceSelection.js"; @@ -25,6 +28,20 @@ export const browserScopeOptions = { commandTimeoutMs: cliDecimalField(webRuntimeCommandTimeoutSchema), }; +/** Shared options for passive browser commands that also accept discovery_timeout_ms. */ +export const passiveBrowserScopeOptions = { + ...browserScopeOptions, + discoveryTimeoutMs: cliDecimalField(browserDiscoveryTimeoutSchema), +}; + +/** Keep an omitted discovery deadline absent so Evidence records the caller selection only. */ +export const selectedDiscoveryTimeout = ( + discoveryTimeoutMs: number | undefined, +): { readonly discovery_timeout_ms?: number } => + discoveryTimeoutMs === undefined + ? {} + : { discovery_timeout_ms: discoveryTimeoutMs }; + /** Keep an omitted command deadline absent so Evidence records the caller selection only. */ export const selectedCommandTimeout = ( commandTimeoutMs: number | undefined, @@ -34,7 +51,7 @@ export const selectedCommandTimeout = ( : { command_timeout_ms: commandTimeoutMs }; export const browserPageInspectionOptions = z.object({ - ...browserScopeOptions, + ...passiveBrowserScopeOptions, observationMs: observationDuration(500), includeAccessibilityText: z .boolean() diff --git a/src/contracts/browserToolContracts.ts b/src/contracts/browserToolContracts.ts index 27b24d8ca6..fdf78819a5 100644 --- a/src/contracts/browserToolContracts.ts +++ b/src/contracts/browserToolContracts.ts @@ -171,7 +171,7 @@ export const BROWSER_TOOL_CONTRACTS = [ name: "list_browser_targets", ...toolContractMetadata("list_browser_targets"), description: - "List page targets exposed by the selected loopback Chrome DevTools Protocol endpoint. Optionally filter by exact origin. URLs preserve query values and fragments; only URL userinfo credentials are removed. HTTP discovery and browser-socket discovery use the fixed 10-second discovery deadline, then the socket closes without a later command. This operation accepts no command_timeout_ms; that listing deadline has no runtime override.", + "List page targets exposed by the selected loopback Chrome DevTools Protocol endpoint. Optionally filter by exact origin. URLs preserve query values and fragments; only URL userinfo credentials are removed. HTTP discovery and browser-socket discovery use optional discovery_timeout_ms (default 10000 ms, range 1..2147483647, applied on the next call) without running a post-discovery target command, so this operation accepts no command_timeout_ms. An admitted browser-socket endpoint stays retained on the active browser provider for later calls and closes on unconfirmed target-session cleanup or when that provider closes; a discovery deadline or cancellation on a clean shared socket releases only that call's lease.", kind: "browser-provider", inputSchema: listBrowserTargetsInputSchema, outputSchema: listOutputSchema, diff --git a/src/domain/browserDeadline.ts b/src/domain/browserDeadline.ts index 8fae18eb15..9925733ac3 100644 --- a/src/domain/browserDeadline.ts +++ b/src/domain/browserDeadline.ts @@ -58,11 +58,33 @@ export const noRuntimeOverrideSentence = ( /** Handshake and per-command deadline shared by CDP browser tools. */ export const webRuntimeCommandTimeoutSchema = optionalBrowserDeadlineMs( - `CDP WebSocket handshake and per-command response deadline in milliseconds (default ${String(DEFAULT_CDP_COMMAND_TIMEOUT_MS)}). Omitted means ${String(DEFAULT_CDP_COMMAND_TIMEOUT_MS)} and applies on the next call. deadlineExpired for a handshake or command is not the caller observation window, not the ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} ms CDP discovery response deadline, and not the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace. It also bounds passive script-source collection and the inspection/source-join phase that share this deadline. Does not raise the fixed protocol, retained-event, or source byte budgets. CDP discovery and cleanup grace have no runtime override.`, + `CDP WebSocket handshake and per-command response deadline in milliseconds (default ${String(DEFAULT_CDP_COMMAND_TIMEOUT_MS)}). Omitted means ${String(DEFAULT_CDP_COMMAND_TIMEOUT_MS)} and applies on the next call. deadlineExpired for a handshake or command is not the caller observation window, not the CDP discovery deadline (default ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} ms, or discovery_timeout_ms where supported), and not the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace. It also bounds passive script-source collection and the inspection/source-join phase that share this deadline. Does not raise the fixed protocol, retained-event, or source byte budgets, or the cleanup grace, which have no runtime override.`, +); + +/** Per-response HTTP discovery and browser-socket discovery deadline. */ +export const browserDiscoveryTimeoutSchema = optionalBrowserDeadlineMs( + `Loopback CDP HTTP discovery per-response deadline and browser-socket handshake plus discovery (Browser.getVersion, Target.getTargets) deadline in milliseconds (default ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)}). Omitted means ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} and applies on the next call. Separate from command_timeout_ms, the caller observation window, and the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace. Does not raise the fixed HTTP discovery document or WebSocket protocol byte budgets, which have no runtime override.`, ); export type CdpCommandDeadlinePhase = "handshake" | "command" | "source-join"; +/** + * Operations whose public input accepts discovery_timeout_ms. Other CDP callers + * keep the fixed numeric discovery deadline and must not advertise a setting they lack. + */ +const DISCOVERY_TIMEOUT_OPERATIONS = new Set([ + "list_browser_targets", + "inspect_web_page", + "analyze_web_bundle", + "observe_web_session", + "discover_webmcp_tools", + "capture_web_screenshot", +]); + +export const cdpDiscoveryTimeoutIsSelectable = ( + operation: BrowserObservationOperation, +): boolean => DISCOVERY_TIMEOUT_OPERATIONS.has(operation); + /** * Operations whose public input accepts command_timeout_ms. Other CDP callers * keep the same numeric deadline and must not advertise a setting they lack. @@ -121,7 +143,43 @@ export const cdpCommandDeadlineRecovery = ( } return { userMessage: `${subject} This is not the caller observation window, not the ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} ms CDP discovery deadline, and not the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace.`, - remediationAction: `${browserDeadlineIncrease("command_timeout_ms", timeoutMs, DEFAULT_CDP_COMMAND_TIMEOUT_MS, "--command-timeout-ms")} ${browserDeadlineRetry} command_timeout_ms does not raise the fixed protocol, retained-event, or source byte budgets, the ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} ms CDP discovery response deadline, or the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace. Those have no runtime override.`, + remediationAction: `${browserDeadlineIncrease("command_timeout_ms", timeoutMs, DEFAULT_CDP_COMMAND_TIMEOUT_MS, "--command-timeout-ms")} ${browserDeadlineRetry} command_timeout_ms does not raise the CDP discovery deadline (default ${String(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)} ms, or discovery_timeout_ms where supported), nor the fixed protocol, retained-event, or source byte budgets or the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace, which have no runtime override.`, + }; +}; + +/** Name the CDP discovery deadline, and name discovery_timeout_ms only when the operation accepts it. */ +export const cdpDiscoveryTimeoutRecovery = ( + operation: BrowserObservationOperation, + timeoutMs: number, + phase: "response" | "browser-socket", + budgetDescription: string, +): { readonly userMessage: string; readonly remediationAction: string } => { + const unrelatedCommand = cdpCommandTimeoutUnrelatedControl(operation); + if (!cdpDiscoveryTimeoutIsSelectable(operation)) { + const subject = + phase === "response" + ? `CDP discovery response exceeded its ${String(timeoutMs)} ms deadline.` + : `CDP browser-socket discovery exceeded its ${String(timeoutMs)} ms deadline.`; + const fixed = noRuntimeOverrideSentence( + `${budgetDescription} and the ${String(timeoutMs)} ms discovery deadline stay fixed.`, + unrelatedCommand, + ); + return { + userMessage: `${subject} ${fixed}`, + remediationAction: fixed, + }; + } + const subject = + phase === "response" + ? `CDP discovery response exceeded its ${String(timeoutMs)} ms discovery_timeout_ms deadline (deadlineExpired).` + : `CDP browser-socket discovery exceeded its ${String(timeoutMs)} ms discovery_timeout_ms deadline (deadlineExpired).`; + const fixedBudget = noRuntimeOverrideSentence( + `${budgetDescription} stays fixed.`, + unrelatedCommand, + ); + return { + userMessage: `${subject} This is not the caller observation window and not the ${String(CDP_CLEANUP_GRACE_MS)} ms cleanup grace. ${fixedBudget}`, + remediationAction: `${browserDeadlineIncrease("discovery_timeout_ms", timeoutMs, CDP_DISCOVERY_RESPONSE_TIMEOUT_MS, "--discovery-timeout-ms")} ${browserDeadlineRetry} ${fixedBudget}`, }; }; @@ -136,3 +194,8 @@ export const selectedCdpCommandTimeoutMs = ( selected: number | undefined, ): number => selectedBrowserDeadlineMs(selected, DEFAULT_CDP_COMMAND_TIMEOUT_MS); + +export const selectedCdpDiscoveryTimeoutMs = ( + selected: number | undefined, +): number => + selectedBrowserDeadlineMs(selected, CDP_DISCOVERY_RESPONSE_TIMEOUT_MS); diff --git a/src/domain/browserObservation.test.ts b/src/domain/browserObservation.test.ts index 6df175f653..4ca3ce2042 100644 --- a/src/domain/browserObservation.test.ts +++ b/src/domain/browserObservation.test.ts @@ -142,6 +142,54 @@ describe("browser observation contracts", () => { }); }); +it("normalizes equivalent loopback browser-socket spellings while keeping IPv4, IPv6, and socket IDs distinct", () => { + expect( + browserObservationEndpointSchema.parse( + "WS://[0:0:0:0:0:0:0:1]:09222/devtools/browser/0b1c-socket-id", + ), + ).toEqual({ + transport: "browser_websocket", + url: "ws://[::1]:9222/devtools/browser/0b1c-socket-id", + recorded: "ws://[::1]:9222/devtools/browser/[REDACTED]", + }); + expect( + browserObservationEndpointSchema.parse( + "WS://127.0.0.1:0080/devtools/browser/0b1c-socket-id", + ), + ).toEqual({ + transport: "browser_websocket", + url: "ws://127.0.0.1:80/devtools/browser/0b1c-socket-id", + recorded: "ws://127.0.0.1:80/devtools/browser/[REDACTED]", + }); + expect( + browserObservationEndpointSchema.parse( + "WS://[0000:0000:0000:0000:0000:0000:0000:0001]:09222/devtools/browser/0b1c-socket-id", + ).url, + ).toBe( + browserObservationEndpointSchema.parse( + "ws://[::1]:9222/devtools/browser/0b1c-socket-id", + ).url, + ); + expect( + browserObservationEndpointSchema.parse( + "ws://127.0.0.1:9222/devtools/browser/0b1c-socket-id", + ).url, + ).not.toBe( + browserObservationEndpointSchema.parse( + "ws://[::1]:9222/devtools/browser/0b1c-socket-id", + ).url, + ); + expect( + browserObservationEndpointSchema.parse( + "ws://127.0.0.1:9222/devtools/browser/Socket-Id", + ).url, + ).not.toBe( + browserObservationEndpointSchema.parse( + "ws://127.0.0.1:9222/devtools/browser/socket-id", + ).url, + ); +}); + describe("browser observation sensitive surfaces and retention", () => { it("removes only URL userinfo and preserves the original query and fragment", () => { expect( diff --git a/src/domain/browserObservation.ts b/src/domain/browserObservation.ts index 503e228ea4..5964ea381e 100644 --- a/src/domain/browserObservation.ts +++ b/src/domain/browserObservation.ts @@ -1,6 +1,9 @@ import { z } from "zod"; -import { webRuntimeCommandTimeoutSchema } from "./browserDeadline.js"; +import { + browserDiscoveryTimeoutSchema, + webRuntimeCommandTimeoutSchema, +} from "./browserDeadline.js"; import { urlDeclaresQueryOrFragment } from "./urlComponents.js"; import { parseRecordedTransportUrl, @@ -194,12 +197,13 @@ export const browserAllowedOriginsSchema = z const browserInput = { cdp_endpoint: browserObservationEndpointSchema, allowed_origins: browserAllowedOriginsSchema, + discovery_timeout_ms: browserDiscoveryTimeoutSchema, }; /** * Public input for complete discovery, optionally filtered by exact origin. - * Discovery uses the fixed deadline and the socket closes without a later - * command, so this operation accepts no command_timeout_ms. + * Discovery uses discovery_timeout_ms (default 10000 ms) without running a + * post-discovery target command, so this operation accepts no command_timeout_ms. */ export const listBrowserTargetsInputSchema = z.strictObject({ ...browserInput, diff --git a/src/domain/browserSession.ts b/src/domain/browserSession.ts index 944a98d14e..24d5a9f4d2 100644 --- a/src/domain/browserSession.ts +++ b/src/domain/browserSession.ts @@ -1,7 +1,10 @@ import { z } from "zod"; import { utcDateTimeSchema } from "./offsetDateTime.js"; -import { webRuntimeCommandTimeoutSchema } from "./browserDeadline.js"; +import { + browserDiscoveryTimeoutSchema, + webRuntimeCommandTimeoutSchema, +} from "./browserDeadline.js"; import { browserAllowedOriginsSchema, browserObservationEndpointSchema, @@ -13,6 +16,7 @@ import { browserVersionSchema } from "./browserObservationSchemas.js"; export const observeWebSessionInputSchema = z.strictObject({ cdp_endpoint: browserObservationEndpointSchema, allowed_origins: browserAllowedOriginsSchema, + discovery_timeout_ms: browserDiscoveryTimeoutSchema, target_id: z.string().min(1), observation_ms: z.int().min(1).default(10_000), command_timeout_ms: webRuntimeCommandTimeoutSchema, diff --git a/src/domain/webDeadlineControls.test.ts b/src/domain/webDeadlineControls.test.ts index 70a57b47e8..54c0632df7 100644 --- a/src/domain/webDeadlineControls.test.ts +++ b/src/domain/webDeadlineControls.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { cdpCleanupGraceExpired, cdpCommandDeadlineRecovery, + cdpDiscoveryTimeoutRecovery, } from "./browserDeadline.js"; import { listBrowserTargetsInputSchema } from "./browserObservation.js"; import { @@ -47,6 +48,7 @@ describe("web deadline controls", () => { expect(parsed.total_analysis_timeout_ms).toBeUndefined(); expect(parsed.max_heap_mb).toBeUndefined(); expect(parsed.source_map_fetch_timeout_ms).toBeUndefined(); + expect(parsed.discovery_timeout_ms).toBeUndefined(); expect(parsed.command_timeout_ms).toBeUndefined(); }); @@ -54,9 +56,22 @@ describe("web deadline controls", () => { expect( analyzeWebBundleInputSchema.safeParse({ ...bundle, + discovery_timeout_ms: 2_147_483_647, command_timeout_ms: 2_147_483_647, }).success, ).toBe(true); + expect( + listBrowserTargetsInputSchema.safeParse({ + cdp_endpoint: "http://127.0.0.1:9222", + discovery_timeout_ms: 15_000, + }).success, + ).toBe(true); + expect( + listBrowserTargetsInputSchema.safeParse({ + cdp_endpoint: "http://127.0.0.1:9222", + discovery_timeout_ms: 0, + }).success, + ).toBe(false); expect( listBrowserTargetsInputSchema.safeParse({ cdp_endpoint: "http://127.0.0.1:9222", @@ -108,6 +123,35 @@ describe("web deadline controls", () => { expect(inspector.userMessage).not.toContain("command_timeout_ms"); expect(inspector.remediationAction).not.toContain("--command-timeout-ms"); expect(inspector.remediationAction).toContain("no runtime override"); + const selectableDiscovery = cdpDiscoveryTimeoutRecovery( + "inspect_web_page", + 15_000, + "browser-socket", + "The 67108864-byte browser-socket message budget", + ); + expect(selectableDiscovery.userMessage).toContain( + "15000 ms discovery_timeout_ms", + ); + expect(selectableDiscovery.remediationAction).toContain( + "Increase discovery_timeout_ms above 15000", + ); + expect(selectableDiscovery.remediationAction).toContain( + "--discovery-timeout-ms", + ); + expect(selectableDiscovery.remediationAction).toContain( + "command_timeout_ms does not raise it", + ); + const fixedDiscovery = cdpDiscoveryTimeoutRecovery( + "observe_web_execution", + 10_000, + "response", + "The 8388608-byte discovery document budget", + ); + expect(fixedDiscovery.userMessage).not.toContain("discovery_timeout_ms"); + expect(fixedDiscovery.remediationAction).not.toContain( + "--discovery-timeout-ms", + ); + expect(fixedDiscovery.remediationAction).toContain("no runtime override"); }); }); diff --git a/src/domain/webMcpDiscovery.ts b/src/domain/webMcpDiscovery.ts index bbcbce5fa2..4be8ab7b7b 100644 --- a/src/domain/webMcpDiscovery.ts +++ b/src/domain/webMcpDiscovery.ts @@ -1,6 +1,9 @@ import { z } from "zod"; -import { webRuntimeCommandTimeoutSchema } from "./browserDeadline.js"; +import { + browserDiscoveryTimeoutSchema, + webRuntimeCommandTimeoutSchema, +} from "./browserDeadline.js"; import { browserCompletenessSchema } from "./browserCompleteness.js"; import { browserAllowedOriginsSchema, @@ -15,6 +18,7 @@ import { digestSchema, prefixedDigestSchema } from "./../domain/digests.js"; export const discoverWebMcpToolsInputSchema = z.strictObject({ cdp_endpoint: browserObservationEndpointSchema, allowed_origins: browserAllowedOriginsSchema, + discovery_timeout_ms: browserDiscoveryTimeoutSchema, target_id: z.string().min(1), observation_ms: z.int().min(0).default(100), command_timeout_ms: webRuntimeCommandTimeoutSchema, diff --git a/src/domain/webScreenshot.ts b/src/domain/webScreenshot.ts index 5cdbd69d99..8147cd2536 100644 --- a/src/domain/webScreenshot.ts +++ b/src/domain/webScreenshot.ts @@ -3,7 +3,10 @@ import { createHash } from "node:crypto"; import { z } from "zod"; import { utcDateTimeSchema } from "./offsetDateTime.js"; -import { webRuntimeCommandTimeoutSchema } from "./browserDeadline.js"; +import { + browserDiscoveryTimeoutSchema, + webRuntimeCommandTimeoutSchema, +} from "./browserDeadline.js"; import { browserCompletenessSchema } from "./browserCompleteness.js"; import { browserAllowedOriginsSchema, @@ -38,6 +41,7 @@ export type WebScreenshotArtifact = z.infer; export const captureWebScreenshotInputSchema = z.strictObject({ cdp_endpoint: browserObservationEndpointSchema, allowed_origins: browserAllowedOriginsSchema, + discovery_timeout_ms: browserDiscoveryTimeoutSchema, target_id: z.string().min(1), command_timeout_ms: webRuntimeCommandTimeoutSchema, }); diff --git a/tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts b/tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts index 9751306a55..cb487a8011 100644 --- a/tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts +++ b/tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts @@ -3,66 +3,211 @@ import { createServer, type Socket } from "node:net"; import { describe, expect, it, vi } from "vitest"; import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; +import { CdpConnection } from "../../../src/browser/CdpConnection.js"; +import { CdpTargetSessionOwner } from "../../../src/browser/CdpTargetSessionOwner.js"; import { openCdpEndpoint } from "../../../src/browser/CdpTargetSession.js"; import { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { browserObservationEndpointSchema, inspectWebPageInputSchema, listBrowserTargetsInputSchema, } from "../../../src/domain/browserObservation.js"; +import { observeWebSessionInputSchema } from "../../../src/domain/browserSession.js"; +import { captureWebScreenshotInputSchema } from "../../../src/domain/webScreenshot.js"; import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; +import { awaitAcknowledgement } from "../../fixtures/lifecycleAcknowledgement.js"; import { trackBrowser } from "./cdpBrowserProvider.support.js"; describe("CdpBrowserProvider: browser socket without HTTP discovery", () => { - it("discovers and inspects a page through one browser socket", async () => { + it("reuses one admitted browser socket across sequential operations and closes on provider shutdown", async () => { const browser = await startFakeCdpBrowser(); trackBrowser(browser); const provider = new CdpBrowserProvider(); - const throughHttp = await provider.listTargets( - listBrowserTargetsInputSchema.parse({ cdp_endpoint: browser.endpoint }), - ); - if (!throughHttp.ok) throw throughHttp.error; - const httpRequests = browser.httpRequests.length; - const openedBefore = browser.openedConnectionCount(); + try { + const throughHttp = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ cdp_endpoint: browser.endpoint }), + ); + if (!throughHttp.ok) throw throughHttp.error; + const httpRequests = browser.httpRequests.length; + const openedBefore = browser.openedConnectionCount(); - const listed = await provider.listTargets( - listBrowserTargetsInputSchema.parse({ - cdp_endpoint: browser.browserWebSocketUrl, - }), - ); - if (!listed.ok) throw listed.error; - expect(listed.value.targets).toEqual(throughHttp.value.targets); - expect(listed.value.excluded).toEqual(throughHttp.value.excluded); - expect(listed.value.browser).toEqual({ - ...throughHttp.value.browser, - revision: "@fake-revision", + const listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!listed.ok) throw listed.error; + expect(listed.value.targets).toEqual(throughHttp.value.targets); + expect(listed.value.excluded).toEqual(throughHttp.value.excluded); + expect(listed.value.browser).toEqual({ + ...throughHttp.value.browser, + revision: "@fake-revision", + }); + expect(browser.openedConnectionCount()).toBe(openedBefore + 1); + expect(browser.activeConnectionCount()).toBe(1); + + const commandsBeforeInspect = browser.commands.length; + const inspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!inspected.ok) throw inspected.error; + expect(inspected.value.target.origin).toBe(browser.allowedOrigin); + expect( + browser.commands + .slice(commandsBeforeInspect, commandsBeforeInspect + 3) + .map(({ method, params }) => ({ method, params })), + ).toEqual([ + { method: "Browser.getVersion", params: {} }, + { method: "Target.getTargets", params: {} }, + { + method: "Target.attachToTarget", + params: { targetId: "allowed-page", flatten: true }, + }, + ]); + expect( + browser.commands + .slice(commandsBeforeInspect) + .map(({ method }) => method), + ).toContain("Target.detachFromTarget"); + expect(browser.openedConnectionCount()).toBe(openedBefore + 1); + expect(browser.activeConnectionCount()).toBe(1); + + const commandsBeforeScreenshot = browser.commands.length; + const screenshot = await provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + }), + ); + if (!screenshot.ok) throw screenshot.error; + expect( + browser.commands + .slice(commandsBeforeScreenshot) + .map(({ method }) => method), + ).toContain("Target.detachFromTarget"); + expect(browser.httpRequests).toHaveLength(httpRequests); + expect(browser.openedConnectionCount()).toBe(openedBefore + 1); + expect(browser.activeConnectionCount()).toBe(1); + } finally { + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + }); +}); + +describe("CdpBrowserProvider: browser socket eviction and per-call scoping", () => { + it("evicts a dropped browser socket and reconnects cleanly on the next call", async () => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + try { + const first = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!first.ok) throw first.error; + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + browser.closeActiveConnections(); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + + const second = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!second.ok) throw second.error; + expect(second.value.target.origin).toBe(browser.allowedOrigin); + expect(browser.openedConnectionCount()).toBe(2); + expect(browser.activeConnectionCount()).toBe(1); + } finally { + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + }); + + it("closes a shared browser socket when Target.detachFromTarget fails so no target session leaks", async () => { + const browser = await startFakeCdpBrowser({ + commandError: ({ method }) => + method === "Target.detachFromTarget" + ? { code: -32_000, message: "Detach rejected" } + : undefined, }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + try { + const inspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!inspected.ok) throw inspected.error; + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + expect(browser.openedConnectionCount()).toBe(1); - const commandsBefore = browser.commands.length; - const inspected = await provider.inspectPage( - inspectWebPageInputSchema.parse({ - cdp_endpoint: browser.browserWebSocketUrl, - target_id: "allowed-page", - observation_ms: 0, - }), - ); - if (!inspected.ok) throw inspected.error; - expect(inspected.value.target.origin).toBe(browser.allowedOrigin); - expect( - browser.commands - .slice(commandsBefore, commandsBefore + 3) - .map(({ method, params }) => ({ method, params })), - ).toEqual([ - { method: "Browser.getVersion", params: {} }, - { method: "Target.getTargets", params: {} }, - { - method: "Target.attachToTarget", - params: { targetId: "allowed-page", flatten: true }, - }, - ]); - expect(browser.httpRequests).toHaveLength(httpRequests); - expect(browser.openedConnectionCount()).toBe(openedBefore + 2); + const listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!listed.ok) throw listed.error; + expect(browser.openedConnectionCount()).toBe(2); + expect(browser.activeConnectionCount()).toBe(1); + } finally { + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + }); + + it("preserves per-operation command_timeout_ms and operation identity on a reused browser socket", async () => { + const browser = await startFakeCdpBrowser({ + hangOnMethod: "Page.enable", + }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + try { + const listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!listed.ok) throw listed.error; + expect(browser.openedConnectionCount()).toBe(1); + + const timedOut = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + command_timeout_ms: 60, + }), + ); + expect(timedOut).toMatchObject({ + ok: false, + error: { + _tag: "BrowserObservationError", + operation: "inspect_web_page", + reason: "timeout", + }, + }); + if (timedOut.ok) throw new Error("Expected command timeout"); + expect(timedOut.error.userMessage).toContain("60 ms command_timeout_ms"); + expect(browser.openedConnectionCount()).toBe(1); + } finally { + await provider.close(); + } await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); }); }); @@ -98,56 +243,78 @@ it("retains browser-socket siblings and reports invalid attachment as unknown, n : undefined, }); trackBrowser(browser); - const result = await new CdpBrowserProvider().listTargets( - listBrowserTargetsInputSchema.parse({ - cdp_endpoint: browser.browserWebSocketUrl, - }), - ); - if (!result.ok) throw result.error; - expect( - result.value.targets.map(({ target_id, attached, title }) => [ - target_id, - attached, - title, - ]), - ).toEqual([ - ["first", null, ""], - ["second", false, "Good"], - ]); - const limitations = result.value.limitations.join("\n"); - expect(limitations).toContain( - 'reported malformed attached metadata "unknown"', - ); - expect(limitations).toContain("reported malformed title metadata 3"); - expect( - result.value.limitations.filter((item) => - item.includes("target identity is malformed"), - ), - ).toHaveLength(2); - expect(limitations).not.toContain("omitted attachment"); + const provider = new CdpBrowserProvider(); + try { + const result = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!result.ok) throw result.error; + expect( + result.value.targets.map(({ target_id, attached, title }) => [ + target_id, + attached, + title, + ]), + ).toEqual([ + ["first", null, ""], + ["second", false, "Good"], + ]); + const limitations = result.value.limitations.join("\n"); + expect(limitations).toContain( + 'reported malformed attached metadata "unknown"', + ); + expect(limitations).toContain("reported malformed title metadata 3"); + expect( + result.value.limitations.filter((item) => + item.includes("target identity is malformed"), + ), + ).toHaveLength(2); + expect(limitations).not.toContain("omitted attachment"); + } finally { + await provider.close(); + } await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); }); describe("CdpBrowserProvider: browser socket failures and cleanup", () => { - it("closes an unadopted browser socket when the target is missing", async () => { + it("retains an admitted browser socket when the target is missing and closes it on shutdown", async () => { const browser = await startFakeCdpBrowser(); trackBrowser(browser); - const result = await new CdpBrowserProvider().inspectPage( - inspectWebPageInputSchema.parse({ - cdp_endpoint: browser.browserWebSocketUrl, - target_id: "missing", - observation_ms: 0, - }), - ); + const provider = new CdpBrowserProvider(); + try { + const result = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "missing", + observation_ms: 0, + }), + ); - expect(result).toMatchObject({ - ok: false, - error: { _tag: "BrowserObservationError", reason: "target_not_found" }, - }); - expect(browser.commands.map(({ method }) => method)).toEqual([ - "Browser.getVersion", - "Target.getTargets", - ]); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "BrowserObservationError", reason: "target_not_found" }, + }); + expect(browser.commands.map(({ method }) => method)).toEqual([ + "Browser.getVersion", + "Target.getTargets", + ]); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + const recovered = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!recovered.ok) throw recovered.error; + expect(browser.openedConnectionCount()).toBe(1); + } finally { + await provider.close(); + } await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); }); @@ -291,3 +458,879 @@ describe("CdpBrowserProvider: browser socket discovery deadline", () => { } }); }); + +describe("CdpBrowserProvider: shared browser socket lifecycle regressions", () => { + it("closes a shared browser socket immediately when detach fails alongside an active observation", async () => { + const browser = await startFakeCdpBrowser({ + commandError: ({ method }) => + method === "Target.detachFromTarget" + ? { code: -32_000, message: "Detach rejected" } + : undefined, + }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + const closeSpy = vi.spyOn(CdpConnection.prototype, "close"); + let armed!: () => void; + const observationArmed = new Promise((resolve) => { + armed = resolve; + }); + const controller = new AbortController(); + let observing: ReturnType | undefined; + try { + observing = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 15_000, + }), + { + signal: controller.signal, + progress: { + report: async (update) => { + if (update.completed === 1) armed(); + }, + }, + }, + ); + await awaitAcknowledgement( + observationArmed, + observing, + "peer session observation start", + ); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + const inspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!inspected.ok) throw inspected.error; + expect(closeSpy).toHaveBeenCalledTimes(1); + expect(browser.openedConnectionCount()).toBe(1); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + + const session = await observing; + if (!session.ok) throw session.error; + expect(session.value.window.end_reason).toBe("transport_disconnected"); + } finally { + controller.abort(new Error("test teardown")); + await observing?.catch(() => undefined); + closeSpy.mockRestore(); + await provider.close(); + } + }); + + it("surfaces cleanupIncomplete when forced shared-socket close fails after detach rejection", async () => { + const browser = await startFakeCdpBrowser({ + commandError: ({ method }) => + method === "Target.detachFromTarget" + ? { code: -32_000, message: "Detach rejected" } + : undefined, + }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + const originalClose = CdpConnection.prototype.close; + let denyClose = false; + const closeSpy = vi + .spyOn(CdpConnection.prototype, "close") + .mockImplementation(function (this: CdpConnection) { + return denyClose + ? Promise.reject(new Error("controlled shared socket close failure")) + : originalClose.call(this); + }); + let armed!: () => void; + const observationArmed = new Promise((resolve) => { + armed = resolve; + }); + const controller = new AbortController(); + let observing: ReturnType | undefined; + try { + observing = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 15_000, + }), + { + signal: controller.signal, + progress: { + report: async (update) => { + if (update.completed === 1) armed(); + }, + }, + }, + ); + await awaitAcknowledgement( + observationArmed, + observing, + "peer session observation start", + ); + denyClose = true; + const inspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (inspected.ok) + throw new Error("Expected incomplete transport cleanup"); + expect(inspected.error.cleanupIncomplete).toBe(true); + expect(inspected.error.partialObservation).toBeDefined(); + + controller.abort(new Error("end peer observation")); + await observing; + denyClose = false; + await provider.close(); + expect(inspected.error.cleanupIncomplete).toBe(false); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + } finally { + denyClose = false; + controller.abort(new Error("test teardown")); + await observing?.catch(() => undefined); + closeSpy.mockRestore(); + await provider.close(); + } + }); +}); + +describe("CdpTargetSessionOwner: callback failures and idle protocol eviction", () => { + it("records cleanup lease ownership when finish() callback rejects and resolves on retryPending", async () => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const owner = new CdpTargetSessionOwner(); + try { + const connection = await owner.connect( + browser.browserWebSocketUrl, + "inspect_web_page", + {}, + "callback-failed-session", + ); + const cleanup = vi.fn(async () => { + throw new Error("controlled cleanup callback failure"); + }); + const failure = await owner + .finish(connection, cleanup) + .catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(ProviderCleanupError); + if (!(failure instanceof AnalysisError)) + throw new Error("Expected typed cleanup error"); + expect(failure.cleanupIncomplete).toBe(true); + expect(failure.cleanupResources).toEqual(["callback-failed-session"]); + expect(browser.activeConnectionCount()).toBe(1); + + await owner.retryPending(); + expect(cleanup).toHaveBeenCalledTimes(1); + expect(failure.cleanupIncomplete).toBe(false); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + } finally { + await owner.close(); + } + }); + + it("evicts and closes an idle retained browser socket on fatal protocol failure", async () => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + const originalClose = CdpConnection.prototype.close; + let denyClose = true; + const closeSpy = vi + .spyOn(CdpConnection.prototype, "close") + .mockImplementation(function (this: CdpConnection) { + return denyClose + ? Promise.reject(new Error("controlled idle protocol close failure")) + : originalClose.call(this); + }); + try { + const first = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!first.ok) throw first.error; + expect(browser.activeConnectionCount()).toBe(1); + + browser.emitRawMessage("{not-json"); + await vi.waitFor(() => expect(closeSpy).toHaveBeenCalledTimes(1)); + + const blocked = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (blocked.ok) throw new Error("Expected pending cleanup refusal"); + expect(blocked.error.cleanupIncomplete).toBe(true); + + denyClose = false; + const recovered = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!recovered.ok) throw recovered.error; + expect(blocked.error.cleanupIncomplete).toBe(false); + expect(browser.openedConnectionCount()).toBe(2); + expect(browser.activeConnectionCount()).toBe(1); + + browser.emitRawMessage("{not-json-idle-second"); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + } finally { + closeSpy.mockRestore(); + await provider.close(); + } + }); +}); + +it("blocks new admissions while forced shared-socket cleanup fails with an active peer, then recovers cleanly", async () => { + const browser = await startFakeCdpBrowser({ + commandError: ({ method }) => + method === "Target.detachFromTarget" + ? { code: -32_000, message: "Detach rejected" } + : undefined, + }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + const originalClose = CdpConnection.prototype.close; + let denyClose = false; + const closeSpy = vi + .spyOn(CdpConnection.prototype, "close") + .mockImplementation(function (this: CdpConnection) { + return denyClose + ? Promise.reject(new Error("controlled shared socket close failure")) + : originalClose.call(this); + }); + let armed!: () => void; + const observationArmed = new Promise((resolve) => { + armed = resolve; + }); + const controller = new AbortController(); + let observing: ReturnType | undefined; + try { + observing = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 15_000, + }), + { + signal: controller.signal, + progress: { + report: async (update) => { + if (update.completed === 1) armed(); + }, + }, + }, + ); + await awaitAcknowledgement( + observationArmed, + observing, + "peer session observation start", + ); + expect(browser.openedConnectionCount()).toBe(1); + + denyClose = true; + const failedInspect = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (failedInspect.ok) + throw new Error("Expected failed forced shared-socket close"); + expect(failedInspect.error.cleanupIncomplete).toBe(true); + expect(browser.openedConnectionCount()).toBe(1); + + // Peer observation is still active; Call C must NOT bypass the unresolved close failure. + const blocked = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (blocked.ok) + throw new Error("Expected admission blocked on failed cleanup"); + expect(blocked.error.cleanupIncomplete).toBe(true); + expect(browser.openedConnectionCount()).toBe(1); + + // Allow close retry to succeed while peer observation is still unwinding. + denyClose = false; + const recovered = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!recovered.ok) throw recovered.error; + expect(failedInspect.error.cleanupIncomplete).toBe(false); + expect(blocked.error.cleanupIncomplete).toBe(false); + expect(browser.openedConnectionCount()).toBe(2); + + const session = await observing; + if (!session.ok) throw session.error; + expect(session.value.window.end_reason).toBe("transport_disconnected"); + } finally { + denyClose = false; + controller.abort(new Error("test teardown")); + await observing?.catch(() => undefined); + closeSpy.mockRestore(); + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); +}); + +it("keeps an admitted shared browser socket alive for concurrent peers when a caller aborts or hits discovery_timeout_ms", async () => { + let slowTargetDiscovery = false; + let notifySlowDiscoveryStarted: (() => void) | undefined; + const browser = await startFakeCdpBrowser({ + commandDelayMs: ({ method }) => { + if (!slowTargetDiscovery || method !== "Target.getTargets") + return undefined; + notifySlowDiscoveryStarted?.(); + return 200; + }, + }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + let armed!: () => void; + const observationArmed = new Promise((resolve) => { + armed = resolve; + }); + const peerController = new AbortController(); + const cancelController = new AbortController(); + let observing: ReturnType | undefined; + let cancelling: ReturnType | undefined; + try { + observing = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 400, + }), + { + signal: peerController.signal, + progress: { + report: async (update) => { + if (update.completed === 1) armed(); + }, + }, + }, + ); + await awaitAcknowledgement( + observationArmed, + observing, + "peer session observation start", + ); + expect(browser.openedConnectionCount()).toBe(1); + + slowTargetDiscovery = true; + const timedOut = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + discovery_timeout_ms: 35, + }), + ); + expect(timedOut).toMatchObject({ + ok: false, + error: { + _tag: "BrowserObservationError", + operation: "inspect_web_page", + reason: "timeout", + }, + }); + if (timedOut.ok) throw new Error("Expected discovery timeout on Call B"); + expect(timedOut.error.userMessage).toContain("35 ms discovery_timeout_ms"); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + let enterCancelDiscovery!: () => void; + const cancelDiscoveryEntered = new Promise((resolve) => { + enterCancelDiscovery = resolve; + }); + notifySlowDiscoveryStarted = enterCancelDiscovery; + cancelling = provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + { signal: cancelController.signal }, + ); + await awaitAcknowledgement( + cancelDiscoveryEntered, + cancelling, + "shared socket discovery start", + ); + cancelController.abort(new Error("caller cancelled discovery")); + expect(await cancelling).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "list_browser_targets", + }, + }); + + slowTargetDiscovery = false; + const session = await observing; + if (!session.ok) throw session.error; + expect(session.value.window.end_reason).toBe("window_elapsed"); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + } finally { + slowTargetDiscovery = false; + peerController.abort(new Error("test teardown")); + cancelController.abort(new Error("test teardown")); + await Promise.allSettled([observing, cancelling]); + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); +}); + +describe("CdpBrowserProvider: coalesced cold-start browser sockets", () => { + it("coalesces concurrent cold-start calls onto one browser WebSocket handshake", async () => { + const browser = await startFakeCdpBrowser({ handshakeDelayMs: 40 }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + let batch: + | Promise< + [ + Awaited>, + Awaited>, + Awaited>, + ] + > + | undefined; + try { + batch = Promise.all([ + provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ), + provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ), + provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + }), + ), + ]); + const [listed, inspected, screenshot] = await batch; + if (!listed.ok) throw listed.error; + if (!inspected.ok) throw inspected.error; + if (!screenshot.ok) throw screenshot.error; + expect(listed.value.targets.length).toBeGreaterThan(0); + expect(inspected.value.target.origin).toBe(browser.allowedOrigin); + expect(screenshot.value.artifact.media_type).toBe("image/png"); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + const targetSessionIds = new Set( + browser.commands.flatMap((command) => + command.sessionId === undefined ? [] : [command.sessionId], + ), + ); + expect(targetSessionIds).toEqual(new Set(["session-1", "session-2"])); + const detachedSessionIds = browser.commands + .filter((command) => command.method === "Target.detachFromTarget") + .map((command) => command.params.sessionId) + .sort(); + expect(detachedSessionIds).toEqual(["session-1", "session-2"]); + } finally { + await batch?.catch(() => undefined); + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + }); + + it("keeps an in-flight cold-start handshake alive for a longer-deadline peer when one waiter times out", async () => { + const browser = await startFakeCdpBrowser({ handshakeDelayMs: 120 }); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + let shortCall: ReturnType | undefined; + let longCall: ReturnType | undefined; + try { + shortCall = provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + discovery_timeout_ms: 35, + }), + ); + longCall = provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + discovery_timeout_ms: 5_000, + }), + ); + + const shortOutcome = await shortCall; + expect(shortOutcome).toMatchObject({ + ok: false, + error: { + _tag: "BrowserObservationError", + operation: "list_browser_targets", + reason: "timeout", + }, + }); + if (shortOutcome.ok) + throw new Error("Expected short cold-start waiter to time out"); + expect(shortOutcome.error.userMessage).toContain( + "35 ms discovery_timeout_ms", + ); + + const longOutcome = await longCall; + if (!longOutcome.ok) throw longOutcome.error; + expect(longOutcome.value.target.origin).toBe(browser.allowedOrigin); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + const followUp = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + ); + if (!followUp.ok) throw followUp.error; + expect(browser.openedConnectionCount()).toBe(1); + } finally { + await Promise.allSettled([shortCall, longCall]); + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + }); +}); + +const createDeferredGate = (): { + readonly promise: Promise; + readonly open: () => void; +} => { + let open = (): void => undefined; + const promise = new Promise((resolve) => { + open = resolve; + }); + return { promise, open }; +}; + +const holdProgressStep = ( + step: number, + entered: () => void, + release: Promise, +) => ({ + async report(update: { readonly completed: number }) { + if (update.completed === step) { + entered(); + await release; + } + }, +}); + +it("isolates concurrent target sessions on one shared browser socket when a shorter call detaches first", async () => { + const browser = await startFakeCdpBrowser(); + trackBrowser(browser); + const provider = new CdpBrowserProvider(); + const controller = new AbortController(); + const observationArmed = createDeferredGate(); + const inspectionHeld = createDeferredGate(); + const peersGate = createDeferredGate(); + let observing: ReturnType | undefined; + let inspecting: ReturnType | undefined; + let capturing: + | ReturnType + | undefined; + try { + observing = provider.observeSession( + observeWebSessionInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 1, + }), + { + signal: controller.signal, + progress: holdProgressStep(1, observationArmed.open, peersGate.promise), + }, + ); + await awaitAcknowledgement( + observationArmed.promise, + observing, + "shared browser-socket observation start", + ); + + inspecting = provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + { + signal: controller.signal, + progress: holdProgressStep(2, inspectionHeld.open, peersGate.promise), + }, + ); + await awaitAcknowledgement( + inspectionHeld.promise, + inspecting, + "shared browser-socket inspection hold", + ); + + capturing = provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + target_id: "allowed-page", + }), + { signal: controller.signal }, + ); + const screenshot = await capturing; + if (!screenshot.ok) throw screenshot.error; + expect(screenshot.value.artifact.media_type).toBe("image/png"); + const detachesAfterScreenshot = browser.commands + .filter((command) => command.method === "Target.detachFromTarget") + .map((command) => command.params.sessionId); + expect(detachesAfterScreenshot).toEqual(["session-3"]); + + browser.emitEvent({ + method: "Page.navigatedWithinDocument", + sessionId: "session-3", + params: { + frameId: "frame-main", + url: `${browser.allowedOrigin}/ignored-detached-peer`, + navigationType: "historyApi", + timestamp: 40, + }, + }); + browser.emitEvent({ + method: "Page.navigatedWithinDocument", + sessionId: "session-1", + params: { + frameId: "frame-main", + url: `${browser.allowedOrigin}/after-peer-detach`, + navigationType: "historyApi", + timestamp: 41, + }, + }); + // Round-trip over the shared WebSocket so both queued server frames are + // processed in FIFO order before releasing the held peer operations. + const barrier = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + }), + { signal: controller.signal }, + ); + if (!barrier.ok) throw barrier.error; + + peersGate.open(); + const [inspected, observed] = await Promise.all([inspecting, observing]); + if (!inspected.ok) throw inspected.error; + if (!observed.ok) throw observed.error; + expect(inspected.value.target.origin).toBe(browser.allowedOrigin); + expect(observed.value.window.end_reason).toBe("window_elapsed"); + expect(observed.value.timeline).toContainEqual( + expect.objectContaining({ + type: "same_document_navigation", + url: `${browser.allowedOrigin}/after-peer-detach`, + }), + ); + expect(observed.value.timeline).not.toContainEqual( + expect.objectContaining({ + url: `${browser.allowedOrigin}/ignored-detached-peer`, + }), + ); + expect(browser.openedConnectionCount()).toBe(1); + expect(browser.activeConnectionCount()).toBe(1); + + const allTargetSessions = new Set( + browser.commands.flatMap((command) => + command.sessionId === undefined ? [] : [command.sessionId], + ), + ); + expect(allTargetSessions).toEqual( + new Set(["session-1", "session-2", "session-3"]), + ); + const allDetachedSessions = browser.commands + .filter((command) => command.method === "Target.detachFromTarget") + .map((command) => command.params.sessionId) + .sort(); + expect(allDetachedSessions).toEqual([ + "session-1", + "session-2", + "session-3", + ]); + } finally { + peersGate.open(); + controller.abort(new Error("test teardown")); + await Promise.allSettled([observing, inspecting, capturing]); + await provider.close(); + } + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); +}); + +it("reuses one browser socket across equivalent normalized spellings while keeping 127.0.0.1 and [::1] distinct", async () => { + const ipv4Browser = await startFakeCdpBrowser(); + const ipv6Browser = await startFakeCdpBrowser(); + trackBrowser(ipv4Browser); + trackBrowser(ipv6Browser); + const port = new URL(ipv4Browser.endpoint).port; + const canonicalIpv6Url = `ws://[::1]:${port}/devtools/browser/fake`; + const originalConnect = CdpConnection.connect.bind(CdpConnection); + const connectUrls: string[] = []; + const connectSpy = vi + .spyOn(CdpConnection, "connect") + .mockImplementation((url, ...rest) => { + connectUrls.push(url); + const wireUrl = + url === canonicalIpv6Url ? ipv6Browser.browserWebSocketUrl : url; + return originalConnect(wireUrl, ...rest); + }); + const provider = new CdpBrowserProvider(); + try { + const ipv6Listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: `WS://[0:0:0:0:0:0:0:1]:0${port}/devtools/browser/fake`, + }), + ); + if (!ipv6Listed.ok) throw ipv6Listed.error; + const ipv6Inspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: canonicalIpv6Url, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!ipv6Inspected.ok) throw ipv6Inspected.error; + expect(ipv6Browser.openedConnectionCount()).toBe(1); + expect(ipv4Browser.openedConnectionCount()).toBe(0); + + const ipv4Listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: `WS://127.0.0.1:0${port}/devtools/browser/fake`, + }), + ); + if (!ipv4Listed.ok) throw ipv4Listed.error; + const ipv4Screenshot = await provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: ipv4Browser.browserWebSocketUrl, + target_id: "allowed-page", + }), + ); + if (!ipv4Screenshot.ok) throw ipv4Screenshot.error; + + expect(ipv6Browser.openedConnectionCount()).toBe(1); + expect(ipv4Browser.openedConnectionCount()).toBe(1); + expect(ipv6Browser.activeConnectionCount()).toBe(1); + expect(ipv4Browser.activeConnectionCount()).toBe(1); + expect(connectUrls).toEqual([ + canonicalIpv6Url, + ipv4Browser.browserWebSocketUrl, + ]); + } finally { + connectSpy.mockRestore(); + await provider.close(); + } + await vi.waitFor(() => { + expect(ipv6Browser.activeConnectionCount()).toBe(0); + expect(ipv4Browser.activeConnectionCount()).toBe(0); + }); +}); + +it("keeps distinct /devtools/browser/ paths on separate browser sockets while reusing each normalized path", async () => { + const browserA = await startFakeCdpBrowser(); + const browserB = await startFakeCdpBrowser(); + const browserCaseA = await startFakeCdpBrowser(); + trackBrowser(browserA); + trackBrowser(browserB); + trackBrowser(browserCaseA); + const port = new URL(browserA.endpoint).port; + const canonicalIdAUrl = `ws://127.0.0.1:${port}/devtools/browser/idA`; + const canonicalIdBUrl = `ws://127.0.0.1:${port}/devtools/browser/idB`; + const canonicalCaseIdAUrl = `ws://127.0.0.1:${port}/devtools/browser/IdA`; + const routedWireUrls = new Map([ + [canonicalIdAUrl, browserA.browserWebSocketUrl], + [canonicalIdBUrl, browserB.browserWebSocketUrl], + [canonicalCaseIdAUrl, browserCaseA.browserWebSocketUrl], + ]); + const originalConnect = CdpConnection.connect.bind(CdpConnection); + const connectUrls: string[] = []; + const connectSpy = vi + .spyOn(CdpConnection, "connect") + .mockImplementation((url, ...rest) => { + connectUrls.push(url); + const wireUrl = routedWireUrls.get(url); + if (wireUrl === undefined) { + throw new Error(`Unexpected unmapped browser socket URL: ${url}`); + } + return originalConnect(wireUrl, ...rest); + }); + const provider = new CdpBrowserProvider(); + try { + const firstIdAListed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: canonicalIdAUrl, + }), + ); + if (!firstIdAListed.ok) throw firstIdAListed.error; + + const firstIdBListed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: canonicalIdBUrl, + }), + ); + if (!firstIdBListed.ok) throw firstIdBListed.error; + + const secondIdAInspected = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: canonicalIdAUrl, + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!secondIdAInspected.ok) throw secondIdAInspected.error; + + const leadingZeroIdBListed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: `WS://127.0.0.1:0${port}/devtools/browser/idB`, + }), + ); + if (!leadingZeroIdBListed.ok) throw leadingZeroIdBListed.error; + + const caseDistinctListed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: canonicalCaseIdAUrl, + }), + ); + if (!caseDistinctListed.ok) throw caseDistinctListed.error; + + const caseDistinctScreenshot = await provider.captureScreenshot( + captureWebScreenshotInputSchema.parse({ + cdp_endpoint: `WS://127.0.0.1:0${port}/devtools/browser/IdA`, + target_id: "allowed-page", + }), + ); + if (!caseDistinctScreenshot.ok) throw caseDistinctScreenshot.error; + + expect(browserA.openedConnectionCount()).toBe(1); + expect(browserB.openedConnectionCount()).toBe(1); + expect(browserCaseA.openedConnectionCount()).toBe(1); + expect(browserA.activeConnectionCount()).toBe(1); + expect(browserB.activeConnectionCount()).toBe(1); + expect(browserCaseA.activeConnectionCount()).toBe(1); + expect(connectUrls).toEqual([ + canonicalIdAUrl, + canonicalIdBUrl, + canonicalCaseIdAUrl, + ]); + } finally { + connectSpy.mockRestore(); + await provider.close(); + } + await vi.waitFor(() => { + expect(browserA.activeConnectionCount()).toBe(0); + expect(browserB.activeConnectionCount()).toBe(0); + expect(browserCaseA.activeConnectionCount()).toBe(0); + }); +}); diff --git a/tests/boundary/browser/cdpTargetDeadlines.test.ts b/tests/boundary/browser/cdpTargetDeadlines.test.ts index 6bca62351a..42ec869824 100644 --- a/tests/boundary/browser/cdpTargetDeadlines.test.ts +++ b/tests/boundary/browser/cdpTargetDeadlines.test.ts @@ -1,3 +1,4 @@ +import { createServer as createHttpServer } from "node:http"; import { createServer, type Socket } from "node:net"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; @@ -11,12 +12,16 @@ import { } from "../../../src/browser/PlaywrightScenarioServiceWorkers.js"; import { AnalysisTimeoutError } from "../../../src/domain/analysisErrorCore.js"; import { CdpWebRuntimeProvider } from "../../../src/browser/execution/CdpWebRuntimeProvider.js"; -import { CDP_DISCOVERY_RESPONSE_TIMEOUT_MS } from "../../../src/domain/browserDeadline.js"; +import { + CDP_DISCOVERY_RESPONSE_TIMEOUT_MS, + MAX_BROWSER_DEADLINE_MS, +} from "../../../src/domain/browserDeadline.js"; import { BrowserObservationError } from "../../../src/domain/browserObservationError.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { browserObservationEndpointSchema, inspectWebPageInputSchema, + listBrowserTargetsInputSchema, } from "../../../src/domain/browserObservation.js"; import { observeWebExecutionInputSchema } from "../../../src/domain/webExecution.js"; import { WEB_RUNTIME_LIMITS } from "../../../src/domain/webRuntime.js"; @@ -709,3 +714,156 @@ it("names the fixed cleanup grace when runtime restoration stalls", async () => await browser.close(); } }); + +it("applies caller-selected discovery_timeout_ms to browser-socket discovery", async () => { + const discoveryTimeoutMs = 45_000; + const browser = await startFakeCdpBrowser({ + hangOnMethod: "Target.getTargets", + }); + const provider = new CdpBrowserProvider(); + try { + const pending = provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + discovery_timeout_ms: discoveryTimeoutMs, + }), + ); + await vi.waitFor( + () => + expect(browser.commands.map(({ method }) => method)).toContain( + "Target.getTargets", + ), + { timeout: 5_000 }, + ); + expect(activeTimers(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)).toHaveLength(0); + expect(activeTimers(discoveryTimeoutMs)).not.toHaveLength(0); + expireEveryTimer(discoveryTimeoutMs); + const result = await pending; + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected browser-socket discovery timeout"); + const projected = projectAnalysisError(result.error); + expect(projected.code).toBe("provider_timeout"); + expect(projected.message).toContain( + `${String(discoveryTimeoutMs)} ms discovery_timeout_ms`, + ); + expect(projected.remediation?.action).toContain( + `Increase discovery_timeout_ms above ${String(discoveryTimeoutMs)}`, + ); + expect(projected.remediation?.action).toContain("--discovery-timeout-ms"); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + } finally { + await provider.close(); + await browser.close(); + } +}); + +it("applies caller-selected discovery_timeout_ms to HTTP discovery independently of command_timeout_ms", async () => { + const discoveryTimeoutMs = 35_000; + const commandTimeoutMs = 25_000; + let versionRequested = false; + const server = createHttpServer((request) => { + if (request.url === "/json/version") versionRequested = true; + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (address === null || typeof address === "string") + throw new Error("Expected an HTTP listener address"); + const provider = new CdpBrowserProvider(); + try { + const pending = provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: `http://127.0.0.1:${String(address.port)}`, + target_id: "allowed-page", + observation_ms: 0, + discovery_timeout_ms: discoveryTimeoutMs, + command_timeout_ms: commandTimeoutMs, + }), + ); + await vi.waitFor(() => expect(versionRequested).toBe(true), { + timeout: 5_000, + }); + expect(activeTimers(CDP_DISCOVERY_RESPONSE_TIMEOUT_MS)).toHaveLength(0); + expect(activeTimers(commandTimeoutMs)).toHaveLength(0); + expect(activeTimers(discoveryTimeoutMs)).toHaveLength(1); + expireTimer(discoveryTimeoutMs); + const result = await pending; + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected HTTP discovery timeout"); + const projected = projectAnalysisError(result.error); + expect(projected.code).toBe("provider_timeout"); + expect(projected.message).toContain( + `${String(discoveryTimeoutMs)} ms discovery_timeout_ms`, + ); + expect(projected.remediation?.action).toContain("--discovery-timeout-ms"); + expect(projected.remediation?.action).toContain( + "command_timeout_ms does not raise it", + ); + } finally { + await provider.close(); + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + } +}); + +it("clamps browser-socket handshake timers at MAX_BROWSER_DEADLINE_MS without 32-bit timer overflow", async () => { + const browser = await startFakeCdpBrowser({ handshakeDelayMs: 60 }); + const provider = new CdpBrowserProvider(); + try { + const listed = await provider.listTargets( + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: browser.browserWebSocketUrl, + discovery_timeout_ms: MAX_BROWSER_DEADLINE_MS, + }), + ); + if (!listed.ok) throw listed.error; + expect(listed.value.targets.length).toBeGreaterThan(0); + const scheduledDelays = vi + .mocked(globalThis.setTimeout) + .mock.calls.flatMap(([, delay]) => + typeof delay === "number" ? [delay] : [], + ); + expect(scheduledDelays).toContain(MAX_BROWSER_DEADLINE_MS); + expect(Math.max(...scheduledDelays)).toBeLessThanOrEqual( + MAX_BROWSER_DEADLINE_MS, + ); + } finally { + await provider.close(); + await browser.close(); + } + + const sockets = new Set(); + const server = createServer((socket) => { + sockets.add(socket); + socket.on("close", () => sockets.delete(socket)); + socket.resume(); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (address === null || typeof address === "string") + throw new Error("Expected a TCP address"); + try { + const pending = openCdpEndpoint( + browserObservationEndpointSchema.parse( + `ws://127.0.0.1:${String(address.port)}/devtools/browser/max-deadline`, + ), + "list_browser_targets", + undefined, + MAX_BROWSER_DEADLINE_MS, + ).catch((cause: unknown) => cause); + await vi.waitFor(() => expect(sockets.size).toBe(1), { timeout: 5_000 }); + expireEveryTimer(MAX_BROWSER_DEADLINE_MS); + const failure = await pending; + expect(failure).toBeInstanceOf(BrowserObservationError); + if (!(failure instanceof BrowserObservationError)) + throw new Error("Expected maximum discovery deadline failure"); + expect(failure.userMessage).toContain( + `CDP browser-socket discovery exceeded its ${String(MAX_BROWSER_DEADLINE_MS)} ms discovery_timeout_ms deadline`, + ); + expect(failure.remediationAction).toContain( + `discovery_timeout_ms is already at the maximum ${String(MAX_BROWSER_DEADLINE_MS)} ms`, + ); + } finally { + for (const socket of sockets) socket.destroy(); + await new Promise((resolve) => server.close(() => resolve())); + } +}); diff --git a/tests/boundary/browser/cdpTargetSessionOwner.test.ts b/tests/boundary/browser/cdpTargetSessionOwner.test.ts index 602dcfb396..8abf21802f 100644 --- a/tests/boundary/browser/cdpTargetSessionOwner.test.ts +++ b/tests/boundary/browser/cdpTargetSessionOwner.test.ts @@ -1,4 +1,5 @@ import { createServer } from "node:http"; +import type { Duplex } from "node:stream"; import { WebSocket, WebSocketServer } from "ws"; import { expect, it, onTestFinished, vi } from "vitest"; @@ -8,8 +9,10 @@ import { CdpConnection } from "../../../src/browser/CdpConnection.js"; import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; import { inspectWebPageInputSchema } from "../../../src/domain/browserObservation.js"; import { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; +import { awaitAcknowledgement } from "../../fixtures/lifecycleAcknowledgement.js"; it("joins a late owned handshake and waits for its active operation", async () => { const http = createServer(); @@ -284,3 +287,192 @@ it("keeps retry aggregates incomplete until every reported socket is released", await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(1)); await owner.finish(active); }); + +it("surfaces cold-start acquireBrowserSocket cleanup failure to live waiters without unhandled rejections", async () => { + const http = createServer(); + http.on("upgrade", (_request, socket) => { + socket.destroy(); + }); + await new Promise((resolve) => http.listen(0, "127.0.0.1", resolve)); + const address = http.address(); + if (address === null || typeof address === "string") + throw new Error("Fixture did not bind a TCP port"); + const url = `ws://127.0.0.1:${String(address.port)}/devtools/browser/rejected`; + const owner = new CdpTargetSessionOwner(); + const originalClose = CdpConnection.prototype.close; + let denyClose = true; + const closeSpy = vi + .spyOn(CdpConnection.prototype, "close") + .mockImplementation(function (this: CdpConnection) { + return denyClose + ? Promise.reject(new Error("controlled cold-connect close failure")) + : originalClose.call(this); + }); + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason); + }; + process.on("unhandledRejection", onUnhandled); + let first: + | ReturnType + | undefined; + let second: + | ReturnType + | undefined; + try { + const limits = { commandTimeoutMs: 1_000, handshakeTimeoutMs: 1_000 }; + first = owner.acquireBrowserSocket(url, "list_browser_targets", {}, limits); + second = owner.acquireBrowserSocket(url, "inspect_web_page", {}, limits); + const [firstError, secondError] = await Promise.all([ + first.then( + () => undefined, + (cause: unknown) => cause, + ), + second.then( + () => undefined, + (cause: unknown) => cause, + ), + ]); + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(unhandled).toEqual([]); + if ( + !(firstError instanceof AnalysisError) || + !(secondError instanceof AnalysisError) + ) + throw new Error("Expected typed cleanup-incomplete handshake errors"); + expect(firstError.cleanupIncomplete).toBe(true); + expect(secondError.cleanupIncomplete).toBe(true); + expect(firstError.cleanupResources).toEqual(["browser-endpoint"]); + expect(secondError.cleanupResources).toEqual(["browser-endpoint"]); + expect(secondError.cleanupPrimary).toMatchObject({ + _tag: "BrowserObservationError", + operation: "inspect_web_page", + }); + + const blockedRetry = await owner + .retryPending() + .catch((cause: unknown) => cause); + expect(blockedRetry).toBeInstanceOf(AnalysisError); + + denyClose = false; + await owner.retryPending(); + expect(firstError.cleanupIncomplete).toBe(false); + expect(secondError.cleanupIncomplete).toBe(false); + } finally { + process.off("unhandledRejection", onUnhandled); + denyClose = false; + await Promise.allSettled([first, second]); + closeSpy.mockRestore(); + await owner.close(); + await new Promise((resolve, reject) => + http.close((cause) => (cause ? reject(cause) : resolve())), + ); + } +}); + +it("surfaces cold-start acquireBrowserSocket cleanup failure when the last waiter aborts without unhandled rejections", async () => { + const http = createServer(); + const upgradeSockets = new Set(); + let upgradeReceived: (() => void) | undefined; + const received = new Promise((resolve) => { + upgradeReceived = resolve; + }); + http.on("upgrade", (_request, socket) => { + upgradeSockets.add(socket); + socket.on("close", () => upgradeSockets.delete(socket)); + socket.on("error", () => undefined); + upgradeReceived?.(); + }); + await new Promise((resolve) => http.listen(0, "127.0.0.1", resolve)); + const address = http.address(); + if (address === null || typeof address === "string") + throw new Error("Fixture did not bind a TCP port"); + const url = `ws://127.0.0.1:${String(address.port)}/devtools/browser/stalled`; + const owner = new CdpTargetSessionOwner(); + const originalClose = CdpConnection.prototype.close; + let denyClose = true; + const closeSpy = vi + .spyOn(CdpConnection.prototype, "close") + .mockImplementation(function (this: CdpConnection) { + return denyClose + ? Promise.reject(new Error("controlled abort close failure")) + : originalClose.call(this); + }); + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown): void => { + unhandled.push(reason); + }; + process.on("unhandledRejection", onUnhandled); + const firstController = new AbortController(); + const secondController = new AbortController(); + let first: + | ReturnType + | undefined; + let second: + | ReturnType + | undefined; + try { + const limits = { commandTimeoutMs: 5_000, handshakeTimeoutMs: 5_000 }; + first = owner.acquireBrowserSocket( + url, + "list_browser_targets", + { signal: firstController.signal }, + limits, + ); + second = owner.acquireBrowserSocket( + url, + "inspect_web_page", + { signal: secondController.signal }, + limits, + ); + await awaitAcknowledgement( + received, + Promise.all([first, second]), + "cold browser-socket upgrade", + ); + + firstController.abort(new Error("first caller cancelled")); + const firstError = await first.then( + () => undefined, + (cause: unknown) => cause, + ); + expect(firstError).toBeInstanceOf(AnalysisCancelledError); + if (!(firstError instanceof AnalysisCancelledError)) + throw new Error("Expected first waiter cancellation"); + expect(firstError.cleanupIncomplete).toBe(false); + + secondController.abort(new Error("last caller cancelled")); + const secondError = await second.then( + () => undefined, + (cause: unknown) => cause, + ); + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(unhandled).toEqual([]); + if (!(secondError instanceof AnalysisError)) + throw new Error("Expected last waiter cleanup-incomplete failure"); + expect(secondError.cleanupIncomplete).toBe(true); + expect(secondError.cleanupResources).toEqual(["browser-endpoint"]); + expect(secondError.cleanupPrimary).toMatchObject({ + _tag: "AnalysisCancelledError", + operation: "inspect_web_page", + }); + + denyClose = false; + await owner.retryPending(); + expect(secondError.cleanupIncomplete).toBe(false); + } finally { + process.off("unhandledRejection", onUnhandled); + denyClose = false; + firstController.abort(new Error("test teardown")); + secondController.abort(new Error("test teardown")); + await Promise.allSettled([first, second]); + closeSpy.mockRestore(); + await owner.close(); + for (const socket of upgradeSockets) socket.destroy(); + await new Promise((resolve, reject) => + http.close((cause) => (cause ? reject(cause) : resolve())), + ); + } +}); diff --git a/tests/boundary/browser/webDeadlineBehavior.test.ts b/tests/boundary/browser/webDeadlineBehavior.test.ts index 5e17e921e9..7739cbef23 100644 --- a/tests/boundary/browser/webDeadlineBehavior.test.ts +++ b/tests/boundary/browser/webDeadlineBehavior.test.ts @@ -95,6 +95,51 @@ it("rejects a command deadline on target listing and leaves it out of Evidence", expect(evidence.parameters).not.toHaveProperty("command_timeout_ms"); }); +it("records an explicit discovery deadline and omits the default", () => { + const explicitListing = createBrowserEvidence( + "list_browser_targets", + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: "ws://127.0.0.1:9222/devtools/browser/fake", + discovery_timeout_ms: 25_000, + }), + targetList, + CDP_BROWSER_PROVIDER_IDENTITY, + ); + expect(explicitListing.parameters.discovery_timeout_ms).toBe(25_000); + const omittedListing = createBrowserEvidence( + "list_browser_targets", + listBrowserTargetsInputSchema.parse({ + cdp_endpoint: "ws://127.0.0.1:9222/devtools/browser/fake", + }), + targetList, + CDP_BROWSER_PROVIDER_IDENTITY, + ); + expect(omittedListing.parameters).not.toHaveProperty("discovery_timeout_ms"); + + const baseBundle = { + cdp_endpoint: "http://127.0.0.1:9222", + target_id: "page", + fetch_source_maps: false, + }; + const explicitBundle = createBrowserEvidence( + "analyze_web_bundle", + analyzeWebBundleInputSchema.parse({ + ...baseBundle, + discovery_timeout_ms: 15_000, + }), + bundle, + CDP_BROWSER_PROVIDER_IDENTITY, + ); + expect(explicitBundle.parameters.discovery_timeout_ms).toBe(15_000); + const omittedBundle = createBrowserEvidence( + "analyze_web_bundle", + analyzeWebBundleInputSchema.parse(baseBundle), + bundle, + CDP_BROWSER_PROVIDER_IDENTITY, + ); + expect(omittedBundle.parameters).not.toHaveProperty("discovery_timeout_ms"); +}); + it("records an explicit source-map fetch deadline and omits the default", () => { const base = { cdp_endpoint: "http://127.0.0.1:9222", diff --git a/tests/boundary/mcp/browserMcp.test.ts b/tests/boundary/mcp/browserMcp.test.ts index cc251e4479..838083d5d9 100644 --- a/tests/boundary/mcp/browserMcp.test.ts +++ b/tests/boundary/mcp/browserMcp.test.ts @@ -812,6 +812,23 @@ it.each(["full", "compact"] as const)( expect(tool?.inputSchema.properties?.cdp_endpoint).toMatchObject({ description: expect.stringContaining("/devtools/browser/ID"), }); + const listed = await connected.client.callTool({ + name: "list_browser_targets", + arguments: { + cdp_endpoint: browser.browserWebSocketUrl, + discovery_timeout_ms: 15_000, + }, + }); + expect(listed.isError).not.toBe(true); + expect(listed.structuredContent).toMatchObject({ + parameters: { + cdp_endpoint: browser.browserWebSocketUrl.replace( + /fake$/u, + "[REDACTED]", + ), + discovery_timeout_ms: 15_000, + }, + }); const inspected = await connected.client.callTool({ name: "inspect_web_page", arguments: { @@ -822,6 +839,7 @@ it.each(["full", "compact"] as const)( }, }); expect(inspected.isError).not.toBe(true); + expect(browser.openedConnectionCount()).toBe(1); expect(inspected.structuredContent).toMatchObject({ parameters: { cdp_endpoint: browser.browserWebSocketUrl.replace( diff --git a/tests/fixtures/fakeCdpBrowser.ts b/tests/fixtures/fakeCdpBrowser.ts index b359dbae57..be92d04762 100644 --- a/tests/fixtures/fakeCdpBrowser.ts +++ b/tests/fixtures/fakeCdpBrowser.ts @@ -60,46 +60,52 @@ export const startFakeCdpBrowser = async ( }); webSockets.on("connection", (socket: WebSocket, request: IncomingMessage) => { const directPage = request.url?.startsWith("/devtools/page/") === true; + const sessionState: SocketSessionState = { + nextSessionNumber: 1, + attachedSessions: new Set(), + }; openedConnections += 1; sockets.add(socket); socket.on("close", () => sockets.delete(socket)); socket.on("message", (raw) => { const command = parseCommand(raw.toString()); commands.push(command); - const commandError = options.commandError?.(command); - if (commandError !== undefined) { - sendCommandReply(socket, command, { error: commandError }); - if (options.closeAfterMethod === command.method) - setImmediate(() => socket.close()); - return; - } - if (options.closeOnMethod === command.method) { - socket.close(); - return; - } - if (options.hangOnMethod === command.method) return; - if (options.malformedMessageOnMethod === command.method) { - socket.send("{not-json"); - return; - } - if (options.unsupportedMethods?.includes(command.method) === true) { - sendCommandReply(socket, command, { - error: { code: -32_601, message: "Method not found" }, - }); - return; - } if ( - directPage && - (command.method === "Target.attachToTarget" || - command.sessionId !== undefined) - ) { - sendCommandReply(socket, command, { - error: { code: -32_600, message: "Direct page socket expected" }, - }); + handleSimulatedWireFault( + socket, + command, + directPage, + options, + sessionState, + ) + ) + return; + if (command.method === "Page.getFrameTree") frameTreeReads += 1; + const delayMs = options.commandDelayMs?.(command); + if (delayMs !== undefined && delayMs > 0) { + const currentFrameReads = frameTreeReads; + const timer = setTimeout(() => { + if (socket.readyState === WebSocket.OPEN) + replyToCommand( + socket, + command, + port, + options, + currentFrameReads, + sessionState, + ); + }, delayMs); + socket.once("close", () => clearTimeout(timer)); return; } - if (command.method === "Page.getFrameTree") frameTreeReads += 1; - replyToCommand(socket, command, port, options, frameTreeReads); + replyToCommand( + socket, + command, + port, + options, + frameTreeReads, + sessionState, + ); }); }); await new Promise((resolve, reject) => { @@ -121,6 +127,9 @@ export const startFakeCdpBrowser = async ( emitEvent(event) { for (const socket of sockets) socket.send(JSON.stringify(event)); }, + closeActiveConnections() { + for (const socket of sockets) socket.close(); + }, httpRequests, async close() { for (const socket of sockets) socket.terminate(); @@ -219,6 +228,65 @@ const sendCommandReply = ( ); }; +interface SocketSessionState { + nextSessionNumber: number; + readonly attachedSessions: Set; +} + +const sessionNotFoundError = { + code: -32_001, + message: "Session with given id not found", +} as const; + +const handleSimulatedWireFault = ( + socket: WebSocket, + command: FakeCdpCommand, + directPage: boolean, + options: FakeOptions, + sessionState: SocketSessionState, +): boolean => { + const commandError = options.commandError?.(command); + if (commandError !== undefined) { + sendCommandReply(socket, command, { error: commandError }); + if (options.closeAfterMethod === command.method) + setImmediate(() => socket.close()); + return true; + } + if (options.closeOnMethod === command.method) { + socket.close(); + return true; + } + if (options.hangOnMethod === command.method) return true; + if (options.malformedMessageOnMethod === command.method) { + socket.send("{not-json"); + return true; + } + if (options.unsupportedMethods?.includes(command.method) === true) { + sendCommandReply(socket, command, { + error: { code: -32_601, message: "Method not found" }, + }); + return true; + } + if ( + directPage && + (command.method === "Target.attachToTarget" || + command.sessionId !== undefined) + ) { + sendCommandReply(socket, command, { + error: { code: -32_600, message: "Direct page socket expected" }, + }); + return true; + } + if ( + command.sessionId !== undefined && + !sessionState.attachedSessions.has(command.sessionId) + ) { + sendCommandReply(socket, command, { error: sessionNotFoundError }); + return true; + } + return false; +}; + /** Emit a configured producer reply and its associated events. */ const replyToCommand = ( socket: WebSocket, @@ -226,13 +294,48 @@ const replyToCommand = ( port: number, options: FakeOptions, frameTreeReads: number, + sessionState: SocketSessionState, ): void => { + if ( + command.sessionId !== undefined && + !sessionState.attachedSessions.has(command.sessionId) + ) { + sendCommandReply(socket, command, { error: sessionNotFoundError }); + return; + } + const customResult = options.commandResult?.( + command, + `http://127.0.0.1:${String(port)}`, + frameTreeReads, + ); + if ( + customResult === undefined && + command.method === "Target.detachFromTarget" + ) { + const sessionId = command.params.sessionId; + if ( + typeof sessionId !== "string" || + !sessionState.attachedSessions.delete(sessionId) + ) { + sendCommandReply(socket, command, { error: sessionNotFoundError }); + return; + } + } const result = - options.commandResult?.( - command, - `http://127.0.0.1:${String(port)}`, - frameTreeReads, - ) ?? resultFor(command, port, options, frameTreeReads); + customResult ?? + resultFor(command, port, options, frameTreeReads, () => { + const sessionId = `session-${String(sessionState.nextSessionNumber)}`; + sessionState.nextSessionNumber += 1; + return sessionId; + }); + if ( + command.method === "Target.attachToTarget" && + result !== undefined && + typeof result.sessionId === "string" && + result.sessionId.length > 0 + ) { + sessionState.attachedSessions.add(result.sessionId); + } if (result === undefined) { sendCommandReply(socket, command, { error: { diff --git a/tests/fixtures/fakeCdpBrowserResults.ts b/tests/fixtures/fakeCdpBrowserResults.ts index e60428b232..8adaf0e42e 100644 --- a/tests/fixtures/fakeCdpBrowserResults.ts +++ b/tests/fixtures/fakeCdpBrowserResults.ts @@ -6,10 +6,11 @@ export const resultFor = ( port: number, options: FakeOptions, frameTreeReads: number, + allocateSessionId: () => string = () => "session-1", ): Readonly> | undefined => { switch (command.method) { case "Target.attachToTarget": - return attachToTargetResult(options); + return attachToTargetResult(options, allocateSessionId); case "Page.getLayoutMetrics": return { cssVisualViewport: { @@ -98,8 +99,11 @@ const browserVersionResult = () => ({ jsVersion: "13.0", }); -const attachToTargetResult = (options: FakeOptions) => ({ - sessionId: options.invalidAttachedSession ? "" : "session-1", +const attachToTargetResult = ( + options: FakeOptions, + allocateSessionId: () => string, +) => ({ + sessionId: options.invalidAttachedSession ? "" : allocateSessionId(), }); const frameTreeResult = ( diff --git a/tests/fixtures/fakeCdpBrowserTypes.ts b/tests/fixtures/fakeCdpBrowserTypes.ts index 1ef4e07d1a..ef2862a412 100644 --- a/tests/fixtures/fakeCdpBrowserTypes.ts +++ b/tests/fixtures/fakeCdpBrowserTypes.ts @@ -20,6 +20,8 @@ export interface FakeCdpBrowser { /** Deliver actual malformed wire data without bypassing the production transport parser. */ emitRawMessage(message: string): void; emitEvent(event: CdpEvent): void; + /** Drop currently open WebSockets from the server side to exercise reconnect paths. */ + closeActiveConnections(): void; readonly httpRequests: readonly { readonly url: string; readonly authorization: string | undefined; @@ -51,6 +53,8 @@ export interface FakeOptions { readonly discoveryDelayMs?: number; /** Delay WebSocket upgrade acceptance so handshake and attach deadlines stay distinct. */ readonly handshakeDelayMs?: number; + /** Delay selected wire replies so one caller's discovery deadline can expire alongside an active peer. */ + readonly commandDelayMs?: (command: FakeCdpCommand) => number | undefined; readonly malformedDiscovery?: boolean; readonly oversizedDiscovery?: boolean; readonly invalidBrowserWebSocket?: boolean;