fix(undo-redo): bound persisted history by size, not just operation count

apps/sim/stores/undo-redo/store.test.ts

@@ -21,8 +21,13 @@ import {
   createUpdateParentEntry,
 } from '@sim/testing'
 import { beforeEach, describe, expect, it } from 'vitest'
-import { runWithUndoRedoRecordingSuspended, useUndoRedoStore } from '@/stores/undo-redo/store'
-import type { UpdateParentOperation } from '@/stores/undo-redo/types'
+import type { PersistedUndoRedoState } from '@/stores/undo-redo/store'
+import {
+  runWithUndoRedoRecordingSuspended,
+  trimPersistedStateToBudget,
+  useUndoRedoStore,
+} from '@/stores/undo-redo/store'
+import type { OperationEntry, UpdateParentOperation } from '@/stores/undo-redo/types'
 
 describe('useUndoRedoStore', () => {
   const workflowId = 'wf-test'
@@ -762,4 +767,145 @@ 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): 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 = persisted({
+        'wf:user': { undo: [entryOfSize(1, 10)], redo: [], lastUpdated: 1 },
+      })
+      expect(trimPersistedStateToBudget(state, 1024 * 1024)).toBe(state)
+    })
+
+    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,
+        },
+        b: { undo: [entryOfSize(3, 5000)], redo: [], lastUpdated: 3 },
+      })
+
+      const budget = 12_000
+      const trimmed = trimPersistedStateToBudget(state, budget)
+
+      expect(JSON.stringify(trimmed).length).toBeLessThanOrEqual(budget)
+      expect(createdAtsOf(trimmed)).toEqual([3, 4])
+    })
+
+    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,
+        },
+      })
+
+      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 = 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()
+      expect(trimmed.stacks.fresh).toBeDefined()
+    })
+  })
 })

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,94 @@ function isOperationApplicable(
   }
 }
 
+/** The slice of {@link UndoRedoState} that `persist` writes to storage. */
+export type PersistedUndoRedoState = Pick<UndoRedoState, 'stacks' | 'capacity'>
+
+/** Serialized length of `value`, or 0 when it is not serializable. */
+function serializedLength(value: unknown): number {
+  return JSON.stringify(value)?.length ?? 0
+}
+
+/**
+ * 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,
+  maxBytes: number
+): PersistedUndoRedoState {
+  if (serializedLength(state) <= maxBytes) return state
+
+  const stacks: PersistedUndoRedoState['stacks'] = {}
+  for (const [key, stack] of Object.entries(state.stacks)) {
+    stacks[key] = { ...stack, undo: [...stack.undo], redo: [...stack.redo] }
+  }
+
+  const removable = Object.entries(stacks)
+    .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]
+    if (!stack) return
+    const arr = stack[list]
+    const idx = arr.indexOf(entry)
+    if (idx !== -1) arr.splice(idx, 1)
+    if (stack.undo.length === 0 && stack.redo.length === 0) delete stacks[key]
+  }
+
+  /*
+   * 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++) {
+    approxBytes -= serializedLength(removable[i].entry) + 1
+    drop(removable[i])
+  }
+
+  /*
+   * 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
+  ) {
+    drop(removable[i])
+    i++
+  }
+
+  return { stacks, capacity: state.capacity }
+}
+
 export const useUndoRedoStore = create<UndoRedoState>()(
   persist(
     (set, get) => ({
@@ -502,10 +608,11 @@ export const useUndoRedoStore = create<UndoRedoState>()(
     {
       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
+        ),
     }
   )
 )