test: add property-based FSM tests for workflow persistence (#9370)

## Summary

- Fixes #9319
- Add [fast-check](https://github.com/dubzzz/fast-check) property-based
testing with FSM (Finite State Machine) traversal to automatically
explore state combinations in the workflow persistence system
- Fix a real bug in `saveDraft()` discovered by the FSM test: orphan
cleanup in `loadIndex()` could delete a just-written payload when the
in-memory cache was empty

## Why this is needed

#9317 exposed a class of bug where two independently correct changes
interact to cause workflow loss. Conventional unit tests verify
specific, hand-picked scenarios and cannot catch these cross-PR
interaction bugs.

### AS IS (before)

| Aspect | Status |
|---|---|
| Testing approach | Example-based: developer picks specific inputs and
expected outputs |
| State coverage | Only explicitly written scenarios are tested |
| Cross-interaction bugs | Not detectable — each test runs one isolated
path |
| Bug in `saveDraft` | Undetected — `loadIndex()` orphan cleanup could
delete a just-written payload after `reset()` |

### TO BE (after)

| Aspect | Status |
|---|---|
| Testing approach | Property-based: fast-check generates **200 random
command sequences** per run |
| State coverage | Random exploration of `SaveDraft → GetDraft →
RemoveDraft → MoveDraft → GetMostRecentPath → Reset` combinations |
| Cross-interaction bugs | Detected automatically — fast-check shrinks
failing sequences to minimal reproductions |
| Bug in `saveDraft` | Found and fixed — `loadIndex()` now runs
**before** `writePayload()` to prevent orphan cleanup race |

## What fast-check does

fast-check is a property-based testing library. Instead of testing "does
this specific input produce this specific output?", it tests "does this
**property** hold for **all possible inputs**?"

For FSM testing specifically, fast-check:
1. Takes a set of **commands** (SaveDraft, GetDraft, RemoveDraft,
MoveDraft, GetMostRecentPath, Reset)
2. Generates **random sequences** of these commands
3. Runs each sequence against both a **model** (simplified oracle) and
the **real system** (store + localStorage)
4. Verifies **invariants** after every mutating command (index/payload
consistency, no orphans, LRU correctness, model agreement)
5. When a failure is found, **shrinks** the sequence to the minimal
reproduction

Example: the bug this PR fixes was shrunk to just 4 commands:
```
SaveDraft(d.json) → RemoveDraft(d.json) → Reset() → SaveDraft(a.json) ✗
```

## Changes

| File | Change |
|---|---|
| `package.json` / `pnpm-workspace.yaml` | Add `fast-check`
devDependency |
| `draftCacheV2.property.test.ts` | 7 property tests for pure index
functions |
| `workflowDraftStoreV2.fsm.test.ts` | FSM test: 6 commands, invariant
checking, 200 runs |
| `workflowDraftStoreV2.ts` | Fix: move `loadIndex()` before
`writePayload()` in `saveDraft()` |

## Test plan

- [x] `pnpm test:unit` — all 117 persistence tests pass (including 7
property + 1 FSM)
- [x] `pnpm typecheck` — clean
- [x] `pnpm lint` — clean
- [x] Pre-commit hooks pass (format, lint, typecheck)
- [x] Pre-push hook passes (knip)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9370-test-add-property-based-FSM-tests-for-workflow-persistence-3196d73d3650813daa98cdd8bef7e975)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Dante
2026-03-13 23:28:38 +09:00
committed by GitHub
parent b5ddc70233
commit 4e85537b15
6 changed files with 590 additions and 5 deletions

View File

@@ -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:",

19
pnpm-lock.yaml generated
View File

@@ -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

View File

@@ -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

View File

@@ -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
}
}
)
)
})
})

View File

@@ -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<string, DraftData>
lruOrder: string[] // paths, oldest→newest
}
interface PersistenceReal {
draftStore: ReturnType<typeof useWorkflowDraftStoreV2>
}
// ── 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<PersistenceModel, PersistenceReal> {
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<PersistenceModel, PersistenceReal> {
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<PersistenceModel, PersistenceReal> {
constructor(readonly path: string) {}
check(model: Readonly<PersistenceModel>) {
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<PersistenceModel, PersistenceReal> {
constructor(
readonly oldPath: string,
readonly newPath: string,
readonly newName: string
) {}
check(model: Readonly<PersistenceModel>) {
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<PersistenceModel, PersistenceReal> {
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<PersistenceModel, PersistenceReal> {
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 }
)
}
)
})

View File

@@ -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,