feat(persistence): fix QuotaExceededError and cross-workspace draft leakage#8520
Merged
christian-byrne merged 8 commits intomainfrom Feb 21, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughImplements V2 per-draft workflow persistence with workspace-scoped keys, a V1→V2 migration, session-pointer tab restoration, per-workspace in-memory index caching, workspace-aware storage I/O, logout confirmation for unsaved edits, and related tests and browser-test updates. Changes
Sequence DiagramsequenceDiagram
participant App as App
participant Persistence as useWorkflowPersistenceV2
participant Migration as migrateV1toV2
participant Store as workflowDraftStoreV2
participant Storage as StorageIO
App->>Persistence: initializeWorkflow()
Persistence->>Migration: isV2MigrationComplete(workspaceId)
alt migration needed
Persistence->>Migration: migrateV1toV2(workspaceId)
Migration->>Storage: read V1 drafts (localStorage)
Migration->>Store: saveDraft(...) per draft
Store->>Storage: write DraftIndex.v2 and payloads
Migration-->>Persistence: return migrated count
end
Persistence->>Store: loadIndex(currentWorkspaceId)
Store->>Storage: read DraftIndex.v2:<workspaceId>
Storage-->>Store: index
Persistence->>Store: loadActiveOrLatestWorkflow(clientId, workspaceId)
Store->>Storage: readActivePath(clientId, targetWorkspaceId)
Storage-->>Store: pointer (may migrate)
Note over Persistence,Store: On graph change → debounced saveDraft → update index & session pointer
Persistence->>Store: saveDraft(path, payload)
Store->>Storage: persist payload & update DraftIndex.v2
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Contributor
Author
This was referenced Feb 1, 2026
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/20/2026, 06:03:53 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
|
Playwright: ✅ 530 passed, 0 failed · 2 flaky 📊 Browser Reports
|
dosubot
bot
added
the
size:XL
|
Related Documentation Checked 0 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
christian-byrne
changed the title
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
fd0b584 to
5a1e9ae
Compare
christian-byrne
requested review from
a team,
Myestery,
comfyui-wiki,
jtydhr88,
ltdrdata and
viva-jinyi
as code owners
christian-byrne
force-pushed
the
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
branch
from
0c88af0 to
07ae110
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
b7118ee to
bec1fb2
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
branch
from
07ae110 to
f1bc97f
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
bec1fb2 to
2cb0251
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
branch
from
f1bc97f to
84ac4bd
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
2cb0251 to
0cda81c
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
branch
from
84ac4bd to
a00870f
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
2 times, most recently
from
2cf9bbe to
b19bb1e
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
0884d1d to
b62ed7e
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
branch
from
b77c98a to
e07f341
Compare
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
3 times, most recently
from
a83bbea to
e5c8c5b
Compare
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
e2902ea to
f20f794
Compare
Base automatically changed from
01-31-feat_persistence_rewrite_store_with_per-draft_keys_3_4_
to
main
February 20, 2026 07:53
Complete the V2 integration: - useWorkflowPersistenceV2: New composable with: - 512ms debounce on graphChanged events - Uses V2 draft store with per-draft keys - Uses tab state composable for session pointers - Clears storage on signout (cloud only) - migrateV1toV2: One-time migration from V1 blob to V2 per-draft keys - Runs on first load if V2 index doesn't exist - Preserves LRU order during migration - Keeps V1 data intact for rollback until 2026-07-15 The V2 composable is ready but not yet wired as the default export. This allows gradual rollout and easy rollback. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c16f4-05a2-779d-aa0e-a0e098308a95
Port two bug fixes to V2 persistence system: From #8854 (clear draft on workflow close): - workflowStore.ts: removeDraft() now runs for ALL workflows, not just temporary - workflowService.ts: remove draft BEFORE tab switch to prevent re-saving From #8851 (skip drafts when Persist disabled): - comfyWorkflow.ts: guard draft loading with Persist setting check - useWorkflowPersistenceV2.ts: watch to clear drafts when Persist toggled off Amp-Thread-ID: https://ampcode.com/threads/T-019c5898-2097-7710-aab9-52c38a97399a
christian-byrne
force-pushed
the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
from
bb7b441 to
816759e
Compare
📦 Bundle: 4.32 MB gzip 🔴 +3.26 kBDetailsSummary
Category Glance App Entry Points — 21.4 kB (baseline 21.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 949 kB (baseline 933 kB) • 🔴 +15.9 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.6 kB (baseline 68.6 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 706 B (baseline 706 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 43.2 kB (baseline 43.2 kB) • ⚪ 0 BReusable component library chunks
Status: 10 added / 10 removed Data & Services — 2.47 MB (baseline 2.47 MB) • 🔴 +231 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 57.6 kB (baseline 57.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • 🔴 +18 BExternal libraries and shared vendor chunks
Status: 1 added / 1 removed Other — 7.38 MB (baseline 7.38 MB) • 🔴 +163 BBundles that do not match a named category
Status: 63 added / 63 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@browser_tests/tests/interaction.spec.ts`:
- Around line 736-739: The fixed 600ms setTimeout in comfyPage.page.evaluate is
brittle; replace it with a persistence-based wait that polls localStorage for
the draft index update (the key used by V2 persistence) instead of sleeping.
Capture the current localStorage draft index value before the change, then use
comfyPage.page.waitForFunction (or an evaluate loop) to wait until
localStorage.getItem('draftIndex') (or the exact V2 draft index key used in the
app) changes/updates, and only proceed once the new value is observed so the
debounced persistence has completed.
In `@src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts`:
- Around line 186-193: The watcher on workflowStore.activeWorkflow?.key can
persist the new workflow before pending debounced writes from the previous
workflow complete; call debouncedPersist.flush() before calling
persistCurrentWorkflow() so any pending edits for the previous workflow are
flushed, e.g. flush debouncedPersist in the watch callback prior to invoking
persistCurrentWorkflow() (referencing debouncedPersist.flush(),
persistCurrentWorkflow(), and the watch on workflowStore.activeWorkflow?.key).
In `@src/platform/workflow/persistence/migration/migrateV1toV2.ts`:
- Around line 73-78: When no V1 data exists the code calls
writeIndex(workspaceId, createEmptyIndex()) but ignores its result and possible
failures; change migrateV1toV2 so that after calling writeIndex you check its
return value or catch errors from it, log or surface the error (using the same
logger used elsewhere), and return a non-zero/failure code on write failure
instead of returning 0. Locate the block around readV1Drafts, createEmptyIndex
and writeIndex and ensure writeIndex's promise/return is awaited/inspected and
migration returns a failure status when the write fails.
src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts
Show resolved
Hide resolved
dosubot
bot
added
size:XXL
…ebounce on switch, handle writeIndex failure, replace brittle timeout - Extract findAndMigratePointer/readSessionPointer to DRY duplicated fallback search in storageIO - Flush debouncedPersist before persisting new workflow on switch to prevent data loss - Handle writeIndex failure when creating empty V2 index during migration - Replace 600ms setTimeout with localStorage-based waitForFunction in browser test - Revert unrelated package.json change Amp-Thread-ID: https://ampcode.com/threads/T-019c7a1f-7f9e-778b-925c-bda5192177f4
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts (1)
64-76: Prefer function declarations for internal helpers.These helper functions don’t require expression form and can be declared as functions to align with repository conventions and improve hoisting clarity.
♻️ Example refactor (apply similarly to other helpers)
- const ensureTemplateQueryFromIntent = async () => { + async function ensureTemplateQueryFromIntent() { hydratePreservedQuery(TEMPLATE_NAMESPACE) const mergedQuery = mergePreservedQueryIntoQuery( TEMPLATE_NAMESPACE, route.query ) @@ - } + }As per coding guidelines: "Do not use function expressions if it's possible to use function declarations instead".
Also applies to: 91-128, 133-184, 240-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts` around lines 64 - 76, The helper currently defined as a const arrow function ensureTemplateQueryFromIntent should be converted to a function declaration (e.g., function ensureTemplateQueryFromIntent() { ... }) to follow repository conventions and enable hoisting; update its definition and any other internal helpers in this file (the other const arrow helpers mentioned) to function declarations as well, keeping the same logic, parameters and return values so callers (like router.replace/mergePreservedQuery/hydratePreservedQuery usage) continue to work unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/workflow/persistence/base/storageIO.ts`:
- Around line 193-213: The findAndMigratePointer function currently calls
JSON.parse on each sessionStorage entry which can throw on malformed JSON and
abort the scan; update findAndMigratePointer to wrap the
JSON.parse(sessionStorage.getItem(storageKey)) in a try/catch, skip (and
optionally remove) entries that fail to parse, and continue the loop so valid
pointers later in sessionStorage can still be migrated; keep the existing
behavior of matching pointer.workspaceId === targetWorkspaceId, setting newKey,
removing the old storageKey, and returning the pointer when a match is found.
- Around line 220-233: readSessionPointer currently returns a parsed pointer
from sessionStorage without checking that the parsed object's workspaceId
matches the provided targetWorkspaceId, allowing potential cross-workspace
leakage; update readSessionPointer to, after JSON.parse, if targetWorkspaceId is
provided validate parsed.workspaceId === targetWorkspaceId and if it does not
match either call findAndMigratePointer<T>(key, prefix, targetWorkspaceId) or
return null (preserve original fallback behavior), ensuring the function
enforces workspace consistency internally; refer to readSessionPointer,
targetWorkspaceId, parsed object's workspaceId, and findAndMigratePointer when
making the change.
---
Nitpick comments:
In `@src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts`:
- Around line 64-76: The helper currently defined as a const arrow function
ensureTemplateQueryFromIntent should be converted to a function declaration
(e.g., function ensureTemplateQueryFromIntent() { ... }) to follow repository
conventions and enable hoisting; update its definition and any other internal
helpers in this file (the other const arrow helpers mentioned) to function
declarations as well, keeping the same logic, parameters and return values so
callers (like router.replace/mergePreservedQuery/hydratePreservedQuery usage)
continue to work unchanged.
…ed entries - readSessionPointer now validates workspaceId on exact match and removes stale cross-workspace pointers before falling back - findAndMigratePointer guards JSON.parse per entry so one corrupt record doesn't abort the scan - Remove redundant workspace validation from useWorkflowTabState (now handled in storageIO) - Add test for cross-workspace exact match removal and fallback Amp-Thread-ID: https://ampcode.com/threads/T-019c7a1f-7f9e-778b-925c-bda5192177f4
christian-byrne
deleted the
01-31-feat_persistence_integrate_v2_system_migration_4_4_
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the workflow persistence overhaul by integrating the new draft system into the app and migrating existing data. Fixes two critical bugs:
Changes
useWorkflowPersistenceV2.ts- Main composable that hooks into graph changes with 512ms debouncemigrateV1toV2.ts- One-time migration of existing drafts to the new scoped formatHow It Works
Migration
Existing V1 drafts are automatically migrated on first load. The migration:
Comfy.Workflow.*keysPart 4 of 4 in the workflow persistence improvements stack