fix: review of #21 — one browser per session, real DPR for maxWidth, GIF follows replaced tab

extension/handlers/gif.js

@@ -6,6 +6,7 @@
  * server writes the file — the GIF bytes never go to the agent.
  */
 import { resolveTab } from '../lib/page-exec.js';
+import { gifRecordings as recordings, replacedTabs } from '../lib/state.js';
 import { encodeGif, drawMarker } from '../lib/gif-encoder.js';
 import { handleScreenshot } from './tabs.js';
 
@@ -19,8 +20,14 @@ export const GIF_FRAME_TOOLS = new Set([
 const MAX_FRAMES_CAP = 500;
 /** Export travels in parts: the daemon's WebSocket frames are capped at 1 MB. */
 export const GIF_PART_BYTES = 600_000;
-/** tabId -> { frames, width, maxFrames, activate, recording, skipped, startedAt } */
-const recordings = new Map();
+/** recordings (lib/state.js): tabId -> { frames, width, maxFrames, activate, recording, skipped, startedAt } */
+
+/** The tab now holding this id's recording (a frozen tab replaced mid-recording hands it on). */
+export function currentTabId(tabId) {
+  let id = tabId;
+  for (let i = 0; i < 10 && !recordings.has(id) && replacedTabs.has(id); i++) id = replacedTabs.get(id);
+  return id;
+}
 
 export function isRecording(tabId) {
   const r = recordings.get(tabId);
@@ -85,8 +92,11 @@ async function encodeRecording(rec) {
 }
 
 export async function handleGif(params) {
-  const { tabId, action } = params;
-  await resolveTab(tabId);
+  const { action } = params;
+  // An id replaced mid-recording resolves to its replacement.
+  const tabId = action === 'start' ? params.tabId : currentTabId(params.tabId);
+  // Only capturing needs a live tab: stop/status/export/clear work on the frames already taken.
+  if (action === 'start' || action === 'frame') await resolveTab(tabId);
   switch (action) {
     case 'start': {
       const rec = {

extension/handlers/tabs.js

@@ -56,6 +56,16 @@ export function imageSize(b64) {
  * one in its window, so the user's view is never switched, and can downscale
  * (`scale`) or cap the width (`maxWidth`) to save image tokens.
  */
+/** Device pixels per CSS pixel: window.devicePixelRatio, else the layout-metrics ratio. */
+async function pageDpr(send, metrics, vv) {
+  try {
+    const r = await withTimeout(send('Runtime.evaluate', { expression: 'window.devicePixelRatio', returnByValue: true }), 1500, 'devicePixelRatio');
+    const v = Number(r?.result?.value);
+    if (v > 0 && v < 16) return v;
+  } catch { /* frozen or restricted page: fall back */ }
+  return metrics.visualViewport?.clientWidth > 0 ? metrics.visualViewport.clientWidth / vv.clientWidth : 1;
+}
+
 async function cdpScreenshot(tabId, { format, quality, scale, maxWidth, fullPage, region }) {
   return withCdp(tabId, async (send) => {
     let metrics = await send('Page.getLayoutMetrics');
@@ -78,25 +88,34 @@ async function cdpScreenshot(tabId, { format, quality, scale, maxWidth, fullPage
       height = Math.max(1, Math.min(region.height, vv.clientHeight - originY));
     }
     let s = Math.min(region ? 4 : 1, Math.max(0.05, scale ?? (region ? 2 : 1)));
-    // The capture comes out at clip.scale × devicePixelRatio (device pixels):
-    // the deprecated device-pixel metrics against the CSS ones give the ratio.
-    const dpr = metrics.visualViewport?.clientWidth > 0 ? metrics.visualViewport.clientWidth / vv.clientWidth : 1;
+    // The capture comes out at clip.scale × devicePixelRatio (device pixels).
+    // The layout metrics can't be trusted for that ratio (real Chrome at DPR 2
+    // reports equal device and CSS viewport widths), so ask the page.
+    const dpr = await pageDpr(send, metrics, vv);
     if (maxWidth && width * s * dpr > maxWidth) s = maxWidth / (width * dpr);
-    const { data } = await withTimeout(send('Page.captureScreenshot', {
+    const capture = (clipScale) => withTimeout(send('Page.captureScreenshot', {
       format,
       ...(format === 'jpeg' ? { quality } : {}),
       captureBeyondViewport: !!fullPage && !region,
       clip: {
         x: fullPage && !region ? 0 : vv.pageX + originX,
         y: fullPage && !region ? 0 : vv.pageY + originY,
-        width, height, scale: s,
+        width, height, scale: clipScale,
       },
     }), CDP_CAPTURE_TIMEOUT_MS, 'Page.captureScreenshot');
+    let { data } = await capture(s);
+    let size = imageSize(data);
+    // maxWidth is a promise about the IMAGE: if the ratio was still off, shrink
+    // by what the real image shows and capture once more.
+    if (maxWidth && size?.width > maxWidth + 1) {
+      s = s * (maxWidth / size.width);
+      ({ data } = await capture(s));
+      size = imageSize(data);
+    }
     // How image pixels map to the viewport coordinates click/hover/scroll take:
     // viewportX = origin[0] + imageX / scale (fullPage: page coordinates instead).
     // The real image size is the ground truth (it includes the device pixel
     // ratio: an 800 px viewport at DPR 2 is a 1600 px image, scale 2).
-    const size = imageSize(data);
     const imgW = size?.width || Math.round(width * s * dpr);
     const imgH = size?.height || Math.round(height * s * dpr);
     const frame = {

extension/lib/page-exec.js

@@ -2,7 +2,7 @@
  * Page-execution primitives (extracted from background.js): tab resolution,
  * the locator guard, and safeExec. Everything a handler needs to touch a page.
  */
-import { fallbackByTab, wedgedTabs, tabLocks, persistSessionState } from './state.js';
+import { fallbackByTab, wedgedTabs, tabLocks, persistSessionState, gifRecordings, replacedTabs } from './state.js';
 import { PAGE_DOM_INSTALL, PAGE_DOM_VERSION } from './page-dom.js';
 
 /**
@@ -111,6 +111,13 @@ export async function replaceFrozenTab(tab, url, sessionId = null) {
     tabLocks.lock(fresh.id, owner);
     persistSessionState();
   }
+  // A GIF recording moves with the tab: frames keep coming, export still works.
+  const rec = gifRecordings.get(tab.id);
+  if (rec) {
+    gifRecordings.delete(tab.id);
+    gifRecordings.set(fresh.id, rec);
+  }
+  replacedTabs.set(tab.id, fresh.id);
   chrome.tabs.remove(tab.id).catch(() => { /* already gone */ });
   return fresh;
 }

extension/lib/router.js

@@ -15,7 +15,7 @@ import { handleTabs, handleConsole, handleNetwork, handleScreenshot, handleResiz
 import { handleRunAction, handleUploadFile } from '../handlers/cdp.js';
 import { handleIntercept } from '../handlers/intercept.js';
 import { handleObserve, handleAct } from '../handlers/agent-api.js';
-import { handleGif, isRecording, recordFrame, GIF_FRAME_TOOLS } from '../handlers/gif.js';
+import { handleGif, isRecording, recordFrame, GIF_FRAME_TOOLS, currentTabId } from '../handlers/gif.js';
 
 // sessionId arrives as a first-class top-level field on the WS message (audit
 // M1) — the daemon no longer injects it into params. We read it here so the
@@ -113,6 +113,13 @@ function extractTabId(_tool, params) {
   return typeof params.tabId === "number" ? params.tabId : null;
 }
 
+/** The new tab id when this call replaced a frozen tab (navigate/reload), else null. */
+function replacementOf(result) {
+  if (typeof result?.replacedTabId !== 'number') return null;
+  const fresh = result.tabId ?? result.reloaded;
+  return typeof fresh === 'number' ? fresh : null;
+}
+
 export async function handleMessage(msg) {
   // Control messages (non-tool) from the daemon. These carry a `type` and no
   // `tool`; handle them here before the tool-dispatch path assumes a tool call.
@@ -170,6 +177,11 @@ export async function handleMessage(msg) {
   if (tool === "browser_navigate" && typeof p.tabId !== "number") {
     p.tabId = (await getActiveTab()).id;
   }
+  // A GIF call naming a replaced frozen tab targets its replacement, so the
+  // lock check and the per-tab queue below apply to the tab it really touches.
+  if (tool === 'browser_gif' && p.action !== 'start' && typeof p.tabId === 'number') {
+    p.tabId = currentTabId(p.tabId);
+  }
   const tabId = extractTabId(tool, p);
 
   // A tool call without an id can never be answered: it used to collide in
@@ -218,6 +230,12 @@ export async function handleMessage(msg) {
         controller.signal,
       );
       sendToolResponse(id, result);
+      // Recovery navigate replaced a frozen tab: record the new page, queued
+      // on the replacement's mutex like any other capture.
+      const fresh = replacementOf(result);
+      if (fresh != null && GIF_FRAME_TOOLS.has(tool) && isRecording(fresh)) {
+        await tabMutex.run(fresh, () => recordFrame(fresh, tool, result));
+      }
     } catch (err) {
       sendResponse(id, { success: false, error: err.message || String(err) });
     } finally {
@@ -250,8 +268,10 @@ export async function handleMessage(msg) {
         sendToolResponse(id, result);
         // GIF recording: capture the page after the action (the reply is already
         // sent; the tab mutex keeps the next call from racing the capture).
-        if (GIF_FRAME_TOOLS.has(tool) && isRecording(tabId) && !(result && result.success === false)) {
-          await recordFrame(tabId, tool, result);
+        // A frozen tab replaced by this call records on its replacement.
+        const frameTab = replacementOf(result) ?? tabId;
+        if (GIF_FRAME_TOOLS.has(tool) && isRecording(frameTab) && !(result && result.success === false)) {
+          await recordFrame(frameTab, tool, result);
         }
       } catch (err) {
         sendResponse(id, { success: false, error: err.message || String(err) });

extension/lib/state.js

@@ -41,6 +41,10 @@ export const fallbackByTab = new Map();
  * Cleared when the tab starts a new navigation or is closed.
  */
 export const wedgedTabs = new Map();
+/** browser_gif recordings: tabId -> recording (here so a replaced frozen tab can hand its recording over). */
+export const gifRecordings = new Map();
+/** Frozen tabs replaced by replaceFrozenTab: old tabId -> new tabId. */
+export const replacedTabs = new Map();
 /**
  * isNew feature: Map<tabId, string[]> of "fingerprints" (role|name) from the
  * PREVIOUS snapshot. The next snapshot marks any ref whose fingerprint isn't

mcp-server/src/bridge-connections.ts

@@ -36,11 +36,16 @@ export function identityOf(info?: { browserId?: unknown; browserLabel?: unknown
  * The set of extension connections plus each session's browser choice.
  * Routing: a session's selected browser, else the default — the most
  * recently connected live browser (so one browser behaves exactly as before).
+ * An "auto" session is pinned to the browser its first call went to: tab ids
+ * belong to one browser, so a browser connecting mid-session (or mid-batch,
+ * or between a call and its retry) must not take over that session's calls.
  */
 export class ExtensionConnections {
   private conns = new Set<ExtensionConnection>();
   /** sessionId -> browserId chosen with browser_select_browser. */
   private sessionBrowser = new Map<string, string>();
+  /** sessionId -> browserId an "auto" session was pinned to by its first call. */
+  private autoBrowser = new Map<string, string>();
 
   add(conn: ExtensionConnection): void { this.conns.add(conn); }
   has(conn: ExtensionConnection): boolean { return this.conns.has(conn); }
@@ -70,8 +75,19 @@ export class ExtensionConnections {
       if (chosen) return chosen;
       throw new Error(`Selected browser "${want}" is not connected. browser_list_browsers shows the connected ones (browser_select_browser "auto" = default).`);
     }
+    const pinned = sessionId ? this.autoBrowser.get(sessionId) : undefined;
+    if (pinned) {
+      const same = this.live().find((c) => c.browserId === pinned);
+      if (same) return same;
+      // Its browser is gone: moving on to another browser is only safe when
+      // no other one could be confused with it — never switch silently.
+      if (this.live().length > 0) {
+        throw new Error(`This session's browser "${pinned}" disconnected. Its tab ids are not valid in the other connected browser(s): reconnect it, or call browser_select_browser (then browser_tabs list) to continue in another one.`);
+      }
+    }
     const primary = this.primary();
     if (!primary) throw new Error('Chrome extension not connected. Make sure the Browser Controller extension is installed and enabled.');
+    if (sessionId) this.autoBrowser.set(sessionId, primary.browserId);
     return primary;
   }
 
@@ -83,7 +99,7 @@ export class ExtensionConnections {
   /** browser_list_browsers: every connected extension, with this session's choice. */
   list(sessionId?: string): Record<string, unknown> {
     const primary = this.primary();
-    const selected = sessionId ? this.sessionBrowser.get(sessionId) : undefined;
+    const selected = sessionId ? (this.sessionBrowser.get(sessionId) ?? this.autoBrowser.get(sessionId)) : undefined;
     return {
       success: true,
       browsers: this.live().map((c) => ({
@@ -94,6 +110,7 @@ export class ExtensionConnections {
         ...((selected ? selected === c.browserId : c === primary) ? { selected: true } : {}),
       })),
       ...(selected ? { selectedBrowserId: selected } : {}),
+      ...(sessionId && !this.sessionBrowser.has(sessionId) && selected ? { pinnedAuto: true } : {}),
     };
   }
 
@@ -102,19 +119,27 @@ export class ExtensionConnections {
     if (!sessionId) throw new Error('Selecting a browser needs a client session (connect through the Browser Controller MCP server).');
     const id = typeof browserId === 'string' ? browserId.trim() : '';
     if (!id || id === 'auto') {
+      // "auto" again re-pins the session to the current default.
+      this.autoBrowser.delete(sessionId);
       this.sessionBrowser.delete(sessionId);
       return { success: true, selected: 'auto', browserId: this.primary()?.browserId ?? null };
     }
     const conn = this.live().find((c) => c.browserId === id || c.label === id);
+    // A failed selection changes nothing: the session keeps its pin.
     if (!conn) throw new Error(`No connected browser "${id}". browser_list_browsers shows the connected ones.`);
+    this.autoBrowser.delete(sessionId);
     this.sessionBrowser.set(sessionId, conn.browserId);
     return { success: true, selected: conn.browserId, label: conn.label };
   }
 
-  releaseSession(sessionId: string): void { this.sessionBrowser.delete(sessionId); }
+  releaseSession(sessionId: string): void {
+    this.sessionBrowser.delete(sessionId);
+    this.autoBrowser.delete(sessionId);
+  }
 
   clear(): void {
     this.conns.clear();
     this.sessionBrowser.clear();
+    this.autoBrowser.clear();
   }
 }

tests/bridge-multibrowser.test.ts

@@ -90,6 +90,34 @@ describe('multi-browser bridge', () => {
     await expect(bridge.callTool('browser_tabs', { action: 'list' }, 's1')).rejects.toThrow(/not connected/);
   });
 
+  it('an auto session stays on its browser when another one connects (batch/retry safe)', async () => {
+    const { bridge, port: p } = await startBridge();
+    const work = await fakeBrowser(p, 'work');
+    // Step 1 of a batch: click in "work" (the only browser).
+    expect(await bridge.callTool('browser_click', { tabId: 7 }, 's1')).toEqual({ from: 'work' });
+    const home = await fakeBrowser(p, 'home'); // newer: becomes the default
+    // Step 2 must not land in "home" with the same tabId.
+    expect(await bridge.callTool('browser_type', { tabId: 7, text: 'x' }, 's1')).toEqual({ from: 'work' });
+    expect(work.calls).toEqual(['browser_click', 'browser_type']);
+    expect(home.calls).toEqual([]);
+    // A new session takes the default; list shows each session's own browser.
+    expect(await bridge.callTool('browser_tabs', { action: 'list' }, 's2')).toEqual({ from: 'home' });
+    const listed = await (bridge.callTool('browser_list_browsers', {}, 's1') as Promise<any>);
+    expect(listed).toMatchObject({ selectedBrowserId: 'work', pinnedAuto: true });
+    expect(listed.browsers.find((b: any) => b.selected).browserId).toBe('work');
+    // A failed selection keeps the pin.
+    await expect(bridge.callTool('browser_select_browser', { browserId: 'nope' }, 's1')).rejects.toThrow(/No connected browser/);
+    expect(await bridge.callTool('browser_type', { tabId: 7, text: 'z' }, 's1')).toEqual({ from: 'work' });
+    // Its browser gone: an error, never a silent switch to the other browser.
+    work.ws.close();
+    await new Promise((r) => setTimeout(r, 60));
+    await expect(bridge.callTool('browser_type', { tabId: 7, text: 'y' }, 's1')).rejects.toThrow(/"work" disconnected/);
+    expect(home.calls).toEqual(['browser_tabs']);
+    // Selecting "auto" again re-pins to the current default.
+    await bridge.callTool('browser_select_browser', { browserId: 'auto' }, 's1');
+    expect(await bridge.callTool('browser_tabs', { action: 'list' }, 's1')).toEqual({ from: 'home' });
+  });
+
   it('releasing a session forgets its browser choice', async () => {
     const { bridge, port: p } = await startBridge();
     await fakeBrowser(p, 'work');