diff --git a/src/application/BrowserObservationPort.ts b/src/application/BrowserObservationPort.ts index a35d8a13cd..6d8b1e9229 100644 --- a/src/application/BrowserObservationPort.ts +++ b/src/application/BrowserObservationPort.ts @@ -35,6 +35,8 @@ import type { ExecutionOptions } from "./AnalysisProvider.js"; /** Provider-neutral application boundary for passive browser observation. */ export interface BrowserObservationPort { + /** Release provider-owned transports during command or server shutdown. */ + close?(): Promise; identity(): ProviderIdentity; listTargets( input: ListBrowserTargetsInput, diff --git a/src/application/WebRuntimePort.ts b/src/application/WebRuntimePort.ts index 608052bc30..c922443fa3 100644 --- a/src/application/WebRuntimePort.ts +++ b/src/application/WebRuntimePort.ts @@ -12,6 +12,8 @@ import type { /** Provider-neutral live source attribution operations with distinct instrumentation authority. */ export interface WebRuntimePort { + /** Release provider-owned transports during command or server shutdown. */ + close?(): Promise; identity(): ProviderIdentity; observeExecution( input: ObserveWebExecutionInput, diff --git a/src/application/javascript/ElectronObservationPort.ts b/src/application/javascript/ElectronObservationPort.ts index 9e17225706..f619424473 100644 --- a/src/application/javascript/ElectronObservationPort.ts +++ b/src/application/javascript/ElectronObservationPort.ts @@ -13,6 +13,8 @@ import type { Result } from "../../domain/result.js"; /** Provider-neutral boundary for root-confined Electron file-page observation. */ export interface ElectronObservationPort { + /** Release provider-owned transports during command or server shutdown. */ + close?(): Promise; identity(): ProviderIdentity; listTargets( input: ListElectronTargetsInput, diff --git a/src/browser/CdpBrowserProvider.ts b/src/browser/CdpBrowserProvider.ts index 64c4815f1f..94f1ba8997 100644 --- a/src/browser/CdpBrowserProvider.ts +++ b/src/browser/CdpBrowserProvider.ts @@ -42,13 +42,18 @@ import { type WebScreenshot, type WebScreenshotDiff, } from "../domain/webScreenshot.js"; -import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { + AnalysisError, + type AnalysisPartialObservation, +} from "../domain/analysisErrorBase.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { AnalysisCancelledError, AnalysisInputError, } from "../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; import { err, ok, type Result } from "../domain/result.js"; import { @@ -73,6 +78,7 @@ import { comparePngScreenshots } from "./PngVisualDiff.js"; import { CDP_BROWSER_PROVIDER_IDENTITY } from "./providerIdentities.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { CdpTargetSessionOwner } from "./CdpTargetSessionOwner.js"; const CLEANUP_DOMAINS = [ "Network", "Debugger", @@ -83,6 +89,12 @@ const CLEANUP_DOMAINS = [ /** Passive browser observation through a selected user-owned CDP endpoint. */ export class CdpBrowserProvider implements BrowserObservationPort { + readonly #owner = new CdpTargetSessionOwner(); + + close(): Promise { + return this.#owner.close(); + } + identity(): ProviderIdentity { return CDP_BROWSER_PROVIDER_IDENTITY; } @@ -96,8 +108,11 @@ export class CdpBrowserProvider implements BrowserObservationPort { input.cdp_endpoint, "list_browser_targets", options.signal, + undefined, + this.#owner, ); - await endpoint.connection?.close(); + if (endpoint.connection !== undefined) + await this.#owner.finish(endpoint.connection); const { discovery } = endpoint; const selected = selectTargets( discovery, @@ -148,12 +163,9 @@ export class CdpBrowserProvider implements BrowserObservationPort { input: AnalyzeWebBundleInput, options: ExecutionOptions = {}, ): Promise> { + let captured: CapturedPage | undefined; try { - const captured = await this.#capture( - input, - options, - "analyze_web_bundle", - ); + captured = await this.#capture(input, options, "analyze_web_bundle"); const sourceMaps = input.fetch_source_maps ? await fetchWebSourceMaps( captured.sourceMapRequests, @@ -176,7 +188,14 @@ export class CdpBrowserProvider implements BrowserObservationPort { // fetchWebSourceMaps rethrows the exact abort reason. Preserve cancellation // even when a caller supplied an AnalysisError as that reason. if (options.signal?.aborted === true && cause === options.signal.reason) { - return err(new AnalysisCancelledError("analyze_web_bundle", { cause })); + return err( + new AnalysisCancelledError("analyze_web_bundle", { + cause, + ...(captured === undefined + ? {} + : { partialObservation: captured.inspection }), + }), + ); } return err(providerError(cause, "analyze_web_bundle", options.signal)); } @@ -194,6 +213,7 @@ export class CdpBrowserProvider implements BrowserObservationPort { "observe_web_session", options, observeCdpSession, + this.#owner, ), ), ); @@ -214,6 +234,7 @@ export class CdpBrowserProvider implements BrowserObservationPort { "discover_webmcp_tools", options, discoverWebMcp, + this.#owner, ), ), ); @@ -246,6 +267,7 @@ export class CdpBrowserProvider implements BrowserObservationPort { "capture_web_screenshot", options, captureCdpScreenshot, + this.#owner, ), ), ); @@ -282,6 +304,8 @@ export class CdpBrowserProvider implements BrowserObservationPort { operation, options, async (session) => await capturePage({ ...session, operation }), + this.#owner, + (captured) => captured.inspection, ); } } @@ -309,14 +333,19 @@ const withTargetSession = async < operation: BrowserObservationOperation, options: ExecutionOptions, work: (session: CdpTargetOperationSession) => Promise, + owner: CdpTargetSessionOwner, + partialObservation?: (output: Output) => AnalysisPartialObservation, ): Promise => { const endpoint = await openCdpEndpoint( input.cdp_endpoint, operation, options.signal, + undefined, + owner, ); let targetSession: CdpTargetSession | undefined; - let output: Output; + let outcome: Result; + let cleanupFailure: AnalysisError | undefined; try { const target = authorizeTarget(endpoint.discovery, input); const scopedInput = scopeToTargetOrigin(input, target); @@ -327,40 +356,97 @@ const withTargetSession = async < target, operation, options.signal, + undefined, + owner, ) : await attachCdpTargetSession( endpoint.connection, target, operation, options.signal, + owner, ); - output = await work({ - connection: targetSession.connection, - sessionId: targetSession.sessionId, - discovery: endpoint.discovery, - target, - input: scopedInput, - ...(options.signal === undefined ? {} : { signal: options.signal }), - ...(options.progress === undefined ? {} : { progress: options.progress }), - }); - } finally { + outcome = ok( + await work({ + connection: targetSession.connection, + sessionId: targetSession.sessionId, + discovery: endpoint.discovery, + target, + input: scopedInput, + ...(options.signal === undefined ? {} : { signal: options.signal }), + ...(options.progress === undefined + ? {} + : { progress: options.progress }), + }), + ); + } catch (cause: unknown) { + outcome = err(cause); + } + try { if (targetSession !== undefined) await closeCdpTargetSession( targetSession, CLEANUP_DOMAINS, options.signal, + owner, ); - // An unadopted browser socket is closed here; closing is idempotent. - await endpoint.connection?.close(); + if ( + endpoint.connection !== undefined && + endpoint.connection !== targetSession?.connection + ) + await owner.finish(endpoint.connection); + } catch (cause: unknown) { + cleanupFailure = + cause instanceof AnalysisError + ? cause + : new ProviderCleanupError( + "rea-cdp-browser", + [input.target_id], + { reason: cause instanceof Error ? cause.message : String(cause) }, + { operation, cause }, + ); } - // Cancellation during teardown must not publish the completed observation. - // Errors from work or cleanup still propagate through the try/finally above. - if (options.signal?.aborted === true) - throw new AnalysisCancelledError(operation, { - cause: options.signal.reason, - }); - return output; + if (outcome.ok && options.signal?.aborted === true) + outcome = err( + partialObservation === undefined + ? new AnalysisCancelledError(operation, { + cause: options.signal.reason, + }) + : new AnalysisCancelledError(operation, { + cause: options.signal.reason, + partialObservation: partialObservation(outcome.value), + }), + ); + + if (cleanupFailure !== undefined) { + if (!outcome.ok && outcome.error instanceof AnalysisError) + throw analysisErrorWithCleanupFailure( + outcome.error, + cleanupFailure, + operation, + ); + if (!outcome.ok) + throw new AggregateError( + [outcome.error, cleanupFailure], + `CDP operation ${operation} failed and cleanup remains incomplete.`, + { cause: outcome.error }, + ); + throw new ProviderCleanupError( + "rea-cdp-browser", + cleanupFailure.cleanupResources, + { cleanup_error: cleanupFailure.message }, + { + operation, + cause: cleanupFailure, + ...(partialObservation === undefined + ? {} + : { partialObservation: partialObservation(outcome.value) }), + }, + ); + } + if (!outcome.ok) throw outcome.error; + return outcome.value; }; const selectTargets = ( diff --git a/src/browser/CdpConnection.ts b/src/browser/CdpConnection.ts index 78511fb8f4..fa2f4f22a3 100644 --- a/src/browser/CdpConnection.ts +++ b/src/browser/CdpConnection.ts @@ -64,6 +64,7 @@ export class CdpConnection { operation: BrowserObservationOperation, signal?: AbortSignal, limits?: { readonly maxPayloadBytes: number }, + onConnected?: (connection: CdpConnection) => void, ): Promise { if (signal?.aborted === true) throw new AnalysisCancelledError(operation); const maxPayloadBytes = @@ -79,14 +80,36 @@ export class CdpConnection { `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, cancellation, cleanup, and typed errors; - // keep ws's timer disabled so handshake failures have one owner. + // waitForOpen owns the deadline and typed handshake failures; the + // connection owner registers this socket before waiting for open. handshakeTimeout: 0, maxPayload: maxPayloadBytes, perMessageDeflate: false, }); - await waitForOpen(socket, operation, signal); - return new CdpConnection(socket, operation, maxPayloadBytes); + const connection = new CdpConnection(socket, operation, maxPayloadBytes); + try { + onConnected?.(connection); + } catch (cause: unknown) { + await connection.close(); + throw cause; + } + try { + await waitForOpen(socket, operation, signal); + } 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. */ @@ -192,17 +215,64 @@ export class CdpConnection { async #closeTransport(): Promise { this.#closed = true; this.#failPending("disconnected"); - if (this.socket.readyState === WebSocket.CLOSED) return; - await new Promise((resolve) => { - const timer = setTimeout(() => { - this.socket.terminate(); - resolve(); - }, 1_000); - this.socket.once("close", () => { + if (socketIsClosed(this.socket)) return; + await new Promise((resolve, reject) => { + let settled = false; + let fallbackStarted = false; + let fallbackTimer: ReturnType | undefined; + const finish = (cause?: unknown): void => { + if (settled) return; + settled = true; clearTimeout(timer); - resolve(); - }); - this.socket.close(); + if (fallbackTimer !== undefined) clearTimeout(fallbackTimer); + this.socket.off("close", onClose); + if (cause === undefined) resolve(); + else reject(cause); + }; + const onClose = (): void => finish(); + const forceClose = (gracefulFailure?: unknown): void => { + if (settled || fallbackStarted) return; + fallbackStarted = true; + clearTimeout(timer); + if (socketIsClosed(this.socket)) { + finish(); + return; + } + try { + this.socket.terminate(); + } catch (cause: unknown) { + gracefulFailure = + gracefulFailure === undefined + ? cause + : new AggregateError( + [gracefulFailure, cause], + "Graceful and forced CDP WebSocket close both failed.", + { cause: gracefulFailure }, + ); + } + if (socketIsClosed(this.socket)) finish(); + else { + fallbackTimer = setTimeout(() => { + if (socketIsClosed(this.socket)) { + finish(); + return; + } + finish( + gracefulFailure ?? + new BrowserObservationError(this.operation, "cleanup_failed", { + detail: "REA could not confirm release of its CDP WebSocket.", + }), + ); + }, 1_000); + } + }; + const timer = setTimeout(() => forceClose(), 1_000); + this.socket.once("close", onClose); + try { + this.socket.close(); + } catch (closeCause: unknown) { + forceClose(closeCause); + } }); } @@ -350,6 +420,9 @@ export class CdpConnection { } } +const socketIsClosed = (socket: WebSocket): boolean => + socket.readyState === WebSocket.CLOSED; + const rawText = (data: RawData): string => Array.isArray(data) ? Buffer.concat(data).toString("utf8") @@ -415,6 +488,9 @@ const waitForOpen = async ( }); const terminateSocket = (socket: WebSocket): void => { - socket.once("error", () => undefined); - socket.terminate(); + try { + socket.terminate(); + } catch { + // The owning connection remains registered and will retry bounded close. + } }; diff --git a/src/browser/CdpElectronProvider.ts b/src/browser/CdpElectronProvider.ts index 9cf63a32d9..fbfc7ebd61 100644 --- a/src/browser/CdpElectronProvider.ts +++ b/src/browser/CdpElectronProvider.ts @@ -12,6 +12,7 @@ import { type ListElectronTargetsInput, } from "../domain/javascript/electronObservation.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; @@ -31,8 +32,17 @@ import { authorizedElectronFile } from "./ElectronFileScope.js"; import { CDP_ELECTRON_PROVIDER_IDENTITY } from "./providerIdentities.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { CdpTargetSessionOwner } from "./CdpTargetSessionOwner.js"; /** Passive Electron provider for local file pages exposed by loopback CDP. */ export class CdpElectronProvider implements ElectronObservationPort { + readonly #owner = new CdpTargetSessionOwner(); + + close(): Promise { + return this.#owner.close(); + } + identity(): ProviderIdentity { return CDP_ELECTRON_PROVIDER_IDENTITY; } @@ -109,6 +119,8 @@ export class CdpElectronProvider implements ElectronObservationPort { options: ExecutionOptions = {}, ): Promise> { let targetSession: CdpTargetSession | undefined; + let cleanupFailure: AnalysisError | undefined; + let result: Result; try { const discovery = await discoverCdpEndpoint( input.cdp_endpoint, @@ -121,8 +133,10 @@ export class CdpElectronProvider implements ElectronObservationPort { target, "inspect_electron_page", options.signal, + undefined, + this.#owner, ); - return ok( + result = ok( electronPageInspectionSchema.parse( await inspectCdpElectronPage({ connection: targetSession.connection, @@ -138,15 +152,71 @@ export class CdpElectronProvider implements ElectronObservationPort { ), ); } catch (cause: unknown) { - return err(providerError(cause, "inspect_electron_page")); + result = err(providerError(cause, "inspect_electron_page")); } finally { if (targetSession !== undefined) - await closeCdpTargetSession( - targetSession, - ["Debugger", "Runtime", "Page"], - options.signal, - ); + try { + await closeCdpTargetSession( + targetSession, + ["Debugger", "Runtime", "Page"], + options.signal, + this.#owner, + ); + } catch (cause: unknown) { + cleanupFailure = + cause instanceof AnalysisError + ? cause + : new ProviderCleanupError( + CDP_ELECTRON_PROVIDER_IDENTITY.id, + [input.target_id], + { + reason: + cause instanceof Error ? cause.message : String(cause), + }, + { operation: "inspect_electron_page", cause }, + ); + } } + const finalCancellation = + result.ok && options.signal?.aborted === true + ? new AnalysisCancelledError("inspect_electron_page", { + cause: options.signal.reason, + partialObservation: result.value, + }) + : undefined; + if (cleanupFailure !== undefined) { + if (finalCancellation !== undefined) + return err( + analysisErrorWithCleanupFailure( + finalCancellation, + cleanupFailure, + "inspect_electron_page", + ), + ); + if (!result.ok) + return err( + analysisErrorWithCleanupFailure( + result.error, + cleanupFailure, + "inspect_electron_page", + ), + ); + return err( + new ProviderCleanupError( + CDP_ELECTRON_PROVIDER_IDENTITY.id, + cleanupFailure.cleanupResources, + { cleanup_error: cleanupFailure.message }, + { + operation: "inspect_electron_page", + cause: cleanupFailure, + partialObservation: result.value, + }, + ), + ); + } + if (!result.ok) return result; + if (finalCancellation !== undefined) return err(finalCancellation); + return result; } } diff --git a/src/browser/CdpTargetSession.ts b/src/browser/CdpTargetSession.ts index 7355e142e0..61f2368acd 100644 --- a/src/browser/CdpTargetSession.ts +++ b/src/browser/CdpTargetSession.ts @@ -1,7 +1,10 @@ import type { BrowserObservationEndpoint } from "../domain/browserObservation.js"; +import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; import { CdpConnection } from "./CdpConnection.js"; +import { CdpTargetSessionOwner } from "./CdpTargetSessionOwner.js"; import { cdpTargetWebSocket, discoverCdpBrowserSocket, @@ -36,6 +39,7 @@ export const openCdpEndpoint = async ( operation: BrowserObservationOperation, signal?: AbortSignal, timeoutMs?: number, + owner?: CdpTargetSessionOwner, ): Promise => { if (endpoint.transport === "http_discovery") return { @@ -46,11 +50,15 @@ export const openCdpEndpoint = async ( operation, signal, async (deadlineSignal) => { - const connection = await CdpConnection.connect( - endpoint.url, - operation, - deadlineSignal, - ); + const connection = + owner === undefined + ? await CdpConnection.connect(endpoint.url, operation, deadlineSignal) + : await owner.connect( + endpoint.url, + operation, + { signal: deadlineSignal }, + "browser-endpoint", + ); try { return { discovery: await discoverCdpBrowserSocket( @@ -62,7 +70,15 @@ export const openCdpEndpoint = async ( connection, }; } catch (cause: unknown) { - await connection.close(); + if (owner === undefined) { + await connection.close(); + throw cause; + } + try { + await owner.finish(connection); + } catch (cleanupCause: unknown) { + throw combineOpenFailure(cause, cleanupCause, operation); + } throw cause; } }, @@ -77,16 +93,37 @@ export const openCdpTargetSession = async ( operation: BrowserObservationOperation, signal?: AbortSignal, limits?: { readonly maxPayloadBytes: number }, + owner?: CdpTargetSessionOwner, ): Promise => { const webSocket = cdpTargetWebSocket(discovery, target, operation); - const connection = await CdpConnection.connect( - webSocket.url, - operation, - signal, - limits, - ); + const connection = + owner === undefined + ? await CdpConnection.connect(webSocket.url, operation, signal, limits) + : await owner.connect( + webSocket.url, + operation, + signal === undefined ? {} : { signal }, + target.id, + limits, + ); if (webSocket.scope === "page") return { connection, sessionId: undefined }; - return await attachCdpTargetSession(connection, target, operation, signal); + try { + return await attachCdpTargetSession( + connection, + target, + operation, + signal, + owner, + ); + } catch (cause: unknown) { + if (owner === undefined) throw cause; + try { + await owner.finish(connection); + } catch (cleanupCause: unknown) { + throw combineOpenFailure(cause, cleanupCause, operation); + } + throw cause; + } }; /** Attach one target through an open browser socket, which the session then owns. */ @@ -95,6 +132,7 @@ export const attachCdpTargetSession = async ( target: CdpEndpointTarget, operation: BrowserObservationOperation, signal?: AbortSignal, + owner?: CdpTargetSessionOwner, ): Promise => { try { const attached = await connection.send( @@ -105,16 +143,48 @@ export const attachCdpTargetSession = async ( ); return { connection, sessionId: attachedSessionId(attached, operation) }; } catch (cause: unknown) { - await connection.close(); + if (owner === undefined) { + await connection.close(); + throw cause; + } throw cause; } }; +const combineOpenFailure = ( + primary: unknown, + cleanup: unknown, + operation: BrowserObservationOperation, +): unknown => + primary instanceof AnalysisError && cleanup instanceof AnalysisError + ? analysisErrorWithCleanupFailure(primary, cleanup, operation) + : new AggregateError( + [primary, cleanup], + `CDP operation ${operation} failed and its transport cleanup is incomplete.`, + { cause: primary }, + ); + /** Disable enabled domains, detach browser sessions, and close REA's socket. */ export const closeCdpTargetSession = async ( targetSession: CdpTargetSession, enabledDomains: readonly string[], signal?: AbortSignal, + owner?: CdpTargetSessionOwner, +): Promise => { + if (owner !== undefined) { + await owner.finish(targetSession.connection, () => + cleanupCdpTargetCommands(targetSession, enabledDomains, signal), + ); + return; + } + await cleanupCdpTargetCommands(targetSession, enabledDomains, signal); + await targetSession.connection.close(); +}; + +const cleanupCdpTargetCommands = async ( + targetSession: CdpTargetSession, + enabledDomains: readonly string[], + signal?: AbortSignal, ): Promise => { const { connection, sessionId } = targetSession; const cleanup = new AbortController(); @@ -156,7 +226,6 @@ export const closeCdpTargetSession = async ( } finally { clearTimeout(timeout); cleanup.abort(); - await connection.close(); } }; diff --git a/src/browser/CdpTargetSessionOwner.ts b/src/browser/CdpTargetSessionOwner.ts new file mode 100644 index 0000000000..50d6535922 --- /dev/null +++ b/src/browser/CdpTargetSessionOwner.ts @@ -0,0 +1,238 @@ +import { AnalysisError } from "../domain/analysisErrorBase.js"; +import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { CdpConnection } from "./CdpConnection.js"; +import type { ExecutionOptions } from "../application/AnalysisProvider.js"; + +interface OwnedConnection { + readonly connection: CdpConnection; + readonly operation: BrowserObservationOperation; + readonly resource: string; + active: boolean; + readonly settled: Promise; + settle: () => void; + cleanup: (() => Promise) | undefined; + cleanupAttempted: boolean; + failure: ProviderCleanupError | undefined; +} + +/** Retains exact REA-owned CDP transports until their close is observed. */ +export class CdpTargetSessionOwner { + readonly #connections = new Map(); + #closing = false; + #admissions = 0; + #closePromise: Promise | undefined; + readonly #admissionWaiters = new Set<() => void>(); + + /** Admit and own one WebSocket from handshake completion onward. */ + async connect( + url: string, + operation: BrowserObservationOperation, + options: ExecutionOptions, + resource: string, + limits?: { readonly maxPayloadBytes: number }, + ): Promise { + if (this.#closing) + throw new ProviderCleanupError( + "rea-cdp-browser", + [resource], + { reason: "CDP provider is closing and cannot admit another session." }, + { operation }, + ); + await this.retryPending(); + if (this.#closing) + throw new ProviderCleanupError( + "rea-cdp-browser", + [resource], + { reason: "CDP provider is closing and cannot admit another session." }, + { operation }, + ); + if (options.signal?.aborted === true) + throw new AnalysisCancelledError(operation, { + cause: options.signal.reason, + }); + this.#admissions += 1; + let connection: CdpConnection | undefined; + try { + return await CdpConnection.connect( + url, + operation, + options.signal, + limits, + (opened) => { + connection = opened; + this.register(opened, operation, resource); + }, + ); + } catch (cause: unknown) { + if (connection === undefined) throw cause; + try { + await this.finish(connection); + } catch (cleanupCause: unknown) { + if ( + cause instanceof AnalysisError && + cleanupCause instanceof AnalysisError + ) + throw analysisErrorWithCleanupFailure(cause, cleanupCause, operation); + throw new AggregateError( + [cause, cleanupCause], + `CDP operation ${operation} failed during handshake and cleanup remains incomplete.`, + { cause }, + ); + } + throw cause; + } finally { + this.#admissions -= 1; + if (this.#admissions === 0) { + for (const resolve of this.#admissionWaiters) resolve(); + this.#admissionWaiters.clear(); + } + } + } + + /** Register synchronously before awaiting the WebSocket handshake. */ + register( + connection: CdpConnection, + operation: BrowserObservationOperation, + resource: string, + ): void { + let settle = (): void => undefined; + const settled = new Promise((resolve) => { + settle = resolve; + }); + this.#connections.set(connection, { + connection, + operation, + resource, + active: true, + settled, + settle, + cleanup: undefined, + cleanupAttempted: false, + failure: undefined, + }); + } + + /** Finish one admitted operation, retaining a failed transport for retry. */ + async finish( + connection: CdpConnection, + cleanup?: () => Promise, + ): Promise { + const record = this.#connections.get(connection); + if (record === undefined) return; + if (record.failure !== undefined) throw record.failure; + record.cleanup ??= cleanup; + try { + if (!record.cleanupAttempted && record.cleanup !== undefined) { + record.cleanupAttempted = true; + await record.cleanup(); + } + await connection.close(); + this.#connections.delete(connection); + } catch (cause: unknown) { + const failure = cleanupError(record, cause); + record.failure = failure; + throw failure; + } finally { + record.active = false; + record.settle(); + } + } + + /** Retry only retained failed transports; active peers finish their own cleanup. */ + async close(): Promise { + this.#closing = true; + this.#closePromise ??= this.#closeAll().catch((cause: unknown) => { + this.#closePromise = undefined; + throw cause; + }); + return await this.#closePromise; + } + + /** Retry retained failures without touching concurrently active sessions. */ + async retryPending(): Promise { + const failures: AnalysisError[] = []; + for (const record of this.#connections.values()) { + if (record.active || record.failure === undefined) continue; + try { + await record.connection.close(); + this.#connections.delete(record.connection); + } catch (cause: unknown) { + const failure = cleanupError(record, cause); + record.failure = failure; + failures.push(failure); + } + } + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) throw aggregateCleanupFailures(failures); + } + + async #closeAll(): Promise { + while (this.#admissions > 0) + await new Promise((resolve) => this.#admissionWaiters.add(resolve)); + while (true) { + const active = [...this.#connections.values()].filter( + ({ active }) => active, + ); + if (active.length === 0) break; + await Promise.all(active.map(({ settled }) => settled)); + } + const failures: AnalysisError[] = []; + for (const record of this.#connections.values()) { + try { + await record.connection.close(); + this.#connections.delete(record.connection); + } catch (cause: unknown) { + const failure = cleanupError(record, cause); + record.failure = failure; + failures.push(failure); + } + } + if (failures.length === 1) throw failures[0]; + if (failures.length > 1) throw aggregateCleanupFailures(failures); + } +} + +const cleanupError = ( + record: OwnedConnection, + cause: unknown, +): ProviderCleanupError => + new ProviderCleanupError( + "rea-cdp-browser", + [record.resource], + { + reason: + cause instanceof AnalysisError + ? (cause.userMessage ?? cause.message) + : cause instanceof Error + ? cause.message + : String(cause), + }, + { + operation: record.operation, + cause, + }, + ); + +const aggregateCleanupFailures = ( + failures: readonly AnalysisError[], +): ProviderCleanupError => + new ProviderCleanupError( + "rea-cdp-browser", + failures.flatMap(({ cleanupResources }) => cleanupResources), + { + failures: failures.map(({ message, diagnostics }) => ({ + message, + diagnostics: diagnostics ?? {}, + })), + }, + { + operation: "close_cdp_transports", + cause: new AggregateError( + failures, + "One or more REA-owned CDP transports remain open.", + ), + }, + ); diff --git a/src/browser/PlaywrightBrowserScenarioProvider.ts b/src/browser/PlaywrightBrowserScenarioProvider.ts index b0833e89ce..8a5cc71cbb 100644 --- a/src/browser/PlaywrightBrowserScenarioProvider.ts +++ b/src/browser/PlaywrightBrowserScenarioProvider.ts @@ -21,6 +21,7 @@ import { type BrowserStepArtifacts, } from "../domain/browserScenarioCaptureValues.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../domain/browserObservationError.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -231,7 +232,7 @@ const runScenario = async ( options: ExecutionOptions, retainCleanup: (close: () => Promise) => void, ): Promise => { - if (options.signal?.aborted === true) + if (options.signal?.aborted) throw new BrowserObservationError(OPERATION, "cancelled"); const startedAt = performance.now(); const session = await openSession(scenario, { @@ -324,6 +325,23 @@ const runScenario = async ( ], }), }); + if (options.signal?.aborted) + throw new AnalysisCancelledError(OPERATION, { + cause: options.signal?.reason, + partialObservation: { + kind: "browser-scenario-observation", + capture: browserScenarioCaptureData({ + session, + scenario, + startedAt, + steps, + cleanup: cleanupOutcome.cleanup, + limitations: [ + "Scenario capture completed, but the request was cancelled during final browser cleanup.", + ], + }), + }, + }); const completeness = globalCompleteness(scenario, session, steps); return browserScenarioCaptureSchema.parse({ ...browserScenarioCaptureData({ diff --git a/src/browser/execution/CdpRuntimeSession.ts b/src/browser/execution/CdpRuntimeSession.ts index 179327cc61..12c8c836d0 100644 --- a/src/browser/execution/CdpRuntimeSession.ts +++ b/src/browser/execution/CdpRuntimeSession.ts @@ -4,6 +4,8 @@ import { AnalysisTimeoutError } from "../../domain/analysisErrorCore.js"; import { BrowserObservationError } from "../../domain/browserObservationError.js"; import type { BrowserObservationOperation } from "../../domain/browserObservationErrors.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; +import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; +import { CdpTargetSessionOwner } from "../CdpTargetSessionOwner.js"; import { sanitizeBrowserUrl } from "../../domain/browserObservation.js"; import { WEB_RUNTIME_LIMITS, @@ -41,6 +43,7 @@ export class CdpRuntimeSession { readonly allowedOrigins: ReadonlySet, readonly operation: BrowserObservationOperation, readonly options: ExecutionOptions, + readonly owner: CdpTargetSessionOwner, ) {} /** Acquire a validated target; failed setup releases its owned socket. */ @@ -48,6 +51,7 @@ export class CdpRuntimeSession { input: Scope, operation: BrowserObservationOperation, options: ExecutionOptions, + owner: CdpTargetSessionOwner, ): Promise { return runtimeDeadline(operation, options.signal, async (signal) => { const discovery = await discoverCdpEndpoint( @@ -66,6 +70,7 @@ export class CdpRuntimeSession { operation, signal, { maxPayloadBytes: WEB_RUNTIME_LIMITS.protocolBytes }, + owner, ); try { const tree = runtimeFrameTreeSchema.parse( @@ -93,9 +98,27 @@ export class CdpRuntimeSession { allowedOrigins, operation, options, + owner, ); } catch (cause: unknown) { - await transport.connection.close(); + try { + await owner.finish(transport.connection); + } catch (cleanupCause: unknown) { + if ( + cause instanceof AnalysisError && + cleanupCause instanceof AnalysisError + ) + throw analysisErrorWithCleanupFailure( + cause, + cleanupCause, + operation, + ); + throw new AggregateError( + [cause, cleanupCause], + `CDP runtime setup ${operation} failed and cleanup remains incomplete.`, + { cause }, + ); + } throw cause; } }); @@ -180,6 +203,7 @@ export class CdpRuntimeSession { WEB_RUNTIME_LIMITS.cleanupTimeoutMs, ); const failures: string[] = []; + let transportFailure: AnalysisError | undefined; const send = async ( method: string, params: Readonly> = {}, @@ -220,7 +244,25 @@ export class CdpRuntimeSession { } } finally { clearTimeout(timer); - await this.transport.connection.close(); + try { + await this.owner.finish(this.transport.connection); + } catch (cause: unknown) { + transportFailure = + cause instanceof AnalysisError + ? cause + : new ProviderCleanupError( + "rea-cdp-browser", + [this.target.target_id], + { + reason: + cause instanceof Error ? cause.message : String(cause), + }, + { operation: this.operation, cause }, + ); + failures.push( + `CDP WebSocket close: ${transportFailure.userMessage ?? transportFailure.message}`, + ); + } } return failures.length === 0 ? undefined @@ -233,7 +275,12 @@ export class CdpRuntimeSession { previous_error: previousError?.userMessage ?? previousError?.message ?? null, }, - { operation: this.operation }, + { + operation: this.operation, + ...(transportFailure === undefined + ? {} + : { cause: transportFailure }), + }, ); } } diff --git a/src/browser/execution/CdpWebRuntimeProvider.ts b/src/browser/execution/CdpWebRuntimeProvider.ts index 1dc4f97e0a..8df29707c3 100644 --- a/src/browser/execution/CdpWebRuntimeProvider.ts +++ b/src/browser/execution/CdpWebRuntimeProvider.ts @@ -1,10 +1,17 @@ import { z } from "zod"; import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import type { WebRuntimePort } from "../../application/WebRuntimePort.js"; -import { AnalysisError } from "../../domain/analysisErrorBase.js"; -import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisError, + type AnalysisPartialObservation, +} from "../../domain/analysisErrorBase.js"; +import { + AnalysisCancelledError, + AnalysisOutputError, +} from "../../domain/analysisErrorCore.js"; import type { BrowserObservationOperation } from "../../domain/browserObservationErrors.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; +import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; import { err, ok, type Result } from "../../domain/result.js"; import { webExecutionSchema, @@ -17,6 +24,7 @@ import { type WebEventListeners, } from "../../domain/webEventListeners.js"; import { CDP_BROWSER_PROVIDER_IDENTITY } from "../providerIdentities.js"; +import { CdpTargetSessionOwner } from "../CdpTargetSessionOwner.js"; import { CdpRuntimeSession } from "./CdpRuntimeSession.js"; import { CdpRuntimeSources } from "./CdpRuntimeSources.js"; import { captureWebExecution } from "./CdpExecutionCapture.js"; @@ -27,6 +35,12 @@ import { /** Native CDP integration with distinct inspection/instrumentation authority and confirmed cleanup. */ export class CdpWebRuntimeProvider implements WebRuntimePort { + readonly #owner = new CdpTargetSessionOwner(); + + close(): Promise { + return this.#owner.close(); + } + /** Identify this adapter independently of the captured browser version. */ identity() { return CDP_BROWSER_PROVIDER_IDENTITY; @@ -61,7 +75,7 @@ export class CdpWebRuntimeProvider implements WebRuntimePort { ); } - private async run( + private async run( input: ObserveWebExecutionInput | InspectWebEventListenersInput, operation: BrowserObservationOperation, options: ExecutionOptions, @@ -72,7 +86,12 @@ export class CdpWebRuntimeProvider implements WebRuntimePort { let session: CdpRuntimeSession | undefined; let result: Result; try { - session = await CdpRuntimeSession.open(input, operation, options); + session = await CdpRuntimeSession.open( + input, + operation, + options, + this.#owner, + ); result = ok(schema.parse(await capture(new CdpRuntimeSources(session)))); } catch (cause: unknown) { result = err( @@ -90,8 +109,29 @@ export class CdpWebRuntimeProvider implements WebRuntimePort { result.ok ? undefined : result.error, group, ); - if (failure !== undefined) return err(failure); + if (failure !== undefined) + return err( + result.ok + ? options.signal?.aborted === true + ? analysisErrorWithCleanupFailure( + new AnalysisCancelledError(operation, { + cause: options.signal.reason, + partialObservation: result.value, + }), + failure, + operation, + ) + : failure.retainPartialObservation(result.value) + : analysisErrorWithCleanupFailure(result.error, failure, operation), + ); } + if (result.ok && options.signal?.aborted === true) + return err( + new AnalysisCancelledError(operation, { + cause: options.signal.reason, + partialObservation: result.value, + }), + ); return result; } } diff --git a/src/cli/browserAdvancedCommands.ts b/src/cli/browserAdvancedCommands.ts index 90f7b35c9f..d73c53f6f4 100644 --- a/src/cli/browserAdvancedCommands.ts +++ b/src/cli/browserAdvancedCommands.ts @@ -28,6 +28,7 @@ import { parseCliJsonInput } from "../cliJsonInput.js"; import type { Logger } from "pino"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Register WebMCP, capture-diff, and screenshot CLI equivalents. */ export const registerAdvancedBrowserCommands = ( @@ -77,12 +78,19 @@ const registerWebMcp = ( "discover_webmcp_tools", projectInputIssues(parsed.error.issues, input), ); - const result = await discoverWebMcpTools( - context.provider, - parsed.data, - { signal }, + return await finishAnalysisCommand( + () => + discoverWebMcpTools(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "discover_webmcp_tools", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); @@ -182,12 +190,19 @@ const registerScreenshot = ( "capture_web_screenshot", projectInputIssues(parsed.error.issues, input), ); - const result = await captureWebScreenshot( - context.provider, - parsed.data, - { signal }, + return await finishAnalysisCommand( + () => + captureWebScreenshot(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "capture_web_screenshot", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); diff --git a/src/cli/browserCommands.ts b/src/cli/browserCommands.ts index d04d3fb550..e1bc055821 100644 --- a/src/cli/browserCommands.ts +++ b/src/cli/browserCommands.ts @@ -23,6 +23,7 @@ import { observationDuration, } from "../cliObservationOptions.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Register CLI equivalents of the passive browser MCP tools. */ export const registerBrowserCommands = ( @@ -65,12 +66,18 @@ const registerTargetList = ( input, ), ); - const result = await listBrowserTargets( - context.provider, - parsed.data, - { signal }, + return await finishAnalysisCommand( + () => listBrowserTargets(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "list_browser_targets", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); @@ -116,10 +123,18 @@ const registerPageInspection = ( input, ), ); - const result = await inspectWebPage(context.provider, parsed.data, { + return await finishAnalysisCommand( + () => inspectWebPage(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "inspect_web_page", + undefined, signal, - }); - return result.ok ? result.value : browserCliError(result.error); + ); }), ), }); @@ -167,10 +182,18 @@ const registerBundleAnalysis = ( input, ), ); - const result = await analyzeWebBundle(context.provider, parsed.data, { + return await finishAnalysisCommand( + () => analyzeWebBundle(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "analyze_web_bundle", + undefined, signal, - }); - return result.ok ? result.value : browserCliError(result.error); + ); }), ), }); @@ -211,12 +234,18 @@ const registerObservationSession = ( input, ), ); - const result = await observeWebSession( - context.provider, - parsed.data, - { signal }, + return await finishAnalysisCommand( + () => observeWebSession(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + context.provider.identity().id, + "observe_web_session", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); diff --git a/src/cli/electronCommands.ts b/src/cli/electronCommands.ts index 1bf8c1066a..df737736e8 100644 --- a/src/cli/electronCommands.ts +++ b/src/cli/electronCommands.ts @@ -22,6 +22,7 @@ import { } from "../cliObservationOptions.js"; import { runCliJavaScriptApplicationAnalysis } from "./javascriptApplicationAnalysis.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** Register CLI equivalents of the Electron MCP tools. */ export const registerElectronCommands = ( @@ -180,17 +181,30 @@ const registerElectronTargetList = ( endpoint: z.string().describe("Literal-loopback Electron CDP endpoint"), }), run: ({ args }) => - logCliCommand(logger, "list-electron-targets", async () => { - const context = await electronObservationContext(); - const parsed = listElectronTargetsInputSchema.safeParse({ - cdp_endpoint: args.endpoint, - }); - if (!parsed.success) return inputError("list_electron_targets"); - const { listElectronTargets } = - await import("../application/javascript/ElectronObservationService.js"); - const result = await listElectronTargets(context.provider, parsed.data); - return result.ok ? result.value : cliError(result.error); - }), + withCommandCancellation((signal) => + logCliCommand(logger, "list-electron-targets", async () => { + const context = await electronObservationContext(); + const parsed = listElectronTargetsInputSchema.safeParse({ + cdp_endpoint: args.endpoint, + }); + if (!parsed.success) return inputError("list_electron_targets"); + const { listElectronTargets } = + await import("../application/javascript/ElectronObservationService.js"); + return await finishAnalysisCommand( + () => + listElectronTargets(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + "rea-cdp-electron", + "list_electron_targets", + undefined, + signal, + ); + }), + ), }); }; @@ -206,20 +220,33 @@ const registerElectronPageInspection = ( }), options: electronPageInspectionOptions, run: ({ args, options }) => - logCliCommand(logger, "inspect-electron-page", async () => { - const context = await electronObservationContext(); - const parsed = inspectElectronPageInputSchema.safeParse({ - cdp_endpoint: args.endpoint, - target_id: args.targetId, - observation_ms: options.observationMs, - include_script_sources: options.includeScriptSources, - }); - if (!parsed.success) return inputError("inspect_electron_page"); - const { inspectElectronPage } = - await import("../application/javascript/ElectronObservationService.js"); - const result = await inspectElectronPage(context.provider, parsed.data); - return result.ok ? result.value : cliError(result.error); - }), + withCommandCancellation((signal) => + logCliCommand(logger, "inspect-electron-page", async () => { + const context = await electronObservationContext(); + const parsed = inspectElectronPageInputSchema.safeParse({ + cdp_endpoint: args.endpoint, + target_id: args.targetId, + observation_ms: options.observationMs, + include_script_sources: options.includeScriptSources, + }); + if (!parsed.success) return inputError("inspect_electron_page"); + const { inspectElectronPage } = + await import("../application/javascript/ElectronObservationService.js"); + return await finishAnalysisCommand( + () => + inspectElectronPage(context.provider, parsed.data, { signal }), + { + close: async () => { + await context.provider.close?.(); + }, + }, + "rea-cdp-electron", + "inspect_electron_page", + undefined, + signal, + ); + }), + ), }); }; diff --git a/src/cli/finishAnalysisCommand.ts b/src/cli/finishAnalysisCommand.ts index 9394ba828b..d370ff52ca 100644 --- a/src/cli/finishAnalysisCommand.ts +++ b/src/cli/finishAnalysisCommand.ts @@ -4,12 +4,13 @@ import { type AnalysisPartialObservation, } from "../domain/analysisErrorBase.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { Result } from "../domain/result.js"; -/** Finish a one-shot CLI operation without discarding its primary result on close failure. */ +/** Finish a one-shot CLI operation while retaining primary results through close. */ export const finishAnalysisCommand = async < Value extends AnalysisPartialObservation, >( @@ -18,6 +19,7 @@ export const finishAnalysisCommand = async < providerId: string, canonicalOperation: string, fallbackResource?: string, + signal?: AbortSignal, ): Promise> => { let result: Result; try { @@ -33,6 +35,8 @@ export const finishAnalysisCommand = async < } try { await service.close(); + if (!result.ok && result.error.cleanupIncomplete) + result.error.resolveCleanup(); } catch (cause) { let cleanup: AnalysisError; if (cause instanceof AnalysisError) cleanup = cause; @@ -65,5 +69,12 @@ export const finishAnalysisCommand = async < ), ); } + if (result.ok && signal?.aborted === true) + return projectAnalysisError( + new AnalysisCancelledError(canonicalOperation, { + cause: signal.reason, + partialObservation: result.value, + }), + ); return result.ok ? result.value : projectAnalysisError(result.error); }; diff --git a/src/cli/webRuntimeCommands.ts b/src/cli/webRuntimeCommands.ts index 2963641d8d..f970078b50 100644 --- a/src/cli/webRuntimeCommands.ts +++ b/src/cli/webRuntimeCommands.ts @@ -10,6 +10,7 @@ import { CLI_COMMANDS } from "../cliCommandNames.js"; import type { Logger } from "pino"; import type { ProgressReporter } from "../application/ProgressReporter.js"; import { withCommandCancellation } from "./commandCancellation.js"; +import { finishAnalysisCommand } from "./finishAnalysisCommand.js"; /** CLI progress exposes the actual armed point on stderr without corrupting JSON results. */ const runtimeProgress: ProgressReporter = { @@ -39,16 +40,28 @@ export const registerWebRuntimeCommands = ( run: ({ args, options }) => withCommandCancellation((signal) => logCliCommand(logger, "observe-web-execution", async () => { - const result = await createWebRuntimeService().observe( + const service = createWebRuntimeService(); + return await finishAnalysisCommand( + () => + service.observe( + { + cdp_endpoint: args.endpoint, + target_id: args.targetId, + allowed_origins: options.allowedOrigins, + observation_ms: options.observationMs, + }, + { progress: runtimeProgress, signal }, + ), { - cdp_endpoint: args.endpoint, - target_id: args.targetId, - allowed_origins: options.allowedOrigins, - observation_ms: options.observationMs, + close: async () => { + await service.provider.close?.(); + }, }, - { progress: runtimeProgress, signal }, + service.provider.identity().id, + "observe_web_execution", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); @@ -68,16 +81,28 @@ export const registerWebRuntimeCommands = ( run: ({ args, options }) => withCommandCancellation((signal) => logCliCommand(logger, "inspect-web-event-listeners", async () => { - const result = await createWebRuntimeService().inspect( + const service = createWebRuntimeService(); + return await finishAnalysisCommand( + () => + service.inspect( + { + cdp_endpoint: args.endpoint, + target_id: args.targetId, + selector: args.selector, + allowed_origins: options.allowedOrigins, + }, + { signal }, + ), { - cdp_endpoint: args.endpoint, - target_id: args.targetId, - selector: args.selector, - allowed_origins: options.allowedOrigins, + close: async () => { + await service.provider.close?.(); + }, }, - { signal }, + service.provider.identity().id, + "inspect_web_event_listeners", + undefined, + signal, ); - return result.ok ? result.value : browserCliError(result.error); }), ), }); diff --git a/src/server/createServer.ts b/src/server/createServer.ts index 0195bcee8c..dcd893e588 100644 --- a/src/server/createServer.ts +++ b/src/server/createServer.ts @@ -358,6 +358,7 @@ export const createServer = ( options.packetCapture ?? createPacketCaptureService(environment); const signalRecording = options.signalRecording ?? createSignalRecordingService(environment); + const webRuntime = options.webRuntime ?? createWebRuntimeService(); const apktool = options.apktoolAnalysis ?? createApktoolResourceAnalysisProvider(environment); @@ -387,6 +388,10 @@ export const createServer = ( peResources.close(), webNetworkCapture.close(), javascriptRecovery.close?.() ?? Promise.resolve(), + options.browserObservation?.close?.() ?? Promise.resolve(), + options.browserScenarioCapture?.close?.() ?? Promise.resolve(), + options.electronObservation?.close?.() ?? Promise.resolve(), + webRuntime.provider.close?.() ?? Promise.resolve(), artifactResources.close(), reverseEngineering.close(), firmwareAnalysis.close(), @@ -535,6 +540,7 @@ export const createServer = ( evmInterface, jeb, firmwareAnalysis, + webRuntime, ); registerAdbTools( server, @@ -638,6 +644,7 @@ const registerConfiguredAnalysisTools = ( evmInterface: EvmInterfaceService, jeb: JebAnalysisPort, firmwareAnalysis: FirmwareAnalysisService, + webRuntime: WebRuntimeService, ): void => { registerAndroidTools( server, @@ -743,7 +750,7 @@ const registerConfiguredAnalysisTools = ( ); registerWebRuntimeTools( server, - options.webRuntime ?? createWebRuntimeService(), + webRuntime, toolLogger, recordEvidence, withAdmittedAnalysis, diff --git a/tests/boundary/browser/cdpCleanupCancellation.test.ts b/tests/boundary/browser/cdpCleanupCancellation.test.ts index 8495313132..5001a73e08 100644 --- a/tests/boundary/browser/cdpCleanupCancellation.test.ts +++ b/tests/boundary/browser/cdpCleanupCancellation.test.ts @@ -7,6 +7,7 @@ import { expect, it, onTestFinished, vi } from "vitest"; import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; import { CdpElectronProvider } from "../../../src/browser/CdpElectronProvider.js"; import { observeWebSessionInputSchema } from "../../../src/domain/browserSession.js"; +import { analyzeWebBundleInputSchema } from "../../../src/domain/webBundleAnalysis.js"; import { inspectElectronPageInputSchema } from "../../../src/domain/javascript/electronObservation.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; @@ -90,6 +91,45 @@ it("releases cleanup when cancellation arrives after capture completes", async ( expect((await fetch(`${browser.endpoint}/json/version`)).ok).toBe(true); }); +it("retains the page inspection when bundle cleanup is cancelled", async () => { + const controller = new AbortController(); + const browser = await startFakeCdpBrowser({ + commandError(command) { + if (command.method !== "Target.detachFromTarget") return undefined; + controller.abort(); + return undefined; + }, + }); + const provider = new CdpBrowserProvider(); + onTestFinished(async () => { + await provider.close(); + await browser.close(); + }); + + const result = await provider.analyzeBundle( + analyzeWebBundleInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + fetch_source_maps: false, + }), + { signal: controller.signal }, + ); + + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "analyze_web_bundle", + partialObservation: { + target: { target_id: "allowed-page" }, + capture_window: expect.any(Object), + }, + }, + }); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); +}); + it("closes an Electron inspection session when cleanup is cancelled", async () => { const root = await createTestTempDirectory("rea-electron-cancel-"); const page = join(root, "index.html"); @@ -142,3 +182,51 @@ it("closes an Electron inspection session when cleanup is cancelled", async () = await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); expect((await fetch(`${browser.endpoint}/json/version`)).ok).toBe(true); }); + +it("classifies cancellation that arrives during final Electron detach", async () => { + const root = await createTestTempDirectory("rea-electron-final-detach-"); + const page = join(root, "index.html"); + await writeFile(page, "
completed fixture
"); + const controller = new AbortController(); + const browser = await startFakeCdpBrowser({ + electronFileUrl: pathToFileURL(page).href, + commandError(command) { + if (command.method !== "Target.detachFromTarget") return undefined; + // This command is issued only after page inspection has completed. + controller.abort(); + return { code: -32_000, message: "controlled detach rejection" }; + }, + }); + onTestFinished(async () => { + await browser.close(); + await rm(root, { recursive: true, force: true }); + }); + + const result = await new CdpElectronProvider().inspectPage( + inspectElectronPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "electron-page", + observation_ms: 0, + }), + { signal: controller.signal }, + ); + + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "inspect_electron_page", + partialObservation: { + target: { target_id: "electron-page", file_path: page }, + dom: { total_nodes: expect.any(Number) }, + }, + }, + }); + const methods = browser.commands.map(({ method }) => method); + expect(methods).toContain("Page.getFrameTree"); + expect(methods).toContain("Target.detachFromTarget"); + expect(methods).not.toContain("Target.closeTarget"); + expect(methods).not.toContain("Browser.close"); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + expect((await fetch(`${browser.endpoint}/json/version`)).ok).toBe(true); +}); diff --git a/tests/boundary/browser/cdpProviderCleanupOwnership.test.ts b/tests/boundary/browser/cdpProviderCleanupOwnership.test.ts new file mode 100644 index 0000000000..7fd85335f0 --- /dev/null +++ b/tests/boundary/browser/cdpProviderCleanupOwnership.test.ts @@ -0,0 +1,120 @@ +import { rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +import { WebSocket } from "ws"; +import { expect, it, onTestFinished, vi } from "vitest"; + +import { CdpElectronProvider } from "../../../src/browser/CdpElectronProvider.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { inspectElectronPageInputSchema } from "../../../src/domain/javascript/electronObservation.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; + +const retryProviderCleanup = async ( + provider: CdpElectronProvider, +): Promise => { + const close = Reflect.get(provider, "close") as + | (() => Promise) + | undefined; + expect(close).toBeTypeOf("function"); + await close?.call(provider); +}; + +it.each([ + ["keeps a completed inspection", false], + ["keeps the primary capture failure", true], +] as const)( + "retains ownership after socket close fails and %s", + async (_label, failCapture) => { + const root = await createTestTempDirectory("rea-electron-close-owner-"); + const page = join(root, "index.html"); + await writeFile(page, "
completed fixture
"); + const browser = await startFakeCdpBrowser({ + electronFileUrl: pathToFileURL(page).href, + ...(failCapture + ? { + commandError(command: { method: string }) { + return command.method === "Page.getFrameTree" + ? { + code: -32_000, + message: "controlled primary capture failure", + } + : undefined; + }, + } + : {}), + }); + const provider = new CdpElectronProvider(); + onTestFinished(async () => { + const closeProvider = Reflect.get(provider, "close") as + | (() => Promise) + | undefined; + if (closeProvider !== undefined) await closeProvider.call(provider); + await browser.close(); + await rm(root, { recursive: true, force: true }); + }); + + const close = vi + .spyOn(WebSocket.prototype, "close") + .mockImplementationOnce(() => { + throw new Error("controlled synchronous socket close failure"); + }); + const terminate = vi + .spyOn(WebSocket.prototype, "terminate") + .mockImplementation(() => undefined); + let result: + | Awaited> + | undefined; + try { + result = await provider.inspectPage( + inspectElectronPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "electron-page", + observation_ms: 0, + }), + ); + } catch { + // The assertion below requires the provider to report cleanup as typed data. + } finally { + close.mockRestore(); + terminate.mockRestore(); + } + + expect(result).toBeDefined(); + if (failCapture) { + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "ProviderAdapterError", + cleanupIncomplete: true, + diagnostics: { + primary_error: { code: "unreadable_output" }, + cleanup_error: expect.any(Object), + }, + }, + }); + expect(browser.commands.map(({ method }) => method)).toContain( + "Page.getFrameTree", + ); + } else { + expect(result).toMatchObject({ ok: false }); + if (!result || result.ok) throw new Error("Expected cleanup failure"); + expect(result.error).toBeInstanceOf(ProviderCleanupError); + expect(result.error).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: expect.arrayContaining(["electron-page"]), + partialObservation: { + target: { target_id: "electron-page", file_path: page }, + dom: { total_nodes: expect.any(Number) }, + }, + }); + } + + await retryProviderCleanup(provider); + const methods = browser.commands.map(({ method }) => method); + expect(methods).not.toContain("Target.closeTarget"); + expect(methods).not.toContain("Browser.close"); + expect((await fetch(`${browser.endpoint}/json/version`)).ok).toBe(true); + }, +); diff --git a/tests/boundary/browser/cdpTargetSessionOwner.test.ts b/tests/boundary/browser/cdpTargetSessionOwner.test.ts new file mode 100644 index 0000000000..8b1b6ed97c --- /dev/null +++ b/tests/boundary/browser/cdpTargetSessionOwner.test.ts @@ -0,0 +1,158 @@ +import { createServer } from "node:http"; + +import { WebSocket, WebSocketServer } from "ws"; +import { expect, it, onTestFinished, vi } from "vitest"; + +import { CdpTargetSessionOwner } from "../../../src/browser/CdpTargetSessionOwner.js"; +import type { CdpConnection } from "../../../src/browser/CdpConnection.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js"; + +it("waits for a late owned handshake and does not close its active peer on retry", async () => { + const http = createServer(); + const webSockets = new WebSocketServer({ noServer: true }); + let port = 0; + let releaseUpgrade: (() => void) | undefined; + let upgradeReceived: (() => void) | undefined; + const received = new Promise((resolve) => { + upgradeReceived = resolve; + }); + const allow = new Promise((resolve) => { + releaseUpgrade = resolve; + }); + http.on("upgrade", (request, socket, head) => { + upgradeReceived?.(); + void allow.then(() => + webSockets.handleUpgrade(request, socket, head, (webSocket) => + webSockets.emit("connection", webSocket, request), + ), + ); + }); + 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"); + port = address.port; + const owner = new CdpTargetSessionOwner(); + let connecting: Promise | undefined; + onTestFinished(async () => { + releaseUpgrade?.(); + if (connecting !== undefined) { + const pendingConnection = await connecting; + await owner.finish(pendingConnection); + } + await owner.close(); + await new Promise((resolve, reject) => + webSockets.close((cause) => (cause ? reject(cause) : resolve())), + ); + await new Promise((resolve, reject) => + http.close((cause) => (cause ? reject(cause) : resolve())), + ); + }); + + connecting = owner.connect( + `ws://127.0.0.1:${String(port)}/devtools/browser/late`, + "inspect_web_page", + {}, + "late-handshake", + ); + await received; + const closing = owner.close(); + releaseUpgrade?.(); + const connection = await connecting; + await owner.finish(connection); + await closing; + expect(connection).toBeDefined(); +}); + +it("retries only the failed transport and does not repeat successful child cleanup", async () => { + const browser = await startFakeCdpBrowser(); + const owner = new CdpTargetSessionOwner(); + let active: CdpConnection | undefined; + onTestFinished(async () => { + if (active !== undefined) await owner.finish(active); + await owner.close(); + await browser.close(); + }); + const failed = await owner.connect( + browser.browserWebSocketUrl, + "inspect_web_page", + {}, + "failed-session", + ); + active = await owner.connect( + browser.browserWebSocketUrl, + "inspect_web_page", + {}, + "active-peer", + ); + const close = vi + .spyOn(WebSocket.prototype, "close") + .mockImplementationOnce(() => { + throw new Error("controlled close failure"); + }); + const terminate = vi + .spyOn(WebSocket.prototype, "terminate") + .mockImplementation(() => undefined); + try { + const failure = await owner.finish(failed).then( + () => undefined, + (cause: unknown) => cause, + ); + expect(failure).toBeInstanceOf(ProviderCleanupError); + expect(failure).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["failed-session"], + }); + } finally { + close.mockRestore(); + terminate.mockRestore(); + } + + await owner.retryPending(); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(1)); + const cleanup = vi.fn(async () => undefined); + await owner.finish(active, cleanup); + await owner.retryPending(); + await owner.close(); + expect(cleanup).toHaveBeenCalledTimes(1); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + expect(browser.commands.map(({ method }) => method)).not.toContain( + "Target.closeTarget", + ); + expect(browser.commands.map(({ method }) => method)).not.toContain( + "Browser.close", + ); +}); + +it("accepts asynchronous terminate success only after the close event", async () => { + const browser = await startFakeCdpBrowser(); + const owner = new CdpTargetSessionOwner(); + onTestFinished(async () => { + await owner.close(); + await browser.close(); + }); + const connection = await owner.connect( + browser.browserWebSocketUrl, + "inspect_web_page", + {}, + "forced-close", + ); + const originalTerminate = WebSocket.prototype.terminate; + const close = vi + .spyOn(WebSocket.prototype, "close") + .mockImplementation(() => undefined); + const terminate = vi + .spyOn(WebSocket.prototype, "terminate") + .mockImplementation(function (this: WebSocket) { + return originalTerminate.call(this); + }); + try { + await owner.finish(connection); + } finally { + close.mockRestore(); + terminate.mockRestore(); + } + await owner.close(); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); +}); diff --git a/tests/boundary/browser/cdpWebRuntimeFinalCollection.test.ts b/tests/boundary/browser/cdpWebRuntimeFinalCollection.test.ts index 77c25e0da3..7ca35912e1 100644 --- a/tests/boundary/browser/cdpWebRuntimeFinalCollection.test.ts +++ b/tests/boundary/browser/cdpWebRuntimeFinalCollection.test.ts @@ -132,7 +132,21 @@ describe("runtime fatal protocol stream", () => { expect(result.error).toMatchObject({ cleanupIncomplete: true, diagnostics: { - previous_error: expect.stringMatching(/protocol|malformed/iu), + primary_error: { + details: { + reason: expect.stringMatching(/protocol|malformed/iu), + }, + }, + cleanup_error: { + details: { + diagnostics: { + previous_error: expect.stringMatching(/protocol|malformed/iu), + cleanup_failures: expect.arrayContaining([ + expect.stringMatching(/protocol|malformed/iu), + ]), + }, + }, + }, }, }); }, diff --git a/tests/boundary/cli/electronCli.test.ts b/tests/boundary/cli/electronCli.test.ts index 8898ad9004..c8a88dab0a 100644 --- a/tests/boundary/cli/electronCli.test.ts +++ b/tests/boundary/cli/electronCli.test.ts @@ -1,10 +1,10 @@ -import { execFile } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import { rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; import { promisify } from "node:util"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -35,103 +35,178 @@ afterEach(async () => { ); }); -describe("Electron CLI parity", () => { - it( - "returns the same endpoint-scoped Electron Evidence contracts", - async () => { - const root = await createTestTempDirectory("rea-electron-cli-"); - temporary.push(root); - await writeFile( - join(root, "index.html"), - "", - ); - await writeFile(join(root, "app.js"), "export const app = true;"); - const browser = await startFakeCdpBrowser({ - electronFileUrl: pathToFileURL(join(root, "index.html")).href, - commandEvents: ({ method }) => - method === "Debugger.enable" - ? [ - { - method: "Debugger.scriptParsed", - sessionId: "session-1", - params: { - scriptId: "partial-script", - url: pathToFileURL(join(root, "app.js")).href, - executionContextId: 1, - }, +it( + "returns the same endpoint-scoped Electron Evidence contracts", + async () => { + const root = await createTestTempDirectory("rea-electron-cli-"); + temporary.push(root); + await writeFile(join(root, "index.html"), ""); + await writeFile(join(root, "app.js"), "export const app = true;"); + const browser = await startFakeCdpBrowser({ + electronFileUrl: pathToFileURL(join(root, "index.html")).href, + commandEvents: ({ method }) => + method === "Debugger.enable" + ? [ + { + method: "Debugger.scriptParsed", + sessionId: "session-1", + params: { + scriptId: "partial-script", + url: pathToFileURL(join(root, "app.js")).href, + executionContextId: 1, }, - ] - : undefined, - }); - browsers.push(browser); - const environment = { ...process.env }; - const listed = await runCli( - ["list-electron-targets", browser.endpoint, "--json"], - environment, - ); - expect(listed).toMatchObject({ - operation: "list_electron_targets", - normalized_result: { - targets: [{ target_id: "electron-page" }], + }, + ] + : undefined, + }); + browsers.push(browser); + const environment = { ...process.env }; + const listed = await runCli( + ["list-electron-targets", browser.endpoint, "--json"], + environment, + ); + expect(listed).toMatchObject({ + operation: "list_electron_targets", + normalized_result: { + targets: [{ target_id: "electron-page" }], + }, + }); + const inspected = await runCli( + [ + "inspect-electron-page", + browser.endpoint, + "electron-page", + "--observation-ms", + "0", + "--json", + ], + environment, + ); + expect(inspected).toMatchObject({ + operation: "inspect_electron_page", + provider: { id: "rea-cdp-electron" }, + normalized_result: { + target: { file_path: join(root, "index.html") }, + scripts: { + items: [{ cdp_hash: null, length: null, is_module: null }], }, - }); - const inspected = await runCli( - [ - "inspect-electron-page", - browser.endpoint, - "electron-page", - "--observation-ms", - "0", - "--json", - ], - environment, - ); - expect(inspected).toMatchObject({ - operation: "inspect_electron_page", - provider: { id: "rea-cdp-electron" }, - normalized_result: { - target: { file_path: join(root, "index.html") }, - scripts: { - items: [{ cdp_hash: null, length: null, is_module: null }], - }, + }, + }); + }, + INTEGRATION_TEST_TIMEOUT_MS, +); + +// On Windows child.kill("SIGINT") terminates the process without a console signal. +it.skipIf(process.platform === "win32")( + "forwards SIGINT through Electron page inspection and completes target cleanup", + async () => { + const root = await createTestTempDirectory("rea-electron-cli-cancel-"); + temporary.push(root); + const page = join(root, "index.html"); + await writeFile(page, "
fixture
"); + const browser = await startFakeCdpBrowser({ + electronFileUrl: pathToFileURL(page).href, + }); + browsers.push(browser); + + const child = spawn( + process.execPath, + [ + "scripts/rea.mjs", + "inspect-electron-page", + browser.endpoint, + "electron-page", + "--observation-ms", + "60000", + "--json", + ], + { + cwd: process.cwd(), + env: { ...process.env }, + stdio: ["ignore", "pipe", "pipe"], + }, + ); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk) => { + stderr += chunk; + }); + const closed = new Promise<{ + readonly code: number | null; + readonly signal: NodeJS.Signals | null; + }>((resolve, reject) => { + child.once("error", reject); + child.once("close", (code, signal) => resolve({ code, signal })); + }); + const hardStop = setTimeout(() => child.kill("SIGKILL"), 15_000); + try { + await vi.waitFor( + () => { + expect( + browser.commands.some(({ method }) => method === "Runtime.enable"), + ).toBe(true); }, - }); - }, - INTEGRATION_TEST_TIMEOUT_MS, - ); - - it( - "returns the static JavaScript application Evidence contract", - async () => { - const root = await createTestTempDirectory("rea-electron-static-cli-"); - temporary.push(root); - await writeElectronBoundaryFixture(root); - const environment = { - ...process.env, - }; - - const analyzed = await runCli( - ["analyze-javascript-application", root, "--json"], - environment, + { timeout: 5_000 }, ); + child.kill("SIGINT"); - expect(analyzed).toMatchObject({ - operation: "analyze_javascript_application", - provider: { id: "rea-javascript-application" }, - normalized_result: { - input_path: root, - summary: { - browser_windows: 3, - context_bridge_apis: 2, - ipc: { paired_renderer_transmissions: 4 }, - }, - }, + const result = await closed; + expect(result, stderr).toEqual({ code: 130, signal: null }); + expect(JSON.parse(stdout)).toMatchObject({ + code: "cancelled", + category: "cancelled", + details: { operation: "inspect_electron_page" }, }); - }, - INTEGRATION_TEST_TIMEOUT_MS, - ); -}); + const methods = browser.commands.map(({ method }) => method); + expect(methods).toContain("Target.detachFromTarget"); + expect(methods).not.toContain("Target.closeTarget"); + expect(methods).not.toContain("Browser.close"); + await vi.waitFor(() => expect(browser.activeConnectionCount()).toBe(0)); + expect((await fetch(`${browser.endpoint}/json/version`)).ok).toBe(true); + } finally { + clearTimeout(hardStop); + if (child.exitCode === null && child.signalCode === null) { + child.kill("SIGKILL"); + await closed; + } + } + }, + INTEGRATION_TEST_TIMEOUT_MS, +); +it( + "returns the static JavaScript application Evidence contract", + async () => { + const root = await createTestTempDirectory("rea-electron-static-cli-"); + temporary.push(root); + await writeElectronBoundaryFixture(root); + const environment = { + ...process.env, + }; + + const analyzed = await runCli( + ["analyze-javascript-application", root, "--json"], + environment, + ); + + expect(analyzed).toMatchObject({ + operation: "analyze_javascript_application", + provider: { id: "rea-javascript-application" }, + normalized_result: { + input_path: root, + summary: { + browser_windows: 3, + context_bridge_apis: 2, + ipc: { paired_renderer_transmissions: 4 }, + }, + }, + }); + }, + INTEGRATION_TEST_TIMEOUT_MS, +); describe("Electron CLI artifact behavior", () => { it( "keeps artifact format separate from structured output format", diff --git a/tests/boundary/cli/finishAnalysisCommand.test.ts b/tests/boundary/cli/finishAnalysisCommand.test.ts new file mode 100644 index 0000000000..00373bd84d --- /dev/null +++ b/tests/boundary/cli/finishAnalysisCommand.test.ts @@ -0,0 +1,53 @@ +import { expect, it } from "vitest"; + +import { finishAnalysisCommand } from "../../../src/cli/finishAnalysisCommand.js"; +import { AnalysisOutputError } from "../../../src/domain/analysisErrorCore.js"; +import { err, ok } from "../../../src/domain/result.js"; +import { scriptCaptureEvidenceFixture } from "../../fixtures/webScriptCapture.js"; + +it("returns completed partial Evidence when cancellation arrives during close", async () => { + const evidence = scriptCaptureEvidenceFixture(); + const controller = new AbortController(); + const result = await finishAnalysisCommand( + async () => ok(evidence), + { + close: async () => { + controller.abort(new Error("cancelled during close")); + }, + }, + "fixture-provider", + "capture_browser_scenario", + undefined, + controller.signal, + ); + + expect(result).toMatchObject({ + code: "cancelled", + details: { partial_observation: { evidence_id: evidence.evidence_id } }, + }); +}); + +it("preserves the primary operation error when cancellation arrives during close", async () => { + const controller = new AbortController(); + const result = await finishAnalysisCommand( + async () => + err(new AnalysisOutputError("inspect_web_page", "primary failure")), + { + close: async () => { + controller.abort(); + }, + }, + "fixture-provider", + "inspect_web_page", + undefined, + controller.signal, + ); + + expect(result).toMatchObject({ + code: "unreadable_output", + details: { + reason: "primary failure", + operation: "inspect_web_page", + }, + }); +}); diff --git a/tests/boundary/mcp/serverLifecycleMcp.test.ts b/tests/boundary/mcp/serverLifecycleMcp.test.ts index cdec09a3ef..c2d63f46a8 100644 --- a/tests/boundary/mcp/serverLifecycleMcp.test.ts +++ b/tests/boundary/mcp/serverLifecycleMcp.test.ts @@ -341,6 +341,7 @@ it("closes the bound Go and web-capture services and retries failed shutdown", a let goCalls = 0; let goCloseAttempts = 0; let captureCloseAttempts = 0; + let scenarioCloseAttempts = 0; let cleanupAllowed = false; const goBinary = new GoBinaryService({ identity: { id: "go-fixture", name: "Go fixture", version: "1" }, @@ -370,7 +371,24 @@ it("closes the bound Go and web-capture services and retries failed shutdown", a }); const server = createServer( { kind: "fixed", analysis: { execute: async () => ok(null) } }, - { goBinary, webNetworkCapture }, + { + goBinary, + webNetworkCapture, + browserScenarioCapture: { + identity: () => ({ + id: "scenario-fixture", + name: "Scenario fixture", + version: "1", + }), + captureScenario: async () => { + throw new Error("No scenario capture requested"); + }, + close: async () => { + scenarioCloseAttempts += 1; + if (!cleanupAllowed) throw new Error("Scenario cleanup refused"); + }, + }, + }, ); const client = new Client({ name: "bound-service-lifecycle", version: "1" }); const [clientTransport, serverTransport] = @@ -386,14 +404,20 @@ it("closes the bound Go and web-capture services and retries failed shutdown", a expect(result.isError).toBe(true); expect(goCalls).toBe(1); - await expect(server.close()).rejects.toBeInstanceOf(AggregateError); + await expect(server.close()).rejects.toMatchObject({ + errors: expect.arrayContaining([ + expect.objectContaining({ message: "Scenario cleanup refused" }), + ]), + }); expect(goCloseAttempts).toBe(1); expect(captureCloseAttempts).toBe(1); + expect(scenarioCloseAttempts).toBe(1); cleanupAllowed = true; await server.close(); expect(goCloseAttempts).toBe(2); expect(captureCloseAttempts).toBe(2); + expect(scenarioCloseAttempts).toBe(2); } finally { cleanupAllowed = true; } diff --git a/tests/process-global/browserScenarioCleanup.test.ts b/tests/process-global/browserScenarioCleanup.test.ts index 815cfe786c..638d188ad0 100644 --- a/tests/process-global/browserScenarioCleanup.test.ts +++ b/tests/process-global/browserScenarioCleanup.test.ts @@ -174,6 +174,54 @@ const serveScenario = async () => { }; }; +it("retains the completed scenario when cancellation arrives during final close", async () => { + let announceClose!: () => void; + const closeStarted = new Promise((resolve) => { + announceClose = resolve; + }); + let releaseClose!: () => void; + const closeBarrier = new Promise((resolve) => { + releaseClose = resolve; + }); + class FinalCloseSession extends RetrySession { + override async close() { + announceClose(); + await closeBarrier; + return super.close(); + } + } + + const session = new FinalCloseSession(0); + const provider = new PlaywrightBrowserScenarioProvider(async () => session); + const cancellation = new AbortController(); + try { + const pending = provider.captureScenario(scenario, { + signal: cancellation.signal, + }); + await closeStarted; + cancellation.abort(); + releaseClose(); + + const result = await pending; + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.partialObservation).toMatchObject({ + kind: "browser-scenario-observation", + capture: { + browser: { cleanup: "terminated-owned-process" }, + steps: [{ step_id: "scenario_start" }, { step_id: "wait" }], + }, + }); + expect(session.closeBrowserCalls).toBe(1); + expect(session.profileRemovalCalls).toBe(1); + } finally { + releaseClose(); + cancellation.abort(); + await provider.close(); + } +}); + it("retries retained browser cleanup before the one-shot CLI releases its provider", async () => { const savedExitCode = process.exitCode; const session = new RetrySession();