fix(showcase/harness): stop pooled-launcher context leaks and abort-listener leaks

A3: the d4 and e2e-parity pooled context-wrappers now delete from the abort
tracking set on normal close() (mirroring d5/d6/demos), and their test fakes'
release() is idempotent (tracks a liveContexts Set, no-ops on unknown/double
release) so a normal-close-then-abort sequence can't double-release or drive
inUse negative. Across all five pooled launchers (d4, d5, d6, e2e-parity,
e2e-readiness): capture the abort listener and detach it in the launcher-level
close() so a post-completion abort can't fire after the run returned, and in
the pre-aborted branch refuse + immediately release any context opened after
the signal already aborted so it can't leak into a torn-down run.

Author: jpr5

showcase/harness/src/probes/drivers/d4-chat-roundtrip.test.ts

@@ -64,18 +64,17 @@ function makePage(script: PageScript = {}): E2ePage {
       }
     },
     async textContent(sel) {
-      if (sel === '[data-testid="copilot-assistant-message"]:last-of-type') {
-        return script.assistantText ?? "";
-      }
+      // The driver reads the assistant message via `evaluate()` (see below),
+      // NOT `textContent(:last-of-type)`, so only the `body` read remains.
       if (sel === "body") {
         return script.bodyText ?? "";
       }
       return "";
     },
     async evaluate<R>(fn: () => R): Promise<R> {
-      // The evaluate() call in the driver reads the last assistant
-      // message's textContent via querySelectorAll. In the fake we
-      // return the same assistantText the old textContent path used.
+      // The driver's evaluate() reads the last assistant message's
+      // textContent via querySelectorAll and returns a string. The fake
+      // returns the scripted assistantText as that string.
       return (script.assistantText ?? "") as unknown as R;
     },
     async close() {
@@ -624,29 +623,58 @@ describe("createPooledE2eSmokeLauncher context checkout + abort release", () =>
     await browser.close(); // no-op
     expect(pool._releaseLog).toHaveLength(1);
   });
+
+  // A3 — a normal close() must remove the context from the abort tracking set
+  // so a SUBSEQUENT abort does not re-release it (double release). Before the
+  // fix, close() released but never `openContexts.delete(ctxHandle)`, so abort
+  // closed the already-released context a second time — driving the pool's
+  // inUse negative with a non-idempotent pool. The fix (delete-on-close + an
+  // idempotent pool) keeps the release count accurate at exactly 1 and inUse
+  // at 0.
+  it("does not double-release a normally-closed context on a later abort", async () => {
+    const pool = makeFakeContextPool(4);
+    const launcher = createPooledE2eSmokeLauncher(
+      pool as unknown as BrowserPool,
+    );
+    const ac = new AbortController();
+    const browser = await launcher(ac.signal);
+    const ctx = await browser.newContext();
+    await ctx.newPage();
+    expect(pool.stats().inUse).toBe(1);
+
+    // Normal close first — releases the context exactly once.
+    await ctx.close();
+    expect(pool.stats().inUse).toBe(0);
+    expect(pool._releaseLog).toHaveLength(1);
+
+    // Now abort. The already-closed context must NOT be released again.
+    ac.abort();
+    await new Promise((r) => setTimeout(r, 10));
+    expect(pool._releaseLog).toHaveLength(1); // still exactly one release
+    expect(pool.stats().inUse).toBe(0); // never driven negative
+  });
 });
 
 // Module-scoped fake context-pool for the createPooledE2eSmokeLauncher tests
 // above — mirrors d6-all-pills.test.ts's helper. Tracks per-CONTEXT
-// acquire/release and the contextOptions each acquire was called with. The
-// release() is idempotent on the inUse counter only in the sense that the
-// real BrowserPool.release is a no-op on a context that's already been
-// released; here each ctxHandle.close() routes to one release call, and the
-// launcher's abort path closes each open context exactly once.
+// acquire/release and the contextOptions each acquire was called with.
+// release() is IDEMPOTENT, mirroring the real BrowserPool.release: it tracks a
+// `liveContexts` Set and no-ops on an unknown / already-released context so a
+// double release can never drive the inUse counter negative and silently mask
+// a double-release bug.
 function makeFakeContextPool(maxContexts: number) {
   let nextCtxId = 0;
-  let live = 0;
+  const liveContexts = new Set<object>();
   const releaseLog: number[] = [];
   const acquireOptions: Array<
     { extraHTTPHeaders?: Record<string, string> } | undefined
   > = [];
   return {
     async acquire(options?: { extraHTTPHeaders?: Record<string, string> }) {
-      if (live >= maxContexts) throw new Error("FakePool: at cap");
+      if (liveContexts.size >= maxContexts) throw new Error("FakePool: at cap");
       const id = nextCtxId++;
-      live++;
       acquireOptions.push(options);
-      return {
+      const ctx = {
         __id: id,
         async newPage() {
           return {
@@ -661,18 +689,23 @@ function makeFakeContextPool(maxContexts: number) {
           } as unknown;
         },
         async close() {},
-      } as unknown as Browser;
+      };
+      liveContexts.add(ctx);
+      return ctx as unknown as Browser;
     },
     async release(ctx: unknown) {
-      const c = ctx as { __id: number };
-      releaseLog.push(c.__id);
-      live--;
+      // Unknown / double release — no-op, mirroring BrowserPool.release.
+      if (typeof ctx !== "object" || ctx === null || !liveContexts.has(ctx)) {
+        return;
+      }
+      liveContexts.delete(ctx);
+      releaseLog.push((ctx as { __id: number }).__id);
     },
     stats() {
       return {
         size: maxContexts,
-        available: maxContexts - live,
-        inUse: live,
+        available: maxContexts - liveContexts.size,
+        inUse: liveContexts.size,
         totalRecycles: 0,
       };
     },

showcase/harness/src/probes/drivers/d4-chat-roundtrip.ts

@@ -29,9 +29,12 @@ import type { ProbeContext, ProbeResult } from "../../types/index.js";
  *
  * In-process vs spawn: the orchestrator already runs as a long-lived Node
  * process with `playwright` installed (see Dockerfile). Launching chromium
- * directly via `playwright.chromium.launch()` gives us first-class
- * AbortSignal propagation into `page.close()` / `browser.close()` rather
- * than having to kill a child process.
+ * directly via `playwright.chromium.launch()` keeps the browser in-process
+ * rather than having to kill a child process. Note: the `defaultLauncher`
+ * does NOT wire the AbortSignal — it dedicates a chromium per call and lets
+ * the driver's teardown close it. The POOLED launcher
+ * (`createPooledE2eSmokeLauncher`) is the path that actually re-targets abort
+ * onto open contexts for prompt pool release.
  *
  * Pluggable launcher: the production default imports `playwright` lazily
  * so the driver module loads cleanly in environments without chromium
@@ -291,33 +294,40 @@ export function createPooledE2eSmokeLauncher(
     // pooled context back to the pool, freeing a context slot.
     const openContexts = new Set<{ close(): Promise<void> }>();
 
-    if (abortSignal) {
-      const onAbort = (): void => {
-        if (aborted) return;
-        aborted = true;
-        const ctxCount = openContexts.size;
-        const stats = pool.stats();
-        logger?.warn("probe.e2e-smoke.pool-abort-release", {
-          openContexts: ctxCount,
-          poolAvailable: stats.available,
-          poolInUse: stats.inUse,
-          poolSize: stats.size,
-        });
-        const contextClosePromises = Array.from(openContexts).map((ctx) =>
-          ctx.close().catch(() => {}),
-        );
-        void Promise.allSettled(contextClosePromises).then(() => {
-          logger?.warn("probe.e2e-smoke.pool-abort-released", {
-            closedContexts: ctxCount,
-            poolAvailable: pool.stats().available,
-          });
+    const onAbort = (): void => {
+      if (aborted) return;
+      aborted = true;
+      const ctxCount = openContexts.size;
+      const stats = pool.stats();
+      logger?.warn("probe.e2e-smoke.pool-abort-release", {
+        openContexts: ctxCount,
+        poolAvailable: stats.available,
+        poolInUse: stats.inUse,
+        poolSize: stats.size,
+      });
+      const contextClosePromises = Array.from(openContexts).map((ctx) =>
+        ctx.close().catch(() => {}),
+      );
+      void Promise.allSettled(contextClosePromises).then(() => {
+        logger?.warn("probe.e2e-smoke.pool-abort-released", {
+          closedContexts: ctxCount,
+          poolAvailable: pool.stats().available,
         });
-      };
+      });
+    };
+    // Capture the listener so launcher-level close() can detach it: without
+    // removeEventListener a post-completion abort would fire onAbort after the
+    // run returned (closing nothing, but leaking the listener for the signal's
+    // lifetime).
+    let detachAbort: (() => void) | undefined;
+    if (abortSignal) {
       if (abortSignal.aborted) {
         aborted = true;
         logger?.warn("probe.e2e-smoke.pool-pre-aborted-release");
       } else {
         abortSignal.addEventListener("abort", onAbort, { once: true });
+        detachAbort = () =>
+          abortSignal.removeEventListener("abort", onAbort);
       }
     }
 
@@ -328,6 +338,14 @@ export function createPooledE2eSmokeLauncher(
         const ctx = await pool.acquire({
           extraHTTPHeaders: ctxOpts?.extraHTTPHeaders,
         });
+        // If the signal was already aborted at launcher construction (the
+        // pre-aborted branch never attached the live abort listener), a context
+        // opened now would never be closed by the abort path. Release it
+        // immediately and refuse so it cannot leak into a torn-down run.
+        if (aborted) {
+          await pool.release(ctx).catch(() => {});
+          throw new Error("e2e-smoke launcher aborted");
+        }
         const ctxHandle = { close: () => pool.release(ctx) };
         openContexts.add(ctxHandle);
         return {
@@ -343,12 +361,19 @@ export function createPooledE2eSmokeLauncher(
               close: () => page.close(),
             };
           },
-          close: () => ctxHandle.close(),
+          close: async () => {
+            openContexts.delete(ctxHandle);
+            await ctxHandle.close();
+          },
         };
       },
-      // Launcher-level close is a no-op: contexts are released individually via
-      // each context-wrapper's close(). There is no Browser held to release.
-      close: async () => {},
+      // Launcher-level close releases nothing itself — contexts are released
+      // individually via each context-wrapper's close() — but it detaches the
+      // abort listener so a post-completion abort can't fire onAbort after the
+      // run returned. There is no Browser held to release.
+      close: async () => {
+        detachAbort?.();
+      },
     };
   };
 }

showcase/harness/src/probes/drivers/d5-single-pill.ts

@@ -478,33 +478,38 @@ export function createPooledE2eDeepLauncher(
     // pooled context back to the pool, freeing a context slot.
     const openContexts = new Set<{ close(): Promise<void> }>();
 
-    if (abortSignal) {
-      const onAbort = (): void => {
-        if (aborted) return;
-        aborted = true;
-        const ctxCount = openContexts.size;
-        const stats = pool.stats();
-        logger?.warn("probe.e2e-deep.pool-abort-release", {
-          openContexts: ctxCount,
-          poolAvailable: stats.available,
-          poolInUse: stats.inUse,
-          poolSize: stats.size,
-        });
-        const contextClosePromises = Array.from(openContexts).map((ctx) =>
-          ctx.close().catch(() => {}),
-        );
-        void Promise.allSettled(contextClosePromises).then(() => {
-          logger?.warn("probe.e2e-deep.pool-abort-released", {
-            closedContexts: ctxCount,
-            poolAvailable: pool.stats().available,
-          });
+    const onAbort = (): void => {
+      if (aborted) return;
+      aborted = true;
+      const ctxCount = openContexts.size;
+      const stats = pool.stats();
+      logger?.warn("probe.e2e-deep.pool-abort-release", {
+        openContexts: ctxCount,
+        poolAvailable: stats.available,
+        poolInUse: stats.inUse,
+        poolSize: stats.size,
+      });
+      const contextClosePromises = Array.from(openContexts).map((ctx) =>
+        ctx.close().catch(() => {}),
+      );
+      void Promise.allSettled(contextClosePromises).then(() => {
+        logger?.warn("probe.e2e-deep.pool-abort-released", {
+          closedContexts: ctxCount,
+          poolAvailable: pool.stats().available,
         });
-      };
+      });
+    };
+    // Capture the listener so launcher-level close() can detach it: without
+    // removeEventListener a post-completion abort would fire onAbort after the
+    // run returned, leaking the listener for the signal's lifetime.
+    let detachAbort: (() => void) | undefined;
+    if (abortSignal) {
       if (abortSignal.aborted) {
         aborted = true;
         logger?.warn("probe.e2e-deep.pool-pre-aborted-release");
       } else {
         abortSignal.addEventListener("abort", onAbort, { once: true });
+        detachAbort = () => abortSignal.removeEventListener("abort", onAbort);
       }
     }
 
@@ -515,6 +520,14 @@ export function createPooledE2eDeepLauncher(
         const ctx = await pool.acquire({
           extraHTTPHeaders: contextOpts?.extraHTTPHeaders,
         });
+        // If the signal was already aborted at launcher construction (the
+        // pre-aborted branch never attached the live abort listener), a context
+        // opened now would never be closed by the abort path. Release it
+        // immediately and refuse so it cannot leak into a torn-down run.
+        if (aborted) {
+          await pool.release(ctx).catch(() => {});
+          throw new Error("e2e-deep launcher aborted");
+        }
         const ctxHandle = { close: () => pool.release(ctx) };
         openContexts.add(ctxHandle);
         return {
@@ -573,8 +586,12 @@ export function createPooledE2eDeepLauncher(
           },
         };
       },
-      // Launcher-level close is a no-op: each context releases itself.
-      close: async () => {},
+      // Launcher-level close releases nothing itself (each context releases
+      // itself) but detaches the abort listener so a post-completion abort
+      // can't fire onAbort after the run returned.
+      close: async () => {
+        detachAbort?.();
+      },
     };
   };
 }