Commit Graph

18 Commits

Author SHA1 Message Date
Christian Byrne
40aa7c5974 feat(persistence): fix QuotaExceededError and cross-workspace draft leakage (#8520)
## Summary

Completes the workflow persistence overhaul by integrating the new draft
system into the app and migrating existing data. Fixes two critical
bugs:

1. **QuotaExceededError** - localStorage fills up with workflow drafts,
breaking auto-save
2. **Cross-workspace data leakage** - Drafts from one ComfyUI instance
appear in another

## Changes

- **What**: 
- `useWorkflowPersistenceV2.ts` - Main composable that hooks into graph
changes with 512ms debounce
- `migrateV1toV2.ts` - One-time migration of existing drafts to the new
scoped format
  - Updated E2E tests for new storage key patterns
- **Why**: Users lose work when storage quota is exceeded, and see
confusing workflows from other instances

## How It Works

- **Workspace scoping**: Each ComfyUI instance (identified by server
URL) has isolated draft storage
- **LRU eviction**: When storage is full, oldest drafts are
automatically removed (keeps 32 most recent)
- **Tab isolation**: Each browser tab tracks its own active/open
workflows via sessionStorage
- **Debounced saves**: Graph changes are batched with 512ms delay to
reduce storage writes

## Migration

Existing V1 drafts are automatically migrated on first load. The
migration:
1. Reads drafts from old `Comfy.Workflow.*` keys
2. Converts to new workspace-scoped format
3. Cleans up old keys after successful migration

---
*Part 4 of 4 in the workflow persistence improvements stack*

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Simula_r <18093452+simula-r@users.noreply.github.com>
2026-02-21 00:57:50 -08:00
Christian Byrne
116685595b feat(persistence): add draft store and tab state management (#8519)
## Summary

Adds the Pinia store for managing workflow drafts and a composable for
tracking open workflow tabs per browser tab. Uses sessionStorage for
tab-specific state to support multiple ComfyUI tabs without conflicts.

## Changes

- **What**: 
- `workflowDraftStoreV2.ts` - Pinia store wrapping the LRU cache with
save/load/remove operations
- `useWorkflowTabState.ts` - Composable for tracking active workflow
path and open tabs in sessionStorage (scoped by clientId)
- **Why**: Browser tabs need independent workflow state, but the current
system uses shared localStorage keys causing tab conflicts

## Review Focus

- Store API design in `workflowDraftStoreV2.ts`
- Session vs local storage split in `useWorkflowTabState.ts`

---
*Part 3 of 4 in the workflow persistence improvements stack*

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-02-19 23:53:02 -08:00
Christian Byrne
351d43a95a feat(persistence): add LRU draft cache with quota management (#8518)
## Summary

Adds an LRU (Least Recently Used) cache layer and storage I/O utilities
that handle localStorage quota limits gracefully. When storage is full,
the oldest drafts are automatically evicted to make room for new ones.

## Changes

- **What**: 
- `draftCacheV2.ts` - In-memory LRU cache with configurable max entries
(default 32)
- `storageIO.ts` - Storage read/write with automatic quota management
and eviction
- **Why**: Users experience `QuotaExceededError` when localStorage fills
up with workflow drafts, breaking auto-save functionality

## Review Focus

- LRU eviction logic in `draftCacheV2.ts`
- Quota error handling and recovery in `storageIO.ts`

---
*Part 2 of 4 in the workflow persistence improvements stack*

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-02-19 22:04:19 -08:00
Johnpaul Chiwetelu
fe78bc6043 chore: remove unused draftTypes.ts to fix knip (#8993)
## Summary
- Remove `src/platform/workflow/persistence/base/draftTypes.ts` which is
not imported anywhere
- Fixes `knip` reporting it as an unused file

The file was added in #8517 but nothing consumes its exports yet.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8993-chore-remove-unused-draftTypes-ts-to-fix-knip-30d6d73d36508151a9e1e936864fd311)
by [Unito](https://www.unito.io)
2026-02-19 20:57:14 -08:00
Christian Byrne
8ab9a7b887 feat(persistence): add workspace-scoped storage keys and types (#8517)
## Summary

Adds the foundational types and key generation utilities for
workspace-scoped workflow draft persistence. This enables storing drafts
per-workspace to prevent data leakage between different ComfyUI
instances.

[Screencast from 2026-02-08
18-17-45.webm](https://github.com/user-attachments/assets/f16226e9-c1db-469d-a0b7-aa6af725db53)

## Changes

- **What**: Type definitions for draft storage (`DraftIndexV2`,
`DraftPayloadV2`, session pointers) and key generation utilities with
workspace/client scoping
- **Why**: The current persistence system stores all drafts globally,
causing cross-workspace data leakage when users work with multiple
ComfyUI instances

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-02-18 19:31:24 -08:00
Yourz
58182ddda7 fix: skip loading drafts when Comfy.Workflow.Persist is disabled (#8851)
## Summary

Skip draft loading and clear stored drafts when `Comfy.Workflow.Persist`
is disabled, preventing unsaved changes from reappearing.

## Changes

- **What**: Guard draft loading in `ComfyWorkflow.load()` with
`Comfy.Workflow.Persist` setting check. Clear all localStorage drafts
when Persist is toggled from true to false.

## Review Focus

`ComfyWorkflow.load()` previously read drafts unconditionally regardless
of the Persist setting. This meant that after disabling Persist,
previously stored drafts would still be applied when opening a saved
workflow. The fix adds a guard in two places:
1. `comfyWorkflow.ts`: `load()` now checks `Comfy.Workflow.Persist`
before calling `getDraft()`
2. `useWorkflowPersistence.ts`: A `watch` on the Persist setting calls
`draftStore.reset()` when disabled

Fixes Comfy-Org/ComfyUI#12323

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8851-fix-skip-loading-drafts-when-Comfy-Workflow-Persist-is-disabled-3066d73d36508119ac2ce13564e18c01)
by [Unito](https://www.unito.io)

Co-authored-by: Amp <amp@ampcode.com>
2026-02-15 10:01:54 +08:00
Johnpaul Chiwetelu
90a701dd67 Road to No Explicit Any Part 11 (#8565)
## Summary

This PR removes `any` types from widgets, services, stores, and test
files, replacing them with proper TypeScript types.

### Key Changes

#### Type Safety Improvements
- Replaced `any` with `unknown`, explicit types, or proper interfaces
across widgets and services
- Added proper type imports (TgpuRoot, Point, StyleValue, etc.)
- Created typed interfaces (NumericWidgetOptions, TestWindow,
ImportFailureDetail, etc.)
- Fixed function return types to be non-nullable where appropriate
- Added type guards and null checks instead of non-null assertions
- Used `ComponentProps` from vue-component-type-helpers for component
testing

#### Widget System
- Added index signature to IWidgetOptions for Record compatibility
- Centralized disabled logic in WidgetInputNumberInput
- Moved template type assertions to computed properties
- Fixed ComboWidget getOptionLabel type assertions
- Improved remote widget type handling with runtime checks

#### Services & Stores
- Fixed getOrCreateViewer to return non-nullable values
- Updated addNodeOnGraph to use specific options type `{ pos?: Point }`
- Added proper type assertions for settings store retrieval
- Fixed executionIdToCurrentId return type (string | undefined)

#### Test Infrastructure
- Exported GraphOrSubgraph from litegraph barrel to avoid circular
dependencies
- Updated test fixtures with proper TypeScript types (TestInfo,
LGraphNode)
- Replaced loose Record types with ComponentProps in tests
- Added proper error handling in WebSocket fixture

#### Code Organization
- Created shared i18n-types module for locale data types
- Made ImportFailureDetail non-exported (internal use only)
- Added @public JSDoc tag to ElectronWindow type
- Fixed console.log usage in scripts to use allowed methods

### Files Changed

**Widgets & Components:**
-
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
- src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue
-
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
- src/lib/litegraph/src/widgets/ComboWidget.ts
- src/lib/litegraph/src/types/widgets.ts
- src/components/common/LazyImage.vue
- src/components/load3d/Load3dViewerContent.vue

**Services & Stores:**
- src/services/litegraphService.ts
- src/services/load3dService.ts
- src/services/colorPaletteService.ts
- src/stores/maskEditorStore.ts
- src/stores/nodeDefStore.ts
- src/platform/settings/settingStore.ts
- src/platform/workflow/management/stores/workflowStore.ts

**Composables & Utils:**
- src/composables/node/useWatchWidget.ts
- src/composables/useCanvasDrop.ts
- src/utils/widgetPropFilter.ts
- src/utils/queueDisplay.ts
- src/utils/envUtil.ts

**Test Files:**
- browser_tests/fixtures/ComfyPage.ts
- browser_tests/fixtures/ws.ts
- browser_tests/tests/actionbar.spec.ts
-
src/workbench/extensions/manager/components/manager/skeleton/PackCardGridSkeleton.test.ts
- src/lib/litegraph/src/subgraph/subgraphUtils.test.ts
- src/components/rightSidePanel/shared.test.ts
- src/platform/cloud/subscription/composables/useSubscription.test.ts
-
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts

**Scripts & Types:**
- scripts/i18n-types.ts (new shared module)
- scripts/diff-i18n.ts
- scripts/check-unused-i18n-keys.ts
- src/workbench/extensions/manager/types/conflictDetectionTypes.ts
- src/types/algoliaTypes.ts
- src/types/simplifiedWidget.ts

**Infrastructure:**
- src/lib/litegraph/src/litegraph.ts (added GraphOrSubgraph export)
- src/lib/litegraph/src/infrastructure/CustomEventTarget.ts
- src/platform/assets/services/assetService.ts

**Stories:**
- apps/desktop-ui/src/views/InstallView.stories.ts
- src/components/queue/job/JobDetailsPopover.stories.ts

**Extension Manager:**
- src/workbench/extensions/manager/composables/useConflictDetection.ts
- src/workbench/extensions/manager/composables/useManagerQueue.ts
- src/workbench/extensions/manager/services/comfyManagerService.ts
- src/workbench/extensions/manager/utils/conflictMessageUtil.ts

### Testing

- [x] All TypeScript type checking passes (`pnpm typecheck`)
- [x] ESLint passes without errors (`pnpm lint`)
- [x] Format checks pass (`pnpm format:check`)
- [x] Knip (unused exports) passes (`pnpm knip`)
- [x] Pre-commit and pre-push hooks pass

Part of the "Road to No Explicit Any" initiative.

### Previous PRs in this series:
- Part 2: #7401
- Part 3: #7935
- Part 4: #7970
- Part 5: #8064
- Part 6: #8083
- Part 7: #8092
- Part 8 Group 1: #8253
- Part 8 Group 2: #8258
- Part 8 Group 3: #8304
- Part 8 Group 4: #8314
- Part 8 Group 5: #8329
- Part 8 Group 6: #8344
- Part 8 Group 7: #8459
- Part 8 Group 8: #8496
- Part 9: #8498
- Part 10: #8499

---------

Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com>
Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-02-05 16:29:28 -08:00
Alexander Brown
6ce60a11a4 test: use createTestingPinia instead of createPinia (#8376)
Replace \createPinia\ with \createTestingPinia({ stubActions: false })\
from \@pinia/testing\ across 45 test files for proper test isolation.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8376-test-use-createTestingPinia-instead-of-createPinia-2f66d73d36508137a9f0daffcddc86f7)
by [Unito](https://www.unito.io)

Co-authored-by: Amp <amp@ampcode.com>
2026-01-28 22:21:38 -08:00
Johnpaul Chiwetelu
3946d7b5ff Road to no explicit any part 8 group 5 (#8329)
## Summary
- Add `createMockLLink` and `createMockLinks` factory functions to
handle hybrid Map/Record types
- Replace `as any` assertions with type-safe factory functions in
minimap tests
- Implement proper Pinia store mocking using `vi.hoisted()` pattern
- Remove unused `createMockSubgraph` export (shadowed by local
implementations)

## Test plan
- [x] All minimap tests pass (29 tests)
- [x] `pnpm typecheck` passes
- [x] `pnpm lint` passes
- [x] `pnpm knip` passes

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8329-Road-to-no-explicit-any-part-8-group-5-2f56d73d365081218882de81d5526220)
by [Unito](https://www.unito.io)

---------

Co-authored-by: AustinMroz <austin@comfy.org>
2026-01-27 19:25:15 +01:00
Christian Byrne
d3e664b2dd Feat: Persist all unsaved workflow tabs (#6050)
## Summary

- Keep all drafts in localStorage, mirroring the logic from VSCode.

- Fix a bug where newly created blank workflow tabs would incorrectly
restore as defaultGraph instead of blankGraph after page refresh.

Resolves https://github.com/Comfy-Org/desktop/issues/910, Resolves
https://github.com/Comfy-Org/ComfyUI_frontend/issues/4057, Fixes
https://github.com/Comfy-Org/ComfyUI_frontend/issues/3665

## Changes

### What
- Fix `restoreWorkflowTabsState` to parse and pass workflow data from
drafts when recreating temporary workflows
- Add error handling for invalid draft data with fallback to default
workflow
- Fix E2E test `should not serialize color adjustments in workflow` to
wait for workflow persistence before assertions
- Add proper validation for workflow nodes array in test assertions

### Breaking
- None

### Dependencies
- No new dependencies added

## Review Focus

1. **Workflow restoration**: Verify that blank workflows correctly
restore as blankGraph after page refresh
2. **Error handling**: Check that invalid draft data gracefully falls
back to default workflow
3. **Test coverage**: Ensure E2E test correctly waits for workflow
persistence before checking node properties
4. **Edge cases**: Test with multiple tabs, switching between tabs, and
rapid refresh scenarios

---------

Co-authored-by: Yourz <crazilou@vip.qq.com>
2026-01-26 09:35:38 -08:00
Alexander Brown
10feb1fd5b chore: migrate tests from tests-ui/ to colocate with source files (#7811)
## Summary

Migrates all unit tests from `tests-ui/` to colocate with their source
files in `src/`, improving discoverability and maintainability.

## Changes

- **What**: Relocated all unit tests to be adjacent to the code they
test, following the `<source>.test.ts` naming convention
- **Config**: Updated `vitest.config.ts` to remove `tests-ui` include
pattern and `@tests-ui` alias
- **Docs**: Moved testing documentation to `docs/testing/` with updated
paths and patterns

## Review Focus

- Migration patterns documented in
`temp/plans/migrate-tests-ui-to-src.md`
- Tests use `@/` path aliases instead of relative imports
- Shared fixtures placed in `__fixtures__/` directories

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7811-chore-migrate-tests-from-tests-ui-to-colocate-with-source-files-2da6d73d36508147a4cce85365dee614)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
2026-01-05 16:32:24 -08:00
AustinMroz
f2a0e5102e Cleanup app.graph usage (#7399)
Prior to the release of subgraphs, there was a single graph accessed
through `app.graph`. Now that there's multiple graphs, there's a lot of
code that needs to be reviewed and potentially updated depending on if
it cares about nearby nodes, all nodes, or something else requiring
specific attention.

This was done by simply changing the type of `app.graph` to unknown so
the typechecker will complain about every place it's currently used.
References were then updated to `app.rootGraph` if the previous usage
was correct, or actually rewritten.

By not getting rid of `app.graph`, this change already ensures that
there's no loss of functionality for custom nodes, but the prior typing
of `app.graph` can always be restored if future dissuasion of
`app.graph` usage creates issues.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7399-Cleanup-app-graph-usage-2c76d73d365081178743dfdcf07f44d0)
by [Unito](https://www.unito.io)
2025-12-11 23:37:34 -07:00
Christian Byrne
8b8f3538bf fix: template query param stripped during login views (#6677)
Fixes issue where query params from
https://github.com/Comfy-Org/ComfyUI_frontend/pull/6593 are stripped
during the login/signup views/flow by storing initial params in session
storage via router plugin.



https://github.com/user-attachments/assets/51642e8c-af5c-43ef-ab7d-133bc7e511aa




┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6677-fix-template-query-param-stripped-during-login-views-2aa6d73d365081a1bdc7d22b35f72a77)
by [Unito](https://www.unito.io)
2025-11-13 20:37:37 -08:00
Christian Byrne
3c550e953a fix: improve template URL loading UX and prevent re-triggering (#6593)
Fixes the janky UX when loading templates via URL query parameters by
moving the loading logic earlier in the app lifecycle (from
GraphView.onGraphReady to
useWorkflowPersistence.restorePreviousWorkflow). The saved workflow now
loads first as a background tab, then the template loads as the active
tab, eliminating the visual flash where the saved workflow briefly
appears before being replaced. After loading, the template and source
query parameters are removed from the URL using router.replace to
prevent the template from re-loading on page refresh. This preserves
user work by keeping both workflows open in separate tabs and matches
the existing behavior when clicking templates from the dialog. All 15
tests pass including 3 new tests for URL cleanup.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6593-fix-improve-template-URL-loading-UX-and-prevent-re-triggering-2a26d73d36508137a0cae6cf92c842fc)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Christian Byrne <c.byrne@comfy.org>
2025-11-11 13:51:56 -07:00
Christian Byrne
b679bfe8f8 logging: log context on session storage write error (QuotaError) (#6636)
adds some context to the storage write errors observed in telemetry data
- in order to pinpoint the exact cause.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6636-logging-log-context-on-session-storage-write-error-QuotaError-2a56d73d365081c28bb0dbfdfef8395a)
by [Unito](https://www.unito.io)
2025-11-08 10:19:12 -07:00
Christian Byrne
f16f5a0bc6 When restoring workflow tabs from previous session, don't skip workflows that were modified (#5857)
## Summary

In https://github.com/Comfy-Org/ComfyUI_frontend/pull/2238, we added
feature to restore open tabs from previous session. At that time, it was
decided to not restore the tabs if they had a modified worklow - the
reason being that we would not be able to restore the actual state.
However, MANY users have expressed frustration with this - so, for now,
restore the tabs even if they were modified in the previous session (and
unsaved).

There are already e2e test cases for this feature here:
26cf45ed36/browser_tests/tests/interaction.spec.ts (L679)

And unit tests for the relevant workflow store methods here:
26cf45ed36/tests-ui/tests/store/workflowStore.test.ts (L184)

Resolves https://github.com/Comfy-Org/ComfyUI_frontend/issues/5848

## Changes

- **What**: Modified filter in `restoreState` computed property to
persist both saved and modified workflows

## Review Focus

Impact on browser storage when users have many modified workflows open
simultaneously.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5857-When-restoring-workflow-tabs-from-previous-session-don-t-skip-workflows-that-were-modifi-27e6d73d36508182a900d2918d0fb163)
by [Unito](https://www.unito.io)
2025-09-29 22:24:44 -07:00
Christian Byrne
4c8c4a1ad4 [refactor] Improve settings domain organization (#5550)
* refactor: move settingStore to platform/settings

Move src/stores/settingStore.ts to src/platform/settings/settingStore.ts
to separate platform infrastructure from domain logic following DDD principles.

Updates all import references across ~70 files to maintain compatibility.

* fix: update remaining settingStore imports after rebase

* fix: complete remaining settingStore import updates

* fix: update vi.mock paths for settingStore in tests

Update all test files to mock the new settingStore location at
@/platform/settings/settingStore instead of @/stores/settingStore

* fix: resolve remaining settingStore imports and unused imports after rebase

* fix: update settingStore mock path in SelectionToolbox test

Fix vi.mock path from @/stores/settingStore to @/platform/settings/settingStore
to resolve failing Load3D viewer button test.

* refactor: complete comprehensive settings migration to platform layer

This commit completes the migration of all settings-related code to the platform layer
as part of the Domain-Driven Design (DDD) architecture refactoring.

- constants/coreSettings.ts → platform/settings/constants/coreSettings.ts
- types/settingTypes.ts → platform/settings/types.ts
- stores/settingStore.ts → platform/settings/settingStore.ts (already moved)

- composables/setting/useSettingUI.ts → platform/settings/composables/useSettingUI.ts
- composables/setting/useSettingSearch.ts → platform/settings/composables/useSettingSearch.ts
- composables/useLitegraphSettings.ts → platform/settings/composables/useLitegraphSettings.ts

- components/dialog/content/SettingDialogContent.vue → platform/settings/components/SettingDialogContent.vue
- components/dialog/content/setting/SettingItem.vue → platform/settings/components/SettingItem.vue
- components/dialog/content/setting/SettingGroup.vue → platform/settings/components/SettingGroup.vue
- components/dialog/content/setting/SettingsPanel.vue → platform/settings/components/SettingsPanel.vue
- components/dialog/content/setting/ColorPaletteMessage.vue → platform/settings/components/ColorPaletteMessage.vue
- components/dialog/content/setting/ExtensionPanel.vue → platform/settings/components/ExtensionPanel.vue
- components/dialog/content/setting/ServerConfigPanel.vue → platform/settings/components/ServerConfigPanel.vue

- ~100+ import statements updated across the codebase
- Test file imports corrected
- Component imports fixed in dialog service and command menubar
- Composable imports updated in GraphCanvas.vue

```
src/platform/settings/
├── components/          # All settings UI components
├── composables/         # Settings-related composables
├── constants/          # Core settings definitions
├── types.ts           # Settings type definitions
└── settingStore.ts    # Central settings state management
```

 TypeScript compilation successful
 All tests passing (settings store, search functionality, UI components)
 Production build successful
 Domain boundaries properly established

This migration consolidates all settings functionality into a cohesive platform domain,
improving maintainability and following DDD principles for better code organization.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: format and lint after rebase conflict resolution

* fix: update remaining import paths to platform settings

- Fix browser test import: extensionAPI.spec.ts
- Fix script import: collect-i18n-general.ts
- Complete settings migration import path updates

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
2025-09-15 03:53:08 -07:00
Christian Byrne
ca312fd1ea [refactor] Improve workflow domain organization (#5584)
* [refactor] move workflow domain to its own folder

* [refactor] Fix workflow platform architecture organization

- Move workflow rendering functionality to renderer/thumbnail domain
- Rename ui folder to management for better semantic clarity
- Update all import paths to reflect proper domain boundaries
- Fix test imports to use new structure

Architecture improvements:
- rendering → renderer/thumbnail (belongs with other rendering logic)
- ui → management (better name for state management and UI integration)

This ensures proper separation of concerns and domain boundaries.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* [fix] Resolve circular dependency between nodeDefStore and subgraphStore

* [fix] Update browser test imports to use new workflow platform paths

---------

Co-authored-by: Claude <noreply@anthropic.com>
2025-09-15 02:22:37 -07:00