* [fix] Consolidate Playwright workflow jobs to fix missing deployment links
The issue in PR #5298 was caused by missing deployment-info artifact
creation. The deploy-reports job was deploying to Cloudflare but wasn't
creating the deployment-info-* artifacts that comment-tests-completed
job expected to download.
This change consolidates the deployment and commenting into a single job,
eliminating the artifact dependency and ensuring links are always available.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor: Split PR deployment workflow for forked vs non-forked repos
- Extract deployment logic to reusable script (scripts/cicd/pr-playwright-deploy-and-comment.sh)
- Non-forked PRs: Use direct pull_request event in test-ui.yaml (faster)
- Forked PRs: Use workflow_run in pr-playwright-deploy.yaml (handles permissions)
- Add starting comment for both forked and non-forked PRs
- Make Cloudflare tokens optional for starting status comments
* refactor: Simplify PR deployment workflow and script
- Consolidate workflow into single job with clearer structure
- Reduce script from 200+ to ~140 lines
- Simplify deployment retry logic and comment generation
- Remove redundant checks and unnecessary complexity
* fix: Add debugging and wrangler installation to deployment script
- Add debug output to identify missing reports
- Install wrangler if not available
- Show deployment attempts and failures
- Log available reports before deployment
* chore: Trigger CI to test deployment workflow
* fix: Fix browser artifact name mismatch in deployment script
- Use dot notation (0.5x) for artifact names as Playwright creates them
- Convert to dash notation (0-5x) for Cloudflare project names
- Properly handle browser name display in comments
* refactor: Convert deployment script to POSIX sh for better compatibility
- Replace bash arrays with space-separated strings
- Use while loops instead of bash-specific for syntax
- Remove bash-specific string manipulation features
- Replace local variables (not required in functions)
- Ensure compatibility with standard /bin/sh
* fix: Fix deployment script output to properly capture URLs
- Redirect debug messages to stderr
- Only output URL to stdout for proper capture
- This fixes the missing deployment links in PR comments
* fix: Add input validation to prevent command injection
- Validate PR number is numeric only
- Sanitize branch name at script start
- Validate status parameter values
- Use pre-sanitized branch throughout script
- Addresses high-severity security issue from PR review
* fix: Add null checks and logging to workflow condition
- Add explicit null checks for head_repository and repository
- Add debug logging to help diagnose workflow trigger issues
- Prevents potential failures from undefined repository objects
- Addresses medium-severity issue from PR review
* fix: Pin wrangler to major version 4 with error handling
- Pin wrangler to major version 4 (^4.0.0) for stability
- Add error handling if wrangler installation fails
- Return 'failed' status if installation fails
- Addresses dependency management issue from PR review
* perf: Implement parallel deployments to reduce CI time
- Deploy all browser reports in parallel using background processes
- Use temporary directory to collect deployment results
- Wait for all deployments to complete before generating comment
- Maintains result order for consistent output
- Significantly reduces deployment time from sequential to parallel execution
* fix: Use specific comment ID for updates instead of edit-last
- Use GitHub API to find exact comment ID
- Update specific comment by ID to avoid editing wrong comment
- Prevents race conditions if user posts between finding and editing
- More reliable comment updates
* fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency
* fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency
* fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity
* fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity
* Update scripts/cicd/pr-playwright-deploy-and-comment.sh
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix(pr-playwright-deploy-and-comment.sh): improve regex for URL extraction to include valid characters and ensure correct URL format
* chore(pr-playwright-deploy-and-comment.sh): move wrangler installation to the beginning of the script to avoid redundancy and improve efficiency
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* feat: add dynamic icon support for NavItem components
- Created NavIcon component with switch-case based icon rendering
- Added iconName prop to NavItem and NavItemData interface
- Updated LeftSidePanel to pass icon names to nav items
- Added sample icons to SampleModelSelector navigation (download, tag, layers, grid)
- Uses i-lucide syntax without imports for better tree-shaking
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* test: add Storybook stories for navigation components
- Add NavIcon.stories.ts with interactive icon selector and all icons gallery
- Add NavItem.stories.ts with text customization and interactive list examples
- Add LeftSidePanel.stories.ts with various navigation configurations
- Remove old Navigation.stories.ts (replaced with component-specific stories)
- Configure slot visibility and hide update:modelValue event in controls
* refactor: simplify NavIcon component and improve type definitions
* fix: add icon size specification for Lucide icons in Storybook
* feature: NavItem story modified
* fix: disable knip unresolved imports rule for virtual icon modules
Add unresolved: 'off' to knip configuration to ignore virtual module imports
from unplugin-icons (~icons/*). These are generated at build time and cannot
be resolved statically.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* chore: v-if condition added
* chore: knip ignoreUnresolved added based on knip issue PR
* refactor: navItem types added & deleting any type on storybook files
---------
Co-authored-by: Claude <noreply@anthropic.com>
* [feat] Refactor overlay compatibility into reusable composable
- Create useTransformCompatOverlayProps composable for centralized overlay prop management
- Update Select, MultiSelect, TreeSelect, and FileUpload components to use composable
- Provides appendTo='self' for transform inheritance in CSS-transformed parents
- Enables easy future additions of other transform compatibility props
- Fix duplicate v-bind attributes by combining props into single computed object
* fix: Keep the canvas container from being scrolled by children
* types: Align the appendTo type with primevue internals
* Update test expectations [skip ci]
---------
Co-authored-by: bymyself <cbyrne@comfy.org>
Co-authored-by: github-actions <github-actions@github.com>
* Remove COMFY_VUE_NODE_DIMENSIONS
* [refactor] Use getSlotPosition for Vue nodes in link rendering
Replace direct node position calls with getSlotPosition utility when Vue nodes mode is enabled. This ensures consistent slot positioning across the canvas rendering system.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix getSlotPosition readonly return value (#5433)
* Update accordingly to new type
* Fix canvas/screen conversion formulas in useTransformState (#5406)
* Fix conversion formulas
* update test expectations
* Remove unused type import
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com>
* switch to schema interface and remove assertions at callsites
* [refactor] improve type safety and code organization - addresses @DrJKL's review feedback
- Remove unnecessary type assertions from REROUTE_DEFAULTS
- Use safer Omit<RerouteData, 'id' < /dev/null | 'parentId'> pattern for defaults to prevent hardcoded ID bugs
- Extract asRerouteId and asLinkId utility functions to module scope as pure functions
- Update getRerouteField to handle partial defaults safely
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* [fix] revert to clean defaults pattern - removes any type usage
Reverted the overcomplicated Omit pattern back to the simple, working approach.
The original pattern was cleaner and didn't introduce unnecessary complexity.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* [feat] add Comfy.Assets.UseAssetAPI to CORE_SETTINGS
* [feat] create AssetService
1. Add service for accessing new Asset API
2. Add fallback model paths logic so empty model directories appear for
the user.
3. Copious tests for them all.
Co-Authored-By: Claude <noreply@anthropic.com>
* [feat] switch between assets and file paths for model data
* [feat] ignore assets with "missing" tag
* [fix] formatting and style
* [fix] call assets API with the correct filters
* [feat] elminate unused modelPath code
* [fix] remove stray comment
* [fix] model manager api was not parsed correctly
---------
Co-authored-by: Claude <noreply@anthropic.com>
* let canvas continue to own selection state management
* fix merge error
* refactor: use computed instead of watcher for selectedNodeIds
Replace watcher pattern with computed for better Vue idioms:
- More reactive and efficient
- Automatically recomputes when dependencies change
- Simpler, more declarative code
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: improve injection error handling for selectedNodeIds
Replace silent fallback with explicit error when SelectedNodeIds
is not provided:
- Fail fast instead of silently using empty Set
- Clear error message for debugging
- Prevents nodes appearing unselected due to missing provider
Addresses DrJKL's concern about injection default behavior.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* test: improve mocking patterns using vi.mockObject
Replace manual mock interfaces with vi.mockObject for better type safety:
- Use Vitest's built-in mocking utilities instead of manual interfaces
- Properly configure mock return values
- Remove unnecessary type assertions
Addresses DrJKL's feedback on test mocking patterns.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* test: extract repeated nodeData for clarity
Extract common test nodeData object to reduce duplication:
- Move repeated VueNodeData object to describe scope
- Replace 6 instances of identical nodeData declarations
- Maintain different nodeData for specific test cases
Addresses DrJKL's suggestion to extract repeated test data.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* add type safety to mocks
---------
Co-authored-by: Claude <noreply@anthropic.com>
* knip: Don't ignore exports that are only used within a given file
* knip: More pruning after rebase
* knip: Vite plugin config fix
* knip: vitest plugin config
* knip: Playwright config, remove unnecessary ignores.
* knip: Simplify project file enumeration.
* knip: simplify the config file patterns ?(.optional_segment)
* knip: tailwind v4 fix
* knip: A little more, explain some of the deps.
Should be good for this PR.
* knip: remove unused disabling of classMembers.
It's opt-in, which we should probably do.
* knip: floating comments
We should probably delete _one_ of these parallell trees, right?
* knip: Add additional entrypoints
* knip: Restore UserData that's exposed via the types for now.
* knip: Add as an entry file even though knip says it's not necessary.
* knip: re-export functions used by nodes (h/t @christian-byrne)
* fix: normalize pack IDs to fix version detection for disabled packs
When a pack is disabled, ComfyUI-Manager returns it with a version suffix
(e.g., "ComfyUI-GGUF@1_1_4") while enabled packs don't have this suffix.
This inconsistency caused disabled packs to incorrectly show as having
updates available even when they were on the latest version.
Changes:
- Add normalizePackId utility to consistently remove version suffixes
- Apply normalization in refreshInstalledList and WebSocket updates
- Use the utility across conflict detection and node help modules
- Ensure pack version info is preserved in the object's ver field
This fixes the "Update Available" indicator incorrectly showing for
disabled packs that are already on the latest version.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* feature: test code added
* test: packUtils test code added
* test: address PR review feedback for test
improvements
- Remove unnecessary .not.toThrow() assertion
in useManagerQueue test
- Add clarifying comments for version
normalization test logic
- Replace 'as any' with vi.mocked() for better
type safety
---------
Co-authored-by: Claude <noreply@anthropic.com>
* add component tests for vue widgets
* [refactor] improve widget test readability and type safety - addresses @DrJKL's review feedback
- Add mountComponent utility function for consistent test setup
- Add setInputValueAndTrigger helper to batch common test operations
- Replace type assertions with proper instanceof checks for type safety
- Remove duplicate test setup code to improve test readability
- Fix TypeScript errors in WidgetSlider tests
These changes address all review comments by making tests easier to read
and understand while ensuring proper type checking.
* [refactor] apply consistent test patterns to WidgetSelect.test.ts
- Add mountComponent utility function for consistent test setup
- Add setSelectValueAndEmit helper to batch select operations
- Remove repetitive mount boilerplate code throughout tests
- Maintain existing test coverage while improving readability
This ensures all widget component tests follow the same patterns
established in WidgetInputText and WidgetSlider tests.
* Implement subgraph publishing
* Add missing null check
* Fix subgraph blueprint display in workflows tab
* Fix demotion of subgraph blueprints on reload
* Update locales [skip ci]
* Update blueprint def on save, cleanup
* Fix skipped tracking on subgraph publish
When a subgraph is first published, it previously was not added to the
subgraphCache. This would cause deletion to fail until a reload occurred.
* Fix failing vite tests
A couple of tests that were mocking classes broke SubgraphBlueprint
inheritance. Since they aren't testing anythign related to subgraph
blueprints, the subgraph store is mocked as well.
* Make blueprint breadcrumb badge clickable
* Add confirmation for overwrite on publish
* Simplify blueprint badge naming
* Swap to promise.allSettled when fetching subgraphs
* Navigate into subgraph on blueprint edit
* Revert mission of value in blueprint breadcrumb
This was causing the blueprint badge to always display
* Misc code quality fixes
* Set subgraphNode title on blueprint add.
When a subgraph blueprint is added to the graph, the title of the
subgraphNode is now set to be the title of the blueprint.
NOTE: The name of the subgraph node when a blueprint is edited is left
unchanged. This may cause minor user confusion.
* Add "Delete Blueprint" option to breadcrumb
When editing a blueprint, the options provided for the root graph of the
breadcrumb included a Delete Workflow option. This still functioned for
deleting the current blueprint when selected, but didn't make sense. It
has been updated to instead describe that it deletes the current
blueprint
* Extract subgraph load code as function
* Fix subgraphs appearing in library after refresh
Subgraph nodes were hidden from the node library and context menu by
setting skip_list to true. Unfortunately, this causes them to be
mistakenly be caught and registered as vue nodes when a refresh is
performed. This is fixed by adding a check for skip_list.
* Add delete button and confirmation for deletion
* Use more specific warning for blueprint deletion
* At success toast on subgraph publish
Will return later to potentially add a node library link to the toast
* Don't apply subgraph context menu to normal nodes
Subgraph blueprints have a right click -> delete option in the node
library. This was incorrectly being dislplayed on non blueprint nodes.
* Remove hardcoded subgraphs path
Rather happy with this change. Rather than trying to introduce a
recursive import to pass a magic string, this solution is both
sufficient AND allows potential future extensions with less breakage.
* Fix nodeDef update on save
Wait to update the node def cache until after a blueprint has been
saved. Before, changes to links weren't actually being made visisble.
* Fix SaveAs with subgraph blueprints
* Remove ugly serialize/deserialize
Thought I had already tested this, and found that the mere existence of
proxies was causing issues, but simply adding a correct annotation is
sufficient now.
* Improve error specificity
* Framework for user defined blueprint descriptions
BlueprintDescription can be added to a workflows extra field to provide
more useful information about a blueprint's purpose
Actually hooking this up in a way that is user accessible is out of
scope for right now, but this will simplify future implementation.
* Cleanup breadcrumb dropdown options
Removes Dupliate for blueprints, adds a publish subgraph option.
The publish subgraph button currently routes through the save as logic.
Unforunately, this results in the prompt for name referencing workflows.
The cleanest way to resolve this is still being considered
* Move blueprint renaming into blueprint load
Blueprints should automatically set the name of the added node to the
filename when added. This mostly worked, but created uglier edgecases:
The subgraph itself wasn't renamed, and it would need to be
reimplemented to apply when editing a blueprint.
Instead, this is now applied when a subgraphBlueprint is first loaded.
This keeps all the logic routed through a single point
* Move saveAs prompt into workflow class
Ensures that the correct publish text is displayed when editing
blueprints without making an awful mess of imports
* Fix tests by making subgraphBlueprint internal
This has the added benefit of forcing better organization.
Reverts the useWorkflowThumbnail patch as it is no longer required.
* Add tests for subgraph blueprints
* Rewrite confirmation dialog
* Fix overwrite on publish new subgraph
1 is used as a placeholder size as -1 indicates the baking userFile is
temporary, not persisted, and therefore, not able to overwrite when
saved.
* When editing blueprint, tint background blue
* Fix blueprint tint at low LOD
* Set node source for blueprints to Blueprint
* Fix publish test
Making subgraph blueprints non temporary on publish made it so the
following load actually occurs. A mock has been added for this load.
* Fix multiple nits
* Further cleanup: error handling, and comments
* Fixing failing test cases
This also moves the bg tinting to a property of the workflow,
which makes things more extensible in the future.
* Fix temporary marking on publish.
The prior fix to allow overwrite of an existing blueprint on publish was
misguided. By marking a not-yet-loaded file as non-temporary, the load
performed prior to saving was actually fetching the file off disk and
discarding the existing changes. This additionally entirely prevented
publishing when a blueprint did not already exist with the current name.
To fix this, the blueprint is not marked as non-temporary until after
the load occurs. Note that this load is still required as it initializes
the change tracker state required for saving.
* Block unloading subgraph blueprints
Will need to be revisited if lazy loading is implemented, but this
requires solving some ugly sync/async issues.
---------
Co-authored-by: github-actions <github-actions@github.com>
* [feat] Add ESLint rule for deprecated PrimeVue components
Adds no-restricted-imports rule to catch usage of deprecated PrimeVue 4+ components and guide developers to use their replacements:
- Dropdown → Select
- OverlayPanel → Popover
- Calendar → DatePicker
- InputSwitch → ToggleSwitch
- Sidebar → Drawer
This prevents accidental usage of deprecated components and ensures consistency across the codebase.
* Add ESLint ignore for existing deprecated Dropdown usage
Adds TODO comment for future migration to Select component in SearchFilterDropdown.
* Update eslint.config.js
Co-authored-by: Alexander Brown <drjkl@comfy.org>
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
* standardize graph cleanup
* test: fix useCoreCommands tests and add regression test
- Fix mocking to properly simulate app.clean() calling graph.clear()
- Add intelligent subgraph detection in mock to match real implementation
- Add regression test for Vue node cleanup bug to prevent future regressions
- Ensures app.clean() properly triggers onNodeRemoved events through graph.clear()
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix unit tests
* move beforeLoadNewGraph to before graph is cleaned
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix: Replace reactive feature flags with non-reactive approach
- Changed managerUIState from computed to getManagerUIState() function
- Ensures fresh computation on each call to avoid timing issues
- Updates all consumers to use the new function-based approach
- Fixes manager UI state determination issues
This change addresses the reactivity issues where feature flags were not
updating properly due to Vue's reactive system limitations with external
API values.
* fix: Add HelpCenter manager state handling and API version switching
- Fixed HelpCenter manager extension to check manager state
- Fixed 'Manager' command to respect manager state
- Added dynamic API prefix switching based on manager state
- Added debug logging for manager state determination
This ensures legacy manager uses /api/ prefix and new manager uses /api/v2/ prefix
* fix: Simplify manager state determination and fix API timing issues
- Remove unnecessary extension check from manager state store
- Use only feature flags (client and server) for state determination
- Default to NEW_UI when server flags not loaded (safer default)
- Fix ImportFailInfoBulk to not send empty requests
- Resolves initial 404 errors on installed API calls
* fix: Correct manager state determination for non-v4 servers
- Fix serverSupportsV4=false returning DISABLED instead of LEGACY_UI
- Server without v4 support should use legacy manager, not disable it
- Clarify condition for server v4 + client non-v4 case
* chore: Remove debug console.log statements
- Remove all debug logging from manager state store
- Remove logging from comfy manager service
- Clean up code for production
* test: Update manager state store tests to match new logic
- Update test expectations for server feature flags undefined case (returns NEW_UI)
- Update test expectations for server not supporting v4 case (returns LEGACY_UI)
- Tests now correctly reflect the actual behavior of the manager state logic
* fix: Remove dynamic API version handling in manager service
- Remove getApiBaseURL() function and axios interceptor
- Always use /api/v2/ for New Manager (hardcoded)
- Add isManagerServiceAvailable() to block service calls when not in NEW_UI state
- Simplify API handling as manager packages are now completely separated
* refactor: Add helper functions to managerStateStore for better code reuse
- Add isManagerEnabled(), isNewManagerUI(), isLegacyManagerUI() helpers
- Add shouldShowInstallButton(), shouldShowManagerButtons() for UI logic
- Update components to use helper functions where applicable
- Add comprehensive tests for new helper functions
- Centralize state checking logic to reduce duplication
* fix: Ensure SystemStats is loaded before conflict detection
- Move conflict detection from App.vue to GraphCanvas.vue
- Check manager state before running conflict detection
- Ensures SystemStats and feature flags are loaded first
- Prevents unnecessary API calls when manager is disabled
* docs: Clarify feature flag default behavior in manager state
- Add detailed comments explaining why NEW_UI is the default
- Clarify that undefined state is temporary during WebSocket connection
- Document graceful error handling when server doesn't support v2 API
* fix: Ensure consistent manager state handling for legacy commands
- Legacy commands now show error toast in NEW_UI mode
- Settings fallback for DISABLED mode
- Consistent error handling across all manager entry points
- Legacy commands only work in LEGACY_UI mode as expected
* refactor: centralize manager opening logic into managerStateStore
- Create openManager() function in managerStateStore to eliminate duplicate code
- Replace 8+ repeated switch statements across different files with single function
- Fix inconsistency where legacy command failure in LEGACY_UI mode incorrectly opened new manager
- Add support for legacy-only commands that should show error in NEW_UI mode
- Ensure all manager entry points behave consistently according to feature flags
- Clean up unused imports and fix ESLint errors
This addresses Christian's code review feedback about duplicate switch statements
and improves maintainability by providing a single source of truth for manager
opening logic.
* fix: use correct i18n import in managerStateStore
- Replace useI18n with direct t import from @/i18n
- Fixes issue where error messages showed as numbers (e.g. '26') instead of text
- Ensures toast messages display correctly in NEW_UI mode when legacy commands are invoked
* feature: initial tab fix
* test: Fix managerStateStore test failures by adding missing mocks
The test was failing because managerStateStore imports dialogService,
which imports ErrorDialogContent.vue that instantiates the app object.
This caused api.addEventListener errors in tests.
Added proper mocks for:
- dialogService
- commandStore
- toastStore
This prevents the problematic import chain and fixes the test failures.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* refactor: convert managerStateStore to composable
- Move managerStateStore from store to composable pattern
- All functions are non-reactive utilities that don't need state management
- Follows Pinia guideline: "If it's not reactive, it shouldn't be in a store"
- Update all import paths across 8 files
- Move and update test file accordingly
This change improves architecture consistency as other utility functions
in the codebase also use composables rather than stores when reactivity
is not required.
* refactor: use readonly computed properties instead of getter methods
- Convert all getter methods to readonly computed properties
- Follows Vue conventions for better performance through caching
- Change access pattern from function calls to .value properties
- Update all usages across 6 files
- Thanks to @DrJKL for the suggestion
This improves performance by caching computed values and aligns
with Vue's reactive system patterns.
* fix: check isManagerEnabled check to GraphCanvas.vue to avoid the side-effects of calling useConflictDetection which include calling useComfyManagerStore
* chore: console.log to console.debug
* chore: useConflictDetection().initializeConflictDetection()
* test: add mockManagerDisabled option to disable manager in Playwright tests
- Add mockManagerDisabled parameter to ComfyPage.setup() (defaults to true)
- Override api.getServerFeature() to return false for manager feature flag
- Prevents manager initialization from interfering with subgraph tests
- Individual tests can still enable manager when needed by passing mockManagerDisabled: false
* chore: text modified
* fix: resolve CI/CD failures by fixing manager initialization timing
## Problem
GraphCanvas.vue was initializing conflict detection during component setup,
causing side effects in test environment where manager is disabled. This led
to 4 Playwright test failures in PR #5317.
## Root Cause
- GraphCanvas.vue called useConflictDetection() in setup phase
- This triggered store side effects even when manager was disabled
- systemStats wasn't ready when checking manager state
## Solution
1. Removed conflict detection initialization from GraphCanvas.vue entirely
2. Refactored systemStatsStore to use VueUse's useAsyncState pattern
3. Added isInitialized check in useManagerState to wait for systemStats
4. Updated useConflictDetection to check manager state internally
## Changes
- **GraphCanvas.vue**: Removed all conflict detection code
- **systemStatsStore**: Implemented useAsyncState for proper async handling
- **useManagerState**: Added isInitialized check before checking manager state
- **useConflictDetection**: Added internal manager state validation
- **App.vue**: Removed unnecessary fetchSystemStats calls
- **Tests**: Updated unit tests for new async behavior
## Test Results
All 4 previously failing Playwright tests now pass:
- featureFlags.spec.ts (feature flag handling)
- subgraph.spec.ts (breadcrumb updates, DOM cleanup)
- widget.spec.ts (image changes)
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* chore: modified the note
* fix: test code modified
* fix: when manager is new manager ui, conflict detectetion should work
* fix: ensure fetch system stats before determine manager stats & when new ui & call legacy ui, open new manger dialog by default
* chore: unnecessary .value deleted & fetch name modified to refetch
* fix: ref type .value needed
* chore: vue use until pattern for waiting initializing
* fix: .value added
* fix: useManagerState test to properly mock reactive refs
The test was failing because it was mocking systemStats and isInitialized as plain values instead of reactive refs. The actual composable uses storeToRefs which returns refs with .value properties.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
* fix: when system stats initialized, use until(systemStatsStore.isInitialized)
* fix: test
---------
Co-authored-by: Claude <noreply@anthropic.com>
Fixes shell script error where 'return' was used outside of a function.
In shell scripts, 'exit 0' should be used to exit with success status.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Claude <noreply@anthropic.com>
* refactor: Extract Vue node entry point logic into focused composables
- Extract logic from GraphCanvas.vue (735→200 lines) into 3 composables:
- useVueNodeLifecycle: Node manager initialization and cleanup
- useViewportCulling: Viewport culling with transform sync
- useNodeEventHandlers: Node selection, collapse, title handlers
- Remove type assertions by using comfyApp.canvas instead of canvasStore.canvas
- Eliminate getter anti-pattern with proper Vue reactive refs
- Fix all TypeScript compatibility issues without workarounds
- Maintain proper separation of concerns and Vue-idiomatic patterns
* style: Remove extra comments from return statement
* [auto-fix] Apply ESLint and Prettier fixes
* style: Remove conversational comments
- Remove temporary comments that only made sense in refactoring context
- Clean up comment wording to be more permanent/professional
- Keep meaningful comments about code behavior and architecture
---------
Co-authored-by: GitHub Action <action@github.com>
Fixed the misuse of exposed template refs in InputSlot and OutputSlot components.
When Vue exposes a Ref via defineExpose, it auto-unwraps it on the parent
component instance. The previous code was incorrectly double-unwrapping by
calling .value on an already unwrapped HTMLElement.
Changes:
- Updated type to ComponentPublicInstance with unwrapped HTMLElement
- Replaced watch with watchEffect for better timing handling
- Removed incorrect .value access on auto-unwrapped ref
This ensures slot elements are properly registered with useDomSlotRegistration,
fixing slot position tracking and hit-testing in the layout system.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Claude <noreply@anthropic.com>
* Change theme "button" to sub menu of all themes
* Add test for theme menu
* Prevent separator being added before View
* Refactor test
* Update locales [skip ci]
* Fix has-text vs text-is change breaking other tests
---------
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: bymyself <cbyrne@comfy.org>
* On conversion of single group, convert children
If convert to subgraph is called on a selection consisting of a single
group, the groups children are also converted to subgraph.
* Update locales [skip ci]
* Create new set to avoid mutating passed argument
---------
Co-authored-by: github-actions <github-actions@github.com>
- Add vue/no-restricted-class ESLint rule to catch dark: prefix usage
- Fix existing violation in FormImageUpload.vue (dark: -> dark-theme:)
- Ensures consistent usage of project's custom Tailwind variant
- Violations will fail builds and be caught during development
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Claude <noreply@anthropic.com>
* use step2 -> step bind on slider widget
* fix: Use step2 instead of legacy step property in WidgetSlider
The WidgetSlider was using the legacy `step` property (10x input spec value)
instead of `step2` (correct input spec value). This caused input spec step
values to appear 10x larger than intended.
- Use `widget.options.step2` (correct input spec value)
- Remove fallback to `widget.options.step` (legacy 10x value)
- Both properties coexist, so step2 should always be preferred
Fixes input spec step values not being respected in Vue node sliders.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Update hotfix release command for modern automated backport workflow
- Add Step 0 to check automated backport status first
- Emphasize this command is fallback when automation fails
- Add critical draft release publishing step (uncheck 'Set as latest')
- Add ComfyUI requirements.txt PR creation with exact template
- Update workflow context for modern automated backports
* Restructure hotfix release command for modern workflow
- Add clear process overview and context at top
- Step 1: Try automated backports first (via labels)
- Fallback to manual cherry-picking only if automation fails
- Add critical draft release publishing step (uncheck 'Set as latest')
- Add ComfyUI requirements.txt PR creation with exact template
- Remove time estimates and reorganize for clarity
- Update step numbering and cross-references
* Enable backport automation for already-merged PRs
- Add 'labeled' trigger to backport workflow
- Allow backport automation when needs-backport label is added to merged PRs
- Supports hotfix workflow where labels are added retroactively
- Maintains existing behavior for PRs merged with labels already present
* Prevent duplicate backport triggers with idempotency check
- Add check for existing backport PRs before starting backport process
- Skip backport work if PRs already exist for the same PR number
- Prevents double execution when both 'labeled' and 'closed' events trigger
- Maintains workflow reliability and avoids duplicate backport PRs
* Add smart backport detection to hotfix command
- Check for existing automated backport PRs and their status
- Path A: Skip to version bump if backports already merged
- Path B: Guide user to merge pending backport PRs first
- Path C: Fall back to manual cherry-picking if no/failed automation
- Add clear workflow path documentation for different scenarios
* Add automated fork handling for ComfyUI requirements.txt PRs
- Check if fork exists, create if needed
- Clone fork to local ComfyUI-fork directory
- Create branch, update requirements.txt with sed
- Create PR from fork using gh CLI with exact template format
- Handle both new and existing fork scenarios
- Keep fork directory for future updates
* [style] improve backport workflow logging and structure
- Change ::notice to ::warning for existing backports per @DrJKL's feedback
- Refactor conditional to use guard clause pattern per @DrJKL's suggestion
- Improves readability and follows conventional shell scripting patterns
Addresses @DrJKL's review comments in PR #5271
- Rename 0004-crdt-based-layout-system.md to 0003-crdt-based-layout-system.md
- Update title from "4. Centralized..." to "3. Centralized..."
- Add ADR 0003 entry to docs/adr/README.md index table
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Branch protection for core/X.XX branches will now be managed through
GitHub repository rulesets with wildcard pattern matching, providing
more consistent and centralized protection rule management.
* refactor: dont need will change on animations
* fix: by disabling parent pointer events and forcing on child element
* fix: color picker watch with immediate option
* Update test expectations [skip ci]
---------
Co-authored-by: Jake Schroeder <jake.schroeder@isophex.com>
Co-authored-by: github-actions <github-actions@github.com>
Double clicking the input slot of a node creates and connects a
primitive widget. However, this code would always add the created
primitive to the root graph.
