From fa6790a34fdd087f78fd6cc0d581730da97e667b Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Sat, 2 May 2026 13:00:40 -0700
Subject: [PATCH 01/10] fix(screenshot): handle 0x0 viewport on hidden targets
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Page.captureScreenshot can stall for ~2 minutes when the connected
target reports a 0x0 layout viewport — observed when the MCP attaches
to the Perplexity sidecar panel (visibilityState='hidden') in some
Comet window states (cold launch, no real browsing tabs in front).
Without captureBeyondViewport, captureScreenshot captures the layout
viewport and waits for compositor frames that never arrive.

When Page.getLayoutMetrics reports a zero-size viewport, supply an
explicit clip + captureBeyondViewport=true so the renderer produces
a frame immediately. No-op on real foreground tabs (where the layout
viewport is non-zero, so the clip path doesn't activate).

Closes #2.
---
 src/cdp-client.ts | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 10eb74e..98f835f 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -1112,7 +1112,39 @@ export class CometCDPClient {
    */
   async screenshot(format: "png" | "jpeg" = "png"): Promise<ScreenshotResult> {
     this.ensureConnected();
-    return this.client!.Page.captureScreenshot({ format }) as Promise<ScreenshotResult>;
+
+    try { await this.client!.Page.bringToFront(); } catch { /* not all targets support it */ }
+
+    // Hidden targets (e.g. the Perplexity sidecar panel) can report a 0x0
+    // layout viewport in some Comet window states (cold launch, no real
+    // browsing tabs in front). When the viewport is 0x0,
+    // Page.captureScreenshot waits for compositor frames that never arrive
+    // and stalls for ~2 minutes. Detect via Page.getLayoutMetrics() and
+    // supply an explicit clip + captureBeyondViewport so the renderer
+    // produces a frame immediately.
+    let clip: { x: number; y: number; width: number; height: number; scale: number } | undefined;
+    try {
+      const metrics = await this.client!.Page.getLayoutMetrics();
+      const v = metrics.cssLayoutViewport ?? metrics.layoutViewport;
+      if (!v?.clientWidth || !v?.clientHeight) {
+        clip = { x: 0, y: 0, width: 1280, height: 800, scale: 1 };
+      }
+    } catch {
+      clip = { x: 0, y: 0, width: 1280, height: 800, scale: 1 };
+    }
+
+    const result = await this.client!.Page.captureScreenshot({
+      format,
+      ...(clip ? { captureBeyondViewport: true, clip } : {}),
+    }) as ScreenshotResult;
+
+    if (!result?.data) {
+      throw new Error(
+        "Screenshot returned empty data. Ensure you're connected to a visible tab with content.",
+      );
+    }
+
+    return result;
   }
 
   /**

From e3d9ae1766a99e0747a0cbb4ffb1179bcf83f249 Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 14:57:48 -0700
Subject: [PATCH 02/10] refactor(cdp-client): extract
 captureScreenshotWithFallback for testability
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Pulls the screenshot-with-fallback logic out of the CometCDPClient
method body and into an exported free function backed by a minimal
ScreenshotPageAPI interface — same seam pattern as CometAIClient,
so unit tests can substitute a hand-written fake page without a live
CDP connection.

Also promotes the inline 1280x800 fallback clip to a named
SCREENSHOT_FALLBACK_CLIP constant, with the explanatory comment
attached to the constant rather than buried mid-method.

No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/cdp-client.ts | 103 +++++++++++++++++++++++++++++++---------------
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 98f835f..7c1ac0c 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -15,6 +15,75 @@ import type {
   TabContext,
 } from "./types.js";
 
+// Hidden targets (e.g. the Perplexity sidecar panel) can report a 0x0 layout
+// viewport in some Comet window states (cold launch, no real browsing tabs in
+// front). When that happens, Page.captureScreenshot waits for compositor
+// frames that never arrive and stalls for ~2 minutes. Detecting any
+// degenerate dimension via Page.getLayoutMetrics() and supplying an explicit
+// clip + captureBeyondViewport=true makes the renderer produce a frame
+// immediately.
+const SCREENSHOT_FALLBACK_CLIP = {
+  x: 0,
+  y: 0,
+  width: 1280,
+  height: 800,
+  scale: 1,
+} as const;
+
+/**
+ * The slice of the CDP Page domain that `captureScreenshotWithFallback` uses.
+ * Lets unit tests substitute a hand-written fake without dragging in the full
+ * `chrome-remote-interface` Page surface.
+ */
+export interface ScreenshotPageAPI {
+  bringToFront(): Promise<unknown>;
+  getLayoutMetrics(): Promise<{
+    cssLayoutViewport?: { clientWidth: number; clientHeight: number };
+    layoutViewport?: { clientWidth: number; clientHeight: number };
+  }>;
+  captureScreenshot(opts: {
+    format: "png" | "jpeg";
+    captureBeyondViewport?: boolean;
+    clip?: typeof SCREENSHOT_FALLBACK_CLIP;
+  }): Promise<ScreenshotResult>;
+}
+
+/**
+ * Capture a screenshot, falling back to an explicit clip when the layout
+ * viewport is degenerate. Extracted as a free function so it can be unit
+ * tested against a fake Page without a live CDP connection.
+ */
+export async function captureScreenshotWithFallback(
+  page: ScreenshotPageAPI,
+  format: "png" | "jpeg" = "png",
+): Promise<ScreenshotResult> {
+  try { await page.bringToFront(); } catch { /* not all targets support it */ }
+
+  let clip: typeof SCREENSHOT_FALLBACK_CLIP | undefined;
+  try {
+    const metrics = await page.getLayoutMetrics();
+    const v = metrics.cssLayoutViewport ?? metrics.layoutViewport;
+    if (!v?.clientWidth || !v?.clientHeight) {
+      clip = SCREENSHOT_FALLBACK_CLIP;
+    }
+  } catch {
+    clip = SCREENSHOT_FALLBACK_CLIP;
+  }
+
+  const result = await page.captureScreenshot({
+    format,
+    ...(clip ? { captureBeyondViewport: true, clip } : {}),
+  });
+
+  if (!result?.data) {
+    throw new Error(
+      "Screenshot returned empty data. Ensure you're connected to a visible tab with content.",
+    );
+  }
+
+  return result;
+}
+
 // Detect if running in WSL (must be before windowsFetch)
 function isWSL(): boolean {
   if (platform() !== 'linux') return false;
@@ -1112,39 +1181,7 @@ export class CometCDPClient {
    */
   async screenshot(format: "png" | "jpeg" = "png"): Promise<ScreenshotResult> {
     this.ensureConnected();
-
-    try { await this.client!.Page.bringToFront(); } catch { /* not all targets support it */ }
-
-    // Hidden targets (e.g. the Perplexity sidecar panel) can report a 0x0
-    // layout viewport in some Comet window states (cold launch, no real
-    // browsing tabs in front). When the viewport is 0x0,
-    // Page.captureScreenshot waits for compositor frames that never arrive
-    // and stalls for ~2 minutes. Detect via Page.getLayoutMetrics() and
-    // supply an explicit clip + captureBeyondViewport so the renderer
-    // produces a frame immediately.
-    let clip: { x: number; y: number; width: number; height: number; scale: number } | undefined;
-    try {
-      const metrics = await this.client!.Page.getLayoutMetrics();
-      const v = metrics.cssLayoutViewport ?? metrics.layoutViewport;
-      if (!v?.clientWidth || !v?.clientHeight) {
-        clip = { x: 0, y: 0, width: 1280, height: 800, scale: 1 };
-      }
-    } catch {
-      clip = { x: 0, y: 0, width: 1280, height: 800, scale: 1 };
-    }
-
-    const result = await this.client!.Page.captureScreenshot({
-      format,
-      ...(clip ? { captureBeyondViewport: true, clip } : {}),
-    }) as ScreenshotResult;
-
-    if (!result?.data) {
-      throw new Error(
-        "Screenshot returned empty data. Ensure you're connected to a visible tab with content.",
-      );
-    }
-
-    return result;
+    return captureScreenshotWithFallback(this.client!.Page, format);
   }
 
   /**

From 5cae7701d597e6ded2212450257f2406e8e6268f Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 14:57:59 -0700
Subject: [PATCH 03/10] test(cdp-client): unit tests for screenshot fallback

Covers:
- normal viewport (1024x768) -> no clip; format forwarded as-is
- cssLayoutViewport missing -> falls back to layoutViewport
- jpeg format forwarded
- bringToFront is invoked before capture (best-effort)
- 0x0, 0xN, Nx0, both-fields-absent viewports -> fallback clip +
  captureBeyondViewport=true
- getLayoutMetrics throws -> fallback clip
- bringToFront throws -> swallowed; capture still runs
- captureScreenshot returns empty data -> throws descriptive error

The 0xN / Nx0 cases document the wrapper's defensive || semantics:
any zero dimension triggers the fallback, on the basis that a
0-pixel screenshot is useless regardless of whether the same
compositor stall reproduces. The hang itself is only confirmed for
0x0 in live testing; the wrapper-logic tests here don't claim
otherwise.

Adds tests/unit/fakes/fake-screenshot-page.ts following the existing
FakeCdpClient style (records calls, programmable responses).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 tests/unit/cdp-client.screenshot.test.ts | 139 +++++++++++++++++++++++
 tests/unit/fakes/fake-screenshot-page.ts |  58 ++++++++++
 2 files changed, 197 insertions(+)
 create mode 100644 tests/unit/cdp-client.screenshot.test.ts
 create mode 100644 tests/unit/fakes/fake-screenshot-page.ts

diff --git a/tests/unit/cdp-client.screenshot.test.ts b/tests/unit/cdp-client.screenshot.test.ts
new file mode 100644
index 0000000..63760d8
--- /dev/null
+++ b/tests/unit/cdp-client.screenshot.test.ts
@@ -0,0 +1,139 @@
+import { describe, it, expect } from "vitest";
+import { captureScreenshotWithFallback } from "../../src/cdp-client.js";
+import { FakeScreenshotPage } from "./fakes/fake-screenshot-page.js";
+
+const FALLBACK_CLIP = {
+  x: 0,
+  y: 0,
+  width: 1280,
+  height: 800,
+  scale: 1,
+};
+
+describe("captureScreenshotWithFallback — normal viewport", () => {
+  it("forwards format and supplies no clip when the cssLayoutViewport is non-zero", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls).toHaveLength(1);
+    expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
+  });
+
+  it("falls back to layoutViewport when cssLayoutViewport is missing", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ layoutViewport: { clientWidth: 800, clientHeight: 600 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
+  });
+
+  it("forwards jpeg format unchanged", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 } });
+
+    await captureScreenshotWithFallback(page, "jpeg");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({ format: "jpeg" });
+  });
+
+  it("calls bringToFront before capture (best-effort)", async () => {
+    const page = new FakeScreenshotPage();
+    await captureScreenshotWithFallback(page, "png");
+    expect(page.bringToFrontCalls).toBe(1);
+  });
+});
+
+describe("captureScreenshotWithFallback — degenerate viewport", () => {
+  it("supplies the fallback clip + captureBeyondViewport when viewport is 0×0", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 0, clientHeight: 0 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({
+      format: "png",
+      captureBeyondViewport: true,
+      clip: FALLBACK_CLIP,
+    });
+  });
+
+  // The wrapper treats any zero dimension as degenerate (`||` semantics).
+  // This is defensive: a 0×N or N×0 viewport can't produce a useful
+  // screenshot anyway, so falling back to a real frame is preferable to
+  // returning a zero-pixel image.
+  it("supplies the fallback clip when width is 0 but height is non-zero", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 0, clientHeight: 800 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({
+      format: "png",
+      captureBeyondViewport: true,
+      clip: FALLBACK_CLIP,
+    });
+  });
+
+  it("supplies the fallback clip when height is 0 but width is non-zero", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 0 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({
+      format: "png",
+      captureBeyondViewport: true,
+      clip: FALLBACK_CLIP,
+    });
+  });
+
+  it("supplies the fallback clip when getLayoutMetrics throws", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetricsToThrow();
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({
+      format: "png",
+      captureBeyondViewport: true,
+      clip: FALLBACK_CLIP,
+    });
+  });
+
+  it("supplies the fallback clip when both viewport fields are absent", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({});
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({
+      format: "png",
+      captureBeyondViewport: true,
+      clip: FALLBACK_CLIP,
+    });
+  });
+});
+
+describe("captureScreenshotWithFallback — error handling", () => {
+  it("swallows bringToFront errors and continues to capture", async () => {
+    const page = new FakeScreenshotPage();
+    page.setBringToFrontToThrow();
+
+    const result = await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls).toHaveLength(1);
+    expect(result.data).toBeTruthy();
+  });
+
+  it("throws a descriptive error when captureScreenshot returns empty data", async () => {
+    const page = new FakeScreenshotPage();
+    page.setScreenshotData(undefined);
+
+    await expect(captureScreenshotWithFallback(page, "png")).rejects.toThrow(
+      /empty data/i,
+    );
+  });
+});
diff --git a/tests/unit/fakes/fake-screenshot-page.ts b/tests/unit/fakes/fake-screenshot-page.ts
new file mode 100644
index 0000000..35e2a71
--- /dev/null
+++ b/tests/unit/fakes/fake-screenshot-page.ts
@@ -0,0 +1,58 @@
+// Minimal fake of the CDP `Page` slice that `captureScreenshotWithFallback`
+// uses. Implements just `bringToFront`, `getLayoutMetrics`, and
+// `captureScreenshot` — enough to satisfy the `ScreenshotPageAPI` type — and
+// records every call so tests can assert on the args.
+
+import type { ScreenshotPageAPI } from "../../../src/cdp-client.js";
+import type { ScreenshotResult } from "../../../src/types.js";
+
+type LayoutMetrics = Awaited<ReturnType<ScreenshotPageAPI["getLayoutMetrics"]>>;
+type CaptureScreenshotArgs = Parameters<ScreenshotPageAPI["captureScreenshot"]>[0];
+
+export class FakeScreenshotPage implements ScreenshotPageAPI {
+  public bringToFrontCalls = 0;
+  public readonly captureScreenshotCalls: CaptureScreenshotArgs[] = [];
+
+  private bringToFrontShouldThrow = false;
+  private metricsResponse: LayoutMetrics = {
+    cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 },
+  };
+  private metricsShouldThrow = false;
+  private screenshotData: string | undefined = "ZmFrZS1zY3JlZW5zaG90LWRhdGE="; // base64 "fake-screenshot-data"
+
+  setMetrics(metrics: LayoutMetrics): void {
+    this.metricsResponse = metrics;
+    this.metricsShouldThrow = false;
+  }
+
+  setMetricsToThrow(): void {
+    this.metricsShouldThrow = true;
+  }
+
+  setBringToFrontToThrow(): void {
+    this.bringToFrontShouldThrow = true;
+  }
+
+  setScreenshotData(data: string | undefined): void {
+    this.screenshotData = data;
+  }
+
+  async bringToFront(): Promise<void> {
+    this.bringToFrontCalls++;
+    if (this.bringToFrontShouldThrow) {
+      throw new Error("bringToFront not supported on this target");
+    }
+  }
+
+  async getLayoutMetrics(): Promise<LayoutMetrics> {
+    if (this.metricsShouldThrow) {
+      throw new Error("getLayoutMetrics unsupported");
+    }
+    return this.metricsResponse;
+  }
+
+  async captureScreenshot(opts: CaptureScreenshotArgs): Promise<ScreenshotResult> {
+    this.captureScreenshotCalls.push(opts);
+    return { data: this.screenshotData } as ScreenshotResult;
+  }
+}

From 4b0c8553e52cc14ea6b68ae0caf6b937baa2494e Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 15:06:37 -0700
Subject: [PATCH 04/10] fix(cdp-client): tighten degenerate-viewport predicate
 to require both dims zero
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Switches the screenshot-fallback condition from `||` to `&&` so the
fallback clip only activates on a strict 0x0 layout viewport — the only
viewport state we have evidence for actually stalling
Page.captureScreenshot.

Live CDP testing against a running Comet instance:

- Sidecar (visibilityState=hidden, cold launch): natural 0x0
  viewport; naked Page.captureScreenshot timed out at 15s; the
  wrapper completed in ~1s with valid PNG data.
- Forced 0x0 via Emulation.setDeviceMetricsOverride on a normal
  page: Chromium silently rejected the zero values and reported the
  natural viewport (1024x768) instead of 0x0.
- Forced 0xN and Nx0 via Emulation.setDeviceMetricsOverride:
  Chromium rejected the single-zero dimension the same way,
  substituting the natural value for the missing axis.

So a 0xN or Nx0 layout viewport is not a state we can observe in
practice. The `||` form was defensive against a hypothetical that
doesn't appear to be reachable; `&&` matches the predicate to the
evidence and avoids guessing on inputs we can't generate.

Unit-test expectations for the 0xN / Nx0 boundary cases move from
"fallback clip is applied" to "fallback clip is not applied",
documented inline. The 0x0, missing-viewport-fields, and
getLayoutMetrics-throws cases still trigger the fallback.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/cdp-client.ts                        | 16 +++++--
 tests/unit/cdp-client.screenshot.test.ts | 59 +++++++++++-------------
 2 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 7c1ac0c..684b504 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -18,10 +18,16 @@ import type {
 // Hidden targets (e.g. the Perplexity sidecar panel) can report a 0x0 layout
 // viewport in some Comet window states (cold launch, no real browsing tabs in
 // front). When that happens, Page.captureScreenshot waits for compositor
-// frames that never arrive and stalls for ~2 minutes. Detecting any
-// degenerate dimension via Page.getLayoutMetrics() and supplying an explicit
-// clip + captureBeyondViewport=true makes the renderer produce a frame
-// immediately.
+// frames that never arrive and stalls for ~2 minutes. Detecting that state
+// via Page.getLayoutMetrics() and supplying an explicit clip +
+// captureBeyondViewport=true makes the renderer produce a frame immediately.
+//
+// The predicate is intentionally tight: both dimensions must be zero. Live
+// CDP testing confirmed that 0x0 is the only reproducible viewport state
+// that triggers the stall — Chromium's Emulation.setDeviceMetricsOverride
+// rejects single-zero dimensions and falls back to the natural viewport,
+// and the natural 0x0 case is "no layout computed yet" (an all-or-nothing
+// state). A 0xN or Nx0 viewport is not a state we can observe in practice.
 const SCREENSHOT_FALLBACK_CLIP = {
   x: 0,
   y: 0,
@@ -63,7 +69,7 @@ export async function captureScreenshotWithFallback(
   try {
     const metrics = await page.getLayoutMetrics();
     const v = metrics.cssLayoutViewport ?? metrics.layoutViewport;
-    if (!v?.clientWidth || !v?.clientHeight) {
+    if (!v?.clientWidth && !v?.clientHeight) {
       clip = SCREENSHOT_FALLBACK_CLIP;
     }
   } catch {
diff --git a/tests/unit/cdp-client.screenshot.test.ts b/tests/unit/cdp-client.screenshot.test.ts
index 63760d8..8b2e143 100644
--- a/tests/unit/cdp-client.screenshot.test.ts
+++ b/tests/unit/cdp-client.screenshot.test.ts
@@ -10,7 +10,16 @@ const FALLBACK_CLIP = {
   scale: 1,
 };
 
-describe("captureScreenshotWithFallback — normal viewport", () => {
+// The predicate that triggers the fallback is `!width && !height` — both
+// dimensions must be zero. Live CDP testing showed 0×0 is the only
+// reproducible state that hangs Page.captureScreenshot: Chromium's
+// Emulation.setDeviceMetricsOverride rejects single-zero dimensions, and
+// the natural 0×0 case (visibilityState='hidden' with no layout computed)
+// is all-or-nothing. So 0×N and N×0 are exercised here for boundary
+// coverage but treated as non-degenerate; if the wrapper ever observed
+// one in the wild, we'd trust the reported dimensions rather than fall
+// back.
+describe("captureScreenshotWithFallback — non-degenerate viewport", () => {
   it("forwards format and supplies no clip when the cssLayoutViewport is non-zero", async () => {
     const page = new FakeScreenshotPage();
     page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 } });
@@ -30,6 +39,24 @@ describe("captureScreenshotWithFallback — normal viewport", () => {
     expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
   });
 
+  it("does NOT supply a fallback clip when only width is 0", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 0, clientHeight: 800 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
+  });
+
+  it("does NOT supply a fallback clip when only height is 0", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 0 } });
+
+    await captureScreenshotWithFallback(page, "png");
+
+    expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
+  });
+
   it("forwards jpeg format unchanged", async () => {
     const page = new FakeScreenshotPage();
     page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 } });
@@ -60,36 +87,6 @@ describe("captureScreenshotWithFallback — degenerate viewport", () => {
     });
   });
 
-  // The wrapper treats any zero dimension as degenerate (`||` semantics).
-  // This is defensive: a 0×N or N×0 viewport can't produce a useful
-  // screenshot anyway, so falling back to a real frame is preferable to
-  // returning a zero-pixel image.
-  it("supplies the fallback clip when width is 0 but height is non-zero", async () => {
-    const page = new FakeScreenshotPage();
-    page.setMetrics({ cssLayoutViewport: { clientWidth: 0, clientHeight: 800 } });
-
-    await captureScreenshotWithFallback(page, "png");
-
-    expect(page.captureScreenshotCalls[0]).toEqual({
-      format: "png",
-      captureBeyondViewport: true,
-      clip: FALLBACK_CLIP,
-    });
-  });
-
-  it("supplies the fallback clip when height is 0 but width is non-zero", async () => {
-    const page = new FakeScreenshotPage();
-    page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 0 } });
-
-    await captureScreenshotWithFallback(page, "png");
-
-    expect(page.captureScreenshotCalls[0]).toEqual({
-      format: "png",
-      captureBeyondViewport: true,
-      clip: FALLBACK_CLIP,
-    });
-  });
-
   it("supplies the fallback clip when getLayoutMetrics throws", async () => {
     const page = new FakeScreenshotPage();
     page.setMetricsToThrow();

From f5b5043f21a0f5d9d8881cc5d708bc3d735ae733 Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 15:09:17 -0700
Subject: [PATCH 05/10] test(cdp-client): model fail-loudly path for
 single-zero-dim boundary tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The 0×N and N×0 boundary tests previously asserted only the predicate
outcome ("no fallback clip applied") while letting the fake return a
canned valid-PNG byte string — which doesn't match what a real
encoder would do. PNG IHDR and JPEG SOF both require both dimensions
to be >= 1, so a zero-dim viewport can't produce a valid image
regardless of which encoder runs.

Update those two tests to:
  1. Configure the fake's captureScreenshot to return empty data
     (modeling the real encoder's behavior on a zero-dim viewport).
  2. Assert that the wrapper surfaces the failure via the existing
     empty-data guard rather than masking it with a synthetic 1280×800
     capture.

The call shape assertion (`{ format: "png" }`, i.e. no clip /
captureBeyondViewport) is preserved so the predicate decision is still
covered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 tests/unit/cdp-client.screenshot.test.ts | 27 +++++++++++++++---------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/unit/cdp-client.screenshot.test.ts b/tests/unit/cdp-client.screenshot.test.ts
index 8b2e143..53655bd 100644
--- a/tests/unit/cdp-client.screenshot.test.ts
+++ b/tests/unit/cdp-client.screenshot.test.ts
@@ -15,10 +15,11 @@ const FALLBACK_CLIP = {
 // reproducible state that hangs Page.captureScreenshot: Chromium's
 // Emulation.setDeviceMetricsOverride rejects single-zero dimensions, and
 // the natural 0×0 case (visibilityState='hidden' with no layout computed)
-// is all-or-nothing. So 0×N and N×0 are exercised here for boundary
-// coverage but treated as non-degenerate; if the wrapper ever observed
-// one in the wild, we'd trust the reported dimensions rather than fall
-// back.
+// is all-or-nothing. PNG/JPEG headers also require both dimensions to be
+// >= 1, so a zero-dim image isn't a valid output format. The 0×N and N×0
+// boundary tests below therefore expect the wrapper to NOT apply the
+// fallback (no synthetic 1280×800 masking the anomaly) and to surface
+// the inevitable encoder failure via the empty-data guard.
 describe("captureScreenshotWithFallback — non-degenerate viewport", () => {
   it("forwards format and supplies no clip when the cssLayoutViewport is non-zero", async () => {
     const page = new FakeScreenshotPage();
@@ -39,21 +40,27 @@ describe("captureScreenshotWithFallback — non-degenerate viewport", () => {
     expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
   });
 
-  it("does NOT supply a fallback clip when only width is 0", async () => {
+  it("passes through (no fallback) and fails loudly when only width is 0", async () => {
     const page = new FakeScreenshotPage();
     page.setMetrics({ cssLayoutViewport: { clientWidth: 0, clientHeight: 800 } });
+    // Model real-browser behavior: no encoder can produce output for a
+    // zero-dim image, so the captureScreenshot call returns empty data.
+    page.setScreenshotData(undefined);
 
-    await captureScreenshotWithFallback(page, "png");
-
+    await expect(captureScreenshotWithFallback(page, "png")).rejects.toThrow(
+      /empty data/i,
+    );
     expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
   });
 
-  it("does NOT supply a fallback clip when only height is 0", async () => {
+  it("passes through (no fallback) and fails loudly when only height is 0", async () => {
     const page = new FakeScreenshotPage();
     page.setMetrics({ cssLayoutViewport: { clientWidth: 1024, clientHeight: 0 } });
+    page.setScreenshotData(undefined);
 
-    await captureScreenshotWithFallback(page, "png");
-
+    await expect(captureScreenshotWithFallback(page, "png")).rejects.toThrow(
+      /empty data/i,
+    );
     expect(page.captureScreenshotCalls[0]).toEqual({ format: "png" });
   });
 

From 4245e5b7e5615e6e8e5673fec37cc3b9e604ff48 Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 16:02:09 -0700
Subject: [PATCH 06/10] fix(cdp-client): tighten getLayoutMetrics catch to
 CDP.ProtocolError
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous catch block applied the fallback clip for any throw from
Page.getLayoutMetrics, which masked transport-level failures (websocket
dropped, target detached) behind a synthetic 1280x800 capture. Tighten
the discriminator to only recover from Chrome-side rejections, and
propagate everything else so the caller sees the real failure.

Mechanics:

- Bump chrome-remote-interface from ^0.33.2 to ^0.34.0 to pick up the
  newly-exposed ProtocolError class (added in 82d1d60b upstream).
- @types/chrome-remote-interface hasn't declared ProtocolError yet
  (DefinitelyTyped/DefinitelyTyped#74992 in flight), so bridge the gap
  with a small local cast at import time. Comment marks the cast for
  removal once the upstream types land.
- Switch the catch block to `if (err instanceof ProtocolError)
  { clip = SCREENSHOT_FALLBACK_CLIP } else { throw err }`.

Tests:

- Fake gains `setMetricsToThrowProtocolError()` and
  `setMetricsToThrowGeneric()` (replaces the single `setMetricsToThrow`).
- Splits the existing "getLayoutMetrics throws -> fallback" test into:
  - ProtocolError throw -> fallback clip applied (recovery path)
  - generic Error throw -> propagates, no captureScreenshot attempted

Safety analysis vs. the broad catch we're replacing:
  - getLayoutMetrics returns 0x0:   fallback (unchanged)
  - ProtocolError on unsupported:    fallback (unchanged)
  - 0x0 + ProtocolError on probe:    fallback (unchanged — captureScreenshot
                                     still gets the clip)
  - websocket dropped mid-probe:     propagates (was: silently fell back, then
                                     captureScreenshot also failed with the
                                     same error one call later — same end
                                     state, cleaner provenance)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 package-lock.json                        |  8 ++---
 package.json                             |  2 +-
 src/cdp-client.ts                        | 26 ++++++++++++++--
 tests/unit/cdp-client.screenshot.test.ts | 21 +++++++++++--
 tests/unit/fakes/fake-screenshot-page.ts | 39 ++++++++++++++++++++----
 5 files changed, 81 insertions(+), 15 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index e1e25bc..af327e7 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -11,7 +11,7 @@
       "dependencies": {
         "@anthropic-ai/sdk": "^0.39.0",
         "@modelcontextprotocol/sdk": "^1.0.0",
-        "chrome-remote-interface": "^0.33.2"
+        "chrome-remote-interface": "^0.34.0"
       },
       "bin": {
         "perplexity-comet-mcp": "dist/index.js"
@@ -1064,9 +1064,9 @@
       }
     },
     "node_modules/chrome-remote-interface": {
-      "version": "0.33.3",
-      "resolved": "https://registry.npmjs.org/chrome-remote-interface/-/chrome-remote-interface-0.33.3.tgz",
-      "integrity": "sha512-zNnn0prUL86Teru6UCAZ1yU1XeXljHl3gj7OrfPcarEfU62OUU4IujDPdTDW3dAWwRqN3ZMG/Chhkh2gPL/wiw==",
+      "version": "0.34.0",
+      "resolved": "https://registry.npmjs.org/chrome-remote-interface/-/chrome-remote-interface-0.34.0.tgz",
+      "integrity": "sha512-rTTcTZ3zemx8I+nvBii7d8BAF0Ms8LLEroypfvwwZOwSpyNGLE28nStXyCA6VwGp2YSQfmCrQH21F/E+oBFvMw==",
       "license": "MIT",
       "dependencies": {
         "commander": "2.11.x",
diff --git a/package.json b/package.json
index ee2876f..5e4b486 100644
--- a/package.json
+++ b/package.json
@@ -52,7 +52,7 @@
   "dependencies": {
     "@anthropic-ai/sdk": "^0.39.0",
     "@modelcontextprotocol/sdk": "^1.0.0",
-    "chrome-remote-interface": "^0.33.2"
+    "chrome-remote-interface": "^0.34.0"
   },
   "devDependencies": {
     "@types/chrome-remote-interface": "^0.33.0",
diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 684b504..636470d 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -15,6 +15,19 @@ import type {
   TabContext,
 } from "./types.js";
 
+// chrome-remote-interface@^0.34.0 exposes `ProtocolError` at runtime
+// (`module.exports.ProtocolError = ...`), but @types/chrome-remote-interface
+// doesn't declare it yet. Cast at import so `instanceof` typechecks; remove
+// the cast once DefinitelyTyped/DefinitelyTyped#74992 lands and we pick up
+// the updated @types.
+interface CdpProtocolError extends Error {
+  request: { method: string; params?: unknown };
+  response: CDP.SendError;
+}
+const ProtocolError = (CDP as unknown as {
+  ProtocolError: new (...args: unknown[]) => CdpProtocolError;
+}).ProtocolError;
+
 // Hidden targets (e.g. the Perplexity sidecar panel) can report a 0x0 layout
 // viewport in some Comet window states (cold launch, no real browsing tabs in
 // front). When that happens, Page.captureScreenshot waits for compositor
@@ -72,8 +85,17 @@ export async function captureScreenshotWithFallback(
     if (!v?.clientWidth && !v?.clientHeight) {
       clip = SCREENSHOT_FALLBACK_CLIP;
     }
-  } catch {
-    clip = SCREENSHOT_FALLBACK_CLIP;
+  } catch (err) {
+    // Chrome-side rejection (e.g. method unsupported on a non-page target):
+    // apply the fallback so captureScreenshot can still produce a frame.
+    // Anything else (websocket dropped, unexpected throw): propagate — the
+    // next CDP call would fail the same way, and masking with a synthetic
+    // 1280x800 capture would hide a real transport-level failure.
+    if (err instanceof ProtocolError) {
+      clip = SCREENSHOT_FALLBACK_CLIP;
+    } else {
+      throw err;
+    }
   }
 
   const result = await page.captureScreenshot({
diff --git a/tests/unit/cdp-client.screenshot.test.ts b/tests/unit/cdp-client.screenshot.test.ts
index 53655bd..413ff9c 100644
--- a/tests/unit/cdp-client.screenshot.test.ts
+++ b/tests/unit/cdp-client.screenshot.test.ts
@@ -94,9 +94,13 @@ describe("captureScreenshotWithFallback — degenerate viewport", () => {
     });
   });
 
-  it("supplies the fallback clip when getLayoutMetrics throws", async () => {
+  // Chrome-side rejection (ProtocolError) on getLayoutMetrics typically
+  // means the method isn't supported on this target type. The wrapper
+  // recovers because captureScreenshot may still succeed at the fallback
+  // dimensions.
+  it("supplies the fallback clip when getLayoutMetrics throws a ProtocolError", async () => {
     const page = new FakeScreenshotPage();
-    page.setMetricsToThrow();
+    page.setMetricsToThrowProtocolError();
 
     await captureScreenshotWithFallback(page, "png");
 
@@ -107,6 +111,19 @@ describe("captureScreenshotWithFallback — degenerate viewport", () => {
     });
   });
 
+  // Transport-level or unexpected errors must NOT be masked with a
+  // synthetic 1280x800 capture — propagate so the caller sees the real
+  // failure (websocket dropped, target detached, etc.).
+  it("propagates non-ProtocolError throws from getLayoutMetrics", async () => {
+    const page = new FakeScreenshotPage();
+    page.setMetricsToThrowGeneric();
+
+    await expect(captureScreenshotWithFallback(page, "png")).rejects.toThrow(
+      /websocket dropped/i,
+    );
+    expect(page.captureScreenshotCalls).toHaveLength(0);
+  });
+
   it("supplies the fallback clip when both viewport fields are absent", async () => {
     const page = new FakeScreenshotPage();
     page.setMetrics({});
diff --git a/tests/unit/fakes/fake-screenshot-page.ts b/tests/unit/fakes/fake-screenshot-page.ts
index 35e2a71..94e82e1 100644
--- a/tests/unit/fakes/fake-screenshot-page.ts
+++ b/tests/unit/fakes/fake-screenshot-page.ts
@@ -3,12 +3,25 @@
 // `captureScreenshot` — enough to satisfy the `ScreenshotPageAPI` type — and
 // records every call so tests can assert on the args.
 
+import CDP from "chrome-remote-interface";
 import type { ScreenshotPageAPI } from "../../../src/cdp-client.js";
 import type { ScreenshotResult } from "../../../src/types.js";
 
+// chrome-remote-interface@^0.34.0 exposes ProtocolError but @types doesn't
+// declare it; mirror the cast pattern used in src/cdp-client.ts so tests can
+// construct one for the discriminator check.
+const ProtocolError = (CDP as unknown as {
+  ProtocolError: new (
+    request: { method: string },
+    response: { code: number; message: string; data?: string },
+  ) => Error;
+}).ProtocolError;
+
 type LayoutMetrics = Awaited<ReturnType<ScreenshotPageAPI["getLayoutMetrics"]>>;
 type CaptureScreenshotArgs = Parameters<ScreenshotPageAPI["captureScreenshot"]>[0];
 
+type ThrowMode = "none" | "protocol-error" | "generic";
+
 export class FakeScreenshotPage implements ScreenshotPageAPI {
   public bringToFrontCalls = 0;
   public readonly captureScreenshotCalls: CaptureScreenshotArgs[] = [];
@@ -17,16 +30,24 @@ export class FakeScreenshotPage implements ScreenshotPageAPI {
   private metricsResponse: LayoutMetrics = {
     cssLayoutViewport: { clientWidth: 1024, clientHeight: 768 },
   };
-  private metricsShouldThrow = false;
+  private metricsThrowMode: ThrowMode = "none";
   private screenshotData: string | undefined = "ZmFrZS1zY3JlZW5zaG90LWRhdGE="; // base64 "fake-screenshot-data"
 
   setMetrics(metrics: LayoutMetrics): void {
     this.metricsResponse = metrics;
-    this.metricsShouldThrow = false;
+    this.metricsThrowMode = "none";
   }
 
-  setMetricsToThrow(): void {
-    this.metricsShouldThrow = true;
+  /** Make `getLayoutMetrics` throw a `CDP.ProtocolError` — models Chrome
+   * rejecting the method (e.g. unsupported on a non-page target). */
+  setMetricsToThrowProtocolError(): void {
+    this.metricsThrowMode = "protocol-error";
+  }
+
+  /** Make `getLayoutMetrics` throw a plain `Error` — models transport-level
+   * or unexpected failures that should propagate, not trigger the fallback. */
+  setMetricsToThrowGeneric(): void {
+    this.metricsThrowMode = "generic";
   }
 
   setBringToFrontToThrow(): void {
@@ -45,8 +66,14 @@ export class FakeScreenshotPage implements ScreenshotPageAPI {
   }
 
   async getLayoutMetrics(): Promise<LayoutMetrics> {
-    if (this.metricsShouldThrow) {
-      throw new Error("getLayoutMetrics unsupported");
+    if (this.metricsThrowMode === "protocol-error") {
+      throw new ProtocolError(
+        { method: "Page.getLayoutMetrics" },
+        { code: -32601, message: "'Page.getLayoutMetrics' wasn't found" },
+      );
+    }
+    if (this.metricsThrowMode === "generic") {
+      throw new Error("websocket dropped");
     }
     return this.metricsResponse;
   }

From c92cde71374bee10e9e5d79f5cbb26dec812b14a Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Sat, 2 May 2026 11:00:06 -0700
Subject: [PATCH 07/10] fix(screenshot): wait for paint via Page.lifecycleEvent

Previously screenshot() proceeded as soon as the connection was up,
which meant a screenshot taken mid-navigation could capture an
unrendered or stale frame. Lighthouse and Puppeteer use the CDP
Page.lifecycleEvent stream to know when the renderer has actually
painted; we now do the same.

- At connect(): enable Page.setLifecycleEventsEnabled and accumulate
  events per (frameId, loaderId).
- New waitForLifecycle(name, timeoutMs) returns immediately if the
  event has already fired for the current document, otherwise waits
  for the next occurrence.
- screenshot() awaits firstContentfulPaint up to 2s before capture.
  Returns immediately for steady-state pages; bounds blast radius if
  the page never paints.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/cdp-client.ts | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 636470d..32b4b35 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -261,6 +261,12 @@ export class CometCDPClient {
   // Tab context registry for multi-tab workflow awareness
   private tabRegistry: Map<string, TabContext> = new Map();
 
+  // Page lifecycle tracking — events accumulated per frame for the current
+  // document (loaderId). Used by waitForLifecycle() so screenshots and other
+  // ops can confirm the renderer has actually painted before they run.
+  private frameLifecycle: Map<string, { loaderId: string; events: Set<string> }> = new Map();
+  private lifecycleListener: ((params: any) => void) | null = null;
+
   get isConnected(): boolean {
     return this.state.connected && this.client !== null;
   }
@@ -1121,6 +1127,25 @@ export class CometCDPClient {
       } catch { /* continue */ }
     }
 
+    // Subscribe to Page.lifecycleEvent so we can wait for paint readiness
+    // (firstContentfulPaint, networkAlmostIdle, etc.) the way Lighthouse and
+    // Puppeteer do, instead of polling document.readyState.
+    this.frameLifecycle.clear();
+    try {
+      await this.client.Page.setLifecycleEventsEnabled({ enabled: true });
+      this.lifecycleListener = (params: any) => {
+        const { frameId, loaderId, name } = params || {};
+        if (!frameId || !loaderId || !name) return;
+        const existing = this.frameLifecycle.get(frameId);
+        if (!existing || existing.loaderId !== loaderId) {
+          this.frameLifecycle.set(frameId, { loaderId, events: new Set([name]) });
+        } else {
+          existing.events.add(name);
+        }
+      };
+      this.client.Page.lifecycleEvent(this.lifecycleListener);
+    } catch { /* lifecycle tracking is best-effort */ }
+
     this.state.connected = true;
     this.state.activeTabId = targetId;
     this.lastTargetId = targetId;
@@ -1132,6 +1157,34 @@ export class CometCDPClient {
     return `Connected to tab: ${this.state.currentUrl}`;
   }
 
+  /**
+   * Wait for any frame in the current document to have fired the named
+   * Page.lifecycleEvent (e.g. 'firstContentfulPaint', 'networkAlmostIdle').
+   * Returns true if the event has fired (or fires before the timeout); false
+   * on timeout. If the event has already fired before this is called,
+   * resolves synchronously.
+   */
+  async waitForLifecycle(eventName: string, timeoutMs: number): Promise<boolean> {
+    if (!this.client) return false;
+    for (const { events } of this.frameLifecycle.values()) {
+      if (events.has(eventName)) return true;
+    }
+    return new Promise<boolean>((resolve) => {
+      let done = false;
+      const finish = (val: boolean) => {
+        if (done) return;
+        done = true;
+        try { (this.client as any)?.removeListener?.('Page.lifecycleEvent', listener); } catch { /* ignore */ }
+        clearTimeout(timer);
+        resolve(val);
+      };
+      const listener = (params: any) => { if (params?.name === eventName) finish(true); };
+      try { (this.client as any).on('Page.lifecycleEvent', listener); }
+      catch { finish(false); return; }
+      const timer = setTimeout(() => finish(false), timeoutMs);
+    });
+  }
+
   /**
    * Disconnect from current tab
    */
@@ -1209,6 +1262,12 @@ export class CometCDPClient {
    */
   async screenshot(format: "png" | "jpeg" = "png"): Promise<ScreenshotResult> {
     this.ensureConnected();
+
+    // Wait for paint readiness via Page.lifecycleEvent (Lighthouse/Puppeteer
+    // approach). If firstContentfulPaint already fired for this document the
+    // call returns synchronously; otherwise we wait up to 2s for the next FCP.
+    await this.waitForLifecycle('firstContentfulPaint', 2000);
+
     return captureScreenshotWithFallback(this.client!.Page, format);
   }
 

From bd6e6837674c8226141d66a81531a52f7c9ceb20 Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 16:15:15 -0700
Subject: [PATCH 08/10] fix(cdp-client): use CRI unsubscribe pattern for
 lifecycle listeners

Addresses review feedback on #7:

- Capture the unsubscribe function returned by client.Page.lifecycleEvent()
  in connect(), store on this.lifecycleUnsubscribe. CRI's domain-event
  callbacks return `() => chrome.removeListener(rawEventName, handler)`
  (api.js:49), which is the idiomatic deregistration path. Replaces the
  prior `(client as any).removeListener(...)` call, which worked (Chrome
  extends EventEmitter) but bypassed the typed API.

- In waitForLifecycle, use the same unsubscribe-function pattern instead
  of `client.on(...)` + `client.removeListener(...)`. Register the
  listener before scanning the cache: single-threaded JS makes the
  synchronous-only path safe today, but ordering matters if anything
  upstream (CRI internals, scheduler) ever inserts a microtask between
  the two operations.

- Mark waitForLifecycle private; only screenshot() uses it.

- Clear frameLifecycle, lifecycleListener, and lifecycleUnsubscribe in
  disconnect() so stale state from the prior tab doesn't carry across a
  disconnect/reconnect cycle.
---
 src/cdp-client.ts | 50 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index 32b4b35..a654ef0 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -266,6 +266,7 @@ export class CometCDPClient {
   // ops can confirm the renderer has actually painted before they run.
   private frameLifecycle: Map<string, { loaderId: string; events: Set<string> }> = new Map();
   private lifecycleListener: ((params: any) => void) | null = null;
+  private lifecycleUnsubscribe: (() => unknown) | null = null;
 
   get isConnected(): boolean {
     return this.state.connected && this.client !== null;
@@ -1131,6 +1132,11 @@ export class CometCDPClient {
     // (firstContentfulPaint, networkAlmostIdle, etc.) the way Lighthouse and
     // Puppeteer do, instead of polling document.readyState.
     this.frameLifecycle.clear();
+    if (this.lifecycleUnsubscribe) {
+      try { this.lifecycleUnsubscribe(); } catch { /* ignore */ }
+      this.lifecycleUnsubscribe = null;
+    }
+    this.lifecycleListener = null;
     try {
       await this.client.Page.setLifecycleEventsEnabled({ enabled: true });
       this.lifecycleListener = (params: any) => {
@@ -1143,7 +1149,10 @@ export class CometCDPClient {
           existing.events.add(name);
         }
       };
-      this.client.Page.lifecycleEvent(this.lifecycleListener);
+      // chrome-remote-interface's domain event callbacks return an unsubscribe
+      // function (api.js: `() => chrome.removeListener(rawEventName, handler)`).
+      // Capture it so disconnect() can deregister cleanly.
+      this.lifecycleUnsubscribe = this.client.Page.lifecycleEvent(this.lifecycleListener);
     } catch { /* lifecycle tracking is best-effort */ }
 
     this.state.connected = true;
@@ -1162,26 +1171,37 @@ export class CometCDPClient {
    * Page.lifecycleEvent (e.g. 'firstContentfulPaint', 'networkAlmostIdle').
    * Returns true if the event has fired (or fires before the timeout); false
    * on timeout. If the event has already fired before this is called,
-   * resolves synchronously.
+   * resolves synchronously (no listener registered).
+   *
+   * Defensive ordering: the listener is registered *before* scanning the
+   * cache, so even if FCP arrived on an I/O turn between calls, it's caught
+   * by the live listener rather than missed. Single-threaded JS makes the
+   * synchronous-only path safe today, but the order matters if anything
+   * upstream (CRI internals, the scheduler) ever inserts a microtask here.
    */
-  async waitForLifecycle(eventName: string, timeoutMs: number): Promise<boolean> {
-    if (!this.client) return false;
-    for (const { events } of this.frameLifecycle.values()) {
-      if (events.has(eventName)) return true;
-    }
+  private async waitForLifecycle(eventName: string, timeoutMs: number): Promise<boolean> {
+    const client = this.client;
+    if (!client) return false;
     return new Promise<boolean>((resolve) => {
       let done = false;
+      let unsubscribe: (() => unknown) | null = null;
+      let timer: NodeJS.Timeout | null = null;
       const finish = (val: boolean) => {
         if (done) return;
         done = true;
-        try { (this.client as any)?.removeListener?.('Page.lifecycleEvent', listener); } catch { /* ignore */ }
-        clearTimeout(timer);
+        if (unsubscribe) { try { unsubscribe(); } catch { /* ignore */ } }
+        if (timer) clearTimeout(timer);
         resolve(val);
       };
       const listener = (params: any) => { if (params?.name === eventName) finish(true); };
-      try { (this.client as any).on('Page.lifecycleEvent', listener); }
-      catch { finish(false); return; }
-      const timer = setTimeout(() => finish(false), timeoutMs);
+      try {
+        unsubscribe = client.Page.lifecycleEvent(listener);
+      } catch { finish(false); return; }
+      // Cache check after listener registration — see method docstring.
+      for (const { events } of this.frameLifecycle.values()) {
+        if (events.has(eventName)) { finish(true); return; }
+      }
+      timer = setTimeout(() => finish(false), timeoutMs);
     });
   }
 
@@ -1189,6 +1209,12 @@ export class CometCDPClient {
    * Disconnect from current tab
    */
   async disconnect(): Promise<void> {
+    if (this.lifecycleUnsubscribe) {
+      try { this.lifecycleUnsubscribe(); } catch { /* ignore */ }
+      this.lifecycleUnsubscribe = null;
+    }
+    this.lifecycleListener = null;
+    this.frameLifecycle.clear();
     if (this.client) {
       await this.client.close();
       this.client = null;

From c62ec7579cb64ddd65804e935392ceb838380fa1 Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 16:32:17 -0700
Subject: [PATCH 09/10] refactor(cdp-client): extract waitForLifecycle as free
 function for testability

Mirrors the captureScreenshotWithFallback pattern: move the lifecycle-wait
logic into a free function that takes a `LifecyclePageAPI` slice and a
`FrameLifecycleMap`, so unit tests can drive it with a hand-written fake
instead of a live CDP connection. Drops the private wrapper method on
CometCDPClient (no other callers) and inlines the call site in screenshot().

No behavior change.
---
 src/cdp-client.ts | 100 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 41 deletions(-)

diff --git a/src/cdp-client.ts b/src/cdp-client.ts
index a654ef0..d524a9b 100644
--- a/src/cdp-client.ts
+++ b/src/cdp-client.ts
@@ -112,6 +112,63 @@ export async function captureScreenshotWithFallback(
   return result;
 }
 
+/** Per-frame lifecycle accumulator: frameId -> { current loaderId, event names seen }. */
+export type FrameLifecycleMap = Map<string, { loaderId: string; events: Set<string> }>;
+
+/**
+ * The slice of the CDP Page domain that `waitForLifecycle` uses. The return
+ * type of `lifecycleEvent(handler)` is CRI's unsubscribe function — api.js:49
+ * returns `() => chrome.removeListener(rawEventName, handler)`.
+ */
+export interface LifecyclePageAPI {
+  lifecycleEvent(handler: (params: { name?: string }) => void): () => unknown;
+}
+
+/**
+ * Wait until any frame in `frameLifecycle` has fired the named
+ * Page.lifecycleEvent (e.g. 'firstContentfulPaint', 'networkAlmostIdle').
+ * Resolves true if the event is in the cache or arrives before the timeout;
+ * false otherwise. Cleans up its listener and timer on every exit path.
+ *
+ * Defensive ordering: the listener is registered *before* scanning the
+ * cache, so even if the event arrived on an I/O turn between calls it's
+ * caught by the live listener rather than missed. Single-threaded JS makes
+ * the synchronous-only path safe today, but the order matters if anything
+ * upstream (CRI internals, scheduler) ever inserts a microtask here.
+ *
+ * Extracted as a free function so it can be unit tested against a fake
+ * Page + map without a live CDP connection.
+ */
+export function waitForLifecycle(
+  page: LifecyclePageAPI,
+  frameLifecycle: FrameLifecycleMap,
+  eventName: string,
+  timeoutMs: number,
+): Promise<boolean> {
+  return new Promise<boolean>((resolve) => {
+    let done = false;
+    let unsubscribe: (() => unknown) | null = null;
+    let timer: NodeJS.Timeout | null = null;
+    const finish = (val: boolean) => {
+      if (done) return;
+      done = true;
+      if (unsubscribe) { try { unsubscribe(); } catch { /* ignore */ } }
+      if (timer) clearTimeout(timer);
+      resolve(val);
+    };
+    const listener = (params: { name?: string }) => {
+      if (params?.name === eventName) finish(true);
+    };
+    try {
+      unsubscribe = page.lifecycleEvent(listener);
+    } catch { finish(false); return; }
+    for (const { events } of frameLifecycle.values()) {
+      if (events.has(eventName)) { finish(true); return; }
+    }
+    timer = setTimeout(() => finish(false), timeoutMs);
+  });
+}
+
 // Detect if running in WSL (must be before windowsFetch)
 function isWSL(): boolean {
   if (platform() !== 'linux') return false;
@@ -264,7 +321,7 @@ export class CometCDPClient {
   // Page lifecycle tracking — events accumulated per frame for the current
   // document (loaderId). Used by waitForLifecycle() so screenshots and other
   // ops can confirm the renderer has actually painted before they run.
-  private frameLifecycle: Map<string, { loaderId: string; events: Set<string> }> = new Map();
+  private frameLifecycle: FrameLifecycleMap = new Map();
   private lifecycleListener: ((params: any) => void) | null = null;
   private lifecycleUnsubscribe: (() => unknown) | null = null;
 
@@ -1166,45 +1223,6 @@ export class CometCDPClient {
     return `Connected to tab: ${this.state.currentUrl}`;
   }
 
-  /**
-   * Wait for any frame in the current document to have fired the named
-   * Page.lifecycleEvent (e.g. 'firstContentfulPaint', 'networkAlmostIdle').
-   * Returns true if the event has fired (or fires before the timeout); false
-   * on timeout. If the event has already fired before this is called,
-   * resolves synchronously (no listener registered).
-   *
-   * Defensive ordering: the listener is registered *before* scanning the
-   * cache, so even if FCP arrived on an I/O turn between calls, it's caught
-   * by the live listener rather than missed. Single-threaded JS makes the
-   * synchronous-only path safe today, but the order matters if anything
-   * upstream (CRI internals, the scheduler) ever inserts a microtask here.
-   */
-  private async waitForLifecycle(eventName: string, timeoutMs: number): Promise<boolean> {
-    const client = this.client;
-    if (!client) return false;
-    return new Promise<boolean>((resolve) => {
-      let done = false;
-      let unsubscribe: (() => unknown) | null = null;
-      let timer: NodeJS.Timeout | null = null;
-      const finish = (val: boolean) => {
-        if (done) return;
-        done = true;
-        if (unsubscribe) { try { unsubscribe(); } catch { /* ignore */ } }
-        if (timer) clearTimeout(timer);
-        resolve(val);
-      };
-      const listener = (params: any) => { if (params?.name === eventName) finish(true); };
-      try {
-        unsubscribe = client.Page.lifecycleEvent(listener);
-      } catch { finish(false); return; }
-      // Cache check after listener registration — see method docstring.
-      for (const { events } of this.frameLifecycle.values()) {
-        if (events.has(eventName)) { finish(true); return; }
-      }
-      timer = setTimeout(() => finish(false), timeoutMs);
-    });
-  }
-
   /**
    * Disconnect from current tab
    */
@@ -1292,7 +1310,7 @@ export class CometCDPClient {
     // Wait for paint readiness via Page.lifecycleEvent (Lighthouse/Puppeteer
     // approach). If firstContentfulPaint already fired for this document the
     // call returns synchronously; otherwise we wait up to 2s for the next FCP.
-    await this.waitForLifecycle('firstContentfulPaint', 2000);
+    await waitForLifecycle(this.client!.Page, this.frameLifecycle, 'firstContentfulPaint', 2000);
 
     return captureScreenshotWithFallback(this.client!.Page, format);
   }

From 191ea5c531966ac5e49fea1802d18abec16dc51b Mon Sep 17 00:00:00 2001
From: Madison Rickert <3495636+madisonrickert@users.noreply.github.com>
Date: Tue, 12 May 2026 16:32:50 -0700
Subject: [PATCH 10/10] test(cdp-client): unit tests for waitForLifecycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Covers the three exit paths (cache hit, listener fires, timeout) plus
error handling when the CRI lifecycleEvent registration throws. Pins the
review-fix invariants:

- listener is unsubscribed on every exit path (no leaked handlers across
  calls — guards the listener-leak finding)
- listener is registered before the cache scan (defensive ordering against
  any future microtask insertion between cache check and registration)
- unrelated lifecycle events on the same connection do not resolve the
  Promise — only the requested eventName matters

Uses a FakeLifecyclePage that satisfies LifecyclePageAPI and records
registrations/unsubscribes/live-handler-count so the leak invariants can
be asserted directly.
---
 tests/unit/cdp-client.lifecycle.test.ts | 203 ++++++++++++++++++++++++
 tests/unit/fakes/fake-lifecycle-page.ts |  49 ++++++
 2 files changed, 252 insertions(+)
 create mode 100644 tests/unit/cdp-client.lifecycle.test.ts
 create mode 100644 tests/unit/fakes/fake-lifecycle-page.ts

diff --git a/tests/unit/cdp-client.lifecycle.test.ts b/tests/unit/cdp-client.lifecycle.test.ts
new file mode 100644
index 0000000..58dd0ad
--- /dev/null
+++ b/tests/unit/cdp-client.lifecycle.test.ts
@@ -0,0 +1,203 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import {
+  type FrameLifecycleMap,
+  waitForLifecycle,
+} from "../../src/cdp-client.js";
+import { FakeLifecyclePage } from "./fakes/fake-lifecycle-page.js";
+
+function makeMap(): FrameLifecycleMap {
+  return new Map();
+}
+
+function seedFrame(
+  map: FrameLifecycleMap,
+  frameId: string,
+  loaderId: string,
+  events: string[],
+): void {
+  map.set(frameId, { loaderId, events: new Set(events) });
+}
+
+afterEach(() => {
+  vi.useRealTimers();
+});
+
+describe("waitForLifecycle — cache-hit path", () => {
+  it("resolves true when the event is already in the cache", async () => {
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+    seedFrame(map, "frame-1", "loader-A", ["firstContentfulPaint"]);
+
+    await expect(
+      waitForLifecycle(page, map, "firstContentfulPaint", 2000),
+    ).resolves.toBe(true);
+  });
+
+  it("unsubscribes its listener even when the cache hit fires synchronously", async () => {
+    // Defensive ordering: the listener is registered before the cache scan,
+    // so a cache hit still has to release it. This guards finding #1 (no
+    // leaked listeners across calls).
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+    seedFrame(map, "frame-1", "loader-A", ["firstContentfulPaint"]);
+
+    await waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+
+    expect(page.registrations).toBe(1);
+    expect(page.unsubscribes).toBe(1);
+    expect(page.liveHandlerCount()).toBe(0);
+  });
+
+  it("ignores cache entries for unrelated events and falls through to the listener path", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+    seedFrame(map, "frame-1", "loader-A", ["load", "domContentLoaded"]);
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    expect(page.liveHandlerCount()).toBe(1);
+
+    vi.advanceTimersByTime(2000);
+    await expect(pending).resolves.toBe(false);
+  });
+});
+
+describe("waitForLifecycle — listener path", () => {
+  it("resolves true when the event arrives after registration", async () => {
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    page.emit("firstContentfulPaint");
+
+    await expect(pending).resolves.toBe(true);
+  });
+
+  it("unsubscribes after firing", async () => {
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    page.emit("firstContentfulPaint");
+    await pending;
+
+    expect(page.unsubscribes).toBe(1);
+    expect(page.liveHandlerCount()).toBe(0);
+  });
+
+  it("ignores unrelated events", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    page.emit("load");
+    page.emit("domContentLoaded");
+    page.emit("networkAlmostIdle");
+    vi.advanceTimersByTime(2000);
+
+    await expect(pending).resolves.toBe(false);
+  });
+
+  it("registers the listener before the cache scan (defensive ordering)", () => {
+    // Single-threaded JS makes this safe today even without the ordering;
+    // the test pins the behavior so a future refactor can't silently
+    // reintroduce a microtask-window race.
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    // The cache is empty — if the scan ran before registration, the
+    // listener would never be registered before the function returned
+    // its pending Promise. Asserting that the handler is live immediately
+    // after the call confirms the order.
+    waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    expect(page.liveHandlerCount()).toBe(1);
+  });
+});
+
+describe("waitForLifecycle — timeout path", () => {
+  it("resolves false when the timeout elapses with no event", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    vi.advanceTimersByTime(2000);
+
+    await expect(pending).resolves.toBe(false);
+  });
+
+  it("unsubscribes on timeout", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    vi.advanceTimersByTime(2000);
+    await pending;
+
+    expect(page.unsubscribes).toBe(1);
+    expect(page.liveHandlerCount()).toBe(0);
+  });
+
+  it("does not resolve before the timeout fires", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+
+    const pending = waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    vi.advanceTimersByTime(1999);
+
+    const settled = await Promise.race([
+      pending.then((v) => ({ resolved: true, value: v })),
+      Promise.resolve({ resolved: false as const }),
+    ]);
+    expect(settled).toEqual({ resolved: false });
+
+    vi.advanceTimersByTime(1);
+    await expect(pending).resolves.toBe(false);
+  });
+});
+
+describe("waitForLifecycle — error handling", () => {
+  it("resolves false (best-effort) when page.lifecycleEvent throws", async () => {
+    // Models the connect()-time guard: if CRI rejects the subscription
+    // (older protocol versions, etc.), `waitForLifecycle` should degrade
+    // to "feature not available" rather than throw.
+    const page = new FakeLifecyclePage();
+    page.setLifecycleEventToThrow();
+    const map = makeMap();
+
+    await expect(
+      waitForLifecycle(page, map, "firstContentfulPaint", 2000),
+    ).resolves.toBe(false);
+  });
+
+  it("does not register a timer when the listener registration throws", async () => {
+    vi.useFakeTimers();
+    const page = new FakeLifecyclePage();
+    page.setLifecycleEventToThrow();
+    const map = makeMap();
+
+    await waitForLifecycle(page, map, "firstContentfulPaint", 2000);
+    // If a timer leaked, advancing time after the Promise settled would
+    // do nothing observable here, but the more important assertion is that
+    // no handler was ever registered:
+    expect(page.registrations).toBe(0);
+    expect(page.liveHandlerCount()).toBe(0);
+  });
+});
+
+describe("waitForLifecycle — scanning frameLifecycle", () => {
+  it("finds an event registered under any frameId", async () => {
+    const page = new FakeLifecyclePage();
+    const map = makeMap();
+    seedFrame(map, "frame-A", "loader-1", ["load"]);
+    seedFrame(map, "frame-B", "loader-2", ["firstContentfulPaint"]);
+    seedFrame(map, "frame-C", "loader-3", ["domContentLoaded"]);
+
+    await expect(
+      waitForLifecycle(page, map, "firstContentfulPaint", 2000),
+    ).resolves.toBe(true);
+  });
+});
diff --git a/tests/unit/fakes/fake-lifecycle-page.ts b/tests/unit/fakes/fake-lifecycle-page.ts
new file mode 100644
index 0000000..ecd7a9b
--- /dev/null
+++ b/tests/unit/fakes/fake-lifecycle-page.ts
@@ -0,0 +1,49 @@
+// Minimal fake of the CDP `Page` slice that `waitForLifecycle` uses.
+// Implements just `lifecycleEvent` — enough to satisfy `LifecyclePageAPI` —
+// and exposes hooks so tests can fire events or assert on unsubscribe.
+
+import type { LifecyclePageAPI } from "../../../src/cdp-client.js";
+
+type Handler = (params: { name?: string }) => void;
+
+export class FakeLifecyclePage implements LifecyclePageAPI {
+  /** Handlers currently registered via `lifecycleEvent`. */
+  private readonly handlers: Set<Handler> = new Set();
+
+  /** Number of times `lifecycleEvent` has been called (handler registrations). */
+  public registrations = 0;
+
+  /** Number of unsubscribe-function invocations. */
+  public unsubscribes = 0;
+
+  /** When true, `lifecycleEvent` throws on next call (simulates CRI rejection). */
+  private throwOnRegister = false;
+
+  setLifecycleEventToThrow(): void {
+    this.throwOnRegister = true;
+  }
+
+  /** Emit a lifecycle event to every currently-registered handler. */
+  emit(name: string): void {
+    for (const handler of this.handlers) {
+      handler({ name });
+    }
+  }
+
+  /** Number of currently-live handlers (registered minus unsubscribed). */
+  liveHandlerCount(): number {
+    return this.handlers.size;
+  }
+
+  lifecycleEvent(handler: Handler): () => unknown {
+    if (this.throwOnRegister) {
+      throw new Error("lifecycleEvent registration failed");
+    }
+    this.registrations++;
+    this.handlers.add(handler);
+    return () => {
+      this.unsubscribes++;
+      this.handlers.delete(handler);
+    };
+  }
+}