From 78e2d702de32ef2bb68d4e15172e61db87246307 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 15 Jan 2026 22:49:37 -0800 Subject: [PATCH 1/6] fix: add LRU eviction for session storage to prevent QuotaExceededError Implement LRU-based session storage service for workflow data to prevent QuotaExceededError that was breaking workspace/teams features on mobile. Changes: - Add sessionStorageLruService with try-evict-retry logic - Store workflow data with embedded accessedAt timestamps for LRU tracking - Evict oldest entries when quota is exceeded (legacy entries first) - Graceful degradation: log warnings instead of throwing on persistent failures - Backward compatibility: legacy unwrapped data treated as oldest Fixes: https://comfy-org.sentry.io/issues/6955016837/ Amp-Thread-ID: https://ampcode.com/threads/T-019bc566-eb91-7545-a462-658c2b33083b Co-authored-by: Amp --- .../composables/useWorkflowPersistence.ts | 87 ++++-- .../services/sessionStorageLruService.test.ts | 238 +++++++++++++++ .../services/sessionStorageLruService.ts | 280 ++++++++++++++++++ 3 files changed, 575 insertions(+), 30 deletions(-) create mode 100644 src/platform/workflow/persistence/services/sessionStorageLruService.test.ts create mode 100644 src/platform/workflow/persistence/services/sessionStorageLruService.ts diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts index 09ffd58950f..732af9ad02a 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts @@ -11,11 +11,19 @@ import { useSettingStore } from '@/platform/settings/settingStore' import { useWorkflowService } from '@/platform/workflow/core/services/workflowService' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { useTemplateUrlLoader } from '@/platform/workflow/templates/composables/useTemplateUrlLoader' +import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' import { api } from '@/scripts/api' import { app as comfyApp } from '@/scripts/app' import { getStorageValue, setStorageValue } from '@/scripts/utils' +import { + getStorageStats, + getWithLruTracking, + setWithLruEviction +} from '@/platform/workflow/persistence/services/sessionStorageLruService' import { useCommandStore } from '@/stores/commandStore' +const WORKFLOW_KEY_PATTERN = /^workflow:/ + export function useWorkflowPersistence() { const workflowStore = useWorkflowStore() const settingStore = useSettingStore() @@ -44,39 +52,45 @@ export function useWorkflowPersistence() { const persistCurrentWorkflow = () => { if (!workflowPersistenceEnabled.value) return - const workflow = JSON.stringify(comfyApp.rootGraph.serialize()) + const workflowData = comfyApp.rootGraph.serialize() + const workflowJson = JSON.stringify(workflowData) try { - localStorage.setItem('workflow', workflow) - if (api.clientId) { - sessionStorage.setItem(`workflow:${api.clientId}`, workflow) - } + localStorage.setItem('workflow', workflowJson) } catch (error) { - // Only log our own keys and aggregate stats - const ourKeys = Object.keys(sessionStorage).filter( - (key) => key.startsWith('workflow:') || key === 'workflow' + console.warn( + '[WorkflowPersistence] Failed to save to localStorage:', + error ) - console.error('QuotaExceededError details:', { - workflowSizeKB: Math.round(workflow.length / 1024), - totalStorageItems: Object.keys(sessionStorage).length, - ourWorkflowKeys: ourKeys.length, - ourWorkflowSizes: ourKeys.map((key) => ({ - key, - sizeKB: Math.round(sessionStorage[key].length / 1024) - })), - error: error instanceof Error ? error.message : String(error) - }) - throw error + } + + if (api.clientId) { + const key = `workflow:${api.clientId}` + const success = setWithLruEviction( + key, + workflowData, + WORKFLOW_KEY_PATTERN + ) + + if (!success) { + const stats = getStorageStats(WORKFLOW_KEY_PATTERN) + console.warn( + '[WorkflowPersistence] Failed to persist workflow after LRU eviction', + { + workflowSizeKB: Math.round(workflowJson.length / 1024), + ...stats + } + ) + } } } - const loadWorkflowFromStorage = async ( - json: string | null, + const loadWorkflowFromData = async ( + workflowData: ComfyWorkflowJSON | null, workflowName: string | null ) => { - if (!json) return false - const workflow = JSON.parse(json) - await comfyApp.loadGraphData(workflow, true, true, workflowName) + if (!workflowData) return false + await comfyApp.loadGraphData(workflowData, true, true, workflowName) return true } @@ -84,17 +98,30 @@ export function useWorkflowPersistence() { const workflowName = getStorageValue('Comfy.PreviousWorkflow') const clientId = api.initialClientId ?? api.clientId - // Try loading from session storage first + // Try loading from session storage first (uses LRU tracking) if (clientId) { - const sessionWorkflow = sessionStorage.getItem(`workflow:${clientId}`) - if (await loadWorkflowFromStorage(sessionWorkflow, workflowName)) { + const sessionWorkflow = getWithLruTracking( + `workflow:${clientId}` + ) + if (await loadWorkflowFromData(sessionWorkflow, workflowName)) { return true } } - // Fall back to local storage - const localWorkflow = localStorage.getItem('workflow') - return await loadWorkflowFromStorage(localWorkflow, workflowName) + // Fall back to local storage (raw JSON, no LRU wrapper) + const localWorkflowJson = localStorage.getItem('workflow') + if (localWorkflowJson) { + try { + const localWorkflow = JSON.parse(localWorkflowJson) as ComfyWorkflowJSON + return await loadWorkflowFromData(localWorkflow, workflowName) + } catch { + console.warn( + '[WorkflowPersistence] Failed to parse localStorage workflow' + ) + } + } + + return false } const loadDefaultWorkflow = async () => { diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts b/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts new file mode 100644 index 00000000000..e74b182cf5e --- /dev/null +++ b/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts @@ -0,0 +1,238 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + getStorageStats, + getWithLruTracking, + removeFromStorage, + setWithLruEviction +} from './sessionStorageLruService' + +describe('sessionStorageLruService', () => { + beforeEach(() => { + sessionStorage.clear() + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe('setWithLruEviction', () => { + it('stores data with LRU metadata wrapper', () => { + const data = { nodes: [], links: [] } + const result = setWithLruEviction('workflow:test', data) + + expect(result).toBe(true) + + const stored = sessionStorage.getItem('workflow:test') + expect(stored).toBeTruthy() + + const parsed = JSON.parse(stored!) + expect(parsed).toHaveProperty('accessedAt') + expect(parsed).toHaveProperty('data') + expect(parsed.data).toEqual(data) + expect(typeof parsed.accessedAt).toBe('number') + }) + + it('returns true on successful storage', () => { + const result = setWithLruEviction('key', { test: 'data' }) + expect(result).toBe(true) + }) + + it('evicts LRU entries when quota is exceeded', () => { + const oldEntry = { accessedAt: 1000, data: { old: 'data' } } + const newEntry = { accessedAt: 2000, data: { new: 'data' } } + + sessionStorage.setItem('workflow:old', JSON.stringify(oldEntry)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + const error = new DOMException('Quota exceeded', 'QuotaExceededError') + throw error + } + return originalSetItem(key, value) + }) + + const result = setWithLruEviction( + 'workflow:current', + { current: 'data' }, + /^workflow:/ + ) + + expect(result).toBe(true) + expect(sessionStorage.getItem('workflow:old')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + + it('returns false after max eviction attempts', () => { + const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + }) + + const result = setWithLruEviction('key', { test: 'data' }) + expect(result).toBe(false) + + spy.mockRestore() + }) + }) + + describe('getWithLruTracking', () => { + it('returns null for non-existent key', () => { + const result = getWithLruTracking('nonexistent') + expect(result).toBeNull() + }) + + it('returns unwrapped data for new format entries', () => { + const entry = { accessedAt: Date.now(), data: { nodes: [], links: [] } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + const result = getWithLruTracking('workflow:test') + expect(result).toEqual({ nodes: [], links: [] }) + }) + + it('handles legacy format (unwrapped data) with accessedAt: 0', () => { + const legacyData = { nodes: [1, 2, 3], links: [] } + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + + const result = getWithLruTracking('workflow:legacy') + expect(result).toEqual(legacyData) + }) + + it('updates accessedAt when reading', () => { + const oldTime = 1000 + const entry = { accessedAt: oldTime, data: { test: 'data' } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + getWithLruTracking('workflow:test', true) + + const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) + expect(stored.accessedAt).toBeGreaterThan(oldTime) + }) + + it('does not update accessedAt when updateAccessTime is false', () => { + const oldTime = 1000 + const entry = { accessedAt: oldTime, data: { test: 'data' } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + getWithLruTracking('workflow:test', false) + + const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) + expect(stored.accessedAt).toBe(oldTime) + }) + }) + + describe('removeFromStorage', () => { + it('removes item from session storage', () => { + sessionStorage.setItem('key', 'value') + expect(sessionStorage.getItem('key')).toBe('value') + + removeFromStorage('key') + expect(sessionStorage.getItem('key')).toBeNull() + }) + }) + + describe('getStorageStats', () => { + it('returns stats for all entries', () => { + const entry1 = { accessedAt: 1000, data: { a: 1 } } + const entry2 = { accessedAt: 2000, data: { b: 2 } } + sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) + sessionStorage.setItem('other:c', 'value') + + const stats = getStorageStats() + + expect(stats.totalItems).toBe(3) + expect(stats.matchingItems).toBe(3) + }) + + it('filters entries by pattern', () => { + const entry1 = { accessedAt: 1000, data: { a: 1 } } + const entry2 = { accessedAt: 2000, data: { b: 2 } } + sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) + sessionStorage.setItem('other:c', 'value') + + const stats = getStorageStats(/^workflow:/) + + expect(stats.totalItems).toBe(3) + expect(stats.matchingItems).toBe(2) + expect(stats.entries).toHaveLength(2) + }) + + it('sorts entries by accessedAt (oldest first)', () => { + const entry1 = { accessedAt: 2000, data: { newer: true } } + const entry2 = { accessedAt: 1000, data: { older: true } } + sessionStorage.setItem('workflow:newer', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:older', JSON.stringify(entry2)) + + const stats = getStorageStats(/^workflow:/) + + expect(stats.entries[0].key).toBe('workflow:older') + expect(stats.entries[1].key).toBe('workflow:newer') + }) + + it('treats legacy entries as accessedAt: 0', () => { + const legacyData = { nodes: [], links: [] } + const newEntry = { accessedAt: 1000, data: { test: true } } + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + const stats = getStorageStats(/^workflow:/) + + expect(stats.entries[0].key).toBe('workflow:legacy') + expect(stats.entries[0].accessedAt).toBe(0) + }) + }) + + describe('LRU eviction order', () => { + it('evicts oldest entries first (legacy before new)', () => { + const legacyData = { nodes: [], links: [] } + const newEntry = { accessedAt: Date.now(), data: { new: true } } + + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + } + return originalSetItem(key, value) + }) + + setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + + expect(sessionStorage.getItem('workflow:legacy')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + + it('evicts oldest new-format entries when no legacy entries exist', () => { + const oldEntry = { accessedAt: 1000, data: { old: true } } + const newEntry = { accessedAt: 2000, data: { new: true } } + + sessionStorage.setItem('workflow:old', JSON.stringify(oldEntry)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + } + return originalSetItem(key, value) + }) + + setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + + expect(sessionStorage.getItem('workflow:old')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + }) +}) diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.ts b/src/platform/workflow/persistence/services/sessionStorageLruService.ts new file mode 100644 index 00000000000..aae6e3f0f9f --- /dev/null +++ b/src/platform/workflow/persistence/services/sessionStorageLruService.ts @@ -0,0 +1,280 @@ +/** + * @fileoverview LRU-based session storage service with automatic eviction + * @module services/sessionStorageLruService + * + * Provides session storage operations with: + * - LRU tracking via embedded timestamps + * - Automatic eviction on quota exceeded errors + * - Graceful degradation when storage is unavailable + * - Backward compatibility with legacy (unwrapped) data + * + * @deprecated-notice Legacy format support (unwrapped data) can be removed after 2026-07-15 + */ + +interface StorageEntry { + accessedAt: number + data: T +} + +interface EvictableEntry { + key: string + accessedAt: number + size: number +} + +const MAX_EVICTION_ATTEMPTS = 3 +const PROTECTED_KEY_PREFIXES = ['workspace.', 'Workspace.'] + +function isProtectedKey(key: string): boolean { + return PROTECTED_KEY_PREFIXES.some((prefix) => key.includes(prefix)) +} + +/** + * Checks if parsed data is in legacy format (raw data without wrapper) + * Legacy format: raw workflow JSON with nodes/links at top level + * New format: { accessedAt: number, data: T } + * + * @deprecated Remove after 2026-07-15 + */ +function isLegacyFormat(parsed: unknown): boolean { + if (typeof parsed !== 'object' || parsed === null) return true + const obj = parsed as Record + return !('accessedAt' in obj && 'data' in obj) +} + +/** + * Wraps data with LRU metadata for storage + */ +function wrapForStorage(data: T): string { + const entry: StorageEntry = { + accessedAt: Date.now(), + data + } + return JSON.stringify(entry) +} + +/** + * Unwraps stored data, handling both legacy and new formats + * Legacy entries are assigned accessedAt: 0 to prioritize them for eviction + * + * @deprecated Legacy handling can be removed after 2026-07-15 + */ +function unwrapFromStorage(raw: string): StorageEntry { + const parsed = JSON.parse(raw) + + if (isLegacyFormat(parsed)) { + return { + accessedAt: 0, + data: parsed as T + } + } + + return parsed as StorageEntry +} + +/** + * Gets all evictable entries from session storage matching a key pattern + */ +function getEvictableEntries(keyPattern: RegExp): EvictableEntry[] { + const entries: EvictableEntry[] = [] + + for (let i = 0; i < sessionStorage.length; i++) { + const key = sessionStorage.key(i) + if (!key || !keyPattern.test(key) || isProtectedKey(key)) continue + + const raw = sessionStorage.getItem(key) + if (!raw) continue + + try { + const entry = unwrapFromStorage(raw) + entries.push({ + key, + accessedAt: entry.accessedAt, + size: raw.length + }) + } catch { + entries.push({ + key, + accessedAt: 0, + size: raw.length + }) + } + } + + return entries +} + +/** + * Evicts the least recently used entries matching a pattern + * @returns Number of bytes freed + */ +function evictLruEntries( + keyPattern: RegExp, + excludeKey?: string, + maxToEvict: number = 1 +): number { + const entries = getEvictableEntries(keyPattern) + .filter((e) => e.key !== excludeKey) + .sort((a, b) => a.accessedAt - b.accessedAt) + + let freedBytes = 0 + const toEvict = entries.slice(0, maxToEvict) + + for (const entry of toEvict) { + sessionStorage.removeItem(entry.key) + freedBytes += entry.size + } + + if (toEvict.length > 0) { + console.warn( + `[SessionStorageLRU] Evicted ${toEvict.length} entries, freed ~${Math.round(freedBytes / 1024)}KB`, + toEvict.map((e) => e.key) + ) + } + + return freedBytes +} + +/** + * Attempts to set a session storage item with LRU eviction on quota exceeded + * + * @param key - Storage key + * @param data - Data to store (will be wrapped with LRU metadata) + * @param evictionPattern - Regex pattern for keys eligible for eviction + * @returns true if stored successfully, false if storage failed after retries + */ +export function setWithLruEviction( + key: string, + data: T, + evictionPattern: RegExp = /^workflow:/ +): boolean { + const wrapped = wrapForStorage(data) + + for (let attempt = 0; attempt <= MAX_EVICTION_ATTEMPTS; attempt++) { + try { + sessionStorage.setItem(key, wrapped) + return true + } catch (error) { + if ( + !(error instanceof DOMException) || + error.name !== 'QuotaExceededError' + ) { + console.error('[SessionStorageLRU] Unexpected storage error:', error) + return false + } + + if (attempt === MAX_EVICTION_ATTEMPTS) { + console.warn( + `[SessionStorageLRU] Failed to store ${key} after ${MAX_EVICTION_ATTEMPTS} eviction attempts` + ) + return false + } + + const entriesToEvict = Math.min(attempt + 1, 3) + const freedBytes = evictLruEntries(evictionPattern, key, entriesToEvict) + + if (freedBytes === 0) { + console.warn( + '[SessionStorageLRU] No entries available for eviction, giving up' + ) + return false + } + } + } + + return false +} + +/** + * Gets data from session storage, updating access time for LRU tracking + * + * @param key - Storage key + * @param updateAccessTime - Whether to update the access timestamp (default: true) + * @returns The stored data, or null if not found + */ +export function getWithLruTracking( + key: string, + updateAccessTime: boolean = true +): T | null { + const raw = sessionStorage.getItem(key) + if (!raw) return null + + try { + const entry = unwrapFromStorage(raw) + + if (updateAccessTime && entry.accessedAt !== Date.now()) { + try { + sessionStorage.setItem(key, wrapForStorage(entry.data)) + } catch { + // Ignore quota errors when updating access time + } + } + + return entry.data + } catch (error) { + console.warn(`[SessionStorageLRU] Failed to parse ${key}:`, error) + return null + } +} + +/** + * Removes an item from session storage + */ +export function removeFromStorage(key: string): void { + sessionStorage.removeItem(key) +} + +/** + * Gets storage statistics for debugging + */ +export function getStorageStats(keyPattern?: RegExp): { + totalItems: number + matchingItems: number + totalSizeKB: number + matchingSizeKB: number + entries: Array<{ key: string; sizeKB: number; accessedAt: number }> +} { + let totalSize = 0 + let matchingSize = 0 + let matchingItems = 0 + const entries: Array<{ key: string; sizeKB: number; accessedAt: number }> = [] + + for (let i = 0; i < sessionStorage.length; i++) { + const key = sessionStorage.key(i) + if (!key) continue + + const raw = sessionStorage.getItem(key) + if (!raw) continue + + const size = raw.length * 2 + totalSize += size + + if (!keyPattern || keyPattern.test(key)) { + matchingItems++ + matchingSize += size + + try { + const entry = unwrapFromStorage(raw) + entries.push({ + key, + sizeKB: Math.round(size / 1024), + accessedAt: entry.accessedAt + }) + } catch { + entries.push({ + key, + sizeKB: Math.round(size / 1024), + accessedAt: 0 + }) + } + } + } + + return { + totalItems: sessionStorage.length, + matchingItems, + totalSizeKB: Math.round(totalSize / 1024), + matchingSizeKB: Math.round(matchingSize / 1024), + entries: entries.sort((a, b) => a.accessedAt - b.accessedAt) + } +} From add7a4fbd497f1d3c008d5c280fb4fe2bd2327e7 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 15 Jan 2026 23:04:51 -0800 Subject: [PATCH 2/6] refactor: simplify session storage service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename to workflowSessionStorageService with semantic names - Remove verbose JSDoc and byte tracking - Streamline logging to specific error cases - Simplify eviction flow (280 → 115 lines) Amp-Thread-ID: https://ampcode.com/threads/T-019bc597-fbea-778a-ae15-92061b3d0812 Co-authored-by: Amp --- .../composables/useWorkflowPersistence.ts | 28 +- .../services/sessionStorageLruService.ts | 280 ------------------ ... => workflowSessionStorageService.test.ts} | 141 +++------ .../services/workflowSessionStorageService.ts | 158 ++++++++++ 4 files changed, 213 insertions(+), 394 deletions(-) delete mode 100644 src/platform/workflow/persistence/services/sessionStorageLruService.ts rename src/platform/workflow/persistence/services/{sessionStorageLruService.test.ts => workflowSessionStorageService.test.ts} (53%) create mode 100644 src/platform/workflow/persistence/services/workflowSessionStorageService.ts diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts index 732af9ad02a..0ad937e61f2 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts @@ -16,10 +16,9 @@ import { api } from '@/scripts/api' import { app as comfyApp } from '@/scripts/app' import { getStorageValue, setStorageValue } from '@/scripts/utils' import { - getStorageStats, - getWithLruTracking, - setWithLruEviction -} from '@/platform/workflow/persistence/services/sessionStorageLruService' + getWithAccessTracking, + setWithEviction +} from '@/platform/workflow/persistence/services/workflowSessionStorageService' import { useCommandStore } from '@/stores/commandStore' const WORKFLOW_KEY_PATTERN = /^workflow:/ @@ -66,22 +65,7 @@ export function useWorkflowPersistence() { if (api.clientId) { const key = `workflow:${api.clientId}` - const success = setWithLruEviction( - key, - workflowData, - WORKFLOW_KEY_PATTERN - ) - - if (!success) { - const stats = getStorageStats(WORKFLOW_KEY_PATTERN) - console.warn( - '[WorkflowPersistence] Failed to persist workflow after LRU eviction', - { - workflowSizeKB: Math.round(workflowJson.length / 1024), - ...stats - } - ) - } + setWithEviction(key, workflowData, WORKFLOW_KEY_PATTERN) } } @@ -98,9 +82,9 @@ export function useWorkflowPersistence() { const workflowName = getStorageValue('Comfy.PreviousWorkflow') const clientId = api.initialClientId ?? api.clientId - // Try loading from session storage first (uses LRU tracking) + // Try loading from session storage first if (clientId) { - const sessionWorkflow = getWithLruTracking( + const sessionWorkflow = getWithAccessTracking( `workflow:${clientId}` ) if (await loadWorkflowFromData(sessionWorkflow, workflowName)) { diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.ts b/src/platform/workflow/persistence/services/sessionStorageLruService.ts deleted file mode 100644 index aae6e3f0f9f..00000000000 --- a/src/platform/workflow/persistence/services/sessionStorageLruService.ts +++ /dev/null @@ -1,280 +0,0 @@ -/** - * @fileoverview LRU-based session storage service with automatic eviction - * @module services/sessionStorageLruService - * - * Provides session storage operations with: - * - LRU tracking via embedded timestamps - * - Automatic eviction on quota exceeded errors - * - Graceful degradation when storage is unavailable - * - Backward compatibility with legacy (unwrapped) data - * - * @deprecated-notice Legacy format support (unwrapped data) can be removed after 2026-07-15 - */ - -interface StorageEntry { - accessedAt: number - data: T -} - -interface EvictableEntry { - key: string - accessedAt: number - size: number -} - -const MAX_EVICTION_ATTEMPTS = 3 -const PROTECTED_KEY_PREFIXES = ['workspace.', 'Workspace.'] - -function isProtectedKey(key: string): boolean { - return PROTECTED_KEY_PREFIXES.some((prefix) => key.includes(prefix)) -} - -/** - * Checks if parsed data is in legacy format (raw data without wrapper) - * Legacy format: raw workflow JSON with nodes/links at top level - * New format: { accessedAt: number, data: T } - * - * @deprecated Remove after 2026-07-15 - */ -function isLegacyFormat(parsed: unknown): boolean { - if (typeof parsed !== 'object' || parsed === null) return true - const obj = parsed as Record - return !('accessedAt' in obj && 'data' in obj) -} - -/** - * Wraps data with LRU metadata for storage - */ -function wrapForStorage(data: T): string { - const entry: StorageEntry = { - accessedAt: Date.now(), - data - } - return JSON.stringify(entry) -} - -/** - * Unwraps stored data, handling both legacy and new formats - * Legacy entries are assigned accessedAt: 0 to prioritize them for eviction - * - * @deprecated Legacy handling can be removed after 2026-07-15 - */ -function unwrapFromStorage(raw: string): StorageEntry { - const parsed = JSON.parse(raw) - - if (isLegacyFormat(parsed)) { - return { - accessedAt: 0, - data: parsed as T - } - } - - return parsed as StorageEntry -} - -/** - * Gets all evictable entries from session storage matching a key pattern - */ -function getEvictableEntries(keyPattern: RegExp): EvictableEntry[] { - const entries: EvictableEntry[] = [] - - for (let i = 0; i < sessionStorage.length; i++) { - const key = sessionStorage.key(i) - if (!key || !keyPattern.test(key) || isProtectedKey(key)) continue - - const raw = sessionStorage.getItem(key) - if (!raw) continue - - try { - const entry = unwrapFromStorage(raw) - entries.push({ - key, - accessedAt: entry.accessedAt, - size: raw.length - }) - } catch { - entries.push({ - key, - accessedAt: 0, - size: raw.length - }) - } - } - - return entries -} - -/** - * Evicts the least recently used entries matching a pattern - * @returns Number of bytes freed - */ -function evictLruEntries( - keyPattern: RegExp, - excludeKey?: string, - maxToEvict: number = 1 -): number { - const entries = getEvictableEntries(keyPattern) - .filter((e) => e.key !== excludeKey) - .sort((a, b) => a.accessedAt - b.accessedAt) - - let freedBytes = 0 - const toEvict = entries.slice(0, maxToEvict) - - for (const entry of toEvict) { - sessionStorage.removeItem(entry.key) - freedBytes += entry.size - } - - if (toEvict.length > 0) { - console.warn( - `[SessionStorageLRU] Evicted ${toEvict.length} entries, freed ~${Math.round(freedBytes / 1024)}KB`, - toEvict.map((e) => e.key) - ) - } - - return freedBytes -} - -/** - * Attempts to set a session storage item with LRU eviction on quota exceeded - * - * @param key - Storage key - * @param data - Data to store (will be wrapped with LRU metadata) - * @param evictionPattern - Regex pattern for keys eligible for eviction - * @returns true if stored successfully, false if storage failed after retries - */ -export function setWithLruEviction( - key: string, - data: T, - evictionPattern: RegExp = /^workflow:/ -): boolean { - const wrapped = wrapForStorage(data) - - for (let attempt = 0; attempt <= MAX_EVICTION_ATTEMPTS; attempt++) { - try { - sessionStorage.setItem(key, wrapped) - return true - } catch (error) { - if ( - !(error instanceof DOMException) || - error.name !== 'QuotaExceededError' - ) { - console.error('[SessionStorageLRU] Unexpected storage error:', error) - return false - } - - if (attempt === MAX_EVICTION_ATTEMPTS) { - console.warn( - `[SessionStorageLRU] Failed to store ${key} after ${MAX_EVICTION_ATTEMPTS} eviction attempts` - ) - return false - } - - const entriesToEvict = Math.min(attempt + 1, 3) - const freedBytes = evictLruEntries(evictionPattern, key, entriesToEvict) - - if (freedBytes === 0) { - console.warn( - '[SessionStorageLRU] No entries available for eviction, giving up' - ) - return false - } - } - } - - return false -} - -/** - * Gets data from session storage, updating access time for LRU tracking - * - * @param key - Storage key - * @param updateAccessTime - Whether to update the access timestamp (default: true) - * @returns The stored data, or null if not found - */ -export function getWithLruTracking( - key: string, - updateAccessTime: boolean = true -): T | null { - const raw = sessionStorage.getItem(key) - if (!raw) return null - - try { - const entry = unwrapFromStorage(raw) - - if (updateAccessTime && entry.accessedAt !== Date.now()) { - try { - sessionStorage.setItem(key, wrapForStorage(entry.data)) - } catch { - // Ignore quota errors when updating access time - } - } - - return entry.data - } catch (error) { - console.warn(`[SessionStorageLRU] Failed to parse ${key}:`, error) - return null - } -} - -/** - * Removes an item from session storage - */ -export function removeFromStorage(key: string): void { - sessionStorage.removeItem(key) -} - -/** - * Gets storage statistics for debugging - */ -export function getStorageStats(keyPattern?: RegExp): { - totalItems: number - matchingItems: number - totalSizeKB: number - matchingSizeKB: number - entries: Array<{ key: string; sizeKB: number; accessedAt: number }> -} { - let totalSize = 0 - let matchingSize = 0 - let matchingItems = 0 - const entries: Array<{ key: string; sizeKB: number; accessedAt: number }> = [] - - for (let i = 0; i < sessionStorage.length; i++) { - const key = sessionStorage.key(i) - if (!key) continue - - const raw = sessionStorage.getItem(key) - if (!raw) continue - - const size = raw.length * 2 - totalSize += size - - if (!keyPattern || keyPattern.test(key)) { - matchingItems++ - matchingSize += size - - try { - const entry = unwrapFromStorage(raw) - entries.push({ - key, - sizeKB: Math.round(size / 1024), - accessedAt: entry.accessedAt - }) - } catch { - entries.push({ - key, - sizeKB: Math.round(size / 1024), - accessedAt: 0 - }) - } - } - } - - return { - totalItems: sessionStorage.length, - matchingItems, - totalSizeKB: Math.round(totalSize / 1024), - matchingSizeKB: Math.round(matchingSize / 1024), - entries: entries.sort((a, b) => a.accessedAt - b.accessedAt) - } -} diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts similarity index 53% rename from src/platform/workflow/persistence/services/sessionStorageLruService.test.ts rename to src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts index e74b182cf5e..d6950560cb2 100644 --- a/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts +++ b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts @@ -1,13 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { - getStorageStats, - getWithLruTracking, + getWithAccessTracking, removeFromStorage, - setWithLruEviction -} from './sessionStorageLruService' + setWithEviction +} from './workflowSessionStorageService' -describe('sessionStorageLruService', () => { +describe('workflowSessionStorageService', () => { beforeEach(() => { sessionStorage.clear() vi.clearAllMocks() @@ -17,29 +16,21 @@ describe('sessionStorageLruService', () => { vi.restoreAllMocks() }) - describe('setWithLruEviction', () => { - it('stores data with LRU metadata wrapper', () => { + describe('setWithEviction', () => { + it('stores data with accessedAt wrapper', () => { const data = { nodes: [], links: [] } - const result = setWithLruEviction('workflow:test', data) + const result = setWithEviction('workflow:test', data) expect(result).toBe(true) const stored = sessionStorage.getItem('workflow:test') - expect(stored).toBeTruthy() - const parsed = JSON.parse(stored!) expect(parsed).toHaveProperty('accessedAt') expect(parsed).toHaveProperty('data') expect(parsed.data).toEqual(data) - expect(typeof parsed.accessedAt).toBe('number') - }) - - it('returns true on successful storage', () => { - const result = setWithLruEviction('key', { test: 'data' }) - expect(result).toBe(true) }) - it('evicts LRU entries when quota is exceeded', () => { + it('evicts oldest entries when quota is exceeded', () => { const oldEntry = { accessedAt: 1000, data: { old: 'data' } } const newEntry = { accessedAt: 2000, data: { new: 'data' } } @@ -51,13 +42,12 @@ describe('sessionStorageLruService', () => { vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { callCount++ if (key === 'workflow:current' && callCount === 1) { - const error = new DOMException('Quota exceeded', 'QuotaExceededError') - throw error + throw new DOMException('Quota exceeded', 'QuotaExceededError') } return originalSetItem(key, value) }) - const result = setWithLruEviction( + const result = setWithEviction( 'workflow:current', { current: 'data' }, /^workflow:/ @@ -73,32 +63,42 @@ describe('sessionStorageLruService', () => { throw new DOMException('Quota exceeded', 'QuotaExceededError') }) - const result = setWithLruEviction('key', { test: 'data' }) + const result = setWithEviction('key', { test: 'data' }) + expect(result).toBe(false) + + spy.mockRestore() + }) + + it('returns false on unexpected errors', () => { + const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + throw new Error('Unexpected error') + }) + + const result = setWithEviction('key', { test: 'data' }) expect(result).toBe(false) spy.mockRestore() }) }) - describe('getWithLruTracking', () => { + describe('getWithAccessTracking', () => { it('returns null for non-existent key', () => { - const result = getWithLruTracking('nonexistent') - expect(result).toBeNull() + expect(getWithAccessTracking('nonexistent')).toBeNull() }) it('returns unwrapped data for new format entries', () => { const entry = { accessedAt: Date.now(), data: { nodes: [], links: [] } } sessionStorage.setItem('workflow:test', JSON.stringify(entry)) - const result = getWithLruTracking('workflow:test') + const result = getWithAccessTracking('workflow:test') expect(result).toEqual({ nodes: [], links: [] }) }) - it('handles legacy format (unwrapped data) with accessedAt: 0', () => { + it('handles legacy format with accessedAt: 0', () => { const legacyData = { nodes: [1, 2, 3], links: [] } sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) - const result = getWithLruTracking('workflow:legacy') + const result = getWithAccessTracking('workflow:legacy') expect(result).toEqual(legacyData) }) @@ -107,7 +107,7 @@ describe('sessionStorageLruService', () => { const entry = { accessedAt: oldTime, data: { test: 'data' } } sessionStorage.setItem('workflow:test', JSON.stringify(entry)) - getWithLruTracking('workflow:test', true) + getWithAccessTracking('workflow:test', true) const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) expect(stored.accessedAt).toBeGreaterThan(oldTime) @@ -118,11 +118,18 @@ describe('sessionStorageLruService', () => { const entry = { accessedAt: oldTime, data: { test: 'data' } } sessionStorage.setItem('workflow:test', JSON.stringify(entry)) - getWithLruTracking('workflow:test', false) + getWithAccessTracking('workflow:test', false) const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) expect(stored.accessedAt).toBe(oldTime) }) + + it('returns null for invalid JSON', () => { + sessionStorage.setItem('workflow:invalid', 'not json') + + const result = getWithAccessTracking('workflow:invalid') + expect(result).toBeNull() + }) }) describe('removeFromStorage', () => { @@ -135,61 +142,8 @@ describe('sessionStorageLruService', () => { }) }) - describe('getStorageStats', () => { - it('returns stats for all entries', () => { - const entry1 = { accessedAt: 1000, data: { a: 1 } } - const entry2 = { accessedAt: 2000, data: { b: 2 } } - sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) - sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) - sessionStorage.setItem('other:c', 'value') - - const stats = getStorageStats() - - expect(stats.totalItems).toBe(3) - expect(stats.matchingItems).toBe(3) - }) - - it('filters entries by pattern', () => { - const entry1 = { accessedAt: 1000, data: { a: 1 } } - const entry2 = { accessedAt: 2000, data: { b: 2 } } - sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) - sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) - sessionStorage.setItem('other:c', 'value') - - const stats = getStorageStats(/^workflow:/) - - expect(stats.totalItems).toBe(3) - expect(stats.matchingItems).toBe(2) - expect(stats.entries).toHaveLength(2) - }) - - it('sorts entries by accessedAt (oldest first)', () => { - const entry1 = { accessedAt: 2000, data: { newer: true } } - const entry2 = { accessedAt: 1000, data: { older: true } } - sessionStorage.setItem('workflow:newer', JSON.stringify(entry1)) - sessionStorage.setItem('workflow:older', JSON.stringify(entry2)) - - const stats = getStorageStats(/^workflow:/) - - expect(stats.entries[0].key).toBe('workflow:older') - expect(stats.entries[1].key).toBe('workflow:newer') - }) - - it('treats legacy entries as accessedAt: 0', () => { - const legacyData = { nodes: [], links: [] } - const newEntry = { accessedAt: 1000, data: { test: true } } - sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) - sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) - - const stats = getStorageStats(/^workflow:/) - - expect(stats.entries[0].key).toBe('workflow:legacy') - expect(stats.entries[0].accessedAt).toBe(0) - }) - }) - - describe('LRU eviction order', () => { - it('evicts oldest entries first (legacy before new)', () => { + describe('eviction order', () => { + it('evicts legacy entries (accessedAt: 0) before new entries', () => { const legacyData = { nodes: [], links: [] } const newEntry = { accessedAt: Date.now(), data: { new: true } } @@ -206,18 +160,21 @@ describe('sessionStorageLruService', () => { return originalSetItem(key, value) }) - setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + setWithEviction('workflow:current', { current: true }, /^workflow:/) expect(sessionStorage.getItem('workflow:legacy')).toBeNull() expect(sessionStorage.getItem('workflow:new')).toBeTruthy() }) - it('evicts oldest new-format entries when no legacy entries exist', () => { - const oldEntry = { accessedAt: 1000, data: { old: true } } - const newEntry = { accessedAt: 2000, data: { new: true } } + it('does not evict protected keys', () => { + const workspaceEntry = { accessedAt: 0, data: { workspace: true } } + const workflowEntry = { accessedAt: 1000, data: { workflow: true } } - sessionStorage.setItem('workflow:old', JSON.stringify(oldEntry)) - sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + sessionStorage.setItem( + 'workspace.settings', + JSON.stringify(workspaceEntry) + ) + sessionStorage.setItem('workflow:old', JSON.stringify(workflowEntry)) let callCount = 0 const originalSetItem = sessionStorage.setItem.bind(sessionStorage) @@ -229,10 +186,10 @@ describe('sessionStorageLruService', () => { return originalSetItem(key, value) }) - setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + setWithEviction('workflow:current', { current: true }, /^workflow:/) + expect(sessionStorage.getItem('workspace.settings')).toBeTruthy() expect(sessionStorage.getItem('workflow:old')).toBeNull() - expect(sessionStorage.getItem('workflow:new')).toBeTruthy() }) }) }) diff --git a/src/platform/workflow/persistence/services/workflowSessionStorageService.ts b/src/platform/workflow/persistence/services/workflowSessionStorageService.ts new file mode 100644 index 00000000000..4434f9762ee --- /dev/null +++ b/src/platform/workflow/persistence/services/workflowSessionStorageService.ts @@ -0,0 +1,158 @@ +/** + * Session storage service for workflow data with automatic eviction on quota exceeded. + * Uses timestamp-based access tracking to evict least recently used entries. + */ + +interface StorageEntry { + accessedAt: number + data: T +} + +const MAX_EVICTION_ATTEMPTS = 3 +const PROTECTED_KEY_PREFIXES = ['workspace.', 'Workspace.'] + +function isProtectedKey(key: string): boolean { + return PROTECTED_KEY_PREFIXES.some((prefix) => key.includes(prefix)) +} + +/** + * Legacy format: raw workflow JSON without wrapper. + * @deprecated Remove after 2026-07-15 + */ +function isLegacyFormat(parsed: unknown): boolean { + if (typeof parsed !== 'object' || parsed === null) return true + const obj = parsed as Record + return !('accessedAt' in obj && 'data' in obj) +} + +function wrapForStorage(data: T): string { + return JSON.stringify({ accessedAt: Date.now(), data }) +} + +/** + * @deprecated Legacy handling can be removed after 2026-07-15 + */ +function unwrapFromStorage(raw: string): StorageEntry { + const parsed = JSON.parse(raw) + if (isLegacyFormat(parsed)) { + return { accessedAt: 0, data: parsed as T } + } + return parsed as StorageEntry +} + +function getEvictableKeys(keyPattern: RegExp, excludeKey?: string): string[] { + const entries: Array<{ key: string; accessedAt: number }> = [] + + for (let i = 0; i < sessionStorage.length; i++) { + const key = sessionStorage.key(i) + if ( + !key || + !keyPattern.test(key) || + isProtectedKey(key) || + key === excludeKey + ) + continue + + const raw = sessionStorage.getItem(key) + if (!raw) continue + + try { + const { accessedAt } = unwrapFromStorage(raw) + entries.push({ key, accessedAt }) + } catch { + entries.push({ key, accessedAt: 0 }) + } + } + + return entries.sort((a, b) => a.accessedAt - b.accessedAt).map((e) => e.key) +} + +function evictOldestEntries( + keyPattern: RegExp, + excludeKey: string, + count: number +): number { + const keys = getEvictableKeys(keyPattern, excludeKey) + const toEvict = keys.slice(0, count) + + for (const key of toEvict) { + sessionStorage.removeItem(key) + } + + return toEvict.length +} + +/** + * Stores data in session storage with automatic eviction on quota exceeded. + * @returns true if stored successfully, false if storage failed after retries + */ +export function setWithEviction( + key: string, + data: T, + evictionPattern: RegExp = /^workflow:/ +): boolean { + const wrapped = wrapForStorage(data) + + for (let attempt = 0; attempt <= MAX_EVICTION_ATTEMPTS; attempt++) { + try { + sessionStorage.setItem(key, wrapped) + return true + } catch (error) { + if ( + !(error instanceof DOMException) || + error.name !== 'QuotaExceededError' + ) { + console.error('[WorkflowStorage] Unexpected storage error') + return false + } + + if (attempt === MAX_EVICTION_ATTEMPTS) { + console.warn( + '[WorkflowStorage] Storage full after max eviction attempts' + ) + return false + } + + const evicted = evictOldestEntries(evictionPattern, key, attempt + 1) + if (evicted === 0) { + console.warn('[WorkflowStorage] No entries available for eviction') + return false + } + } + } + + return false +} + +/** + * Gets data from session storage, updating access time for eviction tracking. + * @returns The stored data, or null if not found + */ +export function getWithAccessTracking( + key: string, + updateAccessTime = true +): T | null { + const raw = sessionStorage.getItem(key) + if (!raw) return null + + try { + const entry = unwrapFromStorage(raw) + + if (updateAccessTime) { + try { + sessionStorage.setItem(key, wrapForStorage(entry.data)) + } catch { + // Ignore quota errors when updating access time + } + } + + return entry.data + } catch { + console.warn('[WorkflowStorage] Failed to parse entry:', key) + return null + } +} + +export function removeFromStorage(key: string): void { + sessionStorage.removeItem(key) +} From fdba5d270f6c3200f85f42a17d96944baa0555c2 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 15 Jan 2026 23:20:28 -0800 Subject: [PATCH 3/6] test: remove redundant mock restoration calls vi.restoreAllMocks() in afterEach already handles cleanup --- .../services/workflowSessionStorageService.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts index d6950560cb2..da2317d8843 100644 --- a/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts +++ b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts @@ -59,25 +59,21 @@ describe('workflowSessionStorageService', () => { }) it('returns false after max eviction attempts', () => { - const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { throw new DOMException('Quota exceeded', 'QuotaExceededError') }) const result = setWithEviction('key', { test: 'data' }) expect(result).toBe(false) - - spy.mockRestore() }) it('returns false on unexpected errors', () => { - const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { throw new Error('Unexpected error') }) const result = setWithEviction('key', { test: 'data' }) expect(result).toBe(false) - - spy.mockRestore() }) }) From 5b6cc8777eb717d495e302ea3b8472aab4a13784 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 15 Jan 2026 23:20:48 -0800 Subject: [PATCH 4/6] fix: use startsWith for prefix matching in protected keys Prevents false positives from substring matches --- .../persistence/services/workflowSessionStorageService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/workflow/persistence/services/workflowSessionStorageService.ts b/src/platform/workflow/persistence/services/workflowSessionStorageService.ts index 4434f9762ee..d560c9dd481 100644 --- a/src/platform/workflow/persistence/services/workflowSessionStorageService.ts +++ b/src/platform/workflow/persistence/services/workflowSessionStorageService.ts @@ -12,7 +12,7 @@ const MAX_EVICTION_ATTEMPTS = 3 const PROTECTED_KEY_PREFIXES = ['workspace.', 'Workspace.'] function isProtectedKey(key: string): boolean { - return PROTECTED_KEY_PREFIXES.some((prefix) => key.includes(prefix)) + return PROTECTED_KEY_PREFIXES.some((prefix) => key.startsWith(prefix)) } /** From f44c06f16ba6f1a303220b353957738d8559119b Mon Sep 17 00:00:00 2001 From: bymyself Date: Tue, 20 Jan 2026 14:03:57 -0800 Subject: [PATCH 5/6] fix: restore mock cleanup in tests mockRestore() calls are needed for these specific mocks --- .../services/workflowSessionStorageService.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts index da2317d8843..d6950560cb2 100644 --- a/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts +++ b/src/platform/workflow/persistence/services/workflowSessionStorageService.test.ts @@ -59,21 +59,25 @@ describe('workflowSessionStorageService', () => { }) it('returns false after max eviction attempts', () => { - vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { throw new DOMException('Quota exceeded', 'QuotaExceededError') }) const result = setWithEviction('key', { test: 'data' }) expect(result).toBe(false) + + spy.mockRestore() }) it('returns false on unexpected errors', () => { - vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { throw new Error('Unexpected error') }) const result = setWithEviction('key', { test: 'data' }) expect(result).toBe(false) + + spy.mockRestore() }) }) From aec1a556672ce5393b084f2f426443edf6e97a50 Mon Sep 17 00:00:00 2001 From: bymyself Date: Tue, 27 Jan 2026 21:02:08 -0800 Subject: [PATCH 6/6] fix: use pointer instead of payload in sessionStorage to prevent QuotaExceededError Instead of storing full workflow JSON in sessionStorage (which can hit quota on mobile), store only a small pointer to the draft path. The actual workflow data lives in the draft store which has LRU eviction. Loading priority: 1. SessionStorage pointer -> draft store 2. Preferred path -> draft store 3. Most recent draft (fallback) 4. Legacy sessionStorage payload (backward compat, remove after 2026-07-15) 5. Legacy localStorage payload (backward compat, remove after 2026-07-15) This maintains duplicate-tab support while eliminating the quota issue. Amp-Thread-ID: https://ampcode.com/threads/T-019bc597-fbea-778a-ae15-92061b3d0812 Co-authored-by: Amp --- .../composables/useWorkflowPersistence.ts | 25 ++++--------------- .../persistence/stores/workflowDraftStore.ts | 12 ++++++++- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts index 06e7bff61d0..507543d2046 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts @@ -81,27 +81,12 @@ export function useWorkflowPersistence() { return } + // Store pointer in sessionStorage for duplicate-tab support (small, won't hit quota) + // The actual workflow data is stored in the draft store which has eviction try { - localStorage.setItem('workflow', workflowJson) - if (api.clientId) { - sessionStorage.setItem(`workflow:${api.clientId}`, workflowJson) - } - } catch (error) { - // Only log our own keys and aggregate stats - const ourKeys = Object.keys(sessionStorage).filter( - (key) => key.startsWith('workflow:') || key === 'workflow' - ) - console.error('QuotaExceededError details:', { - workflowSizeKB: Math.round(workflowJson.length / 1024), - totalStorageItems: Object.keys(sessionStorage).length, - ourWorkflowKeys: ourKeys.length, - ourWorkflowSizes: ourKeys.map((key) => ({ - key, - sizeKB: Math.round(sessionStorage[key].length / 1024) - })), - error: error instanceof Error ? error.message : String(error) - }) - throw error + sessionStorage.setItem('Comfy.Workflow.ActivePath', workflowPath) + } catch { + // Ignore - pointer is best-effort } lastSavedJsonByPath.value[workflowPath] = workflowJson diff --git a/src/platform/workflow/persistence/stores/workflowDraftStore.ts b/src/platform/workflow/persistence/stores/workflowDraftStore.ts index 30280e12468..1fe2ec3cb9c 100644 --- a/src/platform/workflow/persistence/stores/workflowDraftStore.ts +++ b/src/platform/workflow/persistence/stores/workflowDraftStore.ts @@ -124,17 +124,26 @@ export const useWorkflowDraftStore = defineStore('workflowDraft', () => { fallbackToLatestDraft = false } = options + // 1. Try sessionStorage pointer (for duplicate-tab support) + const sessionPath = sessionStorage.getItem('Comfy.Workflow.ActivePath') + if (sessionPath && (await loadDraft(sessionPath))) { + return true + } + + // 2. Try preferred path from caller if (preferredPath && (await loadDraft(preferredPath))) { return true } - if (!preferredPath && fallbackToLatestDraft) { + // 3. Fall back to most recent draft + if (fallbackToLatestDraft) { const fallbackPath = mostRecentDraft.value if (fallbackPath && (await loadDraft(fallbackPath))) { return true } } + // 4. Legacy fallback: sessionStorage payload (remove after 2026-07-15) const clientId = api.initialClientId ?? api.clientId if (clientId) { const sessionPayload = sessionStorage.getItem(`workflow:${clientId}`) @@ -143,6 +152,7 @@ export const useWorkflowDraftStore = defineStore('workflowDraft', () => { } } + // 5. Legacy fallback: localStorage payload (remove after 2026-07-15) const localPayload = localStorage.getItem('workflow') return await tryLoadGraph(localPayload, workflowName) }