From 574bf38661c5d51ad79dc7697629082d8be6f174 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 29 May 2026 23:19:02 +0000 Subject: [PATCH 1/2] refactor(web): migrate domain storage files to typed-storage factory (LUM-2046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate 5 domain storage files to use the typed-storage factory from LUM-2044, eliminating copy-pasted SSR guards, try/catch blocks, JSON parsing, and maxEntries trimming across the codebase. Files migrated: - app-pin-storage.ts → createStorageAccessor - last-viewed-conversation-storage.ts → createKeyedStorageAccessor - context-window-storage.ts → createRecordStorageAccessor - dismissed-surfaces-storage.ts → createRecordStorageAccessor - sidebar-group-collapse-storage.ts → 2x createKeyedStorageAccessor Also: - Extract shared MemoryStorage test helper to DRY up 3 test files - Fix pinApp() to not mutate the cached array returned by load() - Fix pre-existing TS error: inline DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC (missing from generated @vellumai/assistant-api package) Closes LUM-2046 Co-Authored-By: ashlee@vellum.ai --- .../chat/utils/context-window-storage.ts | 90 ++++-------------- .../chat/utils/dismissed-surfaces-storage.ts | 84 +++-------------- .../last-viewed-conversation-storage.test.ts | 63 +------------ .../chat/utils/sanitize-display-messages.ts | 6 +- .../sidebar-group-collapse-storage.test.ts | 63 +------------ .../utils/sidebar-group-collapse-storage.ts | 87 +++++------------ apps/web/src/utils/app-pin-storage.test.ts | 59 +----------- apps/web/src/utils/app-pin-storage.ts | 77 ++++++--------- .../utils/last-viewed-conversation-storage.ts | 32 ++----- .../src/utils/memory-storage.test-helper.ts | 94 +++++++++++++++++++ 10 files changed, 197 insertions(+), 458 deletions(-) create mode 100644 apps/web/src/utils/memory-storage.test-helper.ts diff --git a/apps/web/src/domains/chat/utils/context-window-storage.ts b/apps/web/src/domains/chat/utils/context-window-storage.ts index 24e18fc6580..48a25a67a74 100644 --- a/apps/web/src/domains/chat/utils/context-window-storage.ts +++ b/apps/web/src/domains/chat/utils/context-window-storage.ts @@ -1,25 +1,14 @@ -export interface ContextWindowUsage { - tokens: number; - maxTokens: number | null; - fillRatio: number | null; -} - // Persist per-conversation context window usage to localStorage so the indicator // survives page reloads. The desktop client keeps this state alive via a // long-lived per-conversation ChatViewModel; the web client is a short-lived // browser tab, so we mirror the semantics with localStorage instead. -// -// Shape on disk: { [conversationId]: ContextWindowUsage }, keyed per assistant. -const STORAGE_KEY_PREFIX = "vellum:ctxwindow:"; -// Cap per-assistant entries to keep localStorage footprint bounded. Older -// entries are dropped oldest-first when we exceed the limit. -const MAX_ENTRIES_PER_ASSISTANT = 200; +import { createRecordStorageAccessor } from "@/utils/typed-storage"; -type StoredMap = Record; - -function storageKey(assistantId: string): string { - return `${STORAGE_KEY_PREFIX}${assistantId}`; +export interface ContextWindowUsage { + tokens: number; + maxTokens: number | null; + fillRatio: number | null; } function isValidUsage(value: unknown): value is ContextWindowUsage { @@ -45,37 +34,18 @@ function isValidUsage(value: unknown): value is ContextWindowUsage { return true; } -function safeParse(raw: string | null): StoredMap { - if (!raw) { - return {}; - } - try { - const parsed = JSON.parse(raw) as unknown; - if (!parsed || typeof parsed !== "object") { - return {}; - } - const result: StoredMap = {}; - for (const [key, value] of Object.entries(parsed as Record)) { - if (isValidUsage(value)) { - result[key] = value; - } - } - return result; - } catch { - return {}; - } -} +const storage = createRecordStorageAccessor({ + keyFn: (assistantId) => `vellum:ctxwindow:${assistantId}`, + scope: "user", + parseValue: (value) => (isValidUsage(value) ? value : null), + fallback: {}, + maxEntries: 200, +}); -export function loadContextWindowUsageMap(assistantId: string): Map { - if (typeof window === "undefined") { - return new Map(); - } - try { - const raw = window.localStorage.getItem(storageKey(assistantId)); - return new Map(Object.entries(safeParse(raw))); - } catch { - return new Map(); - } +export function loadContextWindowUsageMap( + assistantId: string, +): Map { + return new Map(Object.entries(storage.load(assistantId))); } export function saveContextWindowUsage( @@ -83,31 +53,5 @@ export function saveContextWindowUsage( conversationId: string, usage: ContextWindowUsage, ): void { - if (typeof window === "undefined") { - return; - } - try { - const key = storageKey(assistantId); - const existing = safeParse(window.localStorage.getItem(key)); - existing[conversationId] = usage; - - const entries = Object.entries(existing); - if (entries.length > MAX_ENTRIES_PER_ASSISTANT) { - // Drop oldest entries. We don't track timestamps, so this relies on - // insertion order being preserved by JSON serialization, which holds - // for all supported browsers. - const trimmed = entries.slice(entries.length - MAX_ENTRIES_PER_ASSISTANT); - const trimmedMap: StoredMap = {}; - for (const [k, v] of trimmed) { - trimmedMap[k] = v; - } - window.localStorage.setItem(key, JSON.stringify(trimmedMap)); - return; - } - - window.localStorage.setItem(key, JSON.stringify(existing)); - } catch { - // Storage can fail in private browsing / quota-exceeded cases. Silently - // drop; the in-memory cache still works for the current session. - } + storage.set(assistantId, conversationId, usage); } diff --git a/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts b/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts index 4ce6d8ef166..fd521669343 100644 --- a/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts +++ b/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts @@ -8,61 +8,30 @@ // (a) reappear as active on reload and wedge the composer, or (b) disappear // entirely even if still pending. We persist resolved IDs here so rehydration // can filter them out safely. -// -// Shape on disk: { [conversationId]: string[] }, keyed per assistant. import type { DisplayMessage } from "@/domains/chat/types/types"; +import { createRecordStorageAccessor } from "@/utils/typed-storage"; -const STORAGE_KEY_PREFIX = "vellum:dismissed-surfaces:"; -const MAX_ENTRIES_PER_ASSISTANT = 200; const MAX_IDS_PER_CONVERSATION = 500; -type StoredMap = Record; - -function storageKey(assistantId: string): string { - return `${STORAGE_KEY_PREFIX}${assistantId}`; -} - function isStringArray(value: unknown): value is string[] { return Array.isArray(value) && value.every((v) => typeof v === "string"); } -function safeParse(raw: string | null): StoredMap { - if (!raw) { - return {}; - } - try { - const parsed = JSON.parse(raw) as unknown; - if (!parsed || typeof parsed !== "object") { - return {}; - } - const result: StoredMap = {}; - for (const [key, value] of Object.entries(parsed as Record)) { - if (isStringArray(value)) { - result[key] = value; - } - } - return result; - } catch { - return {}; - } -} +const storage = createRecordStorageAccessor({ + keyFn: (assistantId) => `vellum:dismissed-surfaces:${assistantId}`, + scope: "user", + parseValue: (value) => (isStringArray(value) ? value : null), + fallback: {}, + maxEntries: 200, +}); export function loadDismissedSurfaceIds( assistantId: string, conversationId: string, ): Set { - if (typeof window === "undefined") { - return new Set(); - } - try { - const raw = window.localStorage.getItem(storageKey(assistantId)); - const map = safeParse(raw); - const ids = map[conversationId]; - return ids ? new Set(ids) : new Set(); - } catch { - return new Set(); - } + const ids = storage.get(assistantId, conversationId); + return ids ? new Set(ids) : new Set(); } export function saveDismissedSurfaceIds( @@ -70,36 +39,11 @@ export function saveDismissedSurfaceIds( conversationId: string, ids: Set, ): void { - if (typeof window === "undefined") { - return; - } - try { - const key = storageKey(assistantId); - const existing = safeParse(window.localStorage.getItem(key)); - - // Cap per-conversation list size; drop oldest on overflow (insertion order). - let idArray = Array.from(ids); - if (idArray.length > MAX_IDS_PER_CONVERSATION) { - idArray = idArray.slice(idArray.length - MAX_IDS_PER_CONVERSATION); - } - existing[conversationId] = idArray; - - const entries = Object.entries(existing); - if (entries.length > MAX_ENTRIES_PER_ASSISTANT) { - const trimmed = entries.slice(entries.length - MAX_ENTRIES_PER_ASSISTANT); - const trimmedMap: StoredMap = {}; - for (const [k, v] of trimmed) { - trimmedMap[k] = v; - } - window.localStorage.setItem(key, JSON.stringify(trimmedMap)); - return; - } - - window.localStorage.setItem(key, JSON.stringify(existing)); - } catch { - // Storage can fail in private browsing / quota-exceeded cases. Silently - // drop; in-memory state still works for the current session. + let idArray = Array.from(ids); + if (idArray.length > MAX_IDS_PER_CONVERSATION) { + idArray = idArray.slice(idArray.length - MAX_IDS_PER_CONVERSATION); } + storage.set(assistantId, conversationId, idArray); } // Strip any surfaces (and matching contentOrder entries) whose IDs the user diff --git a/apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts b/apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts index fd9f3064d29..eb7f49522d0 100644 --- a/apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts +++ b/apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts @@ -4,71 +4,12 @@ import { loadLastViewedConversationId, saveLastViewedConversationId, } from "@/utils/last-viewed-conversation-storage"; +import { installMemoryStorage } from "@/utils/memory-storage.test-helper"; const ASSISTANT_ID = "asst_123"; const STORAGE_KEY = `vellum:lastViewedConversation:${ASSISTANT_ID}`; -class MemoryStorage implements Storage { - private store = new Map(); - - get length(): number { - return this.store.size; - } - - clear(): void { - this.store.clear(); - } - - getItem(key: string): string | null { - return this.store.has(key) ? (this.store.get(key) ?? null) : null; - } - - key(index: number): string | null { - return Array.from(this.store.keys())[index] ?? null; - } - - removeItem(key: string): void { - this.store.delete(key); - } - - setItem(key: string, value: string): void { - this.store.set(key, String(value)); - } -} - -const memoryStorage = new MemoryStorage(); -// Track the original `window` descriptor so we can restore it after this test -// file finishes. Other tests in the same bun worker rely on `typeof window === -// "undefined"` to pick a baseUrl for the HTTP client, so we must not leak a -// defined `window` into unrelated suites. -const ORIGINAL_WINDOW_DESCRIPTOR = Object.getOwnPropertyDescriptor( - globalThis, - "window", -); - -beforeAll(() => { - Object.defineProperty(globalThis, "window", { - value: { localStorage: memoryStorage }, - configurable: true, - writable: true, - }); -}); - -afterAll(() => { - if (ORIGINAL_WINDOW_DESCRIPTOR) { - Object.defineProperty(globalThis, "window", ORIGINAL_WINDOW_DESCRIPTOR); - } else { - delete (globalThis as { window?: unknown }).window; - } -}); - -beforeEach(() => { - memoryStorage.clear(); -}); - -afterEach(() => { - memoryStorage.clear(); -}); +const memoryStorage = installMemoryStorage({ beforeAll, afterAll, beforeEach, afterEach }); describe("loadLastViewedConversationId", () => { test("returns null when no value is stored", () => { diff --git a/apps/web/src/domains/chat/utils/sanitize-display-messages.ts b/apps/web/src/domains/chat/utils/sanitize-display-messages.ts index 33342fff6d1..6842b152def 100644 --- a/apps/web/src/domains/chat/utils/sanitize-display-messages.ts +++ b/apps/web/src/domains/chat/utils/sanitize-display-messages.ts @@ -10,12 +10,14 @@ // only have to delete one file when the backend is fixed. // ----------------------------------------------------------------------------- -import { DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC } from "@vellumai/assistant-api"; - import { sortedByTimestamp } from "@/domains/chat/utils/message-sorting"; import type { ChatMessageToolCall } from "@/domains/chat/api/event-types"; import type { DisplayMessage } from "@/domains/chat/types/types"; +// Canonical default from @vellumai/assistant-api (assistant/src/api/constants/tool-execution.ts). +// Inlined here because the generated package does not yet re-export it. +const DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC = 120; + export function sanitizeDisplayMessages( messages: DisplayMessage[], ): DisplayMessage[] { diff --git a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.test.ts b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.test.ts index 95bbecd67d9..74e046ddb31 100644 --- a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.test.ts +++ b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.test.ts @@ -4,71 +4,12 @@ import { loadOpenCategories, saveOpenCategories, } from "@/domains/chat/utils/sidebar-group-collapse-storage"; +import { installMemoryStorage } from "@/utils/memory-storage.test-helper"; const ASSISTANT_ID = "asst_123"; const STORAGE_KEY = `vellum:sidebar-open-categories:${ASSISTANT_ID}`; -class MemoryStorage implements Storage { - private store = new Map(); - - get length(): number { - return this.store.size; - } - - clear(): void { - this.store.clear(); - } - - getItem(key: string): string | null { - return this.store.has(key) ? (this.store.get(key) ?? null) : null; - } - - key(index: number): string | null { - return Array.from(this.store.keys())[index] ?? null; - } - - removeItem(key: string): void { - this.store.delete(key); - } - - setItem(key: string, value: string): void { - this.store.set(key, String(value)); - } -} - -const memoryStorage = new MemoryStorage(); -// Track the original `window` descriptor so we can restore it after this test -// file finishes. Other tests in the same bun worker rely on `typeof window === -// "undefined"` to pick a baseUrl for the HTTP client, so we must not leak a -// defined `window` into unrelated suites. -const ORIGINAL_WINDOW_DESCRIPTOR = Object.getOwnPropertyDescriptor( - globalThis, - "window", -); - -beforeAll(() => { - Object.defineProperty(globalThis, "window", { - value: { localStorage: memoryStorage }, - configurable: true, - writable: true, - }); -}); - -afterAll(() => { - if (ORIGINAL_WINDOW_DESCRIPTOR) { - Object.defineProperty(globalThis, "window", ORIGINAL_WINDOW_DESCRIPTOR); - } else { - delete (globalThis as { window?: unknown }).window; - } -}); - -beforeEach(() => { - memoryStorage.clear(); -}); - -afterEach(() => { - memoryStorage.clear(); -}); +const memoryStorage = installMemoryStorage({ beforeAll, afterAll, beforeEach, afterEach }); describe("loadOpenCategories", () => { test("returns default [] when no value is stored", () => { diff --git a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts index 7512926dc15..bd04016b51a 100644 --- a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts +++ b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts @@ -2,102 +2,65 @@ // so that the user's last-known toggle state for each collapsible group // (Scheduled, Background, Slack, and custom groups) survives page reloads. // -// Shape on disk: string[] — the list of currently-open category keys, one entry -// per expanded group. The format mirrors the Radix Accordion `value` prop for -// `type="multiple"`. Defaults to [] when no stored state exists. -// // Built-in sections (scheduled / background / slack) and custom groups // are stored under SEPARATE keys so each CollapsibleNavSection.Root manages only // its own items — sharing a single array across two Radix roots would cause one // root's onValueChange to clobber the other. -const STORAGE_KEY_PREFIX = "vellum:sidebar-open-categories:"; -const STORAGE_KEY_PREFIX_CUSTOM = "vellum:sidebar-open-custom-groups:"; -const DEFAULT_OPEN_CATEGORIES: string[] = []; -const DEFAULT_OPEN_CUSTOM_GROUPS: string[] = []; +import { createKeyedStorageAccessor } from "@/utils/typed-storage"; + const OPEN_CATEGORY_KEYS = new Set([ "scheduled", "background", "slack", ]); -function storageKey(assistantId: string): string { - return `${STORAGE_KEY_PREFIX}${assistantId}`; -} - -function storageKeyCustom(assistantId: string): string { - return `${STORAGE_KEY_PREFIX_CUSTOM}${assistantId}`; -} - function isStringArray(value: unknown): value is string[] { return Array.isArray(value) && value.every((v) => typeof v === "string"); } -function readFromStorage(key: string, defaultValue: string[]): string[] { - if (typeof window === "undefined") { - return defaultValue; - } - try { - const raw = window.localStorage.getItem(key); - if (!raw) { - return defaultValue; - } - const parsed: unknown = JSON.parse(raw); - if (isStringArray(parsed)) { - return parsed; - } - return defaultValue; - } catch { - return defaultValue; - } +function parseStringArray(raw: string): string[] | null { + const parsed: unknown = JSON.parse(raw); + return isStringArray(parsed) ? parsed : null; } -function writeToStorage(key: string, value: string[]): void { - if (typeof window === "undefined") { - return; - } - try { - window.localStorage.setItem(key, JSON.stringify(value)); - } catch { - // Storage can fail in private browsing / quota-exceeded cases. Silently - // drop; the in-memory selection still works for the current session. - } -} +const categoriesStorage = createKeyedStorageAccessor({ + keyFn: (assistantId) => `vellum:sidebar-open-categories:${assistantId}`, + scope: "user", + parse: parseStringArray, + serialize: JSON.stringify, + fallback: [], +}); + +const customGroupsStorage = createKeyedStorageAccessor({ + keyFn: (assistantId) => `vellum:sidebar-open-custom-groups:${assistantId}`, + scope: "user", + parse: parseStringArray, + serialize: JSON.stringify, + fallback: [], +}); -/** - * Load the list of open built-in sidebar category keys for a given assistant. - * Returns `[]` when no stored state exists or on any parse error. - */ +/** Load open built-in sidebar category keys, filtering stale values. */ export function loadOpenCategories(assistantId: string): string[] { - return readFromStorage(storageKey(assistantId), DEFAULT_OPEN_CATEGORIES).filter( + return categoriesStorage.load(assistantId).filter( (category) => OPEN_CATEGORY_KEYS.has(category), ); } -/** - * Persist the list of open built-in sidebar category keys for a given assistant. - */ export function saveOpenCategories( assistantId: string, openCategories: string[], ): void { - writeToStorage(storageKey(assistantId), openCategories); + categoriesStorage.save(assistantId, openCategories); } -/** - * Load the list of open custom group IDs for a given assistant. - * Returns `[]` when no stored state exists or on any parse error. - */ export function loadOpenCustomGroups(assistantId: string): string[] { - return readFromStorage(storageKeyCustom(assistantId), DEFAULT_OPEN_CUSTOM_GROUPS); + return customGroupsStorage.load(assistantId); } -/** - * Persist the list of open custom group IDs for a given assistant. - */ export function saveOpenCustomGroups( assistantId: string, openGroups: string[], ): void { - writeToStorage(storageKeyCustom(assistantId), openGroups); + customGroupsStorage.save(assistantId, openGroups); } diff --git a/apps/web/src/utils/app-pin-storage.test.ts b/apps/web/src/utils/app-pin-storage.test.ts index b347ef2416e..23fb7fba697 100644 --- a/apps/web/src/utils/app-pin-storage.test.ts +++ b/apps/web/src/utils/app-pin-storage.test.ts @@ -8,66 +8,11 @@ import { savePinnedApps, unpinApp, } from "@/utils/app-pin-storage"; +import { installMemoryStorage } from "@/utils/memory-storage.test-helper"; const STORAGE_KEY = "vellum:pinnedApps"; -class MemoryStorage implements Storage { - private store = new Map(); - - get length(): number { - return this.store.size; - } - - clear(): void { - this.store.clear(); - } - - getItem(key: string): string | null { - return this.store.has(key) ? (this.store.get(key) ?? null) : null; - } - - key(index: number): string | null { - return Array.from(this.store.keys())[index] ?? null; - } - - removeItem(key: string): void { - this.store.delete(key); - } - - setItem(key: string, value: string): void { - this.store.set(key, String(value)); - } -} - -const memoryStorage = new MemoryStorage(); -const ORIGINAL_WINDOW_DESCRIPTOR = Object.getOwnPropertyDescriptor( - globalThis, - "window", -); - -beforeAll(() => { - Object.defineProperty(globalThis, "window", { - value: { localStorage: memoryStorage }, - configurable: true, - writable: true, - }); -}); - -afterAll(() => { - if (ORIGINAL_WINDOW_DESCRIPTOR) { - Object.defineProperty(globalThis, "window", ORIGINAL_WINDOW_DESCRIPTOR); - } else { - delete (globalThis as { window?: unknown }).window; - } -}); - -beforeEach(() => { - memoryStorage.clear(); -}); - -afterEach(() => { - memoryStorage.clear(); -}); +const memoryStorage = installMemoryStorage({ beforeAll, afterAll, beforeEach, afterEach }); function makeApp(overrides: Partial & { id: string }): AppSummary { return { diff --git a/apps/web/src/utils/app-pin-storage.ts b/apps/web/src/utils/app-pin-storage.ts index 941909007cc..fc38047baaa 100644 --- a/apps/web/src/utils/app-pin-storage.ts +++ b/apps/web/src/utils/app-pin-storage.ts @@ -1,12 +1,9 @@ // Persist pinned-app state to localStorage so the SideMenu can show pinned // apps across page reloads. This mirrors the macOS AppListManager.swift // pattern: pin state is local-only, persisted to disk, survives daemon sync. -// -// Shape on disk: PinnedAppEntry[] import type { AppSummary } from "@/types/app-types"; - -const STORAGE_KEY = "vellum:pinnedApps"; +import { createStorageAccessor } from "@/utils/typed-storage"; export interface PinnedAppEntry { appId: string; @@ -29,68 +26,48 @@ function isValidEntry(value: unknown): value is PinnedAppEntry { ); } -function safeParse(raw: string | null): PinnedAppEntry[] { - if (!raw) { - return []; - } - try { - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) { - return []; - } - return parsed.filter(isValidEntry); - } catch { - return []; - } +function parsePinnedApps(raw: string): PinnedAppEntry[] | null { + const parsed: unknown = JSON.parse(raw); + if (!Array.isArray(parsed)) return null; + return parsed.filter(isValidEntry); } -export function loadPinnedApps(): PinnedAppEntry[] { - if (typeof window === "undefined") { - return []; - } - try { - return safeParse(window.localStorage.getItem(STORAGE_KEY)); - } catch { - return []; - } -} +const storage = createStorageAccessor({ + key: "vellum:pinnedApps", + scope: "user", + parse: parsePinnedApps, + serialize: JSON.stringify, + fallback: [], +}); -export function savePinnedApps(entries: PinnedAppEntry[]): void { - if (typeof window === "undefined") { - return; - } - try { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(entries)); - } catch { - // Storage can fail in private browsing / quota-exceeded cases. Silently - // drop; in-memory state still works for the current session. - } -} +export const loadPinnedApps = storage.load; +export const savePinnedApps = storage.save; export function pinApp(app: AppSummary): void { - const entries = loadPinnedApps(); + const entries = storage.load(); if (entries.some((e) => e.appId === app.id)) { return; } const maxOrder = entries.reduce((max, e) => Math.max(max, e.pinnedOrder), 0); - entries.push({ - appId: app.id, - pinnedOrder: maxOrder + 1, - name: app.name, - icon: app.icon, - }); - savePinnedApps(entries); + storage.save([ + ...entries, + { + appId: app.id, + pinnedOrder: maxOrder + 1, + name: app.name, + icon: app.icon, + }, + ]); } export function unpinApp(appId: string): void { - let entries = loadPinnedApps().filter((e) => e.appId !== appId); - // Re-compact pinnedOrder values so there are no gaps. + let entries = storage.load().filter((e) => e.appId !== appId); entries = entries .sort((a, b) => a.pinnedOrder - b.pinnedOrder) .map((e, i) => ({ ...e, pinnedOrder: i + 1 })); - savePinnedApps(entries); + storage.save(entries); } export function isAppPinned(appId: string): boolean { - return loadPinnedApps().some((e) => e.appId === appId); + return storage.load().some((e) => e.appId === appId); } diff --git a/apps/web/src/utils/last-viewed-conversation-storage.ts b/apps/web/src/utils/last-viewed-conversation-storage.ts index 301988010ee..73b3ae4f448 100644 --- a/apps/web/src/utils/last-viewed-conversation-storage.ts +++ b/apps/web/src/utils/last-viewed-conversation-storage.ts @@ -3,37 +3,25 @@ // restore the previous selection on initial page load instead of always // defaulting to the first conversation in the list. -const STORAGE_KEY_PREFIX = "vellum:lastViewedConversation:"; +import { createKeyedStorageAccessor } from "@/utils/typed-storage"; -function storageKey(assistantId: string): string { - return `${STORAGE_KEY_PREFIX}${assistantId}`; -} +const storage = createKeyedStorageAccessor({ + keyFn: (assistantId) => `vellum:lastViewedConversation:${assistantId}`, + scope: "user", + parse: (raw) => (raw.length > 0 ? raw : null), + serialize: (v) => v ?? "", + fallback: null, +}); export function loadLastViewedConversationId( assistantId: string, ): string | null { - if (typeof window === "undefined") { - return null; - } - try { - const raw = window.localStorage.getItem(storageKey(assistantId)); - return raw && raw.length > 0 ? raw : null; - } catch { - return null; - } + return storage.load(assistantId); } export function saveLastViewedConversationId( assistantId: string, conversationId: string, ): void { - if (typeof window === "undefined") { - return; - } - try { - window.localStorage.setItem(storageKey(assistantId), conversationId); - } catch { - // Storage can fail in private browsing / quota-exceeded cases. Silently - // drop; the in-memory selection still works for the current session. - } + storage.save(assistantId, conversationId); } diff --git a/apps/web/src/utils/memory-storage.test-helper.ts b/apps/web/src/utils/memory-storage.test-helper.ts new file mode 100644 index 00000000000..c4bd358ff74 --- /dev/null +++ b/apps/web/src/utils/memory-storage.test-helper.ts @@ -0,0 +1,94 @@ +/** + * In-memory Storage implementation for unit tests. + * + * Mirrors the Web Storage API so tests can swap `window.localStorage` + * without touching real browser storage. + */ +export class MemoryStorage implements Storage { + private store = new Map(); + + get length(): number { + return this.store.size; + } + + clear(): void { + this.store.clear(); + } + + getItem(key: string): string | null { + return this.store.has(key) ? (this.store.get(key) ?? null) : null; + } + + key(index: number): string | null { + return Array.from(this.store.keys())[index] ?? null; + } + + removeItem(key: string): void { + this.store.delete(key); + } + + setItem(key: string, value: string): void { + this.store.set(key, String(value)); + } +} + +/** + * Install a MemoryStorage instance as both `window.localStorage` and + * the global `localStorage` for the current test file. Returns the + * storage instance. Automatically clears between tests and restores + * the originals after all tests. + * + * Call in the test file's top-level scope (not inside `describe`). + */ +export function installMemoryStorage(hooks: { + beforeAll: (fn: () => void) => void; + afterAll: (fn: () => void) => void; + beforeEach: (fn: () => void) => void; + afterEach: (fn: () => void) => void; +}): MemoryStorage { + const storage = new MemoryStorage(); + const originalWindowDescriptor = Object.getOwnPropertyDescriptor( + globalThis, + "window", + ); + const originalLocalStorageDescriptor = Object.getOwnPropertyDescriptor( + globalThis, + "localStorage", + ); + + hooks.beforeAll(() => { + Object.defineProperty(globalThis, "window", { + value: { localStorage: storage }, + configurable: true, + writable: true, + }); + Object.defineProperty(globalThis, "localStorage", { + value: storage, + configurable: true, + writable: true, + }); + }); + + hooks.afterAll(() => { + if (originalWindowDescriptor) { + Object.defineProperty(globalThis, "window", originalWindowDescriptor); + } else { + delete (globalThis as { window?: unknown }).window; + } + if (originalLocalStorageDescriptor) { + Object.defineProperty(globalThis, "localStorage", originalLocalStorageDescriptor); + } else { + delete (globalThis as { localStorage?: unknown }).localStorage; + } + }); + + hooks.beforeEach(() => { + storage.clear(); + }); + + hooks.afterEach(() => { + storage.clear(); + }); + + return storage; +} From 9b79810717b33d62f010eaec3d6b1d7991232be9 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 29 May 2026 23:26:39 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(web):=20audit=20fixes=20=E2=80=94=20col?= =?UTF-8?q?ocate=20test,=20DRY=20isStringArray,=20restore=20canonical=20im?= =?UTF-8?q?port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move last-viewed-conversation-storage.test.ts from domains/chat/utils/ to utils/ to colocate with its source file (CONVENTIONS.md item 16) - Extract shared isStringArray/parseStringArray to storage-validators.ts (duplicated across dismissed-surfaces-storage and sidebar-group-collapse-storage) - Restore DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC import from @vellumai/assistant-api (generated package was stale locally; regenerating via postinstall fixes it) Co-Authored-By: ashlee@vellum.ai --- .../domains/chat/utils/dismissed-surfaces-storage.ts | 5 +---- .../domains/chat/utils/sanitize-display-messages.ts | 6 ++---- .../chat/utils/sidebar-group-collapse-storage.ts | 10 +--------- apps/web/src/domains/chat/utils/storage-validators.ts | 10 ++++++++++ .../utils/last-viewed-conversation-storage.test.ts | 0 5 files changed, 14 insertions(+), 17 deletions(-) create mode 100644 apps/web/src/domains/chat/utils/storage-validators.ts rename apps/web/src/{domains/chat => }/utils/last-viewed-conversation-storage.test.ts (100%) diff --git a/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts b/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts index fd521669343..14cb961e1ac 100644 --- a/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts +++ b/apps/web/src/domains/chat/utils/dismissed-surfaces-storage.ts @@ -10,14 +10,11 @@ // can filter them out safely. import type { DisplayMessage } from "@/domains/chat/types/types"; +import { isStringArray } from "@/domains/chat/utils/storage-validators"; import { createRecordStorageAccessor } from "@/utils/typed-storage"; const MAX_IDS_PER_CONVERSATION = 500; -function isStringArray(value: unknown): value is string[] { - return Array.isArray(value) && value.every((v) => typeof v === "string"); -} - const storage = createRecordStorageAccessor({ keyFn: (assistantId) => `vellum:dismissed-surfaces:${assistantId}`, scope: "user", diff --git a/apps/web/src/domains/chat/utils/sanitize-display-messages.ts b/apps/web/src/domains/chat/utils/sanitize-display-messages.ts index 6842b152def..33342fff6d1 100644 --- a/apps/web/src/domains/chat/utils/sanitize-display-messages.ts +++ b/apps/web/src/domains/chat/utils/sanitize-display-messages.ts @@ -10,14 +10,12 @@ // only have to delete one file when the backend is fixed. // ----------------------------------------------------------------------------- +import { DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC } from "@vellumai/assistant-api"; + import { sortedByTimestamp } from "@/domains/chat/utils/message-sorting"; import type { ChatMessageToolCall } from "@/domains/chat/api/event-types"; import type { DisplayMessage } from "@/domains/chat/types/types"; -// Canonical default from @vellumai/assistant-api (assistant/src/api/constants/tool-execution.ts). -// Inlined here because the generated package does not yet re-export it. -const DEFAULT_TOOL_EXECUTION_TIMEOUT_SEC = 120; - export function sanitizeDisplayMessages( messages: DisplayMessage[], ): DisplayMessage[] { diff --git a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts index bd04016b51a..bdcc1afb885 100644 --- a/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts +++ b/apps/web/src/domains/chat/utils/sidebar-group-collapse-storage.ts @@ -7,6 +7,7 @@ // its own items — sharing a single array across two Radix roots would cause one // root's onValueChange to clobber the other. +import { parseStringArray } from "@/domains/chat/utils/storage-validators"; import { createKeyedStorageAccessor } from "@/utils/typed-storage"; const OPEN_CATEGORY_KEYS = new Set([ @@ -15,15 +16,6 @@ const OPEN_CATEGORY_KEYS = new Set([ "slack", ]); -function isStringArray(value: unknown): value is string[] { - return Array.isArray(value) && value.every((v) => typeof v === "string"); -} - -function parseStringArray(raw: string): string[] | null { - const parsed: unknown = JSON.parse(raw); - return isStringArray(parsed) ? parsed : null; -} - const categoriesStorage = createKeyedStorageAccessor({ keyFn: (assistantId) => `vellum:sidebar-open-categories:${assistantId}`, scope: "user", diff --git a/apps/web/src/domains/chat/utils/storage-validators.ts b/apps/web/src/domains/chat/utils/storage-validators.ts new file mode 100644 index 00000000000..3f3243ab204 --- /dev/null +++ b/apps/web/src/domains/chat/utils/storage-validators.ts @@ -0,0 +1,10 @@ +/** Shared validation helpers for chat domain storage files. */ + +export function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((v) => typeof v === "string"); +} + +export function parseStringArray(raw: string): string[] | null { + const parsed: unknown = JSON.parse(raw); + return isStringArray(parsed) ? parsed : null; +} diff --git a/apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts b/apps/web/src/utils/last-viewed-conversation-storage.test.ts similarity index 100% rename from apps/web/src/domains/chat/utils/last-viewed-conversation-storage.test.ts rename to apps/web/src/utils/last-viewed-conversation-storage.test.ts