fix(renderer): WebGL longtask fallback hardening (#89)

src/web/public/app.js

@@ -625,6 +625,7 @@ class CodemanApp {
         console.error('[CRASH-DIAG] WebGL context LOST — falling back to canvas renderer');
         _crashDiag.log('WEBGL_LOST');
         this._disableWebGLSticky('context-lost');
+        this._disposeWebGLObserver();
         this._webglAddon?.dispose();
         this._webglAddon = null;
       });
@@ -636,9 +637,10 @@ class CodemanApp {
 
   /**
    * Watch for sustained main-thread stalls that indicate WebGL/GPU trouble.
-   * After 3 long tasks (>=200ms each) within 30s, dispose the WebGL addon and
-   * persist a sticky disable so subsequent reloads also use the DOM renderer.
-   * 5s grace period skips initial-load stalls. Force-re-enable: ?webgl=force.
+   * After WEBGL_FALLBACK.LONGTASK_COUNT long tasks (>=LONGTASK_MS each) within
+   * WINDOW_MS, dispose the WebGL addon and persist a sticky disable so
+   * subsequent reloads also use the DOM renderer. GRACE_MS skips initial-load
+   * stalls. Force-re-enable: ?webgl=force.
    */
   _installWebGLLongTaskGuard() {
     if (typeof PerformanceObserver === 'undefined' || this._webglLongTaskObserver) return;
@@ -648,26 +650,33 @@ class CodemanApp {
       this._webglLongTaskObserver = new PerformanceObserver((list) => {
         if (!this._webglAddon) return;
         const now = performance.now();
-        if (now - installedAt < 5000) return;
-        for (const entry of list.getEntries()) {
-          if (entry.duration >= 200) recent.push(entry.startTime);
-        }
-        while (recent.length && now - recent[0] > 30000) recent.shift();
-        if (recent.length >= 3) {
-          console.warn(`[CRASH-DIAG] WebGL long-task threshold (${recent.length} stalls/30s) — falling back to canvas renderer`);
+        if (now - installedAt < WEBGL_FALLBACK.GRACE_MS) return;
+        if (evaluateWebGLLongTaskTrip(recent, list.getEntries(), now)) {
+          console.warn(`[CRASH-DIAG] WebGL long-task threshold (${recent.length} stalls/${WEBGL_FALLBACK.WINDOW_MS}ms) — falling back to canvas renderer`);
           _crashDiag.log(`WEBGL_FALLBACK: ${recent.length}`);
           this._disableWebGLSticky('long-tasks');
+          this._disposeWebGLObserver();
           this._webglAddon?.dispose();
           this._webglAddon = null;
-          try { this._webglLongTaskObserver.disconnect(); } catch {}
-          this._webglLongTaskObserver = null;
           try { this.terminal.refresh(0, this.terminal.rows - 1); } catch {}
         }
       });
       this._webglLongTaskObserver.observe({ type: 'longtask', buffered: false });
     } catch { /* longtask not supported */ }
   }
 
+  /**
+   * Disconnect the WebGL longtask observer. Idempotent. Called from the trip
+   * path, the onContextLoss handler, and any future terminal-teardown path —
+   * the observer outlives its addon otherwise, holding a closure reference
+   * over `this` for every long task the page emits.
+   */
+  _disposeWebGLObserver() {
+    if (!this._webglLongTaskObserver) return;
+    try { this._webglLongTaskObserver.disconnect(); } catch {}
+    this._webglLongTaskObserver = null;
+  }
+
   _disableWebGLSticky(reason) {
     try {
       localStorage.setItem('codeman-webgl-disabled', JSON.stringify({ reason, at: Date.now() }));

src/web/public/constants.js

@@ -71,6 +71,52 @@ const WINDOW_MIN_WIDTH_PX = 200;
 const WINDOW_MIN_HEIGHT_PX = 200;
 const WINDOW_DEFAULT_WIDTH_PX = 300;
 
+// WebGL renderer auto-fallback thresholds.
+// _installWebGLLongTaskGuard() observes longtask entries and disables WebGL
+// after LONGTASK_COUNT stalls of >= LONGTASK_MS within WINDOW_MS. GRACE_MS
+// suppresses the noisy initial-load stalls. STICKY_EXPIRY_MS is how long
+// localStorage's webgl-disabled marker survives before we retry WebGL on a
+// fresh load (driver/Chrome may have been updated).
+const WEBGL_FALLBACK = {
+  LONGTASK_MS: 200,
+  LONGTASK_COUNT: 3,
+  WINDOW_MS: 30000,
+  GRACE_MS: 5000,
+  STICKY_EXPIRY_MS: 7 * 24 * 60 * 60 * 1000,
+};
+
+/**
+ * Pure rolling-window trip evaluator for the WebGL longtask guard.
+ * Mutates `recent` in place (prunes entries older than `now - WINDOW_MS`)
+ * and appends each new duration's startTime that meets the threshold.
+ * Returns true when the count inside the window reaches `LONGTASK_COUNT`.
+ *
+ * Exposed on `window` for unit testing — the production guard in app.js
+ * inlines this same logic in its PerformanceObserver callback. Splitting it
+ * out keeps the threshold math testable without a real PerformanceObserver.
+ *
+ * @param {number[]} recent - mutable array of startTimes inside the window
+ * @param {{startTime: number, duration: number}[]} entries - new longtask entries
+ * @param {number} now - performance.now() at evaluation time
+ * @param {typeof WEBGL_FALLBACK} [config=WEBGL_FALLBACK] - thresholds
+ * @returns {boolean} true if the rolling window has reached the trip count
+ */
+function evaluateWebGLLongTaskTrip(recent, entries, now, config = WEBGL_FALLBACK) {
+  for (const entry of entries) {
+    if (entry.duration >= config.LONGTASK_MS) recent.push(entry.startTime);
+  }
+  while (recent.length && now - recent[0] > config.WINDOW_MS) recent.shift();
+  return recent.length >= config.LONGTASK_COUNT;
+}
+
+// Expose for tests. `const` declarations at the top of a non-module script
+// are global lexical bindings but not `window` properties, so explicit
+// assignment is the test-visible API surface.
+if (typeof window !== 'undefined') {
+  window.WEBGL_FALLBACK = WEBGL_FALLBACK;
+  window.evaluateWebGLLongTaskTrip = evaluateWebGLLongTaskTrip;
+}
+
 // Scheduler API — prioritize terminal writes over background UI updates.
 // scheduler.postTask('background') defers non-critical work (connection lines, panel renders)
 // so the main thread stays free for terminal rendering at 60fps.

src/web/public/terminal-ui.js

@@ -197,8 +197,9 @@ Object.assign(CodemanApp.prototype, {
         const raw = localStorage.getItem('codeman-webgl-disabled');
         if (!raw) return false;
         const { at } = JSON.parse(raw);
-        // Auto-expire after 7 days so we retry (driver may have been fixed)
-        if (Date.now() - at > 7 * 24 * 60 * 60 * 1000) {
+        // Auto-expire after WEBGL_FALLBACK.STICKY_EXPIRY_MS so we retry
+        // (driver/Chrome may have been updated).
+        if (Date.now() - at > WEBGL_FALLBACK.STICKY_EXPIRY_MS) {
           localStorage.removeItem('codeman-webgl-disabled');
           return false;
         }

test/webgl-fallback.test.ts

@@ -0,0 +1,218 @@
+/**
+ * WebGL longtask auto-fallback tests.
+ *
+ * Covers the three follow-ups from #89:
+ *   1. Pure trip-detection helper — rolling-window arithmetic for the
+ *      "N longtasks of >=Xms within Yms" trip condition. Unit-tested
+ *      independently of PerformanceObserver, which can't be driven
+ *      deterministically from JS (entries arrive from the platform).
+ *   2. Constants are hoisted from inline literals to `WEBGL_FALLBACK`
+ *      in constants.js — assert they exist with the documented values.
+ *   3. Observer disconnect — _disposeWebGLObserver() is idempotent and
+ *      can be called from the onContextLoss path without the addon
+ *      having been initialised. Mirrors the leak case in the issue:
+ *      "observer outlives its addon" when teardown precedes a trip.
+ *
+ * Strategy: load the static app shell in a headless browser and drive
+ * the helper through page.evaluate(). No real PTY/tmux/WebGL needed —
+ * the trip math is pure and the dispose path is a couple of property
+ * mutations, both of which run on any page where app.js loaded.
+ *
+ * Port: 3166 (per MEMORY.md, ports 3150+ for tests)
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { chromium, type Browser, type Page } from 'playwright';
+import { WebServer } from '../src/web/server.js';
+
+const PORT = 3166;
+const BASE_URL = `http://localhost:${PORT}`;
+
+describe('WebGL longtask auto-fallback', () => {
+  let server: WebServer;
+  let browser: Browser;
+  let page: Page;
+
+  beforeAll(async () => {
+    server = new WebServer(PORT, false, true);
+    await server.start();
+    browser = await chromium.launch({ headless: true });
+    page = await browser.newPage();
+    await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
+    // Wait for constants.js + app.js to have loaded — both expose globals.
+    await page.waitForFunction(
+      () =>
+        typeof (window as { WEBGL_FALLBACK?: unknown }).WEBGL_FALLBACK !== 'undefined' &&
+        typeof (window as { evaluateWebGLLongTaskTrip?: unknown }).evaluateWebGLLongTaskTrip === 'function' &&
+        typeof (window as { app?: unknown }).app !== 'undefined'
+    );
+  }, 60000);
+
+  afterAll(async () => {
+    if (browser) await browser.close();
+    if (server) await server.stop();
+  }, 60000);
+
+  describe('constants are hoisted', () => {
+    it('WEBGL_FALLBACK exposes documented thresholds', async () => {
+      const cfg = await page.evaluate(
+        () => (window as unknown as { WEBGL_FALLBACK: Record<string, number> }).WEBGL_FALLBACK
+      );
+      expect(cfg).toEqual({
+        LONGTASK_MS: 200,
+        LONGTASK_COUNT: 3,
+        WINDOW_MS: 30000,
+        GRACE_MS: 5000,
+        STICKY_EXPIRY_MS: 7 * 24 * 60 * 60 * 1000,
+      });
+    });
+  });
+
+  describe('evaluateWebGLLongTaskTrip — rolling window arithmetic', () => {
+    type EvalFn = (
+      recent: number[],
+      entries: { startTime: number; duration: number }[],
+      now: number
+    ) => { tripped: boolean; recent: number[] };
+
+    /** Run the pure helper in the page and return the post-call state. */
+    const run: EvalFn = async (recent, entries, now) =>
+      page.evaluate(
+        ({ r, e, n }) => {
+          const fn = (
+            window as unknown as {
+              evaluateWebGLLongTaskTrip: (
+                rec: number[],
+                ents: { startTime: number; duration: number }[],
+                now: number
+              ) => boolean;
+            }
+          ).evaluateWebGLLongTaskTrip;
+          const recent = [...r];
+          const tripped = fn(recent, e, n);
+          return { tripped, recent };
+        },
+        { r: recent, e: entries, n: now }
+      ) as unknown as { tripped: boolean; recent: number[] };
+
+    it('trips when 3 longtasks fall inside the 30s window', async () => {
+      const entries = [
+        { startTime: 1000, duration: 250 },
+        { startTime: 5000, duration: 300 },
+        { startTime: 10000, duration: 220 },
+      ];
+      const result = await run([], entries, 12000);
+      expect(result.tripped).toBe(true);
+      expect(result.recent).toEqual([1000, 5000, 10000]);
+    });
+
+    it('does not trip when 3 longtasks are spread across 60s', async () => {
+      // 3 longtasks 25s apart — only the most recent two stay inside 30s.
+      const entries = [
+        { startTime: 1000, duration: 250 },
+        { startTime: 26000, duration: 250 },
+        { startTime: 51000, duration: 250 },
+      ];
+      const result = await run([], entries, 51100);
+      expect(result.tripped).toBe(false);
+      // First entry pruned (1000 is >30s before now=51100); 26000 and 51000 stay.
+      expect(result.recent).toEqual([26000, 51000]);
+    });
+
+    it('ignores entries shorter than 200ms', async () => {
+      const entries = [
+        { startTime: 1000, duration: 199 },
+        { startTime: 2000, duration: 100 },
+        { startTime: 3000, duration: 50 },
+      ];
+      const result = await run([], entries, 3500);
+      expect(result.tripped).toBe(false);
+      expect(result.recent).toEqual([]);
+    });
+
+    it('prunes stale entries even when no new ones arrive', async () => {
+      // Existing window has 2 stale + 1 fresh; an empty batch should still
+      // age out the stale ones so the next real batch evaluates correctly.
+      const recent = [1000, 5000, 40000];
+      const result = await run(recent, [], 41000);
+      expect(result.tripped).toBe(false);
+      expect(result.recent).toEqual([40000]);
+    });
+
+    it('counts entries cumulatively across batches', async () => {
+      // Two batches of 2 entries each, all inside the window — second
+      // batch should push the cumulative count to 4 and trip.
+      const recent: number[] = [];
+      const first = await run(
+        recent,
+        [
+          { startTime: 1000, duration: 250 },
+          { startTime: 2000, duration: 250 },
+        ],
+        3000
+      );
+      expect(first.tripped).toBe(false);
+      expect(first.recent).toEqual([1000, 2000]);
+
+      const second = await run(
+        first.recent,
+        [
+          { startTime: 4000, duration: 250 },
+          { startTime: 5000, duration: 250 },
+        ],
+        6000
+      );
+      expect(second.tripped).toBe(true);
+      expect(second.recent).toEqual([1000, 2000, 4000, 5000]);
+    });
+  });
+
+  describe('_disposeWebGLObserver', () => {
+    it('is idempotent — safe to call when no observer was installed', async () => {
+      const ok = await page.evaluate(() => {
+        const app = (
+          window as unknown as { app: { _disposeWebGLObserver: () => void; _webglLongTaskObserver: unknown } }
+        ).app;
+        app._webglLongTaskObserver = null;
+        app._disposeWebGLObserver();
+        app._disposeWebGLObserver();
+        return app._webglLongTaskObserver === null;
+      });
+      expect(ok).toBe(true);
+    });
+
+    it('disconnects a stub observer and nulls the reference', async () => {
+      const result = await page.evaluate(() => {
+        const app = (
+          window as unknown as { app: { _disposeWebGLObserver: () => void; _webglLongTaskObserver: unknown } }
+        ).app;
+        let disconnectCalls = 0;
+        app._webglLongTaskObserver = {
+          disconnect() {
+            disconnectCalls++;
+          },
+        } as unknown;
+        app._disposeWebGLObserver();
+        return { disconnectCalls, ref: app._webglLongTaskObserver };
+      });
+      expect(result.disconnectCalls).toBe(1);
+      expect(result.ref).toBeNull();
+    });
+
+    it('swallows a throwing disconnect — guards against driver quirks', async () => {
+      const result = await page.evaluate(() => {
+        const app = (
+          window as unknown as { app: { _disposeWebGLObserver: () => void; _webglLongTaskObserver: unknown } }
+        ).app;
+        app._webglLongTaskObserver = {
+          disconnect() {
+            throw new Error('synthetic');
+          },
+        } as unknown;
+        app._disposeWebGLObserver();
+        return app._webglLongTaskObserver;
+      });
+      expect(result).toBeNull();
+    });
+  });
+});