diff --git a/package.json b/package.json index 9fe4efefe7..c962055115 100644 --- a/package.json +++ b/package.json @@ -154,6 +154,7 @@ "eslint-plugin-storybook": "catalog:", "eslint-plugin-unused-imports": "catalog:", "eslint-plugin-vue": "catalog:", + "fast-check": "catalog:", "fs-extra": "^11.2.0", "globals": "catalog:", "happy-dom": "catalog:", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5f4e45abac..c42ce90f2f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -204,6 +204,9 @@ catalogs: eslint-plugin-vue: specifier: ^10.6.2 version: 10.6.2 + fast-check: + specifier: ^4.5.3 + version: 4.5.3 firebase: specifier: ^11.6.0 version: 11.6.0 @@ -672,6 +675,9 @@ importers: eslint-plugin-vue: specifier: 'catalog:' version: 10.6.2(@typescript-eslint/parser@8.49.0(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.1(jiti@2.6.1))(vue-eslint-parser@10.4.0(eslint@9.39.1(jiti@2.6.1))) + fast-check: + specifier: 'catalog:' + version: 4.5.3 fs-extra: specifier: ^11.2.0 version: 11.3.2 @@ -5547,6 +5553,10 @@ packages: extendable-media-recorder@9.2.27: resolution: {integrity: sha512-2X+Ixi1cxLek0Cj9x9atmhQ+apG+LwJpP2p3ypP8Pxau0poDnicrg7FTfPVQV5PW/3DHFm/eQ16vbgo5Yk3HGQ==} + fast-check@4.5.3: + resolution: {integrity: sha512-IE9csY7lnhxBnA8g/WI5eg/hygA6MGWJMSNfFRrBlXUciADEhS1EDB0SIsMSvzubzIlOBbVITSsypCsW717poA==} + engines: {node: '>=12.17.0'} + fast-deep-equal@3.1.3: resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==} @@ -7348,6 +7358,9 @@ packages: resolution: {integrity: sha512-LjgDO2zPtoXP2wJpDjZrGdojii1uqO0cnwKoIoUzkfS98HDmbeiGmYiXo3lXeFlq2xvne1QFQhwYXSUCLKtEuA==} engines: {node: '>=12.20'} + pure-rand@7.0.1: + resolution: {integrity: sha512-oTUZM/NAZS8p7ANR3SHh30kXB+zK2r2BPcEn/awJIbOvq82WoMN4p62AWWp3Hhw50G0xMsw1mhIBLqHw64EcNQ==} + qified@0.5.3: resolution: {integrity: sha512-kXuQdQTB6oN3KhI6V4acnBSZx8D2I4xzZvn9+wFLLFCoBNQY/sFnCW6c43OL7pOQ2HvGV4lnWIXNmgfp7cTWhQ==} engines: {node: '>=20'} @@ -13886,6 +13899,10 @@ snapshots: subscribable-things: 2.1.53 tslib: 2.8.1 + fast-check@4.5.3: + dependencies: + pure-rand: 7.0.1 + fast-deep-equal@3.1.3: {} fast-glob@3.3.3: @@ -16109,6 +16126,8 @@ snapshots: dependencies: escape-goat: 4.0.0 + pure-rand@7.0.1: {} + qified@0.5.3: dependencies: hookified: 1.14.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c455964545..793e260854 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -69,6 +69,7 @@ catalog: eslint-plugin-storybook: ^10.2.10 eslint-plugin-unused-imports: ^4.3.0 eslint-plugin-vue: ^10.6.2 + fast-check: ^4.5.3 firebase: ^11.6.0 glob: ^13.0.6 globals: ^16.5.0 diff --git a/src/platform/workflow/persistence/base/draftCacheV2.property.test.ts b/src/platform/workflow/persistence/base/draftCacheV2.property.test.ts new file mode 100644 index 0000000000..86027603ab --- /dev/null +++ b/src/platform/workflow/persistence/base/draftCacheV2.property.test.ts @@ -0,0 +1,188 @@ +import * as fc from 'fast-check' +import { describe, expect, it } from 'vitest' + +import { + createEmptyIndex, + removeEntry, + touchOrder, + upsertEntry +} from './draftCacheV2' + +const arbPath = fc + .stringMatching(/^[a-z0-9]{1,20}$/) + .map((s) => `workflows/${s}.json`) + +const arbMeta = fc.record({ + name: fc.string({ minLength: 1, maxLength: 10 }), + isTemporary: fc.boolean(), + updatedAt: fc.nat() +}) + +describe('draftCacheV2 properties', () => { + it('order length never exceeds limit after arbitrary upserts', () => { + fc.assert( + fc.property( + fc.array(fc.tuple(arbPath, arbMeta), { + minLength: 1, + maxLength: 50 + }), + fc.integer({ min: 1, max: 10 }), + (operations, limit) => { + let index = createEmptyIndex() + for (const [path, meta] of operations) { + index = upsertEntry(index, path, meta, limit).index + } + expect(index.order.length).toBeLessThanOrEqual(limit) + } + ) + ) + }) + + it('order and entries keys are always the same set', () => { + fc.assert( + fc.property( + fc.array(fc.tuple(arbPath, arbMeta), { + minLength: 1, + maxLength: 30 + }), + (operations) => { + let index = createEmptyIndex() + for (const [path, meta] of operations) { + index = upsertEntry(index, path, meta).index + } + const orderSet = new Set(index.order) + const entriesSet = new Set(Object.keys(index.entries)) + expect(orderSet).toEqual(entriesSet) + } + ) + ) + }) + + it('order never contains duplicates', () => { + fc.assert( + fc.property( + fc.array(fc.tuple(arbPath, arbMeta), { + minLength: 1, + maxLength: 30 + }), + (operations) => { + let index = createEmptyIndex() + for (const [path, meta] of operations) { + index = upsertEntry(index, path, meta).index + } + expect(new Set(index.order).size).toBe(index.order.length) + } + ) + ) + }) + + it('upserting same path twice results in exactly one entry', () => { + fc.assert( + fc.property(arbPath, arbMeta, arbMeta, (path, meta1, meta2) => { + let index = createEmptyIndex() + index = upsertEntry(index, path, meta1).index + index = upsertEntry(index, path, meta2).index + const count = index.order.length + expect(count).toBe(1) + }) + ) + }) + + it('remove after upsert leaves empty index', () => { + fc.assert( + fc.property(arbPath, arbMeta, (path, meta) => { + let index = createEmptyIndex() + index = upsertEntry(index, path, meta).index + index = removeEntry(index, path).index + expect(index.order).toHaveLength(0) + expect(Object.keys(index.entries)).toHaveLength(0) + }) + ) + }) + + it('touchOrder always places key at end', () => { + fc.assert( + fc.property( + fc.array(fc.string({ minLength: 1, maxLength: 8 }), { + minLength: 1, + maxLength: 10 + }), + fc.string({ minLength: 1, maxLength: 8 }), + (order, key) => { + const result = touchOrder(order, key) + expect(result[result.length - 1]).toBe(key) + } + ) + ) + }) + + it('upserted entry is always the most recent (last in order)', () => { + fc.assert( + fc.property( + fc.array(fc.tuple(arbPath, arbMeta), { + minLength: 1, + maxLength: 20 + }), + (operations) => { + let index = createEmptyIndex() + let lastPath = '' + for (const [path, meta] of operations) { + lastPath = path + index = upsertEntry(index, path, meta).index + } + const lastEntry = index.entries[index.order[index.order.length - 1]] + expect(lastEntry.path).toBe(lastPath) + } + ) + ) + }) + + it('evicted entries are the oldest in LRU order and current key is never evicted', () => { + fc.assert( + fc.property( + fc.array(fc.tuple(arbPath, arbMeta), { + minLength: 1, + maxLength: 50 + }), + fc.integer({ min: 1, max: 10 }), + (operations, limit) => { + let index = createEmptyIndex() + for (const [path, meta] of operations) { + const orderBefore = [...index.order] + const { index: newIndex, evicted } = upsertEntry( + index, + path, + meta, + limit + ) + + // Current key must never be evicted + const currentKey = newIndex.order[newIndex.order.length - 1] + for (const key of evicted) { + expect(key).not.toBe(currentKey) + } + + // Evicted keys must come from the old order + if (evicted.length > 0) { + const evictedSet = new Set(evicted) + for (const key of evicted) { + expect(orderBefore).toContain(key) + } + // Non-evicted keys preserve their relative order + const remaining = orderBefore.filter((k) => !evictedSet.has(k)) + const newOrderWithoutCurrent = newIndex.order.filter( + (k) => k !== currentKey + ) + const remainingWithoutCurrent = remaining.filter( + (k) => k !== currentKey + ) + expect(newOrderWithoutCurrent).toEqual(remainingWithoutCurrent) + } + + index = newIndex + } + } + ) + ) + }) +}) diff --git a/src/platform/workflow/persistence/stores/workflowDraftStoreV2.fsm.test.ts b/src/platform/workflow/persistence/stores/workflowDraftStoreV2.fsm.test.ts new file mode 100644 index 0000000000..32d46c6698 --- /dev/null +++ b/src/platform/workflow/persistence/stores/workflowDraftStoreV2.fsm.test.ts @@ -0,0 +1,374 @@ +import * as fc from 'fast-check' +import type { Command } from 'fast-check' + +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { MAX_DRAFTS } from '../base/draftTypes' +import { useWorkflowDraftStoreV2 } from './workflowDraftStoreV2' + +vi.mock('@/scripts/api', () => ({ + api: { + clientId: 'test-client', + initialClientId: 'test-client' + } +})) + +vi.mock('@/scripts/app', () => ({ + app: { + loadGraphData: vi.fn().mockResolvedValue(undefined) + } +})) + +// ── Model & Real ──────────────────────────────────────────────────── + +interface DraftData { + data: string + name: string + isTemporary: boolean +} + +interface PersistenceModel { + drafts: Map + lruOrder: string[] // paths, oldest→newest +} + +interface PersistenceReal { + draftStore: ReturnType +} + +// ── Invariant Helpers ─────────────────────────────────────────────── + +function assertIndexPayloadConsistency() { + const indexJson = localStorage.getItem( + 'Comfy.Workflow.DraftIndex.v2:personal' + ) + if (!indexJson) return + + const index = JSON.parse(indexJson) + const prefix = 'Comfy.Workflow.Draft.v2:personal:' + + for (const key of index.order) { + const payloadJson = localStorage.getItem(`${prefix}${key}`) + expect(payloadJson, `Missing payload for index key ${key}`).not.toBeNull() + } + + for (let i = 0; i < localStorage.length; i++) { + const lsKey = localStorage.key(i)! + if (lsKey.startsWith(prefix)) { + const draftKey = lsKey.slice(prefix.length) + expect(index.order, `Orphan payload ${draftKey}`).toContain(draftKey) + } + } + + expect(new Set(index.order).size).toBe(index.order.length) + expect(index.order.length).toBeLessThanOrEqual(MAX_DRAFTS) +} + +function assertModelMatchesReal( + model: PersistenceModel, + real: PersistenceReal +) { + for (const [path, expected] of model.drafts) { + const draft = real.draftStore.getDraft(path) + expect(draft, `Draft missing for ${path}`).not.toBeNull() + expect(draft!.data).toBe(expected.data) + expect(draft!.name).toBe(expected.name) + expect(draft!.isTemporary).toBe(expected.isTemporary) + } + + const indexJson = localStorage.getItem( + 'Comfy.Workflow.DraftIndex.v2:personal' + ) + if (model.drafts.size === 0) { + if (!indexJson) return + const index = JSON.parse(indexJson) + expect(Object.keys(index.entries)).toHaveLength(0) + return + } + const index = JSON.parse(indexJson!) + expect(Object.keys(index.entries)).toHaveLength(model.drafts.size) + + // Reverse check: every real store entry exists in the model + for (const entry of Object.values(index.entries) as { path: string }[]) { + expect( + model.drafts.has(entry.path), + `Real store has extra entry ${entry.path} not in model` + ).toBe(true) + } +} + +// ── Commands ──────────────────────────────────────────────────────── + +class SaveDraftCommand implements Command { + constructor( + readonly path: string, + readonly data: string, + readonly name: string, + readonly isTemporary: boolean + ) {} + + check() { + return true + } + + run(model: PersistenceModel, real: PersistenceReal) { + const result = real.draftStore.saveDraft(this.path, this.data, { + name: this.name, + isTemporary: this.isTemporary + }) + expect(result).toBe(true) + + model.drafts.set(this.path, { + data: this.data, + name: this.name, + isTemporary: this.isTemporary + }) + model.lruOrder = model.lruOrder.filter((p) => p !== this.path) + model.lruOrder.push(this.path) + + while (model.lruOrder.length > MAX_DRAFTS) { + const evicted = model.lruOrder.shift()! + model.drafts.delete(evicted) + } + + assertIndexPayloadConsistency() + assertModelMatchesReal(model, real) + } + + toString() { + return `SaveDraft(${this.path}, temp=${this.isTemporary})` + } +} + +class GetDraftCommand implements Command { + constructor(readonly path: string) {} + + check() { + return true + } + + run(model: PersistenceModel, real: PersistenceReal) { + const draft = real.draftStore.getDraft(this.path) + const expected = model.drafts.get(this.path) + + if (expected) { + expect(draft).not.toBeNull() + expect(draft!.data).toBe(expected.data) + expect(draft!.name).toBe(expected.name) + expect(draft!.isTemporary).toBe(expected.isTemporary) + } else { + expect(draft).toBeNull() + } + } + + toString() { + return `GetDraft(${this.path})` + } +} + +class RemoveDraftCommand implements Command { + constructor(readonly path: string) {} + + check(model: Readonly) { + return model.drafts.has(this.path) + } + + run(model: PersistenceModel, real: PersistenceReal) { + real.draftStore.removeDraft(this.path) + model.drafts.delete(this.path) + model.lruOrder = model.lruOrder.filter((p) => p !== this.path) + + assertIndexPayloadConsistency() + assertModelMatchesReal(model, real) + } + + toString() { + return `RemoveDraft(${this.path})` + } +} + +class MoveDraftCommand implements Command { + constructor( + readonly oldPath: string, + readonly newPath: string, + readonly newName: string + ) {} + + check(model: Readonly) { + return ( + this.oldPath !== this.newPath && + model.drafts.has(this.oldPath) && + !model.drafts.has(this.newPath) + ) + } + + run(model: PersistenceModel, real: PersistenceReal) { + const existing = model.drafts.get(this.oldPath)! + + real.draftStore.moveDraft(this.oldPath, this.newPath, this.newName) + + model.drafts.delete(this.oldPath) + model.drafts.set(this.newPath, { + ...existing, + name: this.newName + }) + model.lruOrder = model.lruOrder.filter((p) => p !== this.oldPath) + model.lruOrder.push(this.newPath) + + assertIndexPayloadConsistency() + assertModelMatchesReal(model, real) + } + + toString() { + return `MoveDraft(${this.oldPath} -> ${this.newPath})` + } +} + +class GetMostRecentPathCommand implements Command< + PersistenceModel, + PersistenceReal +> { + check() { + return true + } + + run(model: PersistenceModel, real: PersistenceReal) { + const result = real.draftStore.getMostRecentPath() + const expected = + model.lruOrder.length > 0 + ? model.lruOrder[model.lruOrder.length - 1] + : null + expect(result).toBe(expected) + } + + toString() { + return 'GetMostRecentPath()' + } +} + +class ResetCommand implements Command { + check() { + return true + } + + run(model: PersistenceModel, real: PersistenceReal) { + // Simulate page reload: new Pinia + new store, but storage persists + setActivePinia(createTestingPinia({ stubActions: false })) + real.draftStore = useWorkflowDraftStoreV2() + + for (const [path, expected] of model.drafts) { + const draft = real.draftStore.getDraft(path) + expect(draft, `Draft lost after reset: ${path}`).not.toBeNull() + expect(draft!.data).toBe(expected.data) + expect(draft!.name).toBe(expected.name) + expect(draft!.isTemporary).toBe(expected.isTemporary) + } + assertIndexPayloadConsistency() + } + + toString() { + return 'Reset()' + } +} + +class StoreResetCommand implements Command { + check() { + return true + } + + run(model: PersistenceModel, real: PersistenceReal) { + // Call reset() directly to clear in-memory cache without recreating Pinia. + // This exercises the orphan cleanup path in loadIndex() on next access. + real.draftStore.reset() + + for (const [path, expected] of model.drafts) { + const draft = real.draftStore.getDraft(path) + expect(draft, `Draft lost after store reset: ${path}`).not.toBeNull() + expect(draft!.data).toBe(expected.data) + expect(draft!.name).toBe(expected.name) + expect(draft!.isTemporary).toBe(expected.isTemporary) + } + assertIndexPayloadConsistency() + } + + toString() { + return 'StoreReset()' + } +} + +// ── Test Suite ────────────────────────────────────────────────────── + +describe('workflowDraftStoreV2 FSM', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + localStorage.clear() + sessionStorage.clear() + vi.clearAllMocks() + }) + + afterEach(() => { + localStorage.clear() + sessionStorage.clear() + }) + + // 33+ unique paths to exceed MAX_DRAFTS (32) and exercise LRU eviction + const pathPool = fc.constantFrom( + ...Array.from({ length: 35 }, (_, i) => `workflows/${i}.json`) + ) + + const dataPool = fc.constantFrom( + '{"nodes":[]}', + '{"nodes":[1]}', + '{"nodes":[1,2]}', + '{"version":1}', + '{"version":2}' + ) + + const namePool = fc.constantFrom('wf-1', 'wf-2', 'wf-3', 'wf-4') + + const allCommands = [ + pathPool.chain((path) => + fc + .tuple(dataPool, namePool, fc.boolean()) + .map( + ([data, name, isTemp]) => + new SaveDraftCommand(path, data, name, isTemp) + ) + ), + pathPool.map((path) => new GetDraftCommand(path)), + pathPool.map((path) => new RemoveDraftCommand(path)), + fc + .tuple(pathPool, pathPool, namePool) + .map(([old, nw, name]) => new MoveDraftCommand(old, nw, name)), + fc.constant(new GetMostRecentPathCommand()), + fc.constant(new ResetCommand()), + fc.constant(new StoreResetCommand()) + ] + + it( + 'maintains all invariants across random command sequences', + { timeout: 30_000 }, + () => { + fc.assert( + fc.property(fc.commands(allCommands, { size: 'medium' }), (cmds) => { + // Clear storage between each fast-check run + localStorage.clear() + sessionStorage.clear() + setActivePinia(createTestingPinia({ stubActions: false })) + + const model: PersistenceModel = { + drafts: new Map(), + lruOrder: [] + } + const real: PersistenceReal = { + draftStore: useWorkflowDraftStoreV2() + } + fc.modelRun(() => ({ model, real }), cmds) + }), + { numRuns: 200 } + ) + } + ) +}) diff --git a/src/platform/workflow/persistence/stores/workflowDraftStoreV2.ts b/src/platform/workflow/persistence/stores/workflowDraftStoreV2.ts index 03adf7a333..0e960aa159 100644 --- a/src/platform/workflow/persistence/stores/workflowDraftStoreV2.ts +++ b/src/platform/workflow/persistence/stores/workflowDraftStoreV2.ts @@ -100,7 +100,7 @@ export const useWorkflowDraftStoreV2 = defineStore('workflowDraftV2', () => { /** * Saves a draft (data + metadata). - * Writes payload first, then updates index. + * Primes index cache, writes payload, then persists updated index. */ function saveDraft(path: string, data: string, meta: DraftMeta): boolean { if (!isStorageAvailable()) return false @@ -109,7 +109,12 @@ export const useWorkflowDraftStoreV2 = defineStore('workflowDraftV2', () => { const draftKey = hashPath(path) const now = Date.now() - // Write payload first (before index update) + // Prime the index cache before writing payload. + // loadIndex() runs orphan cleanup on cache miss, which would + // delete a payload written before the index is updated. + const index = loadIndex() + + // Write payload before persisting the updated index const payloadWritten = writePayload(workspaceId, draftKey, { data, updatedAt: now @@ -119,9 +124,6 @@ export const useWorkflowDraftStoreV2 = defineStore('workflowDraftV2', () => { // Quota exceeded - try eviction loop return handleQuotaExceeded(path, data, meta) } - - // Update index - const index = loadIndex() const { index: newIndex, evicted } = upsertEntry( index, path,