fix(engine): enable browser pool and deduplicate concurrent Chrome launches

packages/cli/src/server/studioServer.ts

@@ -113,10 +113,9 @@ async function reapplyStudioManualEditsToThumbnailPage(
   });
 }
 
-// ── Shared thumbnail browser (singleton per process) ────────────────────────
-// One browser instance is reused across all composition thumbnail requests.
-// Spawning a new Puppeteer process per request adds 2-5s overhead and causes
-// contention when the sidebar requests multiple thumbnails simultaneously.
+// ── Shared thumbnail browser (pool-backed) ──────────────────────────────────
+// Uses the engine's browser pool so the thumbnail browser and render workers
+// share a single Chrome process instead of running two independent ones.
 
 let _thumbnailBrowser: import("puppeteer-core").Browser | null = null;
 let _thumbnailBrowserInitializing: Promise<import("puppeteer-core").Browser | null> | null = null;
@@ -139,14 +138,22 @@ async function getThumbnailBrowser(): Promise<import("puppeteer-core").Browser |
         /* continue — acquireBrowser will try its own resolution */
       }
 
-      const acquired = await acquireBrowser(buildChromeArgs({ width: 1920, height: 1080 }), {
-        enableBrowserPool: false,
-      });
+      const acquired = await acquireBrowser(buildChromeArgs({ width: 1920, height: 1080 }));
       _thumbnailBrowser = acquired.browser;
       _thumbnailBrowser.on("disconnected", () => {
         _thumbnailBrowser = null;
         _thumbnailBrowserInitializing = null;
       });
+      // Release the pool ref on process exit so the browser closes cleanly.
+      const onExit = async () => {
+        const { releaseBrowser } = await import("@hyperframes/engine");
+        if (_thumbnailBrowser) {
+          await releaseBrowser(_thumbnailBrowser).catch(() => {});
+          _thumbnailBrowser = null;
+        }
+      };
+      process.once("SIGTERM", () => void onExit());
+      process.once("SIGINT", () => void onExit());
       return _thumbnailBrowser;
     } catch {
       _thumbnailBrowserInitializing = null;

packages/engine/src/config.ts

@@ -173,7 +173,7 @@ export const DEFAULT_CONFIG: EngineConfig = {
 
   disableGpu: false,
   browserGpuMode: "software",
-  enableBrowserPool: false,
+  enableBrowserPool: true,
   browserTimeout: 120_000,
   protocolTimeout: 300_000,
   forceScreenshot: false,

packages/engine/src/index.ts

@@ -49,6 +49,7 @@ export { resolveConfig, DEFAULT_CONFIG, type EngineConfig } from "./config.js";
 export {
   acquireBrowser,
   releaseBrowser,
+  drainBrowserPool,
   resolveHeadlessShellPath,
   resolveBrowserGpuMode,
   buildChromeArgs,

packages/engine/src/services/browserManager.test.ts

@@ -1,9 +1,16 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+import type { Browser, PuppeteerNode } from "puppeteer-core";
+
 import {
   _resetAutoBrowserGpuModeCacheForTests,
+  _resetBrowserPoolForTests,
+  _setPuppeteerForTests,
+  acquireBrowser,
   buildChromeArgs,
+  drainBrowserPool,
   forceReleaseBrowser,
+  releaseBrowser,
   resolveBrowserGpuMode,
 } from "./browserManager.js";
 
@@ -157,3 +164,145 @@ describe("forceReleaseBrowser", () => {
     expect(disconnectFn).toHaveBeenCalled();
   });
 });
+
+describe("browser pool", () => {
+  function makeMockBrowser(): Browser {
+    return {
+      connected: true,
+      newPage: vi.fn(),
+      version: vi.fn().mockResolvedValue("HeadlessChrome/131.0.0.0"),
+      close: vi.fn().mockResolvedValue(undefined),
+      disconnect: vi.fn(),
+      process: () => ({ kill: vi.fn(), killed: false }),
+    } as unknown as Browser;
+  }
+
+  // forceScreenshot: true bypasses the BeginFrame probe path, which on Linux
+  // CI would trigger a second ppt.launch() when the mock's newPage() doesn't
+  // return a real page and the probe falls back to screenshot mode.
+  const poolCfg = { enableBrowserPool: true, forceScreenshot: true } as const;
+
+  let launchFn: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    _resetBrowserPoolForTests();
+    const mockBrowser = makeMockBrowser();
+    launchFn = vi.fn().mockResolvedValue(mockBrowser);
+    _setPuppeteerForTests({ launch: launchFn } as unknown as PuppeteerNode);
+  });
+
+  afterEach(async () => {
+    await drainBrowserPool();
+    _setPuppeteerForTests(undefined);
+  });
+
+  it("sequential acquires with pool enabled return the same browser", async () => {
+    const first = await acquireBrowser(["--no-sandbox"], poolCfg);
+    const second = await acquireBrowser(["--no-sandbox"], poolCfg);
+
+    expect(first.browser).toBe(second.browser);
+    expect(launchFn).toHaveBeenCalledTimes(1);
+
+    await releaseBrowser(first.browser, poolCfg);
+    await releaseBrowser(second.browser, poolCfg);
+  });
+
+  it("concurrent acquires via Promise.all trigger exactly one launch", async () => {
+    const [a, b, c] = await Promise.all([
+      acquireBrowser(["--no-sandbox"], poolCfg),
+      acquireBrowser(["--no-sandbox"], poolCfg),
+      acquireBrowser(["--no-sandbox"], poolCfg),
+    ]);
+
+    expect(launchFn).toHaveBeenCalledTimes(1);
+    expect(a.browser).toBe(b.browser);
+    expect(b.browser).toBe(c.browser);
+
+    await releaseBrowser(a.browser, poolCfg);
+    await releaseBrowser(b.browser, poolCfg);
+    await releaseBrowser(c.browser, poolCfg);
+  });
+
+  it("pool recovers from a disconnected browser", async () => {
+    const first = await acquireBrowser(["--no-sandbox"], poolCfg);
+    await releaseBrowser(first.browser, poolCfg);
+
+    // Simulate Chrome crash
+    (first.browser as unknown as { connected: boolean }).connected = false;
+
+    const freshBrowser = makeMockBrowser();
+    launchFn.mockResolvedValue(freshBrowser);
+
+    const second = await acquireBrowser(["--no-sandbox"], poolCfg);
+    expect(second.browser).toBe(freshBrowser);
+    expect(second.browser).not.toBe(first.browser);
+    expect(launchFn).toHaveBeenCalledTimes(2);
+
+    await releaseBrowser(second.browser, poolCfg);
+  });
+
+  it("release at refCount 0 closes the browser", async () => {
+    const result = await acquireBrowser(["--no-sandbox"], poolCfg);
+    const closeFn = result.browser.close as ReturnType<typeof vi.fn>;
+
+    await releaseBrowser(result.browser, poolCfg);
+    expect(closeFn).toHaveBeenCalledTimes(1);
+  });
+
+  it("pool returns a separate browser when forceScreenshot mismatches pooled mode", async () => {
+    const first = await acquireBrowser(["--no-sandbox"], poolCfg);
+    expect(first.captureMode).toBe("screenshot");
+
+    // Second acquire with same forceScreenshot — same mode, should reuse
+    const second = await acquireBrowser(["--no-sandbox"], poolCfg);
+    expect(second.browser).toBe(first.browser);
+    expect(launchFn).toHaveBeenCalledTimes(1);
+
+    await releaseBrowser(first.browser, poolCfg);
+    await releaseBrowser(second.browser, poolCfg);
+  });
+
+  it("forceReleaseBrowser does not kill Chrome when other sessions hold refs", async () => {
+    const result = await acquireBrowser(["--no-sandbox"], poolCfg);
+    // Acquire a second ref
+    const second = await acquireBrowser(["--no-sandbox"], poolCfg);
+
+    const disconnectFn = result.browser.disconnect as ReturnType<typeof vi.fn>;
+    forceReleaseBrowser(result.browser);
+
+    // Should NOT have disconnected — other session still holds a ref
+    expect(disconnectFn).not.toHaveBeenCalled();
+
+    // Release the remaining ref normally
+    await releaseBrowser(second.browser, poolCfg);
+  });
+
+  it("drainBrowserPool is safe to call when no browser is pooled", async () => {
+    await drainBrowserPool();
+  });
+
+  it("drainBrowserPool awaits in-flight launch before closing", async () => {
+    let resolveDeferred!: (browser: Browser) => void;
+    const deferred = new Promise<Browser>((resolve) => {
+      resolveDeferred = resolve;
+    });
+    launchFn.mockReturnValue(deferred);
+
+    // Start acquire — it will be pending
+    const acquirePromise = acquireBrowser(["--no-sandbox"], poolCfg);
+
+    // Drain while launch is in-flight
+    const drainPromise = drainBrowserPool();
+
+    // Resolve the pending launch
+    const mockBrowser = makeMockBrowser();
+    resolveDeferred(mockBrowser);
+
+    await drainPromise;
+    const closeFn = mockBrowser.close as ReturnType<typeof vi.fn>;
+    expect(closeFn).toHaveBeenCalled();
+
+    // The acquire should still resolve (the launch completed before drain closed it)
+    await acquirePromise.catch(() => {});
+  });
+});