fix(browser): preserve admission failures and retain policy transport cleanup

docs/browser-observation.md

@@ -598,6 +598,9 @@ validation.
 ## Non-goals and threat model
 
 The browser provider deliberately does not expose generic CDP commands, `Runtime.evaluate`, WebMCP invocation, navigation, input, downloads, page closure, or browser closure. Screenshot capture is the sole pixel surface and returns an image only when requested. REA disables the domains it enabled, detaches the target session, and closes only its own WebSocket.
+Discovery and attachment errors remain primary if closing REA's socket also
+fails. The cleanup failure is reported alongside the original command failure;
+it cannot replace the operation diagnostic.
 
 This feature is not a browser sandbox or network containment mechanism. The attached page and browser continue running with their existing privileges and may make external requests independently of REA. CDP gives deep access to the selected browser profile, so use a dedicated profile, include only origins you intend to inspect, and treat other same-user processes as outside this boundary.
 

docs/browser-scenario-contract.md

@@ -15,6 +15,10 @@ there is no separate origin allowlist or request-interception layer. Environment
 settings (including service workers blocked by default), empty storage, and a
 final sanitized URL capture are supplied by default. Set
 `environment.service_workers` to `allow` when the application requires them.
+The connected service-worker policy owner retains its exact CDP transport
+through discovery, attachment and restoration. Failed admission preserves its
+primary diagnostic when socket cleanup also fails; later policy cleanup retries
+that retained transport and clears cleanup uncertainty only after close succeeds.
 For attached browsers, `block` rejects captures when the selected page or its
 frames are controlled by a service worker or already have a registration,
 including after navigation. Policy checks bracket each artifact capture, including

src/browser/CdpTargetSession.ts

@@ -97,16 +97,12 @@ export const openCdpEndpoint = async (
           connection,
         };
       } catch (cause: unknown) {
-        if (owner === undefined) {
-          await connection.close();
-          throw cause;
-        }
-        try {
-          await owner.finish(connection);
-        } catch (cleanupCause: unknown) {
-          throw combineOpenFailure(cause, cleanupCause, operation);
-        }
-        throw cause;
+        return await failCdpOpenAfterCleanup(
+          connection,
+          cause,
+          operation,
+          owner,
+        );
       }
     },
     discoveryTimeoutMs,
@@ -147,12 +143,7 @@ export const openCdpTargetSession = async (
     );
   } catch (cause: unknown) {
     if (owner === undefined) throw cause;
-    try {
-      await owner.finish(connection);
-    } catch (cleanupCause: unknown) {
-      throw combineOpenFailure(cause, cleanupCause, operation);
-    }
-    throw cause;
+    return await failCdpOpenAfterCleanup(connection, cause, operation, owner);
   }
 };
 
@@ -173,14 +164,28 @@ export const attachCdpTargetSession = async (
     );
     return { connection, sessionId: attachedSessionId(attached, operation) };
   } catch (cause: unknown) {
-    if (owner === undefined) {
-      await connection.close();
-      throw cause;
-    }
+    if (owner === undefined)
+      return await failCdpOpenAfterCleanup(connection, cause, operation);
     throw cause;
   }
 };
 
+/** Finish failed admission without allowing cleanup to replace its primary. */
+export const failCdpOpenAfterCleanup = async (
+  connection: CdpConnection,
+  primary: unknown,
+  operation: BrowserObservationOperation,
+  owner?: CdpTargetSessionOwner,
+): Promise<never> => {
+  try {
+    if (owner === undefined) await connection.close();
+    else await owner.finish(connection);
+  } catch (cleanup: unknown) {
+    throw combineOpenFailure(primary, cleanup, operation);
+  }
+  throw primary;
+};
+
 const combineOpenFailure = (
   primary: unknown,
   cleanup: unknown,

src/browser/PlaywrightScenarioServiceWorkers.ts

@@ -11,10 +11,11 @@ import {
 import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js";
 import { browserObservationEndpointSchema } from "../domain/browserObservation.js";
 import { BrowserObservationError } from "../domain/browserObservationError.js";
-import { CdpConnection } from "./CdpConnection.js";
+import { CdpTargetSessionOwner } from "./CdpTargetSessionOwner.js";
 import { withCdpDiscoveryDeadline } from "./CdpEndpoint.js";
 import {
   attachCdpTargetSession,
+  failCdpOpenAfterCleanup,
   openCdpEndpoint,
   type CdpTargetSession,
 } from "./CdpTargetSession.js";
@@ -60,6 +61,7 @@ type PolicyTarget = {
 
 /** Owns only its frame scripts and registration wrappers, never caller-owned workers. */
 export class PlaywrightScenarioServiceWorkers {
+  readonly #transportOwner = new CdpTargetSessionOwner();
   private readonly marker = JSON.stringify(
     `rea-service-workers-${randomUUID()}`,
   );
@@ -109,7 +111,7 @@ export class PlaywrightScenarioServiceWorkers {
           OPERATION,
           signal,
           undefined,
-          undefined,
+          this.#transportOwner,
           this.commandTimeoutMs,
         ),
     );
@@ -117,25 +119,46 @@ export class PlaywrightScenarioServiceWorkers {
       ({ id }) => id === this.targetId,
     );
     if (target === undefined) {
-      await opened.connection?.close();
-      throw new BrowserObservationError(OPERATION, "target_not_found");
+      const failure = new BrowserObservationError(
+        OPERATION,
+        "target_not_found",
+      );
+      if (opened.connection !== undefined)
+        return await failCdpOpenAfterCleanup(
+          opened.connection,
+          failure,
+          OPERATION,
+          this.#transportOwner,
+        );
+      throw failure;
     }
     const transport =
       opened.connection ??
-      (await CdpConnection.connect(
+      (await this.#transportOwner.connect(
         opened.discovery.webSocket.url,
         OPERATION,
-        this.#setup.signal,
+        { signal: this.#setup.signal },
+        target.id,
         { commandTimeoutMs: this.commandTimeoutMs },
       ));
     if (this.#closed)
       transport.beginCleanupGrace(POLICY_CLEANUP_CONFIRMATION_TIMEOUT_MS);
-    this.#session = await attachCdpTargetSession(
-      transport,
-      target,
-      OPERATION,
-      this.#setup.signal,
-    );
+    try {
+      this.#session = await attachCdpTargetSession(
+        transport,
+        target,
+        OPERATION,
+        this.#setup.signal,
+        this.#transportOwner,
+      );
+    } catch (cause: unknown) {
+      return await failCdpOpenAfterCleanup(
+        transport,
+        cause,
+        OPERATION,
+        this.#transportOwner,
+      );
+    }
     const connection = this.#session.connection;
     this.#removeListener = connection.onEvent((event) => {
       if (
@@ -409,12 +432,17 @@ export class PlaywrightScenarioServiceWorkers {
 
   private async restore(): Promise<void> {
     await this.#initialization?.catch(() => undefined);
+    await this.#transportOwner.retryPending();
     this.#session?.connection.beginCleanupGrace(
       POLICY_CLEANUP_CONFIRMATION_TIMEOUT_MS,
     );
     if (this.#released) return;
     const session = this.#session;
-    if (session === undefined) return;
+    if (session === undefined) {
+      await this.#transportOwner.close();
+      this.#released = true;
+      return;
+    }
     if (!this.page.isClosed()) {
       const mainFrame = this.page.mainFrame();
       // This owner exists even with empty storage seeds. Stopped navigation is
@@ -469,7 +497,8 @@ export class PlaywrightScenarioServiceWorkers {
       this.#detached = true;
     }
     this.#removeListener?.();
-    await session.connection.close();
+    await this.#transportOwner.finish(session.connection);
+    await this.#transportOwner.close();
     this.#released = true;
   }
 }

tests/boundary/browser/cdpFailedAdmission.test.ts

@@ -0,0 +1,136 @@
+import { expect, it, vi } from "vitest";
+import type { Page } from "playwright-core";
+import { CdpConnection } from "../../../src/browser/CdpConnection.js";
+import {
+  attachCdpTargetSession,
+  openCdpEndpoint,
+} from "../../../src/browser/CdpTargetSession.js";
+import { PlaywrightScenarioServiceWorkers } from "../../../src/browser/PlaywrightScenarioServiceWorkers.js";
+import { AnalysisError } from "../../../src/domain/analysisErrorBase.js";
+import { BrowserObservationError } from "../../../src/domain/browserObservationError.js";
+import { browserObservationEndpointSchema } from "../../../src/domain/browserObservation.js";
+import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
+import { startFakeCdpBrowser } from "../../fixtures/fakeCdpBrowser.js";
+
+it.each(["Target.getTargets", "Target.attachToTarget"])(
+  "retains %s setup failure and retries the exact policy transport after rejected close",
+  async (method) => {
+    for (const failClose of [false, true]) {
+      const browser = await startFakeCdpBrowser({
+        commandError: (command) =>
+          command.method === method
+            ? { code: -32000, message: "controlled primary failure" }
+            : undefined,
+      });
+      let reject = failClose;
+      const original = CdpConnection.prototype.close;
+      const close = vi
+        .spyOn(CdpConnection.prototype, "close")
+        .mockImplementation(async function (this: CdpConnection) {
+          await original.call(this);
+          if (reject) {
+            reject = false;
+            throw new BrowserObservationError(
+              "capture_browser_scenario",
+              "cleanup_failed",
+              { detail: "controlled close failure" },
+            );
+          }
+        });
+      const page = {
+        frames: () => [{ evaluate: async () => null, isDetached: () => false }],
+        isClosed: () => true,
+      } as unknown as Page;
+      const policy = new PlaywrightScenarioServiceWorkers(
+        page,
+        browser.browserWebSocketUrl,
+        "allowed-page",
+      );
+      try {
+        const failure: unknown = await policy
+          .initialize()
+          .catch((cause: unknown) => cause);
+        expect(failure).toBeInstanceOf(AnalysisError);
+        if (!(failure instanceof AnalysisError))
+          throw new Error("Expected typed setup failure");
+        expect(projectAnalysisError(failure).message).toContain(
+          `${method} failed`,
+        );
+        expect(projectAnalysisError(failure).message).toContain(
+          "controlled primary failure",
+        );
+        expect(failure.cleanupIncomplete).toBe(failClose);
+        if (failClose)
+          expect(failure.cleanupPrimary).toMatchObject({
+            reason: "protocol_error",
+          });
+        expect(close).toHaveBeenCalledTimes(1);
+        await policy.close();
+        expect(close).toHaveBeenCalledTimes(failClose ? 2 : 1);
+        expect(failure.cleanupIncomplete).toBe(false);
+      } finally {
+        await policy.close();
+        close.mockRestore();
+        await browser.close();
+      }
+    }
+  },
+);
+
+it.each(["Target.getTargets", "Target.attachToTarget"])(
+  "also composes rejected close with %s failure for an ownerless caller",
+  async (method) => {
+    const browser = await startFakeCdpBrowser({
+      commandError: (command) =>
+        command.method === method
+          ? { code: -32000, message: "ownerless primary failure" }
+          : undefined,
+    });
+    const original = CdpConnection.prototype.close;
+    const close = vi
+      .spyOn(CdpConnection.prototype, "close")
+      .mockImplementation(async function (this: CdpConnection) {
+        await original.call(this);
+        throw new BrowserObservationError(
+          "capture_browser_scenario",
+          "cleanup_failed",
+          { detail: "ownerless close failure" },
+        );
+      });
+    try {
+      const admission = async () => {
+        const opened = await openCdpEndpoint(
+          browserObservationEndpointSchema.parse(browser.browserWebSocketUrl),
+          "capture_browser_scenario",
+        );
+        const target = opened.discovery.targets.find(
+          ({ id }) => id === "allowed-page",
+        );
+        if (opened.connection === undefined || target === undefined)
+          throw new Error("Missing fixture target");
+        return await attachCdpTargetSession(
+          opened.connection,
+          target,
+          "capture_browser_scenario",
+        );
+      };
+      const failure: unknown = await admission().catch(
+        (cause: unknown) => cause,
+      );
+      expect(failure).toBeInstanceOf(AnalysisError);
+      if (!(failure instanceof AnalysisError))
+        throw new Error("Expected typed admission failure");
+      expect(failure.cleanupPrimary).toMatchObject({
+        reason: "protocol_error",
+      });
+      expect(projectAnalysisError(failure).message).toContain(
+        "ownerless primary failure",
+      );
+      expect(failure.cleanupIncomplete).toBe(true);
+      expect(close).toHaveBeenCalledOnce();
+    } finally {
+      close.mockRestore();
+      await browser.close();
+    }
+  },
+);