mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-14 01:36:14 +00:00
e79e23ddd1aaadcc5030fee07b45bcf3f7eba15e
741 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
GitHub Action
|
cabcf2d57b | [automated] Apply ESLint and Oxfmt fixes | ||
|
bymyself
|
ebb1930212 |
feat(#3410): add centralized assert() utility in src/base/
- src/base/assert.ts: assert(condition, message) with console.error always, throw in DEV, delegate to registered reporter otherwise - setAssertReporter() registration pattern avoids layer architecture violation (base/ cannot import from platform/) - src/main.ts: registers Sentry+toast reporter after Sentry.init() - src/scripts/changeTracker.ts: replaces inline Sentry+warn in reportInactiveTrackerCall() with assert() call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
jaeone94
|
11432f7d0e |
refactor: extract missing model refresh pipeline (#11751)
## Summary Extracts the missing-model pipeline orchestration out of `ComfyApp` and into an app-independent platform module, while tightening the workflow-flattening type boundary that refresh needs when rescanning the live LiteGraph graph. This PR is intentionally refactor-heavy. It is the follow-up to the earlier missing-model refresh work: instead of keeping refresh-specific candidate recheck logic beside the UI, this change makes the refresh path reuse the existing missing-model pipeline and removes the direct dependency on private `ComfyApp` pipeline methods. Linear: FE-499 Issues covered by this PR: - Fixes #11678 - Fixes #11680 - Partially addresses #11679 by removing the missing-model refresh path's unsafe `graph.serialize() as unknown as ComfyWorkflowJSON` cast and replacing it with the narrower flattenable workflow contract. Broader workflow serialization/type-boundary cleanup outside this missing-model refresh path remains deferred. ## Changes - **What**: - Added `src/platform/missingModel/missingModelPipeline.ts` as the orchestration module for missing-model detection/verification. - `runMissingModelPipeline(...)` now owns the pipeline previously embedded in `ComfyApp`: - candidate scan and enrichment - active ancestor filtering for muted/bypassed subgraph containers - pending warning cache updates - OSS folder path and file-size follow-up work - cloud asset verification follow-up work - surfaced missing-model errors via the existing execution error store - `refreshMissingModelPipeline(...)` handles the refresh-specific flow: - calls the injected `reloadNodeDefs()` first - serializes the current live graph - preserves model metadata by preferring active workflow `models`, then falling back to current missing-model candidate metadata - delegates back into the same pipeline used during workflow load - Kept `ComfyApp` as the compatibility caller instead of the owner of the pipeline. - `loadGraphData(...)` now calls `runMissingModelPipeline(...)` with `graph`, `graphData`, `missingNodeTypes`, and `silent` options. - `refreshMissingModels(...)` is now a thin wrapper around `refreshMissingModelPipeline(...)` and keeps the existing default `silent: true` refresh behavior. - The new pipeline module does not import `@/scripts/app`; app-owned data/actions are passed in as inputs. - Moved the workflow node-flattening helpers out of `workflowSchema.ts` and into `src/platform/workflow/core/utils/workflowFlattening.ts`. - This includes `flattenWorkflowNodes`, `buildSubgraphExecutionPaths`, and `isSubgraphDefinition`. - The move is intentional: these helpers are not zod schema definitions or workflow validation logic. They are core workflow traversal utilities used to flatten root workflow nodes plus nested subgraph definition nodes into the execution-shaped node list needed by missing-model scanning. - The refresh path receives data from `LGraph.serialize()`, whose return type is serialized LiteGraph data rather than validated `ComfyWorkflowJSON`. Previously this forced unsafe typing like `graph.serialize() as unknown as ComfyWorkflowJSON`. - The new `FlattenableWorkflowGraph` / `FlattenableWorkflowNode` structural contract describes only what flattening actually needs: `nodes`, `definitions.subgraphs`, node `id`, `type`, `mode`, `widgets_values`, and `properties`. - This lets both normal workflow-load data (`ComfyWorkflowJSON`) and refresh-time live graph serialization (`LGraph.serialize()`) flow into the same scan/enrichment path without pretending serialized LiteGraph output is a fully validated workflow schema document. - Updated `missingModelScan.ts` to consume that minimal flattenable workflow shape via `MissingModelWorkflowData`. - `MissingModelWorkflowData` extends the flattenable workflow contract with optional workflow-level `models` metadata. - Removed now-unnecessary casts around execution IDs, flattened nodes, and `widgets_values` object access. - Updated `getSelectedModelsMetadata(...)` to accept readonly widget value arrays so flattened workflow data can stay read-only. - Reduced the exported surface of the new pipeline module after `knip` flagged unused exported internal option/store interfaces. - Kept `workflowSchema.ts` focused on validation schemas. The flattening helpers are not re-exported from the schema module because they are internal workflow core utilities, not public schema API. - **Breaking**: None intended. - Internal imports were updated to the new core utility path. - This repo is not exposing these flattening helpers as a public package API, so the old schema-local helper location is treated as an internal implementation detail. - **Dependencies**: None. ## Review Focus - **Pipeline extraction / dependency direction**: - Please verify that `missingModelPipeline.ts` stays independent from `@/scripts/app`. - `ComfyApp` should remain the caller/adapter, not the owner of missing-model pipeline orchestration. - **Workflow flattening type boundary**: - The main type-cleanup goal is removing the refresh-time `graph.serialize() as unknown as ComfyWorkflowJSON` lie. - `LGraph.serialize()` and validated workflow JSON are not the same contract. The new flattenable workflow contract is deliberately smaller and structural because the missing-model enrichment path only needs enough data to flatten nodes and read embedded model metadata. - This is why the flattening helpers moved from `workflowSchema.ts` to `workflow/core/utils`: the logic is reusable workflow traversal, not validation schema. - **Behavior preservation**: - The PR is intended to preserve existing user-facing missing-model behavior while moving ownership out of `app.ts`. - Existing async follow-up behavior remains intentionally fire-and-forget: - cloud asset verification still surfaces after verification completes - OSS folder paths still update asynchronously before surfacing confirmed missing models - file-size metadata fetching remains asynchronous - More invasive behavior changes, such as adding non-cloud post-fetch `isMissingCandidateActive(...)` re-verification or redesigning the fire-and-forget result contract, are intentionally left for follow-up work because they are not pure extraction. - **Downloadable model metadata**: - `missingModels` returned for download metadata now requires both `url` and `directory`. - Candidates without a directory still remain in `confirmedCandidates`, but they are not exposed as downloadable model metadata. This keeps the returned downloadable list aligned with what the download flow can actually use. - **Test ownership**: - Complex missing-model pipeline behavior tests moved out of `src/scripts/app.test.ts` and into `src/platform/missingModel/missingModelPipeline.test.ts`. - `app.test.ts` now only covers thin delegation for `app.refreshMissingModels(...)`. - Workflow flattening tests moved with the helper from schema tests into `src/platform/workflow/core/utils/workflowFlattening.test.ts`. - **Deferred follow-ups**: - Broader function decomposition for cognitive complexity. - Wider dependency-injection/port cleanup for stores and services beyond the app boundary. - Cloud-specific pipeline unit tests, which need a separate `isCloud` mocking strategy. - Additional E2E coverage expansion beyond the existing OSS refresh path. - More general workflow serialization/type-boundary cleanup outside the missing-model refresh path. ## Validation - `pnpm format` - `pnpm lint` - Passed. Existing lint output included a pre-existing `no-misused-spread` warning and icon-name logs, but the command exited successfully. - `pnpm typecheck` - `pnpm test:unit` - `714 passed`, `9514 passed | 8 skipped` - Pre-push `pnpm knip` - Passed after reducing the exported surface of the new pipeline module. ## Screenshots (if applicable) Not applicable. This PR is a pipeline/type-boundary refactor with no UI changes. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11751-refactor-extract-missing-model-refresh-pipeline-3516d73d3650816d9245d4b1324b71c9) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Dante
|
8fe0385a57 |
test: add unit tests for pnginfo wrappers and getLatentMetadata (#11745)
## Summary Extends \`src/scripts/pnginfo.test.ts\` with 5 new tests covering the format-specific delegating wrappers and the safetensors metadata reader. Lifts pnginfo.ts coverage from **17.6% → 23.2%** lines (the remaining gap is \`importA1111\`, which needs a refactor before it can be tested cleanly — left to a follow-up). ## Test Coverage - \`getPngMetadata\`, \`getFlacMetadata\`, \`getAvifMetadata\` delegate to their respective \`metadata/*\` modules (mocked). - \`getLatentMetadata\` returns the \`__metadata__\` object from a hand-built safetensors header. - \`getLatentMetadata\` resolves \`undefined\` when the header has no \`__metadata__\` entry. ## Out of scope \`importA1111\` (lines 176-542) is a 270-line A1111-prompt → ComfyUI-graph builder. Testing it requires either heavy LiteGraph mocks or a refactor that extracts pure parsing helpers from the graph-mutation code. Tracking separately. ## Testing \`\`\`bash pnpm vitest run src/scripts/pnginfo.test.ts pnpm vitest run src/scripts/pnginfo.test.ts --coverage --coverage.include='src/scripts/pnginfo.ts' \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11745-test-add-unit-tests-for-pnginfo-wrappers-and-getLatentMetadata-3516d73d365081c080a6c8146aa1bee8) by [Unito](https://www.unito.io) |
||
|
Dante
|
d078af3a79 |
test: add unit tests for avif metadata parser (#11744)
## Summary
Adds 12 tests for `src/scripts/metadata/avif.ts`, raising line coverage
from **2.3% → 90.4%** (statements 88.5%, functions 93.3%).
## Test Coverage
Happy paths:
- Workflow JSON extracted from EXIF Exif item (LE)
- Prompt JSON extracted
- Big-endian (MM) EXIF parsing
- Both prompt and workflow present in separate EXIF entries
Negative paths (each yields `{}` without throwing):
- AVIF major brand is not "avif"
- Meta box missing
- iinf has no Exif item
- EXIF entry uses an unrecognized key
- EXIF entry has malformed JSON
- infe version is unsupported (1)
- iloc box missing while iinf has Exif
- Buffer too short for valid header
## Testing
\`\`\`bash
pnpm vitest run src/scripts/metadata/avif.test.ts
pnpm vitest run src/scripts/metadata/avif.test.ts --coverage
--coverage.include='src/scripts/metadata/avif.ts'
\`\`\`
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11744-test-add-unit-tests-for-avif-metadata-parser-3516d73d365081c5b29adf7a2b9eff62)
by [Unito](https://www.unito.io)
|
||
|
AustinMroz
|
1c541d8577 |
Short circuit asset reuploads, simplify node dnd (#11691)
When an output is dragged from the assets panel onto a node, outputs were being reuploaded. This logic has been simplified to instead reference the existing asset by resolving the annotated path. As part of this change, async drop handlers on nodes are also fixed. Rather than placing obligation of event handling on client code, not respecting async handlers, or completely ignoring return types, the vue drop handler will now simply set `app.dragOverNode` and allow the `document` drop handler to resolve node drag/drop operations without any of the difficulty from propagation. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11691-Short-circuit-asset-reuploads-simplify-node-dnd-34f6d73d36508157af86e6cf09229781) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
jaeone94
|
b4d209b5f6 |
feat: refresh missing models through pipeline (#11661)
## Summary Follow-up to the closed earlier attempt in #11646. This PR keeps the same user-facing goal, but changes the implementation to reuse the existing missing model pipeline for refresh instead of maintaining a separate candidate-only recheck path. Adds a missing model refresh action in the Errors tab by reusing the existing missing model pipeline, so users can re-check models after downloading or manually placing files without reloading the workflow. ## Changes - **What**: - Adds `app.refreshMissingModels()` as a reusable refresh entry point for the current root graph. - Splits node definition reloading into `app.reloadNodeDefs()` so missing-model refresh can pull fresh `object_info` without showing the generic combo refresh success flow. - Reuses the existing missing model pipeline instead of adding a separate candidate-only checker. The refresh path serializes the current graph, reuses active workflow model metadata when available, falls back to current missing-model metadata, and then reruns the same candidate discovery/enrichment/surfacing flow used during workflow load. - Adds missing model refresh state and error handling to `missingModelStore`. - Adds a Refresh button next to Download all in the missing model card action bar. - Moves Download all from the Errors tab header into the missing model card, so the Download all and Refresh actions render or hide together. - Changes Download all visibility from “more than one downloadable model” to “at least one downloadable model.” - Keeps the action bar hidden when there are no downloadable missing models; Cloud still does not render this action area. - Normalizes active workflow `pendingWarnings` updates so resolved missing model warnings do not get revived by stale empty warning objects. - Adds test IDs and coverage for the new action bar, refresh state, refresh delegation, pending warning sync, and E2E refresh behavior. - **Breaking**: None. - **Dependencies**: None. ## Review Focus The main design choice is intentionally reusing the missing model pipeline for refresh instead of implementing a smaller candidate-only recheck. The earlier candidate-only approach was cheaper, but it created a separate source of truth for missing-model resolution and made edge cases harder to reason about. In particular, it could diverge from the behavior used when a workflow is loaded, and it did not naturally handle the case where a model becomes missing after the workflow is already open. This version pays the cost of refreshing node definitions and rerunning the missing-model scan for the current graph, but keeps the refresh behavior aligned with workflow load semantics. Expected behavior by environment: - OSS browser: - The action bar appears when at least one missing model has a downloadable URL and directory. - Download all uses the existing browser download path. - Refresh reloads `object_info`, refreshes node definitions/combo values, reruns missing-model detection for the current graph, and clears the error if the selected model is now available. - OSS desktop: - The same action bar appears under the same downloadable-model condition. - Download all uses the existing Electron DownloadManager path. - Refresh uses the same missing-model pipeline as browser, so manually placed files or desktop-downloaded files can be rechecked without reloading the workflow. - Cloud: - The action bar remains hidden because model download/import is not supported in this section for Cloud. A few boundaries are intentional: - This PR does not add automatic filesystem watching. Browser OSS cannot reliably observe local model folder changes, so the user-triggered Refresh button remains the cross-environment mechanism. - This PR does not redesign the public `refreshComboInNodes` API beyond extracting `reloadNodeDefs()` for reuse. Further cleanup of toast behavior or a more explicit object-info reload API can be follow-up work. - This PR keeps refresh scoped to missing-model validation; missing media and missing nodes continue to use their existing flows. Linear: FE-417 ## Screenshots (if applicable) https://github.com/user-attachments/assets/2e02799f-1374-4377-b7b3-172241517772 ## Validation - `pnpm format` - `pnpm lint` (passes; existing unrelated warning remains in `src/platform/workspace/composables/useWorkspaceBilling.test.ts`) - `pnpm typecheck` - `pnpm test:unit` - `pnpm test:browser:local -- --project=chromium browser_tests/tests/propertiesPanel/errorsTabMissingModels.spec.ts` - `pnpm build` - `NX_SKIP_NX_CACHE=true DISTRIBUTION=desktop USE_PROD_CONFIG=true NODE_OPTIONS='--max-old-space-size=8192' pnpm exec nx build` - Manual desktop verification through `~/Projects/desktop` after copying the desktop build into `assets/ComfyUI/web_custom_versions/desktop_app`: - confirmed the FE bundle is built with `DISTRIBUTION = "desktop"` - confirmed missing model Download uses the desktop download path instead of browser download - confirmed Refresh can clear the missing model error after the model is available - Push hook: `pnpm knip --cache` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11661-feat-refresh-missing-models-through-pipeline-34f6d73d3650811488defee54a7a6667) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
4b7a027946 |
fix: route progress_text feature flag check through getDevOverride (#11384)
## Summary
Route the `progress_text` binary parser's feature-flag check through
`serverSupportsFeature()` so dev overrides via `localStorage` take
effect.
## Changes
- **What**: Replace
`this.getClientFeatureFlags()?.supports_progress_text_metadata` with
`this.serverSupportsFeature('supports_progress_text_metadata')` in the
`case 3` binary message handler, consistent with all other feature-flag
checks in the class.
## Review Focus
Minimal one-line change. The key consideration is that
`serverSupportsFeature()` routes through `getDevOverride()` first,
enabling `localStorage` overrides (`ff:supports_progress_text_metadata`)
for dev testing of the binary wire format.
Fixes #11187
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11384-fix-route-progress_text-feature-flag-check-through-getDevOverride-3476d73d36508161bca0d6c2ea7c3c55)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
|
||
|
Christian Byrne
|
c5b6fd9c40 |
test: clarify inert getClientFeatureFlags mock in progress_text binary parsing tests (#11385)
## Summary Adds inline comments to three `vi.mocked(api.getClientFeatureFlags).mockReturnValue()` calls in the `progress_text binary message parsing` describe block, clarifying they are intentionally inert — the parser checks `serverFeatureFlags` only. This prevents future readers from being confused about whether the mock has any effect. - Fixes #11186 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11385-test-clarify-inert-getClientFeatureFlags-mock-in-progress_text-binary-parsing-tests-3476d73d365081a98c06c43c4737fdd9) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
b756545f59 |
refactor: clean up ChangeTracker logging, guards, and redundant widget wrapper (#11328)
## Summary Follow-ups to PR #10816. Bundles four review items left open after that PR merged — three inside `ChangeTracker` itself and one in the widget composable that wraps it. ### What changed - **Removed all `loglevel` logging from `src/scripts/changeTracker.ts`** — the logger was set to `info`, so every `logger.debug` call was dead code at runtime. `logger.warn` calls were replaced with direct reporting. The only-downstream dead code (`graphDiff` helper) and its sole dependency (`jsondiffpatch`) are also removed. - **Named the `captureCanvasState()` guard conditions** — `isUndoRedoing` and `isInsideChangeTransaction` now carry the intent that the inline `_restoringState` / `changeCount > 0` expressions used to obscure. - **Surfaced lifecycle violations through a single reporting helper** — `reportInactiveTrackerCall()` logs `console.warn` once per method per session and, on Desktop, emits a `Sentry.addBreadcrumb` with the offending workflow path. `deactivate()` and `captureCanvasState()` share this path so the same invariant is reported consistently. - **Inlined `captureWorkflowState` wrapper in `useWidgetSelectActions`** — the private helper forwarded to `changeTracker.captureCanvasState()` with no added logic. Both call sites now invoke the change tracker directly. ### Issues fixed - Fixes #11249 - Fixes #11259 - Fixes #11258 - Fixes #11248 ### Test plan - [x] `pnpm test:unit src/scripts/changeTracker.test.ts` — 16 tests pass - [x] `pnpm test:unit src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectActions.test.ts` — 6 tests pass - [x] `pnpm typecheck` - [x] `pnpm lint` - [x] `pnpm format` |
||
|
jaeone94
|
a1e6fb36d2 |
refactor: harden ChangeTracker lifecycle with self-defending API (#10816)
## Summary Harden the `ChangeTracker` lifecycle to eliminate the class of bugs where an inactive workflow's tracker silently captures the wrong graph state. Renames `checkState()` to `captureCanvasState()` with a self-defending assertion, introduces `deactivate()` and `prepareForSave()` lifecycle methods, and closes a latent undo-history corruption bug discovered during code review. ## Background ComfyUI supports multiple workflows open as tabs, but only one canvas (`app.rootGraph`) exists at a time. When the user switches tabs, the old workflow's graph is unloaded and the new one is loaded into this shared canvas. The old `checkState()` method serialized `app.rootGraph` into `activeState` to track changes for undo/redo. It had no awareness of *which* workflow it belonged to -- if called on an inactive tab's tracker, it would capture the active tab's graph data and silently overwrite the inactive workflow's state. This caused permanent data loss (fixed in PR #10745 with caller-side `isActive` guards). The caller-side guards were fragile: every new call site had to remember to add the guard, and forgetting would reintroduce the same silent data corruption. Additionally, `beforeLoadNewGraph` only called `store()` (viewport/outputs) without `checkState()`, meaning canvas state could be stale if a tab switch happened without a preceding mouseup event. ### Before (fragile) ``` saveWorkflow(workflow): if (isActive(workflow)) <-- caller must remember this guard workflow.changeTracker.checkState() <-- name implies "read", actually writes ... beforeLoadNewGraph(): activeWorkflow.changeTracker.store() <-- only saves viewport, NOT graph state ``` ### After (self-defending) ``` saveWorkflow(workflow): workflow.changeTracker.prepareForSave() <-- handles active/inactive internally ... beforeLoadNewGraph(): activeWorkflow.changeTracker.deactivate() <-- captures graph + viewport together ``` ## Changes - Rename `checkState` to `captureCanvasState` with active-tracker assertion - Add `deactivate()` and `prepareForSave()` lifecycle methods - Fix undo-history corruption: `captureCanvasState()` guarded by `_restoringState` - Fix viewport regression during undo: `deactivate()` skips `captureCanvasState()` during undo/redo but always calls `store()` to preserve viewport (regression from PR #10247) - Log inactive tracker warnings unconditionally at warn level (not DEV-only) - Deprecated `checkState()` wrapper for extension compatibility - Rename `checkState` to `captureCanvasState` in `useWidgetSelectActions` composable - Add `appModeStore.ts` to manual call sites documentation - Add `checkState()` deprecation note to architecture docs - Add 16 unit tests covering all guard conditions, lifecycle methods, and undo behavior - Add E2E test: "Undo preserves viewport offset" ## New ChangeTracker Public API | Method | Caller | Purpose | |--------|--------|---------| | `captureCanvasState()` | Event handlers, UI interactions | Snapshots canvas into activeState, pushes undo. Asserts active tracker. | | `deactivate()` | `beforeLoadNewGraph` only | `captureCanvasState()` (skipped during undo/redo) + `store()`. Freezes state for tab switch. | | `prepareForSave()` | Save paths only | Active: `captureCanvasState()`. Inactive: no-op. | | `checkState()` | **Deprecated** -- extensions only | Wrapper that delegates to `captureCanvasState()` with deprecation warning. | | `store()` | Internal to `deactivate()` | Saves viewport, outputs, subgraph navigation. | | `restore()` | `afterLoadNewGraph` | Restores viewport, outputs, subgraph navigation. | | `reset()` | `afterLoadNewGraph`, save | Resets initial state (marks as "clean"). | ## Test plan - [x] Unit tests: 16 tests covering all guard conditions, state capture, undo queue behavior - [x] E2E test: "Undo preserves viewport offset" verifies no viewport drift on undo - [x] E2E test: "Prevents captureCanvasState from corrupting workflow state during tab switch" - [x] Existing E2E: "Closing an inactive tab with save preserves its own content" - [ ] Manual: rapidly switch tabs during undo/redo, verify no viewport drift - [ ] Manual: verify extensions calling `checkState()` see deprecation warning in console |
||
|
pythongosssss
|
a8e1fa8bef |
test: add regression test for WEBP RIFF padding (#8527) (#11267)
## Summary Add a regression test for #8527 (handle RIFF padding for odd-sized WEBP chunks). The fix added + (chunk_length % 2) to the chunk-stride calculation in getWebpMetadata so EXIF chunks following an odd-sized chunk are still located correctly. There was no existing unit test covering getWebpMetadata, so without a regression test the fix could silently break in a future refactor. ## Changes - **What**: - New unit test file src/scripts/pnginfo.test.ts covering getWebpMetadata's RIFF chunk traversal. - Helpers build a minimal in-memory WEBP with one VP8 chunk of configurable length followed by an EXIF chunk encoding workflow:<json>. - Odd-length case (regression for #8527): without the % 2 padding adjustment, the parser walks one byte short and returns {}. - Even-length case: guards against an over-correction that always adds 1. - Verified RED→GREEN locally. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11267-test-add-regression-test-for-WEBP-RIFF-padding-8527-3436d73d36508117a66edf3cb108ded0) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
693b8383d6 |
fix: missing-asset correctness follow-ups from #10856 (#11233)
Follow-up to #10856. Four correctness issues and their regression tests. ## Bugs fixed ### 1. ErrorOverlay model count reflected node selection `useErrorGroups` exposed `filteredMissingModelGroups` under the public name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute its model count label, so selecting a node shrank the overlay total. The overlay must always show the whole workflow's errors. Exposed both shapes explicitly: `missingModelGroups` / `missingMediaGroups` (unfiltered totals) and `filteredMissingModelGroups` / `filteredMissingMediaGroups` (selection-scoped). `TabErrors.vue` destructures the filtered variant with an alias. Before https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003 After https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120 ### 2. Bypass → un-bypass dropped url/hash metadata Realtime `scanNodeModelCandidates` only reads widget values, so un-bypass produced a fresh candidate without the url that `enrichWithEmbeddedMetadata` had previously attached from `graphData.models`. `MissingModelRow`'s download/copy-url buttons disappeared after a bypass/un-bypass cycle. Added `enrichCandidateFromNodeProperties` that copies `url`/`hash`/`directory` from the node's own `properties.models` — which persists across mode toggles — into each scanned candidate. Applied to every call site of the per-node scan. A later fix in the same branch also enforces directory agreement to prevent a same-name / different-directory collision from stamping the wrong metadata. Before https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b After https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833 ### 3. Initial full scan surfaced interior errors of a muted/bypassed subgraph container `scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based missing-node scan only check each node's own mode. Interior nodes whose parent container was bypassed passed the filter. Added `isAncestorPathActive(rootGraph, executionId)` to `graphTraversalUtil` and post-filter the three pipelines in `app.ts` after the live rootGraph is configured. The filter uses the execution-ID path (`"65:63"` → check node 65's mode) so it handles both live-scan-produced and JSON-enrichment-produced candidates. Before https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602 After https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8 ### 4. Bypassed subgraph entry re-surfaced interior errors `useGraphNodeManager` replays `graph.onNodeAdded` for each existing interior node when the Vue node manager initializes on subgraph entry. That chain reached `scanSingleNodeErrors` via `installErrorClearingHooks`' `onNodeAdded` override. Each interior node's own mode was active, so the caller guards passed and the scan re-introduced the error that the initial pipeline had correctly suppressed. Added an ancestor-activity gate at the top of `scanSingleNodeErrors`, the single entry point shared by paste, un-bypass, subgraph entry, and subgraph container activation. A later commit also hardens this guard against detached nodes (null execution ID → skip) and applies the same ancestor check to `isCandidateStillActive` in the realtime verification callback. Before https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441 After https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4 ## Tests - 6 unit tests for `isAncestorPathActive` (root, active, immediate-bypass, deep-nested mute, unresolvable ancestor, null rootGraph) - 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment, no-overwrite, name mismatch, directory mismatch) - 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry replaying onNodeAdded) - 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract - 4 E2E tests: - ErrorOverlay model count stays constant when a node is selected (new fixture `missing_models_distinct.json`) - Bypass/un-bypass cycle preserves Copy URL button (uses `missing_models_from_node_properties`) - Loading a workflow with bypassed subgraph suppresses interior missing model error (new fixture `missing_models_in_bypassed_subgraph.json`) - Entering a bypassed subgraph does not resurface interior missing model error (shares the above fixture) `pnpm typecheck`, `pnpm lint`, 206 related unit tests passing. ## Follow-up Several items raised by code review are deferred as pre-existing tech debt or scope-avoided refactors. Tracked via comments on #11215 and #11216. --- Follows up on #10856. |
||
|
Christian Byrne
|
e39468567a |
fix: check server feature flags for progress_text binary format (#10996)
## Problem API node generation status text (sent via `progress_text` WebSocket binary messages) was not showing on local ComfyUI, but worked on cloud. ## Root Cause The binary decoder for `progress_text` messages (eventType 3) checked `getClientFeatureFlags()?.supports_progress_text_metadata` — the **client's own flags** — to decide whether to parse the new format with `prompt_id`. Since the client always advertises `supports_progress_text_metadata: true`, it always tried to parse the new wire format: ``` [4B event_type][4B prompt_id_len][prompt_id][4B node_id_len][node_id][text] ``` But the backend PR that adds `prompt_id` to the binary message ([ComfyUI#12540](https://github.com/Comfy-Org/ComfyUI/pull/12540)) was **closed without merging**, so local ComfyUI still sends the legacy format: ``` [4B event_type][4B node_id_len][node_id][text] ``` The decoder misinterpreted the `node_id_len` as `prompt_id_len`, consuming the actual node_id bytes as a prompt_id, then producing garbled `nodeId` and `text` — silently dropping all progress text updates via the catch handler. Cloud worked because the cloud backend supports and echoes the feature flag. ## Fix One-line change: check `serverFeatureFlags.value` (what the server echoed back) instead of `getClientFeatureFlags()` (what the client advertises). ## Tests Added 3 tests covering: - Legacy format parsing when server doesn't support the flag - New format parsing when server does support the flag - Corruption regression test: client advertises support but server doesn't ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10996-fix-check-server-feature-flags-for-progress_text-binary-format-33d6d73d365081449a0dc918358799de) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
521019d173 |
fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary Muted and bypassed nodes are excluded from execution but were still triggering missing model/media/node warnings. This PR makes the error system mode-aware: muted/bypassed nodes no longer produce missing asset errors, and all error lifecycle events (mode toggle, deletion, paste, undo, tab switch) are handled consistently. - Fixes Comfy-Org/ComfyUI#13256 ## Behavioral notes - **Tab switch overlay suppression (intentional)**: Switching back to a workflow with missing assets no longer re-shows the error overlay. This reverses the behavior introduced in #10190. The error state is still restored silently in the errors tab — users can access it via the properties panel without being interrupted by the overlay on every tab switch. ## Changes ### 1. Scan filtering - `scanAllModelCandidates`, `scanAllMediaCandidates`, `scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS` - `collectMissingNodes` (serialized data): skip error reporting for muted/bypassed nodes while still calling `sanitizeNodeName` for safe `configure()` - `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes; workflow-level `graphData.models` only create candidates when active nodes exist - `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models when all referencing nodes are inactive ### 2. Realtime mode change handling - `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect `node:property:changed` (mode) - Deactivation (active → muted/bypassed): remove missing model/media/node errors for the node - Activation (muted/bypassed → active): scan the node and add confirmed errors, show overlay - Subgraph container deactivation: remove all interior node errors (execution ID prefix match) - Subgraph container activation: scan all active interior nodes recursively - Subgraph interior mode change: resolve node via `localGraph.getNodeById()` then compute execution ID from root graph ### 3. Node deletion - `graph.onNodeRemoved`: remove missing model/media/node errors for the deleted node - Handle `node.graph === null` at callback time by using `String(node.id)` for root-level nodes ### 4. Node paste/duplicate - `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after `node.configure()` restores widget values) - Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab switch handled by pipeline) - Guard: skip muted/bypassed nodes ### 5. Workflow tab switch optimization - `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab switch - Cache missing model/media/node state per workflow via `PendingWarnings` - `beforeLoadNewGraph`: save current store state to outgoing workflow's `pendingWarnings` - `showPendingWarnings`: restore cached errors silently (no overlay), always sync missing nodes store (even when null) - Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using `setMissingModels([])` instead of `clearMissingModels()` - `MissingModelRow.vue`: fetch file size on mount via `fetchModelMetadata` memory cache ### 6. Undo/redo overlay suppression - `silentAssetErrors` option propagated through pipeline → `surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option - `showPendingWarnings` `{ silent }` option for missing nodes overlay - `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo ### 7. Error tab node filtering - Selected node filters missing model/media card contents (not just group visibility) - `isAssetErrorInSelection`: resolve execution ID → graph node for selection matching - Missing nodes intentionally unfiltered (pack-level scope) - `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab visibility - Download All button: show only when 2+ downloadable models exist ### 8. New store functions - `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId` - `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId` - `missingNodesErrorStore`: `removeMissingNodesByNodeId` - `missingModelScan`: `scanNodeModelCandidates` (extracted single-node scan) - `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node scan) ### 9. Test infrastructure improvements - `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`) - Error-related TestIds moved from `dialogs` to `errorsTab` namespace in `selectors.ts` - Removed unused `TestIdValue` type - Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts` - Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab` - Added `aria-label` to pencil edit button and subgraph toggle button ## Test plan ### Unit tests (41 new) - Store functions: `addMissing*`, `removeMissing*ByNodeId` - `executionErrorStore`: `surfaceMissing*` silent option - Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`, `scanNodeMediaCandidates` - `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph` caching ### E2E tests (17 new in `errorsTabModeAware.spec.ts`) **Missing nodes** - [x] Deleting a missing node removes its error from the errors tab - [x] Undo after bypass restores error without showing overlay **Missing models** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Deleting a node with missing model removes its error - [x] Undo after bypass restores error without showing overlay - [x] Pasting a node with missing model increases referencing node count - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Missing media** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Subgraph** - [x] Bypassing a subgraph hides interior errors, un-bypassing restores them - [x] Bypassing a node inside a subgraph hides its error, un-bypassing restores it **Workflow switching** - [x] Does not resurface error overlay when switching back to workflow with missing nodes - [x] Restores missing nodes in errors tab when switching back to workflow # Screenshots https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5 ## Follow-up work - Extract error-detection computed properties from `RightSidePanel.vue` into a composable (e.g. `useErrorsTabVisibility`) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Terry Jia
|
63eab15c4f |
Range editor (#10936)
BE change https://github.com/Comfy-Org/ComfyUI/pull/13322 ## Summary Add RANGE widget for image levels adjustment - Add RangeEditor widget with three display modes: plain, gradient, and histogram - Support optional midpoint (gamma) control for non-linear midtone adjustment - Integrate histogram display from upstream node outputs ## Screenshots (if applicable) <img width="1450" height="715" alt="image" src="https://github.com/user-attachments/assets/864976af-9eb7-4dd0-9ce1-2f5d7f003117" /> <img width="1431" height="701" alt="image" src="https://github.com/user-attachments/assets/7ee2af65-f87a-407b-8bf2-6ec59a1dff59" /> <img width="705" height="822" alt="image" src="https://github.com/user-attachments/assets/7bcb8f17-795f-498a-9f8a-076ed6c05a98" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10936-Range-editor-33b6d73d365081089e8be040b40f6c8a) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
d9466947b2 |
feat: detect and resolve missing media inputs in error tab (#10309)
## Summary Add detection and resolution UI for missing image/video/audio inputs (LoadImage, LoadVideo, LoadAudio nodes) in the Errors tab, mirroring the existing missing model pipeline. ## Changes - **What**: New `src/platform/missingMedia/` module — scan pipeline detects missing media files on workflow load (sync for OSS, async for cloud), surfaces them in the error tab with upload dropzone, thumbnail library select, and 2-step confirm flow - **Detection**: `scanAllMediaCandidates()` checks combo widget values against options; cloud path defers to `verifyCloudMediaCandidates()` via `assetsStore.updateInputs()` - **UI**: `MissingMediaCard` groups by media type; `MissingMediaRow` shows node name (single) or filename+count (multiple), upload dropzone with drag & drop, `MissingMediaLibrarySelect` with image/video thumbnails - **Resolution**: Upload via `/upload/image` API or select from library → status card → checkmark confirm → widget value applied, item removed from error list - **Integration**: `executionErrorStore` aggregates into `hasAnyError`/`totalErrorCount`; `useNodeErrorFlagSync` flags nodes on canvas; `useErrorGroups` renders in error tab - **Shared**: Extract `ACCEPTED_IMAGE_TYPES`/`ACCEPTED_VIDEO_TYPES` to `src/utils/mediaUploadUtil.ts`; extract `resolveComboValues` to `src/utils/litegraphUtil.ts` (shared across missingMedia + missingModel scan) - **Reverse clearing**: Widget value changes on nodes auto-remove corresponding missing media errors (via `clearWidgetRelatedErrors`) ## Testing ### Unit tests (22 tests) - `missingMediaScan.test.ts` (12): groupCandidatesByName, groupCandidatesByMediaType (ordering, multi-name), verifyCloudMediaCandidates (missing/present, abort before/after updateInputs, already resolved true/false, no-pending skip, updateInputs spy) - `missingMediaStore.test.ts` (10): setMissingMedia, clearMissingMedia (full lifecycle with interaction state), missingMediaNodeIds, hasMissingMediaOnNode, removeMissingMediaByWidget (match/no-match/last-entry), createVerificationAbortController ### E2E tests (10 scenarios in `missingMedia.spec.ts`) - Detection: error overlay shown, Missing Inputs group in errors tab, correct row count, dropzone + library select visibility, no false positive for valid media - Upload flow: file picker → uploading status card → confirm → row removed - Library select: dropdown → selected status card → confirm → row removed - Cancel: pending selection → returns to upload/library UI - All resolved: Missing Inputs group disappears - Locate node: canvas pans to missing media node ## Review Focus - Cloud verification path: `verifyCloudMediaCandidates` compares widget value against `asset_hash` — implicit contract - 2-step confirm mirrors missing model pattern (`pendingSelection` → confirm/cancel) - Event propagation guard on dropzone (`@drop.prevent.stop`) to prevent canvas LoadImage node creation - `clearAllErrors()` intentionally does NOT clear missing media (same as missing models — preserves pending repairs) - `runMissingMediaPipeline` is now `async` and `await`-ed, matching model pipeline ## Test plan - [x] OSS: load workflow with LoadImage referencing non-existent file → error tab shows it - [x] Upload file via dropzone → status card shows "Uploaded" → confirm → widget updated, error removed - [x] Select from library with thumbnail preview → confirm → widget updated, error removed - [x] Cancel pending selection → returns to upload/library UI - [x] Load workflow with valid images → no false positives - [x] Click locate-node → canvas navigates to the node - [x] Multiple nodes referencing different missing files → correct row count - [x] Widget value change on node → missing media error auto-removed ## Screenshots https://github.com/user-attachments/assets/631c0cb0-9706-4db2-8615-f24a4c3fe27d |
||
|
Christian Byrne
|
62979e3818 |
refactor: rename firebaseAuthStore to authStore with shared test fixtures (#10483)
## Summary Rename `useFirebaseAuthStore` → `useAuthStore` and `FirebaseAuthStoreError` → `AuthStoreError`. Introduce shared mock factory (`authStoreMock.ts`) to replace 16 independent bespoke mocks. ## Changes - **What**: Mechanical rename of store, composable, class, and store ID (`firebaseAuth` → `auth`). Created `src/stores/__tests__/authStoreMock.ts` — a shared mock factory with reactive controls, used by all consuming test files. Migrated all 16 test files from ad-hoc mocks to the shared factory. - **Files**: 62 files changed (rename propagation + new test infra) ## Review Focus - Mock factory API design in `authStoreMock.ts` — covers all store properties with reactive `controls` for per-test customization - Self-test in `authStoreMock.test.ts` validates computed reactivity Fixes #8219 ## Stack This is PR 1/5 in a stacked refactoring series: 1. **→ This PR**: Rename + shared test fixtures 2. #10484: Extract auth-routing from workspaceApi 3. #10485: Auth token priority tests 4. #10486: Decompose MembersPanelContent 5. #10487: Consolidate SubscriptionTier type --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Benjamin Lu
|
4aae52c2fc |
test: move getNodeDefs spec into src/scripts (#10503)
## Summary Move the `getNodeDefs` unit test out of deprecated `tests-ui` and into `src/scripts` so Vitest discovers and runs it. ## Changes - **What**: Renamed `tests-ui/tests/scripts/app.getNodeDefs.test.ts` to `src/scripts/app.getNodeDefs.test.ts` ## Review Focus Confirm the spec now follows the colocated test convention and is included by the existing Vitest `include` globs. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10503-test-move-getNodeDefs-spec-into-src-scripts-32e6d73d3650816f9211dc4c20daba4b) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
23c22e4c52 |
🧙 feat: wire ComfyHub publish wizard with profile gate, asset upload, and submission (#10128)
## Summary 🎯 Wire the ComfyHub publish flow end-to-end: profile gate, multi-step wizard (describe, examples, finish), asset upload, and workflow submission via hub API. > *A wizard of steps, from describe to the end,* > *Upload your assets, your workflows you'll send!* > *With tags neatly slugged and thumbnails in place,* > *Your ComfyHub publish is ready to race!* 🏁 ## Changes 🔧 - 🌐 **Hub Service** — `comfyHubService` for profile CRUD, presigned asset uploads, and workflow publish - 📦 **Submission** — `useComfyHubPublishSubmission` orchestrates file uploads → publish in one flow - 🧙 **Wizard Steps** — Describe (name/description/tags) → Examples (drag-drop reorderable images) → Thumbnail → Finish (profile card + private-asset warnings) - 🖼️ **ReorderableExampleImage** — Drag-drop *and* keyboard reordering, accessible and fun - 🏷️ **Tag Normalization** — `normalizeTags` slugifies before publishing - 🔄 **Re-publish Prefill** — Fetches hub workflow metadata on re-publish, with in-memory cache fallback - 📐 **Schema Split** — Publish-record schema separated from hub-workflow-metadata schema - 🙈 **Unlisted Skip** — No hub-detail prefill fetch for unlisted records - 👤 **Profile Gate** — Username validation in `useComfyHubProfileGate` - 🧪 **Tests Galore** — New suites for DescribeStep, ExamplesStep, WizardContent, PublishSubmission, comfyHubService, normalizeTags, plus expanded PublishDialog & workflowShareService coverage ## Review Focus 🔍 > *Check the service, the schema, the Zod validation too,* > *The upload orchestration — does it carry things through?* > *The prefill fetch strategy: status → detail → cache,* > *And drag-drop reordering — is it keeping its place?* 🤔 - 🌐 `comfyHubService.ts` — API contract shape, error handling, Zod parsing - 📦 `useComfyHubPublishSubmission.ts` — Upload-then-publish flow, edge cases (no profile, no workflow) - 🗂️ `ComfyHubPublishDialog.vue` — Prefill fetch strategy (publish status → hub detail → cache) - 🖼️ `ReorderableExampleImage.vue` — Drag-drop + keyboard a11y ## Testing 🧪 ```bash pnpm test:unit -- src/platform/workflow/sharing/ pnpm typecheck ``` > *If the tests all turn green and the types all align,* > *Then merge it on in — this publish flow's fine!* ✅ --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Dante <bunggl@naver.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: bymyself <cbyrne@comfy.org> Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
66daa6d645 |
refactor: error system cleanup — store separation, DDD fix, test improvements (#10302)
## Summary Refactors the error system to improve separation of concerns, fix DDD layer violations, and address code quality issues. - Extract `missingNodesErrorStore` from `executionErrorStore`, removing the delegation pattern that coupled missing-node logic into the execution error store - Extract `useNodeErrorFlagSync` composable for node error flag reconciliation (previously inlined) - Extract `useErrorClearingHooks` composable with explicit callback cleanup on node removal - Extract `useErrorActions` composable to deduplicate telemetry+command patterns across error card components - Move `getCnrIdFromNode`/`getCnrIdFromProperties` to `platform/nodeReplacement` layer (DDD fix) - Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD alignment) - Add unmount cancellation guard to `useErrorReport` async `onMounted` - Return watch stop handle from `useNodeErrorFlagSync` - Add `asyncResolvedIds` eviction on `missingNodesError` reset - Add `console.warn` to silent catch blocks and empty array guard - Hoist `useCommandStore` to setup scope, fix floating promises - Add `data-testid` to error groups, image/video error spans, copy button - Update E2E tests to use scoped locators and testids - Add unit tests for `onNodeRemoved` restoration and double-install guard Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085 ## Test plan - [x] Existing unit tests pass with updated imports and mocks - [x] New unit tests for `useErrorClearingHooks` (callback restoration, double-install guard) - [x] E2E tests updated to use scoped locators and `data-testid` - [ ] Manual: verify error tab shows runtime errors and missing nodes correctly - [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons work in error cards ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Matt Miller
|
6a9fb4e1d5 |
fix: show clear error dialog for 403 whitelist failures (#10402)
## Summary - When the cloud backend returns a 403 (user not whitelisted), the frontend showed a generic "Prompt Execution Error" dialog with a cryptic message - Now catches 403 responses specifically and shows an "Access Restricted" dialog with the backend's actual error message - Adds `status` field to `PromptExecutionError` so error handlers can distinguish HTTP status codes ## Changes - `api.ts`: Added optional `status` to `PromptExecutionError`, pass `res.status` from `queuePrompt` - `app.ts`: New `else if` branch in the prompt error handler for `status === 403` — shows "Access Restricted" with the backend message ## Backwards compatible - **Old backend** (`"not authorized"`): Shows "Access Restricted: not authorized" - **New backend** ([cloud#2941](https://github.com/Comfy-Org/cloud/pull/2941), `"your account is not whitelisted for this feature"`): Shows "Access Restricted: your account is not whitelisted for this feature" - No behavior change for non-403 errors ## Related - Backend fix: Comfy-Org/cloud#2941 - Notion: COM-16179 ## Test plan - [ ] Submit a prompt as a non-whitelisted user → should see "Access Restricted" dialog with clear message - [ ] Submit a prompt as a whitelisted user → no change in behavior - [ ] Submit a prompt that fails for other reasons (missing nodes, etc.) → existing error handling unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10402-fix-show-clear-error-dialog-for-403-whitelist-failures-32c6d73d365081eb9528d7feac4e8681) by [Unito](https://www.unito.io) --------- Co-authored-by: Matt Miller <mattmiller@Matts-MacBook-Pro.local> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Alexander Brown
|
4d57c41fdb |
test: subgraph integration contracts and expanded Playwright coverage (#10123)
## Summary Add integration contract tests (unit) and expanded Playwright coverage for subgraph promotion, hydration, navigation, and lifecycle edge behaviors. ## Changes - **What**: 22 unit/integration tests across 9 files covering promotion store sync, widget view lifecycle, input link resolution, pseudo-widget cache, navigation viewport restore, and subgraph operations. 13 Playwright E2E tests covering proxyWidgets hydration stability, promoted source removal cleanup, pseudo-preview unpack/remove, multi-link representative round-trip, nested promotion retarget, and navigation state on workflow switch. - **Helpers**: Added `isPseudoPreviewEntry`, `getPseudoPreviewWidgets`, `getNonPreviewPromotedWidgets` to promotedWidgets helper. Added `SubgraphHelper.getNodeCount()`. ## Review Focus - Test-only PR — no production code changes - Validates existing subgraph behaviors are covered by regression tests before further feature work - Phase 4 (unit/integration contracts) and Phase 5 (Playwright expansion) of the subgraph test coverage plan ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10123-test-subgraph-integration-contracts-and-expanded-Playwright-coverage-3256d73d365081258023e3a763859e00) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Alexander Brown
|
2d44e9d2c0 |
fix: resync vue node layout store after legacy normalization (#10256)
## Summary Fixes a regression introduced in #9680 where groups and nodes could render in different coordinate spaces when loading legacy `workflowRendererVersion: "Vue"` workflows with Vue nodes mode enabled. - Add post-normalization sync to copy normalized LiteGraph node bounds into `layoutStore` during `loadGraphData` - Keep sync scoped to Vue nodes mode and only when normalization actually ran - Add unit tests for the new layout-store sync helper - Add Playwright regression coverage for legacy Vue workflow load path using `groups/nested-groups-1-inner-node`, asserting node/group centering-gap distances remain within baseline tolerances ## Testing - pnpm test:unit src/renderer/core/layout/sync/syncLayoutStoreFromGraph.test.ts - pnpm test:unit src/renderer/extensions/vueNodes/layout/ensureCorrectLayoutScale.test.ts - pnpm exec eslint src/renderer/core/layout/sync/syncLayoutStoreFromGraph.ts src/renderer/core/layout/sync/syncLayoutStoreFromGraph.test.ts - pnpm exec oxlint src/scripts/app.ts src/renderer/core/layout/sync/syncLayoutStoreFromGraph.ts src/renderer/core/layout/sync/syncLayoutStoreFromGraph.test.ts - pnpm typecheck - pnpm typecheck:browser - pnpm exec eslint browser_tests/tests/vueNodes/groups/groups.spec.ts - pnpm exec oxlint browser_tests/tests/vueNodes/groups/groups.spec.ts - pnpm exec playwright test browser_tests/tests/vueNodes/groups/groups.spec.ts --grep "legacy Vue workflows" ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10256-fix-resync-vue-node-layout-store-after-legacy-normalization-3276d73d365081568eebc6aa0827d943) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
AustinMroz
|
efa1b68c38 |
Allow graph navigation by browser forward/backward (#6811)
- On graph change, set the `graph.id` as location hash - On hash change, navigate to the target `graph.id` either in the current, or any other loaded workflow. `canvasStore.currentGraph` does not trigger when `app.loadGraphData` is called. A trigger could be forced here, but I'm concerned about side effects. Instead `updateHash` is manually called. Code search shows that there are no current custom nodes using `onhashchange` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6811-Allow-graph-navigation-by-browser-forward-backward-2b26d73d365081bb8414fdf7c3686124) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Terry Jia
|
2e5e04efd5 |
fix: enable 3D thumbnail support for cloud environments (#10121)
## Summary
The 3D thumbnail logic was gated behind `api.getServerFeature('assets',
false)` which only covers local servers. Use `isAssetPreviewSupported()`
to also cover cloud via `assetService.isAssetAPIEnabled()`.
follow up https://github.com/Comfy-Org/ComfyUI_frontend/pull/9471
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10121-fix-enable-3D-thumbnail-support-for-cloud-environments-3256d73d365081c18975e917c604582b)
by [Unito](https://www.unito.io)
|
||
|
Matt Miller
|
e85fc6390a |
fix: broken Firebase auth gate in API layer (#10115)
## Summary - `waitForAuthInitialization` in `api.ts` was silently passing through without actually waiting for Firebase auth - `authStore.isInitialized` is unwrapped by Pinia (plain boolean, not a ref), so `until()` received a static value - `until()` without `.toBe()` returns a builder object, not a promise — `Promise.race` treated it as immediately resolved - Fixed with `storeToRefs` to preserve the ref and `.toBe(true)` to return an actual promise ## Test plan - [ ] Verify cloud mode API calls wait for Firebase initialization before firing - [ ] Verify non-cloud mode is unaffected - [ ] Verify no 401s on initial page load in cloud mode 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10115-fix-broken-Firebase-auth-gate-in-API-layer-3256d73d365081e39b0df4d644f38c84) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
a321d66583 |
refactor: remove legacy missing nodes dialog (#10102)
## Summary - Remove the legacy missing nodes modal dialog and migrate all functionality to the existing Error Overlay / TabErrors system - Migrate core node version warning from `MissingCoreNodesMessage.vue` to `MissingNodeCard` in the errors tab - Remove `Comfy.Workflow.ShowMissingNodesWarning` setting (errors tab always surfaces missing nodes) - Delete 6 legacy files: `useMissingNodesDialog.ts`, `MissingNodesContent.vue`, `MissingNodesFooter.vue`, `MissingNodesHeader.vue`, `MissingCoreNodesMessage.vue` and its test - Rename `showMissingNodesDialog`/`showMissingModelsDialog` params to `showMissingNodes`/`showMissingModels` - Add `errorOverlay` and `missingNodeCard` to centralized `TestIds` - Migrate all E2E tests from legacy dialog selectors to error overlay testIds - Add new E2E test: MissingNodeCard visible via "See Errors" button flow - Add new E2E test: subgraph missing node type verified by expanding pack row - Add `surfaceMissingNodes` unit tests to `executionErrorStore` - Guard `semver.compare` against invalid version strings - Add `role="alert"`, `aria-hidden` for accessibility - Use reactive props destructuring in `MissingNodeCard` and `MissingPackGroupRow` **Net change: -669 lines** (19 files, +323 / -992) <img width="733" height="579" alt="image" src="https://github.com/user-attachments/assets/c497809d-b176-43bf-9872-34bd74c6ea0d" /> ## Test plan - [x] Unit tests: MissingNodeCard core node warning (7 tests) - [x] Unit tests: surfaceMissingNodes (4 tests) - [x] Unit tests: workflowService showPendingWarnings (updated) - [x] E2E: Error overlay visible on missing nodes workflow - [x] E2E: Error overlay visible on subgraph missing nodes - [x] E2E: MissingNodeCard visible via See Errors button - [x] E2E: Subgraph node type visible after expanding pack row - [x] E2E: Error overlay does not resurface on undo/redo - [x] E2E: Error overlay does not reappear on workflow tab switch - [x] Typecheck, lint, knip all passing ## Related issues - Closes #9923 (partially — `errorOverlay` and `missingNodeCard` added to TestIds) - References #10027 (mock hoisting inconsistency) - References #10033 (i18n-based test selectors) - References #10085 (DDD layer violation + focusedErrorNodeId) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10102-refactor-remove-legacy-missing-nodes-dialog-3256d73d365081c194d2e90bc6401846) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
e2ef041170 |
feat: surface missing models in Error Tab for OSS and remove legacy dialog (#9921)
## Summary - Surface missing models in the Error Tab for OSS environments, replacing the legacy modal dialog - Add Download button per model and Download All button in group header with file size display - Move download business logic from `components/dialog/content` to `platform/missingModel` - Remove legacy missing models dialog components and composable ## Changes - **Pipeline**: Remove `isCloud` guard from `scanAllModelCandidates` and `surfaceMissingModels` so OSS detects missing models - **Grouping**: Group non-asset-supported models by directory in OSS instead of lumping into UNSUPPORTED - **UI**: Add Download button (matching Install Node Pack design) and Download All header button - **Store**: Add `folderPaths`/`fileSizes` state with setter methods, race condition guard - **Cleanup**: Delete `MissingModelsContent`, `MissingModelsHeader`, `MissingModelsFooter`, `useMissingModelsDialog`, `missingModelsUtils` - **Tests**: Add OSS/Cloud grouping tests, migrate Playwright E2E to Error Tab, improve test isolation - **Snapshots**: Reset Playwright screenshot expectations since OSS missing model error detection now causes red highlights on affected nodes - **Accessibility**: Add `aria-label` with model name, `aria-expanded` on toggle, warning icon for unknown category ## Test plan - [x] Unit tests pass (86 tests) - [x] TypeScript typecheck passes - [x] knip passes - [x] Load workflow with missing models in OSS → Error Tab shows missing models grouped by directory - [x] Download button triggers browser download with correct URL - [x] Download All button downloads all downloadable models - [x] Cloud environment behavior unchanged - [x] Playwright E2E: `pnpm test:browser:local -- --grep "Missing models in Error Tab"` ## Screenshots https://github.com/user-attachments/assets/12f15e09-215a-4c58-87ed-39bbffd1359c ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9921-feat-surface-missing-models-in-Error-Tab-for-OSS-and-remove-legacy-dialog-3236d73d365081f0a9dfc291978f5ecf) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
woctordho
|
82556f02a9 |
fix: respect 'always snap to grid' when auto-scale layout from nodes 1.0 to 2.0 (#9332)
## Summary Previously when I switch from nodes 1.0 to 2.0, positions and sizes of nodes do not follow 'always snap to grid'. You can guess what a mess it is for people relying on snap to grid to retain sanity. This PR fixes it. ## Changes In `ensureCorrectLayoutScale`, we call `snapPoint` after the position and the size are updated. We also need to ensure that the snapped size is larger than the minimal size required by the content, so I've added 'ceil' mode to `snapPoint`, and the patch is larger than I thought first. I'd happily try out nodes 2.0 once this is addressed :) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9332-fix-respect-always-snap-to-grid-when-auto-scale-layout-from-nodes-1-0-to-2-0-3176d73d365081f5b6bcc035a8ffa648) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
1280d4110d |
fix: simplify ensureCorrectLayoutScale and fix link sync during Vue node drag (#9680)
## Summary Fix node layout drift from repeated `ensureCorrectLayoutScale` scaling, simplify it to a pure one-time normalizer, and fix links not following Vue nodes during drag. ## Changes - **What**: - `ensureCorrectLayoutScale` simplified to a one-time normalizer: unprojects legacy Vue-scaled coordinates back to canonical LiteGraph coordinates, marks the graph as corrected, and does nothing else. No longer touches the layout store, syncs reroutes, or changes canvas scale. - Removed no-op calls from `useVueNodeLifecycle.ts` (a renderer version string was passed where an `LGraph` was expected). - `layoutStore.finalizeOperation` now calls `notifyChange` synchronously instead of via `setTimeout`. This ensures `useLayoutSync`'s `onChange` callback pushes positions to LiteGraph `node.pos` and calls `canvas.setDirty()` within the same RAF frame as a drag update, fixing links not following Vue nodes during drag. - **Tests**: Added tests for `ensureCorrectLayoutScale` (idempotency, round-trip, unknown-renderer no-op) and `graphRenderTransform` (project/unproject round-trips, anchor caching). ## Review Focus - The `setTimeout(() => this.notifyChange(change), 0)` → `this.notifyChange(change)` change in `layoutStore.ts` is the key fix for the drag-link-sync bug. The listener (`useLayoutSync`) only writes to LiteGraph, not back to the layout store, so synchronous notification is safe. - `ensureCorrectLayoutScale` no longer has any side effects beyond normalizing coordinates and setting `workflowRendererVersion` metadata. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com> Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: Hunter <huntcsg@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
de866a15d2 |
Add prompt_id support to progress_text WS messages (#9002)
## Summary Add frontend support for `prompt_id` in `progress_text` binary WS messages, enabling parallel workflow execution to route progress text to the correct active prompt. Backend PR: https://github.com/Comfy-Org/ComfyUI/pull/12540 ## Changes - Advertise `supports_progress_text_metadata` in client feature flags - Decode `prompt_id` from new binary format when flag is active - Add optional `prompt_id` to `zProgressTextWsMessage` schema - Filter `progress_text` events by `activePromptId` — skip messages for non-active prompts ## Deployment Can be deployed independently in any order — feature flag negotiation ensures graceful degradation. Part of COM-12671 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9002-Add-prompt_id-support-to-progress_text-WS-messages-30d6d73d365081acabbcdac939e0c751) by [Unito](https://www.unito.io) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Progress text updates can include optional metadata so richer context is available. * **Bug Fixes / Improvements** * Progress updates are now filtered to show only the currently active prompt, reducing cross-talk from concurrent operations and improving update accuracy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
2f7f3c4e56 |
[feat] Surface missing models in Errors tab (Cloud) (#9743)
## Summary When a workflow is loaded with missing models, users currently have no way to identify or resolve them from within the UI. This PR adds a full missing-model detection and resolution pipeline that surfaces missing models in the Errors tab, allowing users to install or import them without leaving the editor. ## Changes ### Missing Model Detection - Scan all COMBO widgets across root graph and subgraphs for model-like filenames during workflow load - Enrich candidates with embedded workflow metadata (url, hash, directory) when available - Verify asset-supported candidates against the asset store asynchronously to confirm installation status - Propagate missing model state to `executionErrorStore` alongside existing node/prompt errors ### Errors Tab UI — Model Resolution - Group missing models by directory (e.g. `checkpoints`, `loras`, `vae`) with collapsible category cards - Each model row displays: - Model name with copy-to-clipboard button - Expandable list of referencing nodes with locate-on-canvas button - **Library selector**: Pick an alternative from the user's existing models to substitute the missing model with one click - **URL import**: Paste a Civitai or HuggingFace URL to import a model directly; debounced metadata fetch shows filename and file size before confirming; type-mismatch warnings (e.g. importing a LoRA into checkpoints directory) are surfaced with an "Import Anyway" option - **Upgrade prompt**: In cloud environment, free-tier subscribers are shown an upgrade modal when attempting URL import - Separate "Import Not Supported" section for custom-node models that cannot be auto-resolved - Status card with live download progress, completion, failure, and category-mismatch states ### Canvas Integration - Highlight nodes and widgets that reference missing models with error indicators - Propagate missing-model badges through subgraph containers so issues are visible at every graph level ### Code Cleanup - Simplify `surfacePendingWarnings` in workflowService, remove stale widget-detected model merging logic - Add `flattenWorkflowNodes` utility to workflowSchema for traversing nested subgraph structures - Extract `MissingModelUrlInput`, `MissingModelLibrarySelect`, `MissingModelStatusCard` as focused single-responsibility components ## Testing - Unit tests for scan pipeline (`missingModelScan.test.ts`): enrichment, skip-installed, subgraph flattening - Unit tests for store (`missingModelStore.test.ts`): state management, removal helpers - Unit tests for interactions (`useMissingModelInteractions.test.ts`): combo select, URL input, import flow, library confirm - Component tests for `MissingModelCard` and error grouping (`useErrorGroups.test.ts`) - Updated `workflowService.test.ts` and `workflowSchema.test.ts` for new logic ## Review Focus - Missing model scan + enrichment pipeline in `missingModelScan.ts` - Interaction composable `useMissingModelInteractions.ts` — URL metadata fetch, library install, upload fallback - Store integration and canvas-level error propagation ## Screenshots https://github.com/user-attachments/assets/339a6d5b-93a3-43cd-98dd-0fb00681b66f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9743-feat-Surface-missing-models-in-Errors-tab-Cloud-3206d73d365081678326d3a16c2165d8) by [Unito](https://www.unito.io) |
||
|
Jin Yi
|
240b54419b |
fix: load API format workflows with missing node types (#9694)
## Summary `loadApiJson` early-returns when missing node types are detected, preventing the entire API-format workflow from loading onto the canvas. ## Changes - **What**: Remove early `return` in `loadApiJson` so missing nodes are skipped while the rest of the workflow loads normally, consistent with how `loadGraphData` handles missing nodes in standard workflow format. ## Review Focus The existing code already handles missing nodes gracefully: - `LiteGraph.createNode()` returns `null` for unregistered types - `if (!node) continue` skips missing nodes during graph construction - `if (!fromNode) continue` skips connections to missing nodes - `if (!node) return` skips input processing for missing nodes The early `return` was unnecessarily preventing the entire load. The warning modal is still shown via `showMissingNodesError`. ## Test workflow & screen recording [04wan2.2smoothmix图生视频 (3).json](https://github.com/user-attachments/files/25858354/04wan2.2smoothmix.3.json) [screen-capture.webm](https://github.com/user-attachments/assets/9c396f80-fff1-4d17-882c-35ada86542c1) |
||
|
jaeone94
|
370003da94 |
fix: add isGraphReady guard to prevent premature graph access error logs (#9672)
## Summary Adds `isGraphReady` getter to `ComfyApp` and uses it in `executionErrorStore` guards to prevent false 'ComfyApp graph accessed before initialization' error logs during early store evaluation. ## Changes - **What**: Added `isGraphReady` boolean getter to `ComfyApp` that safely checks graph initialization without triggering the `rootGraph` getter's error log. Updated 5 guard sites in `executionErrorStore` to use `app.isGraphReady` instead of `app.rootGraph`. - **Why**: The `rootGraph` getter logs an error when accessed before initialization. Computed properties and watch callbacks in `executionErrorStore` are evaluated early (before graph init), causing false error noise in the console. ## Review Focus - `isGraphReady` is intentionally minimal — just `!!this.rootGraphInternal` — to avoid duplicating the error-logging behavior of `rootGraph` - The `watch(lastNodeErrors, ...)` callback now checks `isGraphReady` at the top and early-returns, consistent with the computed property pattern ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9672-fix-add-isGraphReady-guard-to-prevent-premature-graph-access-error-logs-31e6d73d365081be8e1fc77114ce9382) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Christian Byrne
|
725a0a2b89 |
fix: remove timeouts from error toasts so they persist until dismissed (#9543)
## Summary
Remove `life` (timeout) property from all error-severity toast calls so
they persist until manually dismissed, preventing users from missing
important error messages.
## Changes
- **What**: Removed `life` property from 86 error toast calls across 46
files. Error toasts now use PrimeVue's default behavior (no
auto-dismiss). Non-error toasts (success, warn, info) are unchanged.
- Also fixed a pre-existing lint issue in `TaskListPanel.vue` (`import {
t } from '@/i18n'` → `useI18n()`)
## Review Focus
- One conditional toast in `useMediaAssetActions.ts` intentionally keeps
`life` because its severity alternates between `warn` and `error`
Fixes
https://www.notion.so/comfy-org/Implement-Remove-timeouts-for-all-error-toasts-31b6d73d365081cead54fddc77ae7c3d
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9543-fix-remove-timeouts-from-error-toasts-so-they-persist-until-dismissed-31c6d73d365081fa8d30f6366e9bfe38)
by [Unito](https://www.unito.io)
|
||
|
pythongosssss
|
ec129de63d |
fix: Prevent corruption of workflow data due to checkState during graph loading (#9531)
## Summary
During workflow loading, the workflow data & active workflow object can
be out of sync, meaning any checkState calls will overwrite data into
the wrong workflow.
Recreation steps:
* Open 2-3 workflows
* Enter builder mode > select step
* Select some different inputs on each
* Quickly tap the shift key (this triggers checkState) while switching
tabs
* After a while, you'll see the wrong inputs on the workflows
Alternatively, register an extension that guarantees to call checkState
during the bad phase, run this in browser devtools and switch tabs:
```
window.app.registerExtension({
name: 'bad',
async afterConfigureGraph() {
window.app.extensionManager.workflow.activeWorkflow.changeTracker.checkState()
}
})
```
## Changes
- **What**:
- Add loading graph flag
- Prevent checkState calls while loading
- Prevent app mode data sync while loading
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9531-fix-Prevent-corruption-of-workflow-data-due-to-checkState-during-graph-loading-31c6d73d365081e2ab91d9145bf1d025)
by [Unito](https://www.unito.io)
|
||
|
pythongosssss
|
4ff14b5eb9 |
feat/fix: App mode QA updates (#9439)
## Summary Various fixes from app mode QA ## Changes - **What**: - fix: prevent inserting nodes from workflow/apps sidebar tabs - fix: hide json extension in workflow tab - fix: hide apps nav button in apps tab when already in apps mode - fix: center text on arrange page - fix: prevent IoItems from "jumping" due to stale transform after drag and drop op - fix: refactor side panels and add custom stable pixel based sizing - fix: make outputs/inputs lists in app builder scrollable - fix: fix rerun not working correctly - feat: add text to interrupt button - feat: add enter app mode button to builder toolbar - feat: add tooltip to download button on linear view - feat: show last output of workflow in arrange tab if available - feat: show download count in download all button, hide if only 1 asset to download ## Review Focus - Rerun - I am not sure why it was triggering widget actions, removing it seemed like the correct fix - useStablePrimeVueSplitter - this is a workaround for the fact it uses percent sizing, I also tried switching to reka-ui splitters, but they also only support % sizing in our version [pixel based looks to have been added in a newer version, will log an issue to upgrade & replace splitters with this] ## Screenshots (if applicable) <img width="1314" height="1129" alt="image" src="https://github.com/user-attachments/assets/c430f9d6-7c29-4853-803e-5b6fe7086fca" /> <img width="511" height="283" alt="image" src="https://github.com/user-attachments/assets/b7e594d4-70a1-41e3-8ba1-78512f2a5c8b" /> <img width="254" height="232" alt="image" src="https://github.com/user-attachments/assets/1d146399-39ea-4b0e-928c-340b74957535" /> <img width="487" height="198" alt="image" src="https://github.com/user-attachments/assets/e2ba7f5d-8ff5-47f4-9526-61ebb99514b8" /> <img width="378" height="647" alt="image" src="https://github.com/user-attachments/assets/a47a3054-9320-4327-bdc0-b0a16e19f83d" /> <img width="1016" height="476" alt="image" src="https://github.com/user-attachments/assets/479ae50e-d380-4d56-a5c9-5df142b14ed0" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9439-feat-fix-App-mode-QA-updates-31a6d73d365081b38337d63207b88817) by [Unito](https://www.unito.io) |
||
|
Johnpaul Chiwetelu
|
5e17bbbf85 |
feat: expose litegraph internal keybindings (#9459)
## Summary Migrate hardcoded litegraph canvas keybindings (Ctrl+A/C/V, Delete, Backspace) into the customizable keybinding system so users can remap them via Settings > Keybindings. ## Changes - **What**: Register Ctrl+A (SelectAll), Ctrl+C (CopySelected), Ctrl+V (PasteFromClipboard), Ctrl+Shift+V (PasteFromClipboardWithConnect), Delete/Backspace (DeleteSelectedItems) as core keybindings in `defaults.ts`. Add new `PasteFromClipboardWithConnect` command. Remove hardcoded handling from litegraph `processKey()`, the `app.ts` Ctrl+C/V monkey-patch, and the `keybindingService` canvas forwarding logic. Fixes #1082 Fixes #2015 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9459-feat-expose-litegraph-internal-keybindings-31b6d73d3650819a8499fd96c8a6678f) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
a0abe3e36f |
chore: add deprecation warning for legacy queue/history menu (#9460)
## Summary Add console.warn deprecation notice when the legacy ComfyList queue/history menu is instantiated. ## Changes - **What**: Log a deprecation warning in the `ComfyList` constructor telling users the legacy menu is deprecated, may break, and won't receive support. Includes instructions to switch via Settings → "Use new menu" → "Top". ## Review Focus Wording of the user-facing console warning. Fixes #8100 (Phase 1) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9460-chore-add-deprecation-warning-for-legacy-queue-history-menu-31b6d73d365081ffa041cad33e8cd9a7) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
1bac5d9bdd |
feat: workflow sharing and ComfyHub publish flow (#8951)
## Summary Add workflow sharing by URL and a multi-step ComfyHub publish wizard, gated by feature flags and an optional profile gate. ## Changes - **What**: Share dialog with URL generation and asset warnings; ComfyHub publish wizard (Describe → Examples → Finish) with thumbnail upload and tags; profile gate flow; shared workflow URL loader with confirmation dialog - **Dependencies**: None (new `sharing/` module under `src/platform/workflow/`) ## Review Focus - Three new feature flags: `workflow_sharing_enabled`, `comfyhub_upload_enabled`, `comfyhub_profile_gate_enabled` - Share service API contract and stale-share detection (`workflowShareService.ts`) - Publish wizard and profile gate state management - Shared workflow URL loading and query-param preservation ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8951-feat-share-workflow-by-URL-30b6d73d3650813ebbfafdad775bfb33) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
e7588c33e1 |
refactor: rename imagePreviewStore to nodeOutputStore (#9416)
## Summary Rename `imagePreviewStore.ts` → `nodeOutputStore.ts` to match the store it houses (`useNodeOutputStore`, Pinia ID `nodeOutput`). ## Changes - **What**: Rename file + test file, update all 21 import paths, mock paths, and describe labels - **Breaking**: None — exported symbol (`useNodeOutputStore`) and Pinia store ID (`nodeOutput`) are unchanged ## Custom Node Ecosystem Audit Searched the ComfyUI custom node ecosystem for `imagePreviewStore` and `useNodeOutputStore`: - **Not part of the public API** — neither filename nor export appear in `comfyui_frontend_package` or `vite.types.config.mts` - **1 external repo found:** `wallen0322/ComfyUI-AE-Animation` — contains a full fork of the frontend source tree; it copies the file internally and does not import from the published package. **No breakage.** - **No custom nodes import this store via the extension API.** This is a safe internal-only rename. ## Review Focus Pure mechanical rename — no logic changes. Verify no stale `imagePreviewStore` references remain. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9416-refactor-rename-imagePreviewStore-to-nodeOutputStore-31a6d73d3650816086c5e62959861ddb) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
5376b7ed1e |
feat: App mode empty graph handling (#9393)
## Summary Adds handling for entering app mode with an empty graph prompting the user to load a template as a starting point ## Changes - **What**: - app mode handle empty workflows, disable builder button, show different message - fix fitView when switching from app mode to graph ## Review Focus Moving the fitView since the canvas is hidden in app mode until after the workflow is loaded and the mode has been switched back to graph, I don't see how this could cause any issues but worth a closer eye ## Screenshots (if applicable) <img width="1057" height="916" alt="image" src="https://github.com/user-attachments/assets/2ffe2b6d-9ce1-4218-828a-b7bc336c365a" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9393-feat-App-mode-empty-graph-handling-3196d73d3650812cab0ce878109ed5c9) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
6dc4a3ed1e |
refactor(changeTracker): use shared clone util and centralize nodeOutputs snapshot (#9387)
## Summary Follow-up to #9380. Replaces local `clone()` with shared util and centralizes output snapshotting. ## Changes - **What**: Replaced local `JSON.parse(JSON.stringify)` clone in `changeTracker.ts` with shared `clone()` from `@/scripts/utils` (prefers `structuredClone` with JSON fallback). Added `snapshotOutputs()` to `useNodeOutputStore` as symmetric counterpart to existing `restoreOutputs()`, and wired `changeTracker.store()` to use it. - **Breaking**: None ## Review Focus Symmetry between `snapshotOutputs()` and `restoreOutputs()` in the node output store. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9387-refactor-changeTracker-use-shared-clone-util-and-centralize-nodeOutputs-snapshot-3196d73d365081a289c3cb414f57929e) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
1e86e8c4d5 |
[Bug] Node preview images are lost when switching between multiple workflow tabs (#9380)
## Summary When working with multiple workflow tabs, the internal preview (image thumbnail) of nodes like Load Image disappears after navigating away from and back to a tab. This affects all active tabs once the switch occurs. ## Screenshot before https://github.com/user-attachments/assets/99466123-37db-406f-9e17-0a9ff22311c3 after https://github.com/user-attachments/assets/bdad0ef1-72b7-46c7-aa61-0a557688e55e --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
0d7dc15916 |
App mode output feed to only show current session results for outputs defined in the app (#9307)
## Summary Updates app mode to only show images from: - the workflow that generated the image - in the current session - for the outputs selected in the builder ## Changes - **What**: - adds new mapping of jobid -> workflow path [cant use id here as it is not guaranteed unique], capped at 4k entries - fix bug where executing a workflow then quickly switching tabs associated incorrect workflow - add missing output history tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9307-App-mode-output-feed-to-only-show-current-session-results-for-outputs-defined-in-the-app-3156d73d36508142b4bbca3f938fc5c2) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
a0e518aa98 |
refactor(node-replacement): reorganize domain components and expand comprehensive test suite (#9301)
## Summary Resolves six open issues by reorganizing node replacement components into a domain-driven folder structure, refactoring event handling to follow the emit pattern, and adding comprehensive test coverage across all affected modules. ## Changes - **What**: - Moved `SwapNodeGroupRow.vue` and `SwapNodesCard.vue` from `src/components/rightSidePanel/errors/` to `src/platform/nodeReplacement/components/` (Issues #9255) - Moved `useMissingNodeScan.ts` from `src/composables/` to `src/platform/nodeReplacement/missingNodeScan.ts`, renamed to reflect it is a plain function not a Vue composable (Issues #9254) - Refactored `SwapNodeGroupRow.vue` to emit a `'replace'` event instead of calling `useNodeReplacement()` and `useExecutionErrorStore()` directly; replacement logic now handled in `TabErrors.vue` (Issue #9267) - Added unit tests for `removeMissingNodesByType` (`executionErrorStore.test.ts`), `scanMissingNodes` (`missingNodeScan.test.ts`), and `swapNodeGroups` computed (`swapNodeGroups.test.ts`, `useErrorGroups.test.ts`) (Issue #9270) - Added placeholder detection tests covering unregistered-type detection when `has_errors` is false, and exclusion of registered types (`useNodeReplacement.test.ts`) (Issue #9271) - Added component tests for `MissingNodeCard` and `MissingPackGroupRow` covering rendering, expand/collapse, events, install states, and edge cases (Issue #9231) - Added component tests for `SwapNodeGroupRow` and `SwapNodesCard` (Issues #9255, #9267) ## Additional Changes (Post-Review) - **Edge case guard in placeholder detection** (`useNodeReplacement.ts`): When `last_serialization.type` is absent (old serialization format), the predicate falls back to `n.type`, which the app may have already run through `sanitizeNodeName` — stripping HTML special characters (`& < > " ' \` =`). In that case, a `Set.has()` lookup against the original unsanitized type name would silently miss, causing replacement to be skipped. Fixed by including sanitized variants of each target type in the `targetTypes` Set at construction time. For the overwhelmingly common case (no special characters in type names), the Set deduplicates the entries and runtime behavior is identical to before. A regression test was added to cover the specific scenario: `last_serialization.type` absent + live `n.type` already sanitized. ## Review Focus - `TabErrors.vue`: confirm the new `@replace` event handler correctly replaces nodes and removes them from missing nodes list (mirrors the old inline logic in `SwapNodeGroupRow`) - `missingNodeScan.ts`: filename/export name change from `useMissingNodeScan` — verify all call sites updated via `app.ts` - Test mocking strategy: module-level `vi.mock()` factories use closures over `ref`/plain objects to allow per-test overrides without global mutable state - Fixes #9231 - Fixes #9254 - Fixes #9255 - Fixes #9267 - Fixes #9270 - Fixes #9271 |
||
|
pythongosssss
|
f83daa6f3b |
App mode - discard slow preview messages to prevent overwriting output image (#9261)
## Summary Prevent latent previews received after the job/node has already finished processing overwriting the actual output display ## Changes - **What**: - updates job preview store to also track which node the preview was for - updates linear progress tracking to store executed nodes enabling skipping previews of these ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9261-App-mode-discard-slow-preview-messages-to-prevent-overwriting-output-image-3136d73d3650817884c2ce2ff5993b9e) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
1c3984a178 |
feat: add node replacement UI to Errors Tab (#9253)
## Summary Adds a node replacement UI to the Errors Tab so users can swap missing nodes with compatible alternatives directly from the error panel, without opening a separate dialog. ## Changes - **What**: New `SwapNodesCard` and `SwapNodeGroupRow` components render swap groups in the Errors Tab; each group shows the missing node type, its instances (with locate buttons), and a Replace button. Added `useMissingNodeScan` composable to scan the graph for missing nodes and populate `executionErrorStore`. Added `removeMissingNodesByType()` to `executionErrorStore` so replaced nodes are pruned from the error list reactively. ## Bug Fixes Found During Implementation ### Bug 1: Replaced nodes render as empty shells until page refresh `replaceWithMapping()` directly mutates `_nodes[idx]`, bypassing the Vue rendering pipeline entirely. Because the replacement node reuses the same ID, `vueNodeData` retains the stale entry from the old placeholder (`hasErrors: true`, empty widgets/inputs). `graph.setDirtyCanvas()` only repaints the LiteGraph canvas and has no effect on Vue. **Fix**: After `replaceWithMapping()`, manually call `nodeGraph.onNodeAdded?.(newNode)` to trigger `handleNodeAdded` in `useGraphNodeManager`, which runs `extractVueNodeData(newNode)` and updates `vueNodeData` correctly. Also added a guard in `handleNodeAdded` to skip `layoutStore.createNode()` when a layout for the same ID already exists, preventing a duplicate `spatialIndex.insert()`. ### Bug 2: Missing node error list overwritten by incomplete server response Two compounding issues: (A) the server's `missing_node_type` error only reports the *first* missing node — the old handler parsed this and called `surfaceMissingNodes([singleNode])`, overwriting the full list collected at load time. (B) `queuePrompt()` calls `clearAllErrors()` before the API request; if the subsequent rescan used the stale `has_errors` flag and found nothing, the missing nodes were permanently lost. **Fix**: Created `useMissingNodeScan.ts` which scans `LiteGraph.registered_node_types` directly (not `has_errors`). The `missing_node_type` catch block in `app.ts` now calls `rescanAndSurfaceMissingNodes(this.rootGraph)` instead of parsing the server's partial response. ## Review Focus - `handleReplaceNode` removes the group from the store only when `replaceNodesInPlace` returns at least one replaced node — should we always clear, or only on full success? - `useMissingNodeScan` re-scans on every execution-error change; confirm no performance concerns for large graphs with many subgraphs. ## Screenshots https://github.com/user-attachments/assets/78310fc4-0424-4920-b369-cef60a123d50 https://github.com/user-attachments/assets/3d2fd5e1-5e85-4c20-86aa-8bf920e86987 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9253-feat-add-node-replacement-UI-to-Errors-Tab-3136d73d365081718d4ddfd628cb4449) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
6a08e4ddde |
Revert "fix: sync DOM widget values to widgetValueStore on registration" (#9205)
Reverts Comfy-Org/ComfyUI_frontend#9166 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9205-Revert-fix-sync-DOM-widget-values-to-widgetValueStore-on-registration-3126d73d365081df8944d3c6508d2372) by [Unito](https://www.unito.io) |