From 7e769c8e41a17fae310383f6523f52403beef13b Mon Sep 17 00:00:00 2001 From: rohitsudhakar1 Date: Wed, 5 Aug 2026 20:47:25 -0700 Subject: [PATCH 1/2] fix(undo-redo): bound persisted history by size, not just operation count The undo-redo store persists operation snapshots to localStorage bounded only by MAX_STACKS (5) x DEFAULT_CAPACITY (100) operations. A single operation can carry several KB of block state, so active editing lets the persisted history grow to multiple megabytes and exhaust the origin's shared ~5 MB localStorage budget. Once that budget is gone, the next setItem from any other persisted store throws QuotaExceededError, so the store that surfaces the error is the victim rather than the culprit. The existing safeStorageAdapter only stops this store's own writes from throwing; it does nothing to stop the store from starving its siblings. Cap the serialized footprint at MAX_PERSISTED_BYTES and trim inside partialize, evicting the oldest operations across stacks and dropping any stack left empty. Only the persisted copy is trimmed, so the in-memory history is untouched and the current session stays fully undoable. Tests cover the reproduction (heavy editing stays within budget, newest operations survive) and the trim function directly (no-op when under budget, oldest-first eviction, input not mutated, empty stacks dropped). Fixes #4737 --- apps/sim/stores/undo-redo/store.test.ts | 133 +++++++++++++++++++++++- apps/sim/stores/undo-redo/store.ts | 93 ++++++++++++++++- 2 files changed, 221 insertions(+), 5 deletions(-) diff --git a/apps/sim/stores/undo-redo/store.test.ts b/apps/sim/stores/undo-redo/store.test.ts index add86259615..91ef4198950 100644 --- a/apps/sim/stores/undo-redo/store.test.ts +++ b/apps/sim/stores/undo-redo/store.test.ts @@ -21,7 +21,11 @@ import { createUpdateParentEntry, } from '@sim/testing' import { beforeEach, describe, expect, it } from 'vitest' -import { runWithUndoRedoRecordingSuspended, useUndoRedoStore } from '@/stores/undo-redo/store' +import { + runWithUndoRedoRecordingSuspended, + trimPersistedStateToBudget, + useUndoRedoStore, +} from '@/stores/undo-redo/store' import type { UpdateParentOperation } from '@/stores/undo-redo/types' describe('useUndoRedoStore', () => { @@ -762,4 +766,131 @@ describe('useUndoRedoStore', () => { expect(parentEntry?.operation.type).toBe('update-parent') }) }) + + describe('persisted size budget (issue #4737)', () => { + // Build an entry whose operation + inverse each carry a large block snapshot, + // approximating the multi-KB snapshots real editing produces. + const bigEntry = (index: number, approxBytes = 15_000) => { + const snapshot = { + id: `block-${index}`, + type: 'action', + name: `Block ${index}`, + position: { x: index, y: index }, + subBlocks: { note: { id: 'note', type: 'long-input', value: 'x'.repeat(approxBytes) } }, + } + return createRemoveBlockEntry(`block-${index}`, snapshot, { + workflowId, + userId, + createdAt: index, + }) + } + + it('keeps the persisted payload within a sane byte budget under heavy editing', () => { + const { push } = useUndoRedoStore.getState() + + // A single stack at default capacity (100) of ~30 KB entries serializes to + // ~3 MB — enough on its own to exhaust the origin's shared ~5 MB budget and + // make an unrelated persisted store throw QuotaExceededError on its next + // write. The persisted footprint must stay bounded regardless of op count. + for (let i = 0; i < 100; i++) { + push(workflowId, userId, bigEntry(i)) + } + + const persisted = global.localStorage.getItem('workflow-undo-redo') + expect(persisted).not.toBeNull() + + // A sane share of the ~5 MB origin budget, leaving room for sibling stores. + const SANE_BUDGET_BYTES = 2 * 1024 * 1024 + expect(persisted!.length).toBeLessThanOrEqual(SANE_BUDGET_BYTES) + }) + + it('keeps recent history usable after trimming (newest ops survive)', () => { + const { push, getStackSizes } = useUndoRedoStore.getState() + + for (let i = 0; i < 100; i++) { + push(workflowId, userId, bigEntry(i)) + } + + // Trimming targets storage only; the in-memory stack stays fully intact, so + // the current session remains undoable to its capacity. + expect(getStackSizes(workflowId, userId).undoSize).toBe(100) + + const persisted = JSON.parse(global.localStorage.getItem('workflow-undo-redo')!) + const undo = persisted.state.stacks[`${workflowId}:${userId}`].undo + // Some history is persisted, and it's the newest slice (oldest evicted first). + expect(undo.length).toBeGreaterThan(0) + expect(undo.length).toBeLessThan(100) + const createdAts = undo.map((e: { createdAt: number }) => e.createdAt) + expect(createdAts[createdAts.length - 1]).toBe(99) + expect(Math.min(...createdAts)).toBeGreaterThan(0) + }) + }) + + describe('trimPersistedStateToBudget', () => { + const entryOfSize = (createdAt: number, chars: number) => ({ + id: `e-${createdAt}`, + createdAt, + operation: { id: `op-${createdAt}`, data: { blob: 'x'.repeat(chars) } }, + inverse: { id: `inv-${createdAt}`, data: {} }, + }) + + it('returns the state unchanged when already within budget', () => { + const state = { + capacity: 100, + stacks: { 'wf:user': { undo: [entryOfSize(1, 10)], redo: [], lastUpdated: 1 } }, + } as any + expect(trimPersistedStateToBudget(state, 1024 * 1024)).toBe(state) + }) + + it('evicts oldest-first across stacks until under budget', () => { + const state = { + capacity: 100, + stacks: { + a: { + undo: [entryOfSize(1, 5000), entryOfSize(4, 5000)], + redo: [entryOfSize(2, 5000)], + lastUpdated: 4, + }, + b: { undo: [entryOfSize(3, 5000)], redo: [], lastUpdated: 3 }, + }, + } as any + + const budget = 12_000 + const trimmed = trimPersistedStateToBudget(state, budget) + + expect(JSON.stringify(trimmed).length).toBeLessThanOrEqual(budget) + // The oldest entries (createdAt 1, 2) are gone; the newest (3, 4) survive. + const survivors = Object.values(trimmed.stacks) + .flatMap((s: any) => [...s.undo, ...s.redo]) + .map((e: any) => e.createdAt) + .sort((x, y) => x - y) + expect(survivors).toEqual([3, 4]) + }) + + it('does not mutate the input state', () => { + const state = { + capacity: 100, + stacks: { + a: { undo: [entryOfSize(1, 5000), entryOfSize(2, 5000)], redo: [], lastUpdated: 2 }, + }, + } as any + + trimPersistedStateToBudget(state, 6000) + expect(state.stacks.a.undo).toHaveLength(2) + }) + + it('drops stacks emptied by eviction', () => { + const state = { + capacity: 100, + stacks: { + old: { undo: [entryOfSize(1, 8000)], redo: [], lastUpdated: 1 }, + fresh: { undo: [entryOfSize(2, 100)], redo: [], lastUpdated: 2 }, + }, + } as any + + const trimmed = trimPersistedStateToBudget(state, 4000) + expect(trimmed.stacks.old).toBeUndefined() + expect(trimmed.stacks.fresh).toBeDefined() + }) + }) }) diff --git a/apps/sim/stores/undo-redo/store.ts b/apps/sim/stores/undo-redo/store.ts index 30ec1a3a237..844c6768f2c 100644 --- a/apps/sim/stores/undo-redo/store.ts +++ b/apps/sim/stores/undo-redo/store.ts @@ -20,6 +20,24 @@ const logger = createLogger('UndoRedoStore') const DEFAULT_CAPACITY = 100 const MAX_STACKS = 5 +/** + * Upper bound on the *serialized* size of the persisted history. + * + * DEFAULT_CAPACITY and MAX_STACKS bound how many operations are retained, not how + * many bytes they occupy — and a single operation can carry several KB of block + * snapshots. Left unbounded by size, the history can still grow to multiple + * megabytes and exhaust the origin's shared ~5 MB localStorage budget; the + * resulting QuotaExceededError then surfaces in whichever *other* persisted store + * writes next (notification-storage, panel state, …), so the store that throws is + * the victim, not the culprit. Capping the footprint here keeps undo/redo from + * starving its siblings. Only the persisted copy is trimmed (oldest operations + * first); the in-memory history is left intact, so the current session stays + * fully undoable. + */ +// ~2 MB: a conservative minority of the typical ~5 MB origin budget, so the +// majority stays available to sibling persisted stores. +export const MAX_PERSISTED_BYTES = 2 * 1024 * 1024 + let recordingSuspendDepth = 0 function isRecordingSuspended(): boolean { @@ -120,6 +138,72 @@ function isOperationApplicable( } } +type PersistedUndoRedoState = Pick + +function serializedLength(value: unknown): number { + return JSON.stringify(value)?.length ?? 0 +} + +/** + * Returns a copy of the persisted state trimmed so its serialized size fits + * `maxBytes`, evicting the oldest operations (by `createdAt`) across every stack + * first and dropping any stack left empty. The input is never mutated, so the + * live in-memory history is unaffected — only what gets written to storage shrinks. + */ +export function trimPersistedStateToBudget( + state: PersistedUndoRedoState, + maxBytes: number +): PersistedUndoRedoState { + if (serializedLength(state) <= maxBytes) return state + + // Clone stacks (and their arrays) so eviction never touches the live state. + const stacks: PersistedUndoRedoState['stacks'] = {} + for (const [key, stack] of Object.entries(state.stacks)) { + stacks[key] = { ...stack, undo: [...stack.undo], redo: [...stack.redo] } + } + + // Every removable entry, oldest first. + const removable = Object.entries(stacks) + .flatMap(([key, stack]) => [ + ...stack.undo.map((entry) => ({ key, list: 'undo' as const, entry })), + ...stack.redo.map((entry) => ({ key, list: 'redo' as const, entry })), + ]) + .sort((a, b) => a.entry.createdAt - b.entry.createdAt) + + const drop = ({ key, list, entry }: (typeof removable)[number]): void => { + const stack = stacks[key] + if (!stack) return + const arr = stack[list] + const idx = arr.indexOf(entry) + if (idx !== -1) arr.splice(idx, 1) + // Reclaim the key/overhead of a stack emptied by eviction. + if (stack.undo.length === 0 && stack.redo.length === 0) delete stacks[key] + } + + // Pass 1: estimate-based bulk eviction. Subtracting each entry's own serialized + // length (plus a separating comma) avoids re-serializing the whole payload on + // every eviction, which would be O(n²) over the megabytes involved. + let approxBytes = serializedLength(state) + let i = 0 + for (; i < removable.length && approxBytes > maxBytes; i++) { + approxBytes -= serializedLength(removable[i].entry) + 1 + drop(removable[i]) + } + + // Pass 2: the estimate can undershoot the true reduction (structural overhead it + // doesn't attribute to entries), so verify exactly and keep dropping the oldest + // survivors until the invariant holds. In practice this runs zero or one times. + while ( + i < removable.length && + serializedLength({ stacks, capacity: state.capacity }) > maxBytes + ) { + drop(removable[i]) + i++ + } + + return { stacks, capacity: state.capacity } +} + export const useUndoRedoStore = create()( persist( (set, get) => ({ @@ -502,10 +586,11 @@ export const useUndoRedoStore = create()( { name: 'workflow-undo-redo', storage: createJSONStorage(() => safeStorageAdapter), - partialize: (state) => ({ - stacks: state.stacks, - capacity: state.capacity, - }), + partialize: (state) => + trimPersistedStateToBudget( + { stacks: state.stacks, capacity: state.capacity }, + MAX_PERSISTED_BYTES + ), } ) ) From 20d75659c1ee36da7d079a33db3e82b52b8a1f9f Mon Sep 17 00:00:00 2001 From: rohitsudhakar1 Date: Wed, 5 Aug 2026 22:45:05 -0700 Subject: [PATCH 2/2] fix(undo-redo): evict furthest-from-use first so redo order survives trimming Review caught that ordering eviction purely by createdAt is correct for the undo stack but inverted for redo. Both stacks are consumed from their end, so redo's next entry is its oldest, and trimming oldest-first removed the operation redo needed next while keeping the later ones that depend on it. After a reload redo could skip a step or replay against the wrong graph. Evict by depth from the end of each array instead, which drops the front first and matches the capacity policy that keeps the tail via slice(-capacity). createdAt now only breaks ties. Also type the test fixtures against PersistedUndoRedoState and OperationEntry instead of any, and add a regression test asserting the next redo operation survives while the front of the redo stack is evicted. --- apps/sim/stores/undo-redo/store.test.ts | 95 ++++++++++++++----------- apps/sim/stores/undo-redo/store.ts | 60 +++++++++++----- 2 files changed, 96 insertions(+), 59 deletions(-) diff --git a/apps/sim/stores/undo-redo/store.test.ts b/apps/sim/stores/undo-redo/store.test.ts index 91ef4198950..05a162827b3 100644 --- a/apps/sim/stores/undo-redo/store.test.ts +++ b/apps/sim/stores/undo-redo/store.test.ts @@ -21,12 +21,13 @@ import { createUpdateParentEntry, } from '@sim/testing' import { beforeEach, describe, expect, it } from 'vitest' +import type { PersistedUndoRedoState } from '@/stores/undo-redo/store' import { runWithUndoRedoRecordingSuspended, trimPersistedStateToBudget, useUndoRedoStore, } from '@/stores/undo-redo/store' -import type { UpdateParentOperation } from '@/stores/undo-redo/types' +import type { OperationEntry, UpdateParentOperation } from '@/stores/undo-redo/types' describe('useUndoRedoStore', () => { const workflowId = 'wf-test' @@ -827,66 +828,80 @@ describe('useUndoRedoStore', () => { }) describe('trimPersistedStateToBudget', () => { - const entryOfSize = (createdAt: number, chars: number) => ({ - id: `e-${createdAt}`, - createdAt, - operation: { id: `op-${createdAt}`, data: { blob: 'x'.repeat(chars) } }, - inverse: { id: `inv-${createdAt}`, data: {} }, + const entryOfSize = (createdAt: number, chars: number): OperationEntry => + ({ + id: `e-${createdAt}`, + createdAt, + operation: { id: `op-${createdAt}`, data: { blob: 'x'.repeat(chars) } }, + inverse: { id: `inv-${createdAt}`, data: {} }, + }) as unknown as OperationEntry + + const persisted = (stacks: PersistedUndoRedoState['stacks']): PersistedUndoRedoState => ({ + capacity: 100, + stacks, }) + const createdAtsOf = (state: PersistedUndoRedoState): number[] => + Object.values(state.stacks) + .flatMap((stack) => [...stack.undo, ...stack.redo]) + .map((entry) => entry.createdAt) + .sort((x, y) => x - y) + it('returns the state unchanged when already within budget', () => { - const state = { - capacity: 100, - stacks: { 'wf:user': { undo: [entryOfSize(1, 10)], redo: [], lastUpdated: 1 } }, - } as any + const state = persisted({ + 'wf:user': { undo: [entryOfSize(1, 10)], redo: [], lastUpdated: 1 }, + }) expect(trimPersistedStateToBudget(state, 1024 * 1024)).toBe(state) }) - it('evicts oldest-first across stacks until under budget', () => { - const state = { - capacity: 100, - stacks: { - a: { - undo: [entryOfSize(1, 5000), entryOfSize(4, 5000)], - redo: [entryOfSize(2, 5000)], - lastUpdated: 4, - }, - b: { undo: [entryOfSize(3, 5000)], redo: [], lastUpdated: 3 }, + it('evicts the entries furthest from the next use until under budget', () => { + const state = persisted({ + a: { + undo: [entryOfSize(1, 5000), entryOfSize(4, 5000)], + redo: [entryOfSize(2, 5000)], + lastUpdated: 4, }, - } as any + b: { undo: [entryOfSize(3, 5000)], redo: [], lastUpdated: 3 }, + }) const budget = 12_000 const trimmed = trimPersistedStateToBudget(state, budget) expect(JSON.stringify(trimmed).length).toBeLessThanOrEqual(budget) - // The oldest entries (createdAt 1, 2) are gone; the newest (3, 4) survive. - const survivors = Object.values(trimmed.stacks) - .flatMap((s: any) => [...s.undo, ...s.redo]) - .map((e: any) => e.createdAt) - .sort((x, y) => x - y) - expect(survivors).toEqual([3, 4]) + expect(createdAtsOf(trimmed)).toEqual([3, 4]) }) - it('does not mutate the input state', () => { - const state = { - capacity: 100, - stacks: { - a: { undo: [entryOfSize(1, 5000), entryOfSize(2, 5000)], redo: [], lastUpdated: 2 }, + it('keeps the next redo operation and evicts from the front of the redo stack', () => { + // redo is replayed from the end, so redo[length - 1] (createdAt 1) is next. + const state = persisted({ + a: { + undo: [], + redo: [entryOfSize(3, 5000), entryOfSize(2, 5000), entryOfSize(1, 5000)], + lastUpdated: 3, }, - } as any + }) + + const trimmed = trimPersistedStateToBudget(state, 11_000) + const redo = trimmed.stacks.a.redo + + expect(redo.map((entry) => entry.createdAt)).toEqual([2, 1]) + expect(redo[redo.length - 1].createdAt).toBe(1) + }) + + it('does not mutate the input state', () => { + const state = persisted({ + a: { undo: [entryOfSize(1, 5000), entryOfSize(2, 5000)], redo: [], lastUpdated: 2 }, + }) trimPersistedStateToBudget(state, 6000) expect(state.stacks.a.undo).toHaveLength(2) }) it('drops stacks emptied by eviction', () => { - const state = { - capacity: 100, - stacks: { - old: { undo: [entryOfSize(1, 8000)], redo: [], lastUpdated: 1 }, - fresh: { undo: [entryOfSize(2, 100)], redo: [], lastUpdated: 2 }, - }, - } as any + const state = persisted({ + old: { undo: [entryOfSize(1, 8000)], redo: [], lastUpdated: 1 }, + fresh: { undo: [entryOfSize(2, 100)], redo: [], lastUpdated: 2 }, + }) const trimmed = trimPersistedStateToBudget(state, 4000) expect(trimmed.stacks.old).toBeUndefined() diff --git a/apps/sim/stores/undo-redo/store.ts b/apps/sim/stores/undo-redo/store.ts index 844c6768f2c..167c87779ec 100644 --- a/apps/sim/stores/undo-redo/store.ts +++ b/apps/sim/stores/undo-redo/store.ts @@ -138,17 +138,32 @@ function isOperationApplicable( } } -type PersistedUndoRedoState = Pick +/** The slice of {@link UndoRedoState} that `persist` writes to storage. */ +export type PersistedUndoRedoState = Pick +/** Serialized length of `value`, or 0 when it is not serializable. */ function serializedLength(value: unknown): number { return JSON.stringify(value)?.length ?? 0 } /** - * Returns a copy of the persisted state trimmed so its serialized size fits - * `maxBytes`, evicting the oldest operations (by `createdAt`) across every stack - * first and dropping any stack left empty. The input is never mutated, so the - * live in-memory history is unaffected — only what gets written to storage shrinks. + * Trims a copy of the persisted state so its serialized size fits `maxBytes`. + * + * Both stacks are consumed from their end: `undo()` takes `undo[undo.length - 1]` + * and `redo()` takes `redo[redo.length - 1]`. Eviction therefore removes entries + * furthest from the next use first, which is the front of each array, matching the + * capacity policy that keeps the tail via `slice(-capacity)`. Ordering purely by + * `createdAt` would be correct for `undo` but inverted for `redo`, whose next entry + * is its oldest, and would drop the operation redo needs next while keeping the + * later ones that depend on it. + * + * Any stack emptied by eviction is removed so its key and overhead are reclaimed. + * The input is never mutated, so the live in-memory history is unaffected and only + * what gets written to storage shrinks. + * + * @param state - The persisted slice to trim. Left untouched. + * @param maxBytes - Serialized-size ceiling for the returned state. + * @returns `state` itself when already within budget, otherwise a trimmed copy. */ export function trimPersistedStateToBudget( state: PersistedUndoRedoState, @@ -156,19 +171,23 @@ export function trimPersistedStateToBudget( ): PersistedUndoRedoState { if (serializedLength(state) <= maxBytes) return state - // Clone stacks (and their arrays) so eviction never touches the live state. const stacks: PersistedUndoRedoState['stacks'] = {} for (const [key, stack] of Object.entries(state.stacks)) { stacks[key] = { ...stack, undo: [...stack.undo], redo: [...stack.redo] } } - // Every removable entry, oldest first. const removable = Object.entries(stacks) - .flatMap(([key, stack]) => [ - ...stack.undo.map((entry) => ({ key, list: 'undo' as const, entry })), - ...stack.redo.map((entry) => ({ key, list: 'redo' as const, entry })), - ]) - .sort((a, b) => a.entry.createdAt - b.entry.createdAt) + .flatMap(([key, stack]) => + (['undo', 'redo'] as const).flatMap((list) => + stack[list].map((entry, index) => ({ + key, + list, + entry, + depth: stack[list].length - 1 - index, + })) + ) + ) + .sort((a, b) => b.depth - a.depth || a.entry.createdAt - b.entry.createdAt) const drop = ({ key, list, entry }: (typeof removable)[number]): void => { const stack = stacks[key] @@ -176,13 +195,14 @@ export function trimPersistedStateToBudget( const arr = stack[list] const idx = arr.indexOf(entry) if (idx !== -1) arr.splice(idx, 1) - // Reclaim the key/overhead of a stack emptied by eviction. if (stack.undo.length === 0 && stack.redo.length === 0) delete stacks[key] } - // Pass 1: estimate-based bulk eviction. Subtracting each entry's own serialized - // length (plus a separating comma) avoids re-serializing the whole payload on - // every eviction, which would be O(n²) over the megabytes involved. + /* + * Pass 1 evicts in bulk against an estimate. Subtracting each entry's own + * serialized length (plus a separating comma) avoids re-serializing the whole + * payload on every eviction, which would be O(n^2) over the megabytes involved. + */ let approxBytes = serializedLength(state) let i = 0 for (; i < removable.length && approxBytes > maxBytes; i++) { @@ -190,9 +210,11 @@ export function trimPersistedStateToBudget( drop(removable[i]) } - // Pass 2: the estimate can undershoot the true reduction (structural overhead it - // doesn't attribute to entries), so verify exactly and keep dropping the oldest - // survivors until the invariant holds. In practice this runs zero or one times. + /* + * The estimate can undershoot the true reduction because it does not attribute + * structural overhead to entries, so pass 2 verifies exactly and keeps dropping + * survivors until the invariant holds. In practice it runs zero or one times. + */ while ( i < removable.length && serializedLength({ stacks, capacity: state.capacity }) > maxBytes