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

## Summary

- **Enable browser pool by default** (`enableBrowserPool: true`) — parallel capture workers now share a single Chrome process via reference-counted pool instead of each spawning their own (~256MB each). A 6-worker render drops from 7+ browser parent processes to 1 shared pool.
- **Add launch-promise deduplication** in `acquireBrowser` — when multiple workers race into the pool simultaneously (via `Promise.all`), they await the same launch Promise instead of each triggering a separate Chrome spawn. Same pattern as the existing `_autoBrowserGpuModeCache` for GPU probes.
- **Add `connected` health check** on pool hit — if Chrome crashes mid-render, subsequent acquires detect the dead browser and launch fresh instead of returning a stale reference.
- **Add `drainBrowserPool()`** for explicit cleanup between independent render jobs.
- **CLI studio server** now uses the shared pool instead of its own redundant `enableBrowserPool: false` singleton, so thumbnail generation shares Chrome with render workers.

## Problem

The engine had a reference-counted browser pool (`browserManager.ts:73-75`) but it was **disabled by default** (`enableBrowserPool: false`). This meant:

1. **Every parallel worker spawned its own Chrome** — a `--workers 6` render launched 7+ independent Chrome processes (1 probe + 6 workers), each ~256MB.
2. **The pool had a race condition** — even if manually enabled, concurrent workers calling `acquireBrowser()` via `Promise.all` could all see `pooledBrowser === null` before the first launch completed, spawning N Chromes instead of 1.
3. **No crash recovery** — if Chrome died, the pool still held the dead reference. Subsequent acquires got a disconnected browser.
4. **CLI studio server ran its own singleton** — `studioServer.ts` explicitly set `enableBrowserPool: false` and managed a separate browser, so thumbnails and renders could never share.

Over time, orphaned Chrome processes accumulated across renders and previews. We observed **344 headless Chrome processes** consuming **569% CPU and 20% memory** on a dev machine.

## Before / After (6-worker parallel render)

| Metric | Before (pool off) | After (pool on) |
|--------|-------------------|-----------------|
| Browser parent processes | 7+ (1 probe + 6 workers) | **2** (1 GPU probe + 1 shared) |
| Total Chrome processes (with helpers) | 40-50+ | **14** |
| Memory during capture | ~20%+ | **4.6%** |
| Render time (1200 frames, 30fps) | ~64s | **53s** (~17% faster) |
| Post-render orphans | Accumulated over time | **0** |

## Changes

| File | Change |
|------|--------|
| `engine/src/config.ts` | `enableBrowserPool` default `false` → `true` |
| `engine/src/services/browserManager.ts` | Extract `launchBrowser()`, add `_pooledBrowserLaunchPromise` dedup, add `connected` check on pool hit, add `drainBrowserPool()` and `_resetBrowserPoolForTests()` |
| `engine/src/index.ts` | Export `drainBrowserPool` |
| `engine/src/services/browserManager.test.ts` | Pool dedup and drain tests |
| `cli/src/server/studioServer.ts` | Remove `enableBrowserPool: false` override — thumbnails now share the pool |
| `producer/src/services/browserManager.ts` | Re-export `drainBrowserPool` |

## Backward compatibility

- `PRODUCER_ENABLE_BROWSER_POOL=false` env var disables pooling (same as before).
- Callers passing `{ enableBrowserPool: false }` explicitly still get isolated browsers.
- Tests that set `enableBrowserPool: false` in their config fixtures continue to work.

## Test plan

- [x] Engine tests pass (597/597)
- [x] Producer tests pass (406/407, 1 pre-existing flaky test in `pngDecodeBlitWorkerPool`)
- [x] Build passes (lint, format, typecheck all green via lefthook pre-commit)
- [x] Manual render: `shortform-financial` with `--workers 6` → 1200 frames in 53s, 0 orphaned Chrome processes after completion
- [x] Process monitoring during render confirmed 2 browser parents (1 GPU probe + 1 shared pool) instead of 7+

Author: miguel-heygen

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(() => {});
+  });
+});