diff --git a/packages/ui/src/components/sections/skills/catalog/AddCatalogDialog.tsx b/packages/ui/src/components/sections/skills/catalog/AddCatalogDialog.tsx index bcc00670..42b1c3d7 100644 --- a/packages/ui/src/components/sections/skills/catalog/AddCatalogDialog.tsx +++ b/packages/ui/src/components/sections/skills/catalog/AddCatalogDialog.tsx @@ -85,6 +85,8 @@ export const AddCatalogDialog: React.FC = ({ open, onOpen const { t } = useI18n(); const scanRepo = useSkillsCatalogStore((s) => s.scanRepo); const loadCatalog = useSkillsCatalogStore((s) => s.loadCatalog); + const loadSource = useSkillsCatalogStore((s) => s.loadSource); + const setSelectedSource = useSkillsCatalogStore((s) => s.setSelectedSource); const isScanning = useSkillsCatalogStore((s) => s.isScanning); const defaultGitIdentityId = useGitIdentitiesStore((s) => s.defaultGitIdentityId); const loadDefaultGitIdentityId = useGitIdentitiesStore((s) => s.loadDefaultGitIdentityId); @@ -248,6 +250,8 @@ export const AddCatalogDialog: React.FC = ({ open, onOpen setExistingCatalogs(updated); toast.success(t('settings.skills.catalog.add.toast.catalogAdded')); await loadCatalog({ refresh: true }); + await loadSource(next.id, { refresh: true }); + setSelectedSource(next.id); onOpenChange(false); } catch (error) { toast.error(error instanceof Error ? error.message : t('settings.skills.catalog.add.toast.saveFailed')); diff --git a/packages/ui/src/lib/persistence.test.ts b/packages/ui/src/lib/persistence.test.ts index 2d63e89e..5f11b95f 100644 --- a/packages/ui/src/lib/persistence.test.ts +++ b/packages/ui/src/lib/persistence.test.ts @@ -1,10 +1,41 @@ import { afterAll, beforeEach, describe, expect, test } from 'bun:test'; -import { applyPersistedHomeDirectoryToWindow } from './persistence'; +import type { RuntimeAPIs, SettingsPayload } from '@/lib/api/types'; +import { registerRuntimeAPIs } from '@/contexts/runtimeAPIRegistry'; +import { applyPersistedHomeDirectoryToWindow, updateDesktopSettings } from './persistence'; -type TestWindow = { __OPENCHAMBER_HOME__?: string }; +type TestWindow = { + __OPENCHAMBER_HOME__?: string; + dispatchEvent: (event: Event) => boolean; +}; let createdWindow = false; +let createdLocalStorage = false; + +const ensureLocalStorage = (): void => { + if (typeof localStorage !== 'undefined') { + return; + } + + const values = new Map(); + Object.defineProperty(globalThis, 'localStorage', { + value: { + getItem: (key: string) => values.get(key) ?? null, + setItem: (key: string, value: string) => { + values.set(key, value); + }, + removeItem: (key: string) => { + values.delete(key); + }, + clear: () => { + values.clear(); + }, + }, + configurable: true, + writable: true, + }); + createdLocalStorage = true; +}; const getWindow = (): TestWindow => { if (typeof window === 'undefined') { @@ -15,22 +46,41 @@ const getWindow = (): TestWindow => { }); createdWindow = true; } - return window as unknown as TestWindow; + const testWindow = window as unknown as Partial; + testWindow.dispatchEvent ??= () => true; + ensureLocalStorage(); + return testWindow as TestWindow; }; +const delay = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +const registerSettingsSave = (save: (changes: Partial) => Promise): void => { + registerRuntimeAPIs({ + runtime: { platform: 'web', isDesktop: false, isVSCode: false }, + settings: { + load: async () => ({ settings: {}, source: 'web' }), + save, + }, + } as unknown as RuntimeAPIs); +}; + +afterAll(() => { + registerRuntimeAPIs(null); + if (createdWindow) { + delete (globalThis as { window?: unknown }).window; + } else if (typeof window !== 'undefined') { + delete getWindow().__OPENCHAMBER_HOME__; + } + if (createdLocalStorage) { + delete (globalThis as { localStorage?: unknown }).localStorage; + } +}); + describe('applyPersistedHomeDirectoryToWindow', () => { beforeEach(() => { delete getWindow().__OPENCHAMBER_HOME__; }); - afterAll(() => { - if (createdWindow) { - delete (globalThis as { window?: unknown }).window; - } else { - delete getWindow().__OPENCHAMBER_HOME__; - } - }); - test('does not overwrite an injected desktop home directory', () => { getWindow().__OPENCHAMBER_HOME__ = '/Users/example'; @@ -45,3 +95,79 @@ describe('applyPersistedHomeDirectoryToWindow', () => { expect(getWindow().__OPENCHAMBER_HOME__).toBe('/Users/example/projects/app'); }); }); + +describe('updateDesktopSettings', () => { + beforeEach(() => { + getWindow(); + registerRuntimeAPIs(null); + }); + + test('waits for the debounced settings save to finish before resolving', async () => { + let saveStarted = false; + let saveFinished = false; + let updateResolved = false; + + registerSettingsSave(async () => { + saveStarted = true; + await delay(100); + saveFinished = true; + return {}; + }); + + const update = updateDesktopSettings({ + skillCatalogs: [{ id: 'custom:test', label: 'Test', source: 'owner/repo' }], + }); + update.then(() => { + updateResolved = true; + }).catch(() => { + updateResolved = true; + }); + + await delay(50); + expect(saveStarted).toBe(false); + expect(updateResolved).toBe(false); + + await delay(200); + expect(saveStarted).toBe(true); + expect(saveFinished).toBe(false); + expect(updateResolved).toBe(false); + + await update; + expect(saveFinished).toBe(true); + expect(updateResolved).toBe(true); + }); + + test('coalesces rapid settings updates and resolves every caller after one merged save', async () => { + const saveCalls: Array> = []; + let firstResolved = false; + let secondResolved = false; + + registerSettingsSave(async (changes) => { + saveCalls.push(changes); + await delay(50); + return {}; + }); + + const first = updateDesktopSettings({ themeVariant: 'dark' }); + first.then(() => { + firstResolved = true; + }).catch(() => { + firstResolved = true; + }); + + await delay(50); + + const second = updateDesktopSettings({ fontSize: 14 }); + second.then(() => { + secondResolved = true; + }).catch(() => { + secondResolved = true; + }); + + await Promise.all([first, second]); + + expect(saveCalls).toEqual([{ themeVariant: 'dark', fontSize: 14 }]); + expect(firstResolved).toBe(true); + expect(secondResolved).toBe(true); + }); +}); diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index 88e944fe..076e02a7 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -1254,13 +1254,19 @@ export const syncDesktopSettings = async (): Promise => { // Coalesce rapid updateDesktopSettings calls into a single PUT let _pendingSettingsChanges: Partial | null = null; let _settingsFlushTimer: ReturnType | null = null; +let _settingsFlushWaiters: Array<() => void> = []; const SETTINGS_DEBOUNCE_MS = 200; const _flushSettingsUpdate = async (): Promise => { const changes = _pendingSettingsChanges; + const waiters = _settingsFlushWaiters; _pendingSettingsChanges = null; _settingsFlushTimer = null; - if (!changes || Object.keys(changes).length === 0) return; + _settingsFlushWaiters = []; + if (!changes || Object.keys(changes).length === 0) { + waiters.forEach((resolve) => resolve()); + return; + } const runtimeSettings = getRuntimeSettingsAPI(); if (runtimeSettings) { @@ -1270,7 +1276,9 @@ const _flushSettingsUpdate = async (): Promise => { persistToLocalStorage(updated); applyDesktopUiPreferences(updated); dispatchSettingsSynced(updated); + _settingsCache = null; } + waiters.forEach((resolve) => resolve()); return; } catch (error) { console.warn('Failed to update settings via runtime settings API:', error); @@ -1302,6 +1310,8 @@ const _flushSettingsUpdate = async (): Promise => { } } catch (error) { console.warn('Failed to update shared settings via API:', error); + } finally { + waiters.forEach((resolve) => resolve()); } }; @@ -1315,7 +1325,11 @@ export const updateDesktopSettings = async (changes: Partial): if (_settingsFlushTimer) { clearTimeout(_settingsFlushTimer); } + const flushed = new Promise((resolve) => { + _settingsFlushWaiters.push(resolve); + }); _settingsFlushTimer = setTimeout(() => void _flushSettingsUpdate(), SETTINGS_DEBOUNCE_MS); + return flushed; }; export const initializeAppearancePreferences = async (): Promise => {