diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index 620f21d3..0e9ee7c1 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -32,7 +32,15 @@ const persistToLocalStorage = (settings: DesktopSettings) => { } if (settings.homeDirectory) { localStorage.setItem('homeDirectory', settings.homeDirectory); - window.__OPENCHAMBER_HOME__ = settings.homeDirectory; + // Electron's preload exposes __OPENCHAMBER_HOME__ as a read-only + // contextBridge property; assignment throws TypeError there. In VSCode + // webview and plain web runtime the property is writable. Swallow the + // error in Electron — preload already seeded the value correctly. + try { + window.__OPENCHAMBER_HOME__ = settings.homeDirectory; + } catch { + /* read-only contextBridge property — leave preload-seeded value */ + } } if (Array.isArray(settings.projects) && settings.projects.length > 0) { localStorage.setItem('projects', JSON.stringify(settings.projects)); @@ -969,20 +977,47 @@ export const syncDesktopSettings = async (): Promise => { const persistApi = getPersistApi(); - const applySettings = (settings: DesktopSettings) => { - persistToLocalStorage(settings); - const apply = () => applyDesktopUiPreferences(settings); + // Wait for Zustand persist hydration before applying server settings. + // Otherwise `set()`-calls race with hydration: we set X, then hydration + // reads localStorage and overwrites back to the persisted value. + const waitForHydration = (): Promise => { + if (!persistApi?.hasHydrated || persistApi.hasHydrated()) { + return Promise.resolve(); + } + if (!persistApi.onFinishHydration) { + return Promise.resolve(); + } + return new Promise((resolve) => { + let settled = false; + const finish = () => { + if (settled) return; + settled = true; + resolve(); + }; + const unsubscribe = persistApi.onFinishHydration!(() => { + unsubscribe?.(); + finish(); + }); + // Guard: hydration may have flipped to true between the hasHydrated + // check and the onFinishHydration subscription — resolve immediately. + if (persistApi.hasHydrated?.()) finish(); + }); + }; - if (persistApi?.hasHydrated?.()) { - apply(); - } else { - apply(); - if (persistApi?.onFinishHydration) { - const unsubscribe = persistApi.onFinishHydration(() => { - unsubscribe?.(); - apply(); - }); - } + // Each step is wrapped in try/catch so a failure in one side-effect (e.g. + // a TypeError from writing to a contextBridge-protected global) doesn't + // prevent server settings from reaching the Zustand store. + const applySettings = async (settings: DesktopSettings) => { + try { + persistToLocalStorage(settings); + } catch (error) { + console.warn('persistToLocalStorage failed:', error); + } + await waitForHydration(); + try { + applyDesktopUiPreferences(settings); + } catch (error) { + console.warn('applyDesktopUiPreferences failed:', error); } if (typeof window !== 'undefined') { @@ -993,7 +1028,7 @@ export const syncDesktopSettings = async (): Promise => { try { const webSettings = await fetchWebSettings(); if (webSettings) { - applySettings(webSettings); + await applySettings(webSettings); } } catch (error) { console.warn('Failed to synchronise settings:', error); diff --git a/packages/ui/src/stores/globalSessions.ts b/packages/ui/src/stores/globalSessions.ts index 83ae9c9f..36eb6294 100644 --- a/packages/ui/src/stores/globalSessions.ts +++ b/packages/ui/src/stores/globalSessions.ts @@ -69,6 +69,7 @@ export async function listGlobalSessionPages( }, ): Promise { const all: GlobalSessionRecord[] = []; + const seenIds = new Set(); let cursor: number | undefined; while (true) { @@ -76,23 +77,41 @@ export async function listGlobalSessionPages( () => apiClient.experimental.session.list({ archived: options.archived, limit: options.pageSize, - ...(cursor ? { cursor } : {}), + ...(cursor !== undefined ? { cursor } : {}), }), { attempts: 3, delay: 500, retryIf: () => true }, ); const payload = Array.isArray(response.data) ? (response.data as GlobalSessionRecord[]) : []; - if (payload.length === 0) { - break; + if (payload.length === 0) break; + + let appended = 0; + for (const session of payload) { + if (!session?.id || seenIds.has(session.id)) continue; + seenIds.add(session.id); + all.push(session); + appended += 1; + } + if (appended > 0) { + options.onPage?.(payload); } - all.push(...payload); - options.onPage?.(payload); + // Stop on partial page — nothing more to fetch. + if (payload.length < options.pageSize) break; + + // Prefer server header; fall back to last session's `time.updated` + // (cursor semantics on server = "updated strictly before this timestamp"). + const headerCursor = toNumber(readResponseHeader(response, "x-next-cursor")); + const lastUpdated = payload[payload.length - 1]?.time?.updated; + const nextCursor = headerCursor + ?? (typeof lastUpdated === "number" && Number.isFinite(lastUpdated) ? lastUpdated : undefined); + + if (nextCursor === undefined) break; + // Loop guard: cursor must move backwards in time. + if (cursor !== undefined && nextCursor >= cursor) break; + // Every id in this page already seen — stop to avoid spinning. + if (appended === 0) break; - const nextCursor = toNumber(readResponseHeader(response, "x-next-cursor")); - if (!nextCursor) { - break; - } cursor = nextCursor; } diff --git a/packages/vscode/src/bridge-settings-runtime.ts b/packages/vscode/src/bridge-settings-runtime.ts index a0897ee7..cfe7d688 100644 --- a/packages/vscode/src/bridge-settings-runtime.ts +++ b/packages/vscode/src/bridge-settings-runtime.ts @@ -165,12 +165,19 @@ const writeSharedSettingsToDisk = async (changes: Record): Prom await fs.promises.mkdir(path.dirname(OPENCHAMBER_SHARED_SETTINGS_PATH), { recursive: true }); const current = readSharedSettingsFromDisk(); const next: Record = { ...current, ...changes }; - await fs.promises.writeFile(OPENCHAMBER_SHARED_SETTINGS_PATH, JSON.stringify(next, null, 2), 'utf8'); + // Atomic write: tmp file + rename. Readers never see a partial/truncated + // JSON that would fail to parse and silently get coerced to {}. + const tmp = `${OPENCHAMBER_SHARED_SETTINGS_PATH}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + await fs.promises.writeFile(tmp, JSON.stringify(next, null, 2), 'utf8'); + await fs.promises.rename(tmp, OPENCHAMBER_SHARED_SETTINGS_PATH); } catch { // ignore } }; +// Fields derived from runtime context — never persisted, always recomputed. +const DERIVED_FIELDS = new Set(['themeVariant', 'lastDirectory']); + const sanitizeMagicPromptOverrides = (input: unknown): Record => { if (!input || typeof input !== 'object' || Array.isArray(input)) { return {}; @@ -207,12 +214,50 @@ const writeMagicPromptFile = async (state: { version: number; overrides: Record< await fs.promises.writeFile(OPENCHAMBER_MAGIC_PROMPTS_PATH, JSON.stringify(state, null, 2), 'utf8'); }; +const stripDerived = (source: Record): Record => { + const next: Record = { ...source }; + for (const key of DERIVED_FIELDS) { + delete next[key]; + } + return next; +}; + +let eagerMigrationAttempted = false; + +// Read the merged persisted settings: shared file is canonical (synced with +// Desktop and Web clients), globalState is kept as a migration fallback for +// users upgrading from the pre-shared-sync era. Disk wins on conflicts. +// +// On first read per process, if globalState has keys that are missing on +// disk, copy them to disk so other clients see them immediately — without +// waiting for the user to save again. +const readPersistedSettings = (ctx?: BridgeContext): Record => { + const fromGlobalState = stripDerived( + ctx?.context?.globalState.get>(SETTINGS_KEY) || {}, + ); + const fromDisk = stripDerived(readSharedSettingsFromDisk()); + + if (!eagerMigrationAttempted) { + eagerMigrationAttempted = true; + const missingFromDisk: Record = {}; + for (const [key, value] of Object.entries(fromGlobalState)) { + if (!(key in fromDisk)) { + missingFromDisk[key] = value; + } + } + if (Object.keys(missingFromDisk).length > 0) { + // Fire-and-forget; readers already have an in-memory merged view. + void writeSharedSettingsToDisk(missingFromDisk); + } + } + + return { ...fromGlobalState, ...fromDisk }; +}; + export const readSettings = (ctx?: BridgeContext): Record => { - const stored = ctx?.context?.globalState.get>(SETTINGS_KEY) || {}; - const restStored = { ...stored }; - delete (restStored as Record).lastDirectory; - const shared = readSharedSettingsFromDisk(); - const sharedOpencodeBinary = typeof shared.opencodeBinary === 'string' ? shared.opencodeBinary.trim() : ''; + const persisted = readPersistedSettings(ctx); + const persistedOpencodeBinary = + typeof persisted.opencodeBinary === 'string' ? String(persisted.opencodeBinary).trim() : ''; const workspaceFolder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath || ''; const themeVariant = vscode.window.activeColorTheme.kind === vscode.ColorThemeKind.Light || @@ -221,20 +266,16 @@ export const readSettings = (ctx?: BridgeContext): Record => { : 'dark'; return { + ...persisted, themeVariant, lastDirectory: workspaceFolder, - ...restStored, - opencodeBinary: - typeof restStored.opencodeBinary === 'string' - ? String(restStored.opencodeBinary).trim() - : (sharedOpencodeBinary || undefined), + opencodeBinary: persistedOpencodeBinary || undefined, }; }; export const persistSettings = async (changes: Record, ctx?: BridgeContext): Promise> => { const current = readSettings(ctx); - const restChanges = { ...(changes || {}) }; - delete restChanges.lastDirectory; + const restChanges = stripDerived({ ...(changes || {}) }); const keysToClear = new Set(); @@ -256,19 +297,33 @@ export const persistSettings = async (changes: Record, ctx?: Br delete restChanges.usageRefreshIntervalMs; } - const merged = { ...current, ...restChanges, lastDirectory: current.lastDirectory } as Record; + if (typeof restChanges.opencodeBinary === 'string') { + restChanges.opencodeBinary = restChanges.opencodeBinary.trim(); + } + + // Persistable state = current persisted (no derived fields) + sanitized changes. + const persistedCurrent = readPersistedSettings(ctx); + const persistable: Record = { ...persistedCurrent, ...restChanges }; for (const key of keysToClear) { - delete merged[key]; - } - await ctx?.context?.globalState.update(SETTINGS_KEY, merged); - - if (keysToClear.has('opencodeBinary')) { - await writeSharedSettingsToDisk({ opencodeBinary: '' }); - } else if (typeof restChanges.opencodeBinary === 'string') { - await writeSharedSettingsToDisk({ opencodeBinary: restChanges.opencodeBinary.trim() }); + delete persistable[key]; } - return merged; + // Write to the shared file (canonical, cross-client). Also mirror into + // globalState so older builds can still read recent values if a user + // downgrades the extension. + await writeSharedSettingsToDisk(persistable); + await ctx?.context?.globalState.update(SETTINGS_KEY, persistable); + + // Return the same shape as readSettings (with derived fields re-applied). + return { + ...persistable, + themeVariant: current.themeVariant, + lastDirectory: current.lastDirectory, + opencodeBinary: + typeof persistable.opencodeBinary === 'string' && persistable.opencodeBinary.length > 0 + ? persistable.opencodeBinary + : undefined, + }; }; export const readMagicPromptOverrides = (): { version: number; overrides: Record } => {