fix(settings): persist per-model visibility and sibling selector state (#1700)

* fix(settings): persist per-model visibility and sibling selector state The server-side settings sanitizer only allowlisted favoriteModels and recentModels, so hiddenModels, collapsedModelProviders, recentAgents, and recentEfforts were stripped on every write to settings.json — per-model visibility and collapsed-provider state silently reset on every container redeploy or settings reload. Add the four missing fields to sanitizeSettingsUpdate: - hiddenModels: sanitizeModelRefs(..., 1024) — same shape as favoriteModels; 1024 covers dense multi-provider setups while bounding persistence/memory. - collapsedModelProviders: normalizeStringArray with Array.isArray gate (matches usageDropdownProviders). - recentAgents: normalizeStringArray (Array<string> per ui-store). - recentEfforts: new sanitizeRecentEfforts validating Record<string, string[]> (shape confirmed in ui-store + addRecentEffort action); trims/dedupes keys and variants, caps at 128 keys x 5 variants/key (5 matches client slice). No ui-store version bump or migration: zustand's default merge spreads persisted state over defaults, so missing fields fall back to [] / {} until the next toggle. favoriteModels and recentModels are untouched. Tests: 8 new cases in settings-helpers.test.js using the real sanitizeModelRefs / normalizeStringArray — round-trips, empty-[] parity with favoriteModels, garbage rejection, and a full-payload regression test. * fix: sync model selector settings --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
2026-06-23 21:46:23 +03:00
parent 307808bec2
commit eae09d4576
6 changed files with 464 additions and 26 deletions
@@ -26,6 +26,9 @@ export const createSettingsHelpers = (dependencies) => {
  const SHORTCUT_OVERRIDE_VALUE_MAX_LENGTH = 128;
  const PWA_ORIENTATION_VALUES = new Set(['system', 'portrait', 'landscape']);
  const MOBILE_KEYBOARD_MODE_VALUES = new Set(['native', 'resize-content']);
+  const HIDDEN_MODELS_MAX = 1024;
+  const RECENT_EFFORTS_MAX_KEYS = 128;
+  const RECENT_EFFORTS_MAX_VARIANTS_PER_KEY = 5;

  const sanitizeShortcutOverrides = (value) => {
    if (!value || typeof value !== 'object' || Array.isArray(value)) {
@@ -41,6 +44,35 @@ export const createSettingsHelpers = (dependencies) => {
    return result;
  };

+  const sanitizeRecentEfforts = (value) => {
+    if (!value || typeof value !== 'object' || Array.isArray(value)) {
+      return null;
+    }
+    const result = {};
+    const seenKeys = new Set();
+    let count = 0;
+    for (const [rawKey, rawVariants] of Object.entries(value)) {
+      const key = typeof rawKey === 'string' ? rawKey.trim() : '';
+      if (!key || seenKeys.has(key)) continue;
+      if (!Array.isArray(rawVariants)) continue;
+      const variants = [];
+      const seenVariants = new Set();
+      for (const rawVariant of rawVariants) {
+        const variant = typeof rawVariant === 'string' ? rawVariant.trim() : '';
+        if (!variant || seenVariants.has(variant)) continue;
+        seenVariants.add(variant);
+        variants.push(variant);
+        if (variants.length >= RECENT_EFFORTS_MAX_VARIANTS_PER_KEY) break;
+      }
+      if (variants.length === 0) continue;
+      seenKeys.add(key);
+      result[key] = variants;
+      count += 1;
+      if (count >= RECENT_EFFORTS_MAX_KEYS) break;
+    }
+    return count > 0 ? result : null;
+  };
+
  const normalizePwaAppName = (value, fallback = '') => {
    if (typeof value !== 'string') {
      return fallback;
@@ -474,6 +506,28 @@ export const createSettingsHelpers = (dependencies) => {
    if (recentModels) {
      result.recentModels = recentModels;
    }
+
+    // Cap at 1024: users with several providers (anthropic, openai, google,
+    // bedrock, azure, etc.) each exposing dozens-to-hundreds of models can
+    // exceed 256 hidden entries quickly. 1024 covers dense multi-provider
+    // setups while still bounding persistence/memory.
+    const hiddenModels = sanitizeModelRefs(candidate.hiddenModels, HIDDEN_MODELS_MAX);
+    if (hiddenModels) {
+      result.hiddenModels = hiddenModels;
+    }
+
+    if (Array.isArray(candidate.collapsedModelProviders)) {
+      result.collapsedModelProviders = normalizeStringArray(candidate.collapsedModelProviders);
+    }
+
+    if (Array.isArray(candidate.recentAgents)) {
+      result.recentAgents = normalizeStringArray(candidate.recentAgents);
+    }
+
+    const recentEfforts = sanitizeRecentEfforts(candidate.recentEfforts);
+    if (recentEfforts) {
+      result.recentEfforts = recentEfforts;
+    }
    if (typeof candidate.diffLayoutPreference === 'string') {
      const mode = candidate.diffLayoutPreference.trim();
      if (mode === 'dynamic' || mode === 'inline' || mode === 'side-by-side') {
@@ -1,6 +1,7 @@
 import { describe, expect, it } from 'vitest';

 import { createSettingsHelpers } from './settings-helpers.js';
+import { createSettingsNormalizationRuntime } from './settings-normalization-runtime.js';

 const createTestHelpers = () => createSettingsHelpers({
  normalizePathForPersistence: (value) => value,
@@ -20,6 +21,42 @@ const createTestHelpers = () => createSettingsHelpers({
  sanitizeProjects: () => undefined,
 });

+const createTestHelpersWithRealSanitizers = () => {
+  const runtime = createSettingsNormalizationRuntime({
+    os: { homedir: () => '/home/testuser' },
+    path: {
+      resolve: (...args) => args[args.length - 1],
+      sep: '/',
+      dirname: (p) => p.split('/').slice(0, -1).join('/') || '/',
+    },
+    processLike: { platform: 'linux', env: {} },
+    realpathSync: (p) => p,
+    tunnelBootstrapTtlDefaultMs: 600000,
+    tunnelBootstrapTtlMinMs: 60000,
+    tunnelBootstrapTtlMaxMs: 3600000,
+    tunnelSessionTtlDefaultMs: 86400000,
+    tunnelSessionTtlMinMs: 3600000,
+    tunnelSessionTtlMaxMs: 604800000,
+  });
+  return createSettingsHelpers({
+    normalizePathForPersistence: (value) => value,
+    normalizeDirectoryPath: (value) => value,
+    normalizeTunnelBootstrapTtlMs: (value) => value,
+    normalizeTunnelSessionTtlMs: (value) => value,
+    normalizeTunnelProvider: (value) => value,
+    normalizeTunnelMode: (value) => value,
+    normalizeOptionalPath: (value) => value,
+    normalizeManagedRemoteTunnelHostname: (value) => value,
+    normalizeManagedRemoteTunnelPresets: () => undefined,
+    normalizeManagedRemoteTunnelPresetTokens: () => undefined,
+    sanitizeTypographySizesPartial: () => undefined,
+    normalizeStringArray: runtime.normalizeStringArray,
+    sanitizeModelRefs: runtime.sanitizeModelRefs,
+    sanitizeSkillCatalogs: () => undefined,
+    sanitizeProjects: () => undefined,
+  });
+};
+
 describe('settings helpers', () => {
  it('accepts messageStreamTransport as a persisted shared setting', () => {
    const helpers = createTestHelpers();
@@ -188,4 +225,121 @@ describe('settings helpers', () => {
      else delete process.env.OPENCHAMBER_DESKTOP_LAN_ACCESS_BLOCKED_REASON;
    }
  });
+
+  describe('previously-dropped model selector persistence fields', () => {
+    it('round-trips hiddenModels through the sanitizer', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+      const input = [
+        { providerID: 'anthropic', modelID: 'claude-opus-4' },
+        { providerID: 'openai', modelID: 'gpt-5' },
+      ];
+
+      expect(helpers.sanitizeSettingsUpdate({ hiddenModels: input })).toEqual({
+        hiddenModels: input,
+      });
+    });
+
+    it('handles empty hiddenModels the same way as empty favoriteModels', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+
+      const hiddenResult = helpers.sanitizeSettingsUpdate({ hiddenModels: [] });
+      const favoriteResult = helpers.sanitizeSettingsUpdate({ favoriteModels: [] });
+
+      expect(hiddenResult.hiddenModels).toEqual([]);
+      expect(favoriteResult.favoriteModels).toEqual([]);
+      expect(hiddenResult.hiddenModels).toEqual(favoriteResult.favoriteModels);
+    });
+
+    it('round-trips collapsedModelProviders and recentAgents as string arrays', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+
+      expect(helpers.sanitizeSettingsUpdate({ collapsedModelProviders: ['anthropic', 'openai'] })).toEqual({
+        collapsedModelProviders: ['anthropic', 'openai'],
+      });
+      expect(helpers.sanitizeSettingsUpdate({ recentAgents: ['build', 'plan'] })).toEqual({
+        recentAgents: ['build', 'plan'],
+      });
+    });
+
+    it('round-trips recentEfforts as a Record<string, string[]>', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+      const input = {
+        'anthropic/claude-opus-4': ['high', 'default'],
+        'openai/gpt-5': ['low'],
+      };
+
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: input })).toEqual({
+        recentEfforts: input,
+      });
+    });
+
+    it('rejects garbage hiddenModels input the same way sanitizeModelRefs rejects bad refs', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+
+      expect(helpers.sanitizeSettingsUpdate({ hiddenModels: 'not-an-array' })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ hiddenModels: null })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ hiddenModels: 123 })).toEqual({});
+      expect(
+        helpers.sanitizeSettingsUpdate({
+          hiddenModels: [
+            { providerID: 'anthropic' },
+            { modelID: 'gpt-5' },
+            'not-an-object',
+            null,
+            { providerID: '  ', modelID: 'x' },
+            { providerID: 'openai', modelID: '' },
+          ],
+        })
+      ).toEqual({ hiddenModels: [] });
+    });
+
+    it('rejects garbage collapsedModelProviders and recentAgents input', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+
+      expect(helpers.sanitizeSettingsUpdate({ collapsedModelProviders: 'anthropic' })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ collapsedModelProviders: null })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentAgents: 42 })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentAgents: { build: 1 } })).toEqual({});
+    });
+
+    it('rejects garbage recentEfforts input', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: 'not-an-object' })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: [] })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: null })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: { 'anthropic/claude-opus-4': 'high' } })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: { '': ['high'] } })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: { 'anthropic/claude-opus-4': [] } })).toEqual({});
+      expect(helpers.sanitizeSettingsUpdate({ recentEfforts: { 'anthropic/claude-opus-4': [123, ''] } })).toEqual({});
+    });
+
+    it('survives a full settings.json payload containing all four previously-dropped fields (regression)', () => {
+      const helpers = createTestHelpersWithRealSanitizers();
+      const payload = {
+        themeId: 'default',
+        hiddenModels: [
+          { providerID: 'anthropic', modelID: 'claude-opus-4' },
+          { providerID: 'openai', modelID: 'gpt-5' },
+        ],
+        collapsedModelProviders: ['anthropic', 'openai'],
+        recentAgents: ['build', 'plan'],
+        recentEfforts: {
+          'anthropic/claude-opus-4': ['high', 'default'],
+          'openai/gpt-5': ['low'],
+        },
+        favoriteModels: [{ providerID: 'anthropic', modelID: 'claude-haiku-4' }],
+        recentModels: [{ providerID: 'openai', modelID: 'gpt-5' }],
+      };
+
+      const sanitized = helpers.sanitizeSettingsUpdate(payload);
+
+      expect(sanitized.hiddenModels).toEqual(payload.hiddenModels);
+      expect(sanitized.collapsedModelProviders).toEqual(payload.collapsedModelProviders);
+      expect(sanitized.recentAgents).toEqual(payload.recentAgents);
+      expect(sanitized.recentEfforts).toEqual(payload.recentEfforts);
+      expect(sanitized.favoriteModels).toEqual(payload.favoriteModels);
+      expect(sanitized.recentModels).toEqual(payload.recentModels);
+    });
+  });
 });