fix(review): make WebGL toggle per-device and fix sticky-marker semantics (PR #140)

- saveAppSettings no longer sends webglRendererEnabled on the settings PUT:
  the key is absent from the .strict() SettingsUpdateSchema, so every save
  400'd with INVALID_INPUT, silently killing all server-side settings
  persistence. Stripped in the per-device destructure alongside
  localEchoEnabled/skin/etc.
- shouldSkipWebGL now treats a stored true like the untouched default w.r.t.
  the sticky marker: the checkbox defaults checked on desktop, so any
  unrelated save stored true and every page load then cleared the
  'codeman-webgl-disabled' marker, permanently defeating the GPU-stall
  auto-fallback. Only ?webgl=force clears the marker at init.
- The marker is instead retired on a real OFF->ON toggle flip detected at
  save time (mirrors the _prevGestureEnabled pattern in settings-ui.js).
- webglRendererEnabled added to the displayKeys per-device set in
  loadAppSettingsFromServer (renderer choice is device/GPU-specific; syncing
  would leak mobile's hidden-checkbox false onto desktop).
- Tests: stored true + sticky marker -> still skips WebGL; OFF->ON save
  clears the marker and keeps the key off the wire; default-checked save
  leaves the marker alone; ?webgl=force / ?nowebgl behavior unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/web/public/constants.js

@@ -120,8 +120,14 @@ function evaluateWebGLLongTaskTrip(recent, entries, now, config = WEBGL_FALLBACK
  * Precedence (desktop only — mobile always skips):
  *   1. user toggle OFF        -> skip (one-shot opt-out, sticky untouched)
  *   2. ?nowebgl               -> skip (one-shot opt-out, sticky untouched)
- *   3. user toggle ON / ?webgl=force -> enable + clear stale sticky marker
- *   4. untouched (default on) -> respect the auto-fallback sticky marker
+ *   3. ?webgl=force           -> enable + clear stale sticky marker
+ *   4. toggle ON / untouched  -> respect the auto-fallback sticky marker
+ *
+ * A stored `true` is treated like the untouched default here: the checkbox
+ * ships checked on desktop, so any unrelated settings save stores `true` —
+ * letting it clear the marker would permanently defeat the GPU-stall
+ * auto-fallback safety net. The marker is only retired by ?webgl=force or by
+ * a real OFF->ON toggle flip, which saveAppSettings() detects at save time.
  *
  * @param {{deviceType?: string, noWebglParam?: boolean, forceParam?: boolean,
  *          stickyDisabled?: boolean, userPrefEnabled?: (boolean|undefined)}} [input]
@@ -129,10 +135,9 @@ function evaluateWebGLLongTaskTrip(recent, entries, now, config = WEBGL_FALLBACK
  */
 function shouldSkipWebGL(input = {}) {
   if (input.deviceType !== 'desktop') return { skip: true, clearSticky: false };
-  const pref = input.userPrefEnabled; // true | false | undefined (default on)
-  if (pref === false) return { skip: true, clearSticky: false };
+  if (input.userPrefEnabled === false) return { skip: true, clearSticky: false };
   if (input.noWebglParam) return { skip: true, clearSticky: false };
-  if (pref === true || input.forceParam) return { skip: false, clearSticky: true };
+  if (input.forceParam) return { skip: false, clearSticky: true };
   return { skip: !!input.stickyDisabled, clearSticky: false };
 }
 

src/web/public/settings-ui.js

@@ -1359,6 +1359,9 @@ Object.assign(CodemanApp.prototype, {
     // only takes effect on reload — remember the prior value to decide below.
     const _prev = this.loadAppSettingsFromStorage();
     const _prevGestureEnabled = (_prev.gestureControlEnabled ?? false) === true;
+    // WebGL toggle: default ON (desktop), so only an explicit stored false counts
+    // as "previously off" — used below to detect a real OFF→ON flip.
+    const _prevWebglEnabled = (_prev.webglRendererEnabled ?? true) === true;
     const settings = {
       defaultClaudeMdPath: document.getElementById('appSettingsClaudeMdPath').value.trim(),
       defaultWorkingDir: document.getElementById('appSettingsDefaultDir').value.trim(),
@@ -1417,6 +1420,15 @@ Object.assign(CodemanApp.prototype, {
     this.saveAppSettingsToStorage(settings);
     this._updateLocalEchoState();
 
+    // A real OFF→ON flip of the WebGL toggle retires the GPU-stall auto-fallback
+    // marker so the next reload actually re-tries WebGL. Only the transition
+    // clears it — an incidental save with the checkbox default-checked must NOT
+    // defeat the sticky safety net (shouldSkipWebGL treats stored true like the
+    // untouched default at page load).
+    if (!_prevWebglEnabled && settings.webglRendererEnabled) {
+      try { localStorage.removeItem('codeman-webgl-disabled'); } catch {}
+    }
+
     // Save voice settings to localStorage + include in server payload for cross-device sync
     const voiceSettings = {
       apiKey: document.getElementById('voiceDeepgramKey').value.trim(),
@@ -1527,6 +1539,10 @@ Object.assign(CodemanApp.prototype, {
     // Strip device-specific DISPLAY keys so they never sync across devices —
     // localEcho/cjk/extendedKeyboard/skin are per-platform, and showPlanUsageLimits
     // is per-device too (desktop can show the usage chip while mobile stays hidden).
+    // webglRendererEnabled is per-device as well (renderer choice is GPU-specific,
+    // and syncing would leak mobile's hidden-checkbox false onto desktop); it's
+    // also absent from SettingsUpdateSchema, which is .strict() — sending it
+    // would 400 the whole settings PUT.
     // Telemetry COLLECTION is requested out-of-band via statusLineTelemetry (sent on
     // ENABLE only, so a device with the chip OFF never strips the exporter that
     // another device's chip depends on — see system-routes settings handler).
@@ -1537,6 +1553,7 @@ Object.assign(CodemanApp.prototype, {
       skin: _skin,
       showPlanUsageLimits: _pul,
       showAttachmentsButton: _ahb,
+      webglRendererEnabled: _wgl,
       ...serverSettings
     } = settings;
     try {
@@ -2037,7 +2054,7 @@ Object.assign(CodemanApp.prototype, {
           'showLifecycleLog', 'showResponseViewer',
           'showMonitor', 'showProjectInsights', 'showFileBrowser', 'showSubagents',
           'subagentActiveTabOnly', 'tabTwoRows', 'localEchoEnabled', 'cjkInputEnabled', 'extendedKeyboardBar',
-          'skin', 'showPlanUsageLimits', 'showAttachmentsButton',
+          'skin', 'showPlanUsageLimits', 'showAttachmentsButton', 'webglRendererEnabled',
         ]);
         // The plan-usage chip is a PER-DEVICE display setting (default OFF): desktop
         // can show it while mobile stays hidden. It used to sync, so an older

src/web/public/terminal-ui.js

@@ -272,7 +272,9 @@ Object.assign(CodemanApp.prototype, {
       stickyDisabled: _stickyDisabled,
       userPrefEnabled: _webglPref,
     });
-    // Explicit opt-in (toggle ON) or ?webgl=force retires a stale auto-fallback marker.
+    // Only ?webgl=force retires the auto-fallback marker at init — a stored
+    // toggle ON is incidental (checkbox defaults checked) and must not defeat
+    // the sticky safety net. An OFF→ON flip clears it in saveAppSettings().
     if (_clearWebglSticky) {
       try { localStorage.removeItem('codeman-webgl-disabled'); } catch {}
     }

test/webgl-fallback.test.ts

@@ -236,9 +236,7 @@ describe('WebGL longtask auto-fallback', () => {
       );
 
     it('exposes shouldSkipWebGL on window', async () => {
-      const t = await page.evaluate(
-        () => typeof (window as unknown as { shouldSkipWebGL?: unknown }).shouldSkipWebGL
-      );
+      const t = await page.evaluate(() => typeof (window as unknown as { shouldSkipWebGL?: unknown }).shouldSkipWebGL);
       expect(t).toBe('function');
     });
 
@@ -263,10 +261,19 @@ describe('WebGL longtask auto-fallback', () => {
       });
     });
 
-    it('explicit opt-in enables and clears a stale sticky marker', async () => {
+    it('stored toggle ON still respects the sticky marker (incidental saves must not defeat auto-fallback)', async () => {
+      // The checkbox defaults checked on desktop, so any unrelated settings save
+      // stores true — that must NOT act like ?webgl=force and clear the marker.
       expect(await run({ deviceType: 'desktop', userPrefEnabled: true, stickyDisabled: true })).toEqual({
+        skip: true,
+        clearSticky: false,
+      });
+    });
+
+    it('stored toggle ON enables WebGL when no sticky marker is set', async () => {
+      expect(await run({ deviceType: 'desktop', userPrefEnabled: true, stickyDisabled: false })).toEqual({
         skip: false,
-        clearSticky: true,
+        clearSticky: false,
       });
     });
 
@@ -291,4 +298,70 @@ describe('WebGL longtask auto-fallback', () => {
       });
     });
   });
+
+  describe('saveAppSettings — per-device WebGL toggle semantics', () => {
+    type SaveHarnessApp = {
+      loadAppSettingsFromStorage: () => Record<string, unknown>;
+      saveAppSettingsToStorage: (s: Record<string, unknown>) => void;
+      saveAppSettings: () => Promise<void>;
+      _apiPut: (path: string, body: Record<string, unknown>) => Promise<unknown>;
+      saveModelConfigFromSettings: () => Promise<void>;
+      _syncPushPreferences: () => void;
+    };
+
+    /**
+     * Drive the real saveAppSettings() in the page with the settings PUT stubbed
+     * out. Seeds the stored blob (via saveAppSettingsToStorage so the in-memory
+     * cache stays coherent), sets the sticky marker + checkbox, saves, and
+     * reports what happened to the marker and the captured server payload.
+     */
+    const runSave = async (opts: { storedPref: boolean | undefined }) =>
+      page.evaluate(async (o) => {
+        const app = (window as unknown as { app: SaveHarnessApp }).app;
+        const prevStored = { ...app.loadAppSettingsFromStorage() };
+        const origPut = app._apiPut;
+        const origModel = app.saveModelConfigFromSettings;
+        const origSync = app._syncPushPreferences;
+        let captured: Record<string, unknown> | null = null;
+        try {
+          const seeded = { ...prevStored };
+          if (o.storedPref === undefined) delete seeded.webglRendererEnabled;
+          else seeded.webglRendererEnabled = o.storedPref;
+          app.saveAppSettingsToStorage(seeded);
+          localStorage.setItem('codeman-webgl-disabled', JSON.stringify({ at: Date.now() }));
+          (document.getElementById('appSettingsWebglRenderer') as HTMLInputElement).checked = true;
+          app._apiPut = async (_path, body) => {
+            captured = body;
+            return { ok: true };
+          };
+          app.saveModelConfigFromSettings = async () => {};
+          app._syncPushPreferences = () => {};
+          await app.saveAppSettings();
+          return {
+            markerCleared: localStorage.getItem('codeman-webgl-disabled') === null,
+            payloadHasWebglKey: captured ? 'webglRendererEnabled' in captured : null,
+          };
+        } finally {
+          app._apiPut = origPut;
+          app.saveModelConfigFromSettings = origModel;
+          app._syncPushPreferences = origSync;
+          localStorage.removeItem('codeman-webgl-disabled');
+          app.saveAppSettingsToStorage(prevStored);
+        }
+      }, opts);
+
+    it('a real OFF→ON flip clears the sticky marker and keeps the key off the settings PUT', async () => {
+      const result = await runSave({ storedPref: false });
+      expect(result.markerCleared).toBe(true);
+      expect(result.payloadHasWebglKey).toBe(false);
+    });
+
+    it('a save with the toggle merely default-checked leaves the sticky marker alone', async () => {
+      // No stored OFF → this is the "unrelated settings save stores true" case;
+      // clearing here would permanently defeat the GPU-stall auto-fallback.
+      const result = await runSave({ storedPref: undefined });
+      expect(result.markerCleared).toBe(false);
+      expect(result.payloadHasWebglKey).toBe(false);
+    });
+  });
 });