fix(cdp): serialize event consumer; persist dialog until taken; validate page info shape

2 files changed, 61 insertions, 17 deletions

diff --git a/src/cdp/session.ts b/src/cdp/session.ts
index 71f7a76..fffa241 100644
--- a/src/cdp/session.ts
+++ b/src/cdp/session.ts
@@ -21,6 +21,8 @@ export const createCdpSession = (transport: CdpTransport): CdpSession => {
   let dialog: DialogInfo | null = null;
   let pageInfoDirty = false;
 
+  let activeConsumer: Promise<void> = Promise.resolve();
+
   const consumeEvents = async (): Promise<void> => {
     for await (const ev of transport.events()) {
       if (ev.method === "Page.javascriptDialogOpening") {
@@ -32,25 +34,33 @@ export const createCdpSession = (transport: CdpTransport): CdpSession => {
         };
         continue;
       }
-      if (ev.method === "Page.javascriptDialogClosed") {
-        dialog = null;
-        continue;
-      }
+      // Page.javascriptDialogClosed is intentionally NOT cleared here —
+      // the dialog stays in the buffer until takeDialog() is called.
+      // This prevents fast dismiss flows from dropping a dialog the agent
+      // was about to read. (Fix for spec §7 predictability bug #2.)
       if (ev.method === "Page.frameNavigated" || ev.method === "Page.loadEventFired") {
         pageInfoDirty = true;
       }
     }
   };
 
-  void consumeEvents();
+  const restartConsumer = (): void => {
+    activeConsumer = activeConsumer.then(() => consumeEvents()).catch(() => undefined);
+  };
+
+  restartConsumer();
   transport.onClose(() => {
     sessionId = null;
     targetId = null;
-    dialog = null;
     pageInfoDirty = false;
-    void consumeEvents();
+    // Do NOT clear `dialog` here — same rationale as inside consumeEvents:
+    // the agent may have a pending takeDialog() call that should still see it.
+    restartConsumer();
   });
 
+  // TODO(perf): the four enable calls are sequential here for predictability.
+  // Switching to Promise.all over a single WS pipelines the round-trips and
+  // saves ~3× on tab-switch latency. Defer until session.ts has tests.
   const enableDomains = async (sid: string): Promise<void> => {
     for (const d of ["Page", "DOM", "Runtime", "Network"]) {
       await transport.request(`${d}.enable`, {}, { sessionId: sid });
diff --git a/src/client.ts b/src/client.ts
index 637114b..1fcef67 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -32,6 +32,34 @@ export type BrowserClient = {
 const HEALTH_TTL_MS = 30_000;
 const PAGE_INFO_TTL_MS = 1_000;
 
+const parsePageInfoPayload = (v: unknown): Result<PageInfo, CdpError> => {
+  if (typeof v !== "object" || v === null) {
+    return err(cdpError("invalid_response", "page info payload is not an object"));
+  }
+  const o = v as Readonly<Record<string, unknown>>;
+  const fields: ReadonlyArray<readonly [string, "string" | "number"]> = [
+    ["url", "string"], ["title", "string"],
+    ["w", "number"], ["h", "number"],
+    ["sx", "number"], ["sy", "number"],
+    ["pw", "number"], ["ph", "number"],
+  ];
+  for (const [k, t] of fields) {
+    if (typeof o[k] !== t) {
+      return err(cdpError("invalid_response", `page info field ${k} has wrong type (expected ${t})`));
+    }
+  }
+  return ok({
+    url: o["url"] as string,
+    title: o["title"] as string,
+    width: o["w"] as number,
+    height: o["h"] as number,
+    scrollX: o["sx"] as number,
+    scrollY: o["sy"] as number,
+    pageWidth: o["pw"] as number,
+    pageHeight: o["ph"] as number,
+  });
+};
+
 export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient => {
   const transport = createCdpTransport();
   const session = createCdpSession(transport);
@@ -110,15 +138,16 @@ export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient =
     const raw = await evaluateJs(expr);
     if (!raw.success) return raw;
     if (typeof raw.data !== "string") return err(cdpError("invalid_response", "page info evaluation did not return a string"));
-    const parsedRaw = JSON.parse(raw.data) as { url: string; title: string; w: number; h: number; sx: number; sy: number; pw: number; ph: number };
-    const info: PageInfo = {
-      url: parsedRaw.url, title: parsedRaw.title,
-      width: parsedRaw.w, height: parsedRaw.h,
-      scrollX: parsedRaw.sx, scrollY: parsedRaw.sy,
-      pageWidth: parsedRaw.pw, pageHeight: parsedRaw.ph,
-    };
-    pageCache = { info, at: Date.now() };
-    return ok(info);
+    let parsedRaw: unknown;
+    try {
+      parsedRaw = JSON.parse(raw.data);
+    } catch (e) {
+      return err(cdpError("invalid_response", `page info JSON.parse failed: ${e instanceof Error ? e.message : String(e)}`));
+    }
+    const info = parsePageInfoPayload(parsedRaw);
+    if (!info.success) return info;
+    pageCache = { info: info.data, at: Date.now() };
+    return ok(info.data);
   };
 
   const pageInfo = async (): Promise<Result<PageInfo | { readonly dialog: DialogInfo }, CdpError>> => {
@@ -142,7 +171,12 @@ export const createBrowserClient = (opts: BrowserClientOptions): BrowserClient =
     const r = await session.switchTo(targetId);
     if (!r.success) return r;
     pageCache = null;
-    await session.call("Runtime.evaluate", { expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title` });
+    // Best-effort: mark the tab title with a green circle so the user can
+    // see which tab the agent attached to. CSP or detached frames may
+    // block the eval; we don't surface that as a switchTab failure.
+    await session.call("Runtime.evaluate", {
+      expression: safeJs`if(!document.title.startsWith('🟢'))document.title='🟢 '+document.title`,
+    });
     return ok(undefined);
   };