- nodeSearchBox.spec.ts: Use NodeReference.getOutput() for CLIP output slot position
- rightClickMenu.spec.ts: Use NodeReference.getTitlePosition() for node dragging
- Add getTitlePosition() helper to NodeReference for drag operations
Amp-Thread-ID: https://ampcode.com/threads/T-019c1367-6a59-7752-9329-17cbf152fc62
Co-authored-by: Amp <amp@ampcode.com>
Migrate all test callers to use helper classes directly:
- nodeOps: getGraphNodesCount, getNodeRefById, selectNodes, etc.
- canvasOps: dragAndDrop, zoom, pan, resetView, etc.
- subgraph: rightClickInputSlot, connectToOutput, etc.
Reduces ComfyPage.ts from 1335 to 1085 lines (-250 lines).
Amp-Thread-ID: https://ampcode.com/threads/T-019c1318-58b7-757f-801c-5b676f0d3421
Co-authored-by: Amp <amp@ampcode.com>
- Create browser_tests/fixtures/constants/defaultGraphPositions.ts with all hardcoded default graph positions
- Create browser_tests/fixtures/constants/types.ts with Position and Size interfaces
- Update ComfyPage.ts to import and use DefaultGraphPositions
- Reduces duplication and prepares for further modular extraction
Amp-Thread-ID: https://ampcode.com/threads/T-019c1201-adec-7363-96d5-446d0a98e4cf
Co-authored-by: Amp <amp@ampcode.com>
Per Playwright best practices, assertions belong in tests, not fixtures.
Fixture methods should use waitFor() for preconditions instead.
- vueNodeFixtures.ts: setTitle(), cancelTitleEdit() use waitFor()
- Topbar.ts: closeTopbarMenu() uses waitFor({ state: 'hidden' })
- ComfyPage.ts: ConfirmDialog.click(), closeDialog(), clickDialogButton(),
closeToasts() all converted to waitFor()
- templates.ts: renamed waitForMinimumCardCount to expectMinimumCardCount
to clarify it contains an assertion (uses expect().toPass() pattern)
Amp-Thread-ID: https://ampcode.com/threads/T-019c11f8-acd2-7429-8bec-525ad47a47c4
Co-authored-by: Amp <amp@ampcode.com>
- widget.spec.ts: use expect().toPass() for combo refresh
- subgraph.spec.ts: use comfyPage.setup() after reload
- nodeLibrary.spec.ts: use toHaveCount() for debounced search
- remoteWidgets.spec.ts: use expect().toPass() for backoff test
- litegraphUtils.ts: refactor navigateIntoSubgraph to use toPass()
Amp-Thread-ID: https://ampcode.com/threads/T-019c11f1-e839-772c-8890-b77cfdf70124
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Disables the legacy "node templates" feature on ComfyUI Cloud. This
feature is accessed via right-clicking on the canvas and is distinct
from both subgraphs and workflow templates.
## Problem
The legacy node templates feature presents false-positive errors on
cloud:
- Errors appear when no existing templates exist initially
- After refresh, workflow progress is lost from workflow tabs
- Node templates fail to delete (except the first one)
## Solution
Conditionally load the `nodeTemplates` extension only for non-cloud
builds by wrapping the import in `if (!isCloud)`.
## Testing
- [ ] Verify right-click canvas menu no longer shows "Node Templates"
submenu on cloud
- [ ] Verify right-click canvas menu no longer shows "Save Selected as
Template" on cloud
- [ ] Verify the feature still works on desktop/OSS builds
## Related
- Refs: COM-14105
- Related issue: #4056 (Migrate Node Templates to Workflows)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8462-fix-cloud-disable-legacy-node-templates-feature-on-cloud-2f86d73d36508162948cc897d4c7ef5e)
by [Unito](https://www.unito.io)
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Fix for COM-14110: Preview image does not display new outputs in
vue-nodes.
## Problem
The merge logic in `setOutputsByLocatorId` updated `app.nodeOutputs` but
returned early without updating the reactive `nodeOutputs.value` ref.
This caused Vue components to never receive merged output updates
because only the non-reactive `app.nodeOutputs` was being updated.
## Solution
Added `nodeOutputs.value[nodeLocatorId] = existingOutput` after the
merge loop, before the return statement.
## Testing
- Added 2 unit tests covering the merge behavior
- All 4076 existing unit tests pass
- Typechecks pass
- Lint passes
## Notes
- Related open PRs touching same files: #8143, #8366 - potential minor
conflicts possible
Fixes COM-14110
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8479-fix-update-reactive-ref-after-merge-in-imagePreviewStore-2f86d73d365081f1a145fa5a9782515f)
by [Unito](https://www.unito.io)
Co-authored-by: Amp <amp@ampcode.com>
## Summary
This PR removes unsafe type assertions ("as unknown as Type") from test
files and improves type safety across the codebase.
### Key Changes
#### Type Safety Improvements
- Removed improper `as unknown as Type` patterns from test files in
Group 8 part 8
- Replaced with proper TypeScript patterns using Pinia store testing
patterns
- Fixed parameter shadowing issue in typeGuardUtil.test.ts (constructor
→ nodeConstructor)
- Fixed stale mock values in useConflictDetection.test.ts using getter
functions
- Refactored useManagerState tests to follow proper Pinia store testing
patterns with createTestingPinia
### Files Changed
Test files (Group 8 part 8 - utils and manager composables):
- src/utils/typeGuardUtil.test.ts - Fixed parameter shadowing
- src/utils/graphTraversalUtil.test.ts - Removed unsafe type assertions
- src/utils/litegraphUtil.test.ts - Improved type handling
- src/workbench/extensions/manager/composables/useManagerState.test.ts -
Complete rewrite using Pinia testing patterns
-
src/workbench/extensions/manager/composables/useConflictDetection.test.ts
- Fixed stale mock values with getters
- src/workbench/extensions/manager/composables/useManagerQueue.test.ts -
Type safety improvements
-
src/workbench/extensions/manager/composables/nodePack/useMissingNodes.test.ts
- Removed unsafe casts
-
src/workbench/extensions/manager/composables/nodePack/usePacksSelection.test.ts
- Type improvements
-
src/workbench/extensions/manager/composables/nodePack/usePacksStatus.test.ts
- Type improvements
- src/workbench/extensions/manager/utils/versionUtil.test.ts - Type
safety fixes
Source files (minor type fixes):
- src/utils/fuseUtil.ts - Type improvements
- src/utils/linkFixer.ts - Type safety fixes
- src/utils/syncUtil.ts - Type improvements
-
src/workbench/extensions/manager/composables/nodePack/useWorkflowPacks.ts
- Type fix
-
src/workbench/extensions/manager/composables/useConflictAcknowledgment.ts
- Type fix
### Testing
- All TypeScript type checking passes (`pnpm typecheck`)
- All affected test files pass (`pnpm test:unit`)
- Linting passes without errors (`pnpm lint`)
- Code formatting applied (`pnpm format`)
Part of the "Road to No Explicit Any" initiative, cleaning up type
casting issues from branch `fix/remove-any-types-part8`.
### 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 (this PR)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8496-Road-to-No-explicit-any-Group-8-part-8-test-files-2f86d73d365081f3afdcf8d01fba81e1)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
When bypassing or muting a subgraph the contents are no longer bypassed
along with it. This behaviour was less than ideal because it meant that
toggling the bypass of a single subgraph node twice could change the
behaviour of the node.
It is entirely intended that a subgraph node which is bypassed does not
have it's children execute. As part of testing this behaviour, it was
found that nodes inside of a bypassed subgraph are still considered for
execution even if boundry links are treated as disconnected. The
following example would execute even if the subgraph is muted.
<img width="826" height="476" alt="image"
src="https://github.com/user-attachments/assets/7b282873-e114-494d-b8f1-74c373859151"
/>
To resolve this, the PR does not add the contents of a subgraphNode
which is muted or bypassed to the execution map.
Resolves#8489
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8494-Don-t-bypass-subgraph-contents-with-subgraph-2f86d73d365081aeba8dd2990b6ba0ad)
by [Unito](https://www.unito.io)
## Summary
Add `PR_GH_TOKEN` to ci-lint-format and i18n-update-core workflows to
ensure commits trigger e2e test runs.
## Changes
- Add `token: ${{ secrets.PR_GH_TOKEN }}` to checkout step in
`.github/workflows/ci-lint-format.yaml`
- Add `token: ${{ secrets.PR_GH_TOKEN }}` to checkout step in
`.github/workflows/i18n-update-core.yaml`
## Context
This matches the pattern used consistently across all other workflows in
the repository (release-version-bump, version-bump-desktop-ui,
api-update-registry-api-types, i18n-update-nodes, etc.).
Without this token, commits made by these workflows don't trigger
downstream e2e tests, which can lead to issues being missed.
## Original Work
Original idea & implementation by @drjkl in commits:
- be3a8d61c - fix: use GitHub App token for lint/i18n workflows to
trigger e2e tests
- 50ccb254b - Add github app token action to pinact
This PR simplifies the approach to use the existing `PR_GH_TOKEN` secret
instead of GitHub App tokens, for consistency with the rest of the
repository.
## Test Plan
- [x] Verify workflows can checkout code successfully
- [x] Confirm commits from these workflows trigger e2e test runs
## It works!
<img width="1075" height="402" alt="image"
src="https://github.com/user-attachments/assets/31762d68-a37f-4926-b463-84d184d4309d"
/>
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
## Summary
Fixes a bug where socket map data was not properly removed when sockets
are dynamically added/removed via DynamicCombo widgets in Vue mode
(Nodes 2.0).
## Problem
When DynamicCombo widgets (e.g., `should_remesh` on Meshy nodes) change
their selection, inputs are dynamically added/removed. The Vue `v-for`
loop in `NodeSlots.vue` was using array index as the `:key`, causing Vue
to **reuse** slot components instead of properly unmounting them.
This led to:
- Socket map entries leaking (never cleaned up)
- Socket positions becoming desynced
- Stale cached offset data
## Solution
1. **Use slot `name` as Vue key** instead of array index in
`NodeSlots.vue`
- Slot names are unique per node (enforced by ComfyUI backend)
- When a slot is removed, Vue sees the key disappear and properly
unmounts the component
- `onUnmounted` cleanup in `useSlotElementTracking` now runs correctly
2. **Add defensive cleanup** in `useSlotElementTracking.ts`
- Before registering a new slot, check if a stale entry exists with the
same key
- Clean up stale entry to handle any edge cases
## Related
- Fixes COM-12970
- Related to #7837 (fixed LiteGraph version of this bug, but not Vue
mode)
## Testing
- Quality checks pass (typecheck, lint, format)
- Manual testing with DynamicCombo nodes (Meshy, nodes_logic)
recommended
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8469-fix-Vue-mode-socket-map-data-not-cleaned-up-on-dynamic-input-changes-2f86d73d365081e599eadca4f15e6b6e)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Improve Template search input performance issue #8134
This was caused by the search logic running too frequently (throttled at
50ms), causing the main thread to block on every few keystrokes.
## Changes
Use debouncing that wait until you stop typing for a specific time
(300ms) before running.
It makes the searching function more smoothly.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8343-Improve-template-search-input-performance-issue-2f56d73d36508144bdf9fa5e0cd76818)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary
Migrate keybindings domain to `src/platform/keybindings/` following DDD
principles.
## Changes
- **What**: Consolidate keybinding-related code (types, store, service,
defaults, reserved keys) into a single domain module with flat structure
- Extracted `KeyComboImpl` and `KeybindingImpl` classes into separate
files
- Updated all consumers to import from new location
- Colocated tests with source files
- Updated stores/README.md and services/README.md to remove migrated
entries
## Review Focus
- Verify all import paths were updated correctly
- Check that the flat structure is appropriate (vs nested core/data/ui
layers)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8369-refactor-migrate-keybindings-to-DDD-structure-2f66d73d36508120b169dc737075fb45)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
## Summary
This PR removes unsafe type assertions ("as unknown as Type") from test
files and improves type safety across the codebase.
### Key Changes
#### Type Safety Improvements
- Removed improper `as unknown as Type` patterns from 17 test files in
Group 8 part 7
- Replaced with proper TypeScript patterns using factory functions and
Mock types
- Fixed createTestingPinia usage in test files (was incorrectly using
createPinia)
- Fixed vi.hoisted pattern for mockSetDirty in viewport tests
- Fixed vi.doMock lint issues with vi.mock and vi.hoisted pattern
- Retained necessary `as unknown as` casts only for complex mock objects
where direct type assertions would fail
### Files Changed
Test files (Group 8 part 7 - services, stores, utils):
- src/services/nodeOrganizationService.test.ts
- src/services/providers/algoliaSearchProvider.test.ts
- src/services/providers/registrySearchProvider.test.ts
- src/stores/comfyRegistryStore.test.ts
- src/stores/domWidgetStore.test.ts
- src/stores/executionStore.test.ts
- src/stores/firebaseAuthStore.test.ts
- src/stores/modelToNodeStore.test.ts
- src/stores/queueStore.test.ts
- src/stores/subgraphNavigationStore.test.ts
- src/stores/subgraphNavigationStore.viewport.test.ts
- src/stores/subgraphStore.test.ts
- src/stores/systemStatsStore.test.ts
- src/stores/workspace/nodeHelpStore.test.ts
- src/utils/colorUtil.test.ts
- src/utils/executableGroupNodeChildDTO.test.ts
Source files:
- src/stores/modelStore.ts - Improved type handling
### Testing
- All TypeScript type checking passes (`pnpm typecheck`)
- All affected test files pass (`pnpm test:unit`)
- Linting passes without errors (`pnpm lint`)
- Code formatting applied (`pnpm format`)
Part of the "Road to No Explicit Any" initiative, cleaning up type
casting issues from branch `fix/remove-any-types-part8`.
### 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 (this PR)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8459-Road-to-No-explicit-any-Group-8-part-7-test-files-2f86d73d36508114ad28d82e72a3a5e9)
by [Unito](https://www.unito.io)
## Summary
Migrates ECMAScript private fields (`#`) to TypeScript private
(`private`) across LiteGraph to fix Vue Proxy reactivity
incompatibility.
## Problem
ES private fields (`#field`) are incompatible with Vue's Proxy-based
reactivity system - accessing `#field` through a Proxy throws
`TypeError: Cannot read private member from an object whose class did
not declare it`.
## Solution
- Converted all `#field` to `private _field` across 10 phases
- Added `toJSON()` methods to `LGraph`, `NodeSlot`, `NodeInputSlot`, and
`NodeOutputSlot` to prevent circular reference errors during
serialization (TypeScript private fields are visible to `JSON.stringify`
unlike true ES private fields)
- Made `DragAndScale.element.data` non-enumerable to break canvas
circular reference chain
## Testing
- All 4027 unit tests pass
- Added 9 new serialization tests to catch future circular reference
issues
- Browser tests (undo/redo, save workflows) verified working
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8440-refactor-migrate-ES-private-fields-to-TypeScript-private-for-Vue-Proxy-compatibility-2f76d73d365081a3bd82d429a3e0fcb7)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Adds support for creating Chatterbox TTS nodes when clicking Chatterbox
models in the Cloud asset browser.
## Changes
### modelToNodeStore.ts
- Add `findProvidersWithFallback()` helper for hierarchical model type
lookups (e.g., `parent/child` falls back to `parent`)
- Register 4 Chatterbox model directories with empty widget keys:
- `chatterbox/chatterbox` → `FL_ChatterboxTTS`
- `chatterbox/chatterbox_turbo` → `FL_ChatterboxTurboTTS`
- `chatterbox/chatterbox_multilingual` → `FL_ChatterboxMultilingualTTS`
- `chatterbox/chatterbox_vc` → `FL_ChatterboxVC`
### createModelNodeFromAsset.ts
- Skip widget assignment when `provider.key` is empty (for nodes that
auto-load models without a widget selector)
### Tests
- Add tests for hierarchical fallback behavior
- Add tests for empty widget key (auto-load nodes)
- Add Chatterbox node types to mock data
## Notes
- Empty `key` convention: Chatterbox nodes auto-load their models and
don't have a model selector widget, so we register them with `key: ''`
and skip the widget assignment step
- Hierarchical fallback only goes one level deep (`a/b/c` → `a`, not
`a/b`)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8418-feat-add-Chatterbox-model-support-for-Cloud-asset-browser-2f76d73d365081be822bc369b155f099)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: GitHub Action <action@github.com>
Previously the save keybind action would
- apply the new keybind
- wait for a network request to persist the change
- close the dialogue regardless of the results of the above changes
During this network request, the dialog would show a warning that the
keybind is invalid because the dialogue "contains a modified keybind
which conflicts with an existing keybind"
<img width="506" height="261" alt="image"
src="https://github.com/user-attachments/assets/e46150ce-9349-4f8e-b3b5-fb0b20dd3db9"
/>
This PR changes the order these actions are applied in.
- The dialogue is immediately closed
- The keybinding is updated if valid
- The keybinding is persisted.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8435-Fix-invalid-keybind-flash-2f76d73d3650815c9657f35e77d331fe)
by [Unito](https://www.unito.io)
## Summary
Fix shift+click range selection not properly deselecting assets when
selecting a smaller range, and improve selection performance.
## Changes
- **Bug Fix**: Shift+click now replaces selection with the new range
instead of combining with existing selection
- **Performance**: Remove unnecessary `.every()` check in `setSelection`
(O(n) → O(1))
- **Tests**: Add 23 unit tests for asset selection logic
## Test Plan
- [x] Click 1st asset → only 1st selected
- [x] Shift+click 3rd asset → items 1-3 selected
- [x] Shift+click 1st asset → only 1st selected (was broken, now fixed)
- [x] All 23 new unit tests pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8396-bugfix-Fix-shift-click-deselection-in-asset-panel-2f76d73d3650814ca060d1e6a40cf6d4)
by [Unito](https://www.unito.io)
## Summary
When removing a SubgraphNode via `LGraph.remove()`:
- Fire `onRemoved` for all nodes inside the subgraph
- Fire `onNodeRemoved` callback on the subgraph for each inner node
- Remove subgraph definition from `rootGraph.subgraphs` when no other
nodes reference it (checks both root graph nodes and nodes inside other
subgraphs)
This ensures proper cleanup of subgraph definitions and lifecycle
callbacks for nested nodes when subgraph nodes are deleted.
## Changes
### LGraph.ts
Added SubgraphNode-specific cleanup in `remove()` method that:
1. Iterates inner nodes and fires their `onRemoved` callbacks
2. Fires `onNodeRemoved` on the subgraph for downstream listeners (e.g.,
minimap)
3. Garbage collects the subgraph definition when no other nodes
reference it
### SubgraphNode.ts
Fixed `graph` property to match `LGraphNode` lifecycle contract.
Previously it was declared as `override readonly graph` via constructor
parameter promotion, which prevented `LGraph.remove()` from setting
`node.graph = null`. Changed to a regular mutable property with null
guard in `rootGraph` getter.
### LGraph.test.ts
Added 4 tests:
- `removing SubgraphNode fires onRemoved for inner nodes`
- `removing SubgraphNode fires onNodeRemoved callback`
- `subgraph definition is removed when last referencing node is removed`
- `subgraph definition is retained when other nodes still reference it`
## Related
- Fixes#8145
- Part of the subgraph lifecycle cleanup plan (Slice 2: Definition
garbage collection)
## Summary
Shows tab-specific empty state messages in Node Manager instead of
generic "No search results found" message.
## Changes
- Added computed properties to determine empty state messages based on
current tab and search state
- Display tab-specific messages when a tab is empty without active
search (e.g., "No Missing Nodes" for Missing tab)
- Fall back to search-related messages only when there's an active
search query
- Added Korean translations for empty state messages
| Tab | Empty State Title |
|-----|-------------------|
| All Installed | No Extensions Installed |
| Update Available | All Up to Date |
| Conflicting | No Conflicts Detected |
| Workflow | No Extensions in Workflow |
| Missing | No Missing Nodes |
## Review Focus
- Verify i18n key structure matches existing patterns
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8415-feat-Show-context-appropriate-empty-state-messages-in-Manager-tabs-2f76d73d3650817ab8a0d41b45df3411)
by [Unito](https://www.unito.io)
