1134 Commits

Author	SHA1	Message	Date
AustinMroz	e97c4b6ab9	Remove flake screenshot (#12529)	2026-05-29 21:08:46 +00:00
jaeone94	fb58a76a53	fix: preserve validation errors on execution start (#12493) ... ## Summary Preserve validation node errors and their overlay when a valid active root starts execution, so partial workflow runs no longer hide validation failures. ## Changes - **What**: Split execution-start clearing from full error clearing; `execution_start` now clears transient execution/prompt state without clearing validation `lastNodeErrors`. - **What**: Keep the ErrorOverlay open when validation errors are still present, and show it for successful prompt responses that include `node_errors`. - **Dependencies**: None. ## Review Focus Please check the error-clearing boundary between prompt submission/workflow changes and WebSocket `execution_start`. Full clearing still happens through `clearAllErrors`; execution start now uses the narrower clearing path and only dismisses the overlay when there are no validation node errors to show. Linear: FE-851 ## Red-Green Verification - Red: `76bcf34c4 test: add failing validation error preservation e2e` - Green: `9766172ea fix: preserve validation errors on execution start` - Follow-up: `321c95aba fix: keep validation error overlay during execution start` - Coverage: `7b5fab577 test: cover prompt node error overlay` ## Test Plan - `pnpm exec vitest run src/scripts/app.test.ts` - `pnpm exec vitest run src/stores/executionStore.test.ts` - `pnpm exec vitest run src/scripts/app.test.ts src/stores/executionStore.test.ts --coverage` - `pnpm format:check -- src/stores/executionErrorStore.ts src/stores/executionStore.ts src/stores/executionStore.test.ts src/scripts/app.ts src/scripts/app.test.ts browser_tests/fixtures/helpers/ExecutionHelper.ts browser_tests/tests/execution.spec.ts` - `pnpm exec oxlint src/stores/executionErrorStore.ts src/stores/executionStore.ts src/stores/executionStore.test.ts src/scripts/app.ts src/scripts/app.test.ts browser_tests/tests/execution.spec.ts --type-aware` - `pnpm typecheck` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://127.0.0.1:5175 pnpm exec playwright test browser_tests/tests/execution.spec.ts:132` ## Screenshots (Before/After) Before https://github.com/user-attachments/assets/04a212b6-66f9-4c77-9056-58bdc642d96e After https://github.com/user-attachments/assets/db7813c7-bf8a-4e19-9b66-7f49fd01c305	2026-05-29 17:20:54 +00:00
AustinMroz	b7990f7645	Fix ghost links on IO remove slot (#12473) ... Context menu operations on subgraph IO slots only set the foreground canvas as dirty, so links would visually persist until a different operation caused a background draw.	2026-05-29 16:57:52 +00:00
Dante	c57944f315	fix: hide duplicate LiteGraph Resize/Collapse/Expand entries from Vue node menu (FE-867) (#12487) ... ## Summary https://linear.app/comfyorg/issue/FE-867/bug-node-expand-menu-doesnt-work-nodes-immediately-collapse-after Recreates #12175 on a fresh `main` base (original branch's CI failed only because its `frontend-dist` artifact had expired — not a code issue). Original work by @christian-byrne / Glary-Bot, cherry-picked here so it can land while he's offline. The Vue right-click "More Options" node menu shows duplicates for collapse/expand functionality: - **Vue source**: `Minimize Node` / `Expand Node` (works) - **LiteGraph source**: `Resize`, `Collapse`, `Expand` (silently no-op in this menu — the converter wrapper invokes `LGraphCanvas.onMenuNodeCollapse` without the `node` arg it expects) Suppress the LiteGraph duplicates in `convertContextMenuToOptions` by matching the built-in **callback identity** (`LGraphCanvas.onMenuResizeNode`, `LGraphCanvas.onMenuNodeCollapse`), not the raw label. Matching by identity avoids accidentally hiding extension-provided items that share those labels. Also align `CORE_MENU_ITEMS` / `MENU_ORDER` on the Vue label `Expand Node` so the toggled Minimize/Expand pair sorts correctly. ## Scope of suppression Only the Vue node menu (via `convertContextMenuToOptions`) is affected. The raw `LGraphCanvas.getNodeMenuOptions` output is untouched, so: - The legacy right-click menu (`Comfy.UseNewMenu` disabled) still has `Collapse` / `Resize`. - `useLoad3d.ts`, which calls `new LiteGraph.ContextMenu(app.canvas.getNodeMenuOptions(node), ...)`, is unaffected. - Extensions that monkey-patch `getNodeMenuOptions` continue to receive the full option list. ## Tests - `contextMenuConverter.test.ts`: covers both that built-in entries are dropped by identity AND that extension-provided items with the same labels survive. - E2E `selectionToolboxMoreActions.spec.ts`: asserts the Vue "More Options" menu shows `Minimize Node` but no `Resize`/`Collapse`/`Expand`. - `pnpm typecheck` clean. Supersedes #12175. --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>	2026-05-29 22:28:08 +09:00
jaeone94	d86483a6af	refactor: consolidate middle-button pan handling (#12491) ... ## Summary Refactors middle mouse button pan handling around the intent of #11409, dropping the outdated implementation details from that PR and aligning the core behavior with the current main branch. ## Changes - **What**: Centralized phase-specific middle mouse button handling in `src/base/pointerUtils.ts`, added a shared Vue widget forwarding helper, and updated canvas, LiteGraph, Vue node, and mask editor call sites to use the same semantics. - **Breaking**: None expected. This keeps existing middle-click pan behavior while making pointerdown, pointermove, pointerup, and auxclick checks explicit for their event phases. - **Dependencies**: None. ## Review Focus This PR is intentionally narrower than #11409. That PR had the right goal, but its implementation became outdated against main: mask editor tests now have helper coverage on main, Vue node/widget code has shifted, and a blanket replacement with `isMiddlePointerInput` would lose the bitmask behavior needed during pointermove drags. The core difference is that this PR preserves the useful part of #11409, namely removing scattered ad-hoc MMB checks, while avoiding stale changes that no longer fit the current codebase. Key behavior changes: - `isMiddlePointerInput` is the conservative pointerdown-style check: changed middle button or strict middle-only `buttons === 4`. - `isMiddleButtonHeld` handles pointermove-style held-button bitmasks so chorded drags with the middle button still pan. - `isMiddleButtonEvent` handles pointerup/auxclick-style changed-button events. - Call sites now choose the phase-specific helper directly instead of routing through an event-type dispatcher. - String and markdown widgets now share `forwardMiddleButtonToCanvas(...)` instead of duplicating three pointer listeners each. - The widget helper intentionally keeps the existing `app.canvas.processMouseDown/Move/Up` forwarding route and only centralizes the duplicated listener logic. - Mask editor pan handling, Vue node pointer forwarding, graph canvas pan forwarding, LiteGraph middle-click checks, input indicators, and transform settling now use the centralized helpers. Coverage added or updated: - Unit coverage for middle-button helper semantics, including chorded pointermove drags and pointercancel held-bit behavior. - Unit coverage for widget forwarding helper down/move/up routing. - Regression coverage for canvas, mask editor, Vue node media preview, and transform-settling pointer handling. - Browser coverage for middle-click drag panning on a Vue node, a multiline string widget, and the mask editor canvas. Validation run: - `pnpm format` - `pnpm lint` - `pnpm typecheck` - `pnpm test:unit src/base/pointerUtils.test.ts src/renderer/extensions/vueNodes/widgets/utils/forwardMiddleButtonToCanvas.test.ts src/renderer/extensions/vueNodes/widgets/composables/useStringWidget.test.ts src/renderer/extensions/vueNodes/widgets/composables/useMarkdownWidget.test.ts src/renderer/core/canvas/useCanvasInteractions.test.ts src/composables/maskeditor/useToolManager.test.ts src/renderer/core/layout/transform/useTransformSettling.test.ts src/composables/node/useNodeImage.test.ts src/composables/node/useNodeAnimatedImage.test.ts src/components/graph/SelectionToolbox.test.ts src/lib/litegraph/src/LGraphCanvas.slotHitDetection.test.ts` - `pnpm typecheck:browser` - `pnpm test:browser:local browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts browser_tests/tests/vueNodes/widgets/text/multilineStringWidget.spec.ts browser_tests/tests/maskEditor.spec.ts --project chromium --grep "Middle-click drag"` - Commit hook: staged file format/lint, `pnpm typecheck` ## Screenshots (if applicable) Not applicable; this is interaction behavior covered by unit and browser tests.	2026-05-29 16:12:12 +09:00
Alexis Rolland	dc1bc4c9f8	Update utils category to utilities (#12498) ... ## Summary Update frontend only nodes categories to consolidate utility nodes into a `utilities` category (instead of utils). Paired with changes done in the core repo here: https://github.com/Comfy-Org/ComfyUI/pull/14145 ## Changes - **What**: - Rename frontend only nodes category from `utils` to `utilities` - Move frontend only Primitive node from `utils` to `utilities/primitive` ## Screenshots <img width="563" height="352" alt="image" src="https://github.com/user-attachments/assets/a768ec48-fb87-4fa3-934a-bd593bb35f3d" /> <img width="1181" height="773" alt="image" src="https://github.com/user-attachments/assets/a3e09e25-3412-4d23-abe8-220948b87258" />	2026-05-29 02:03:44 +00:00
AustinMroz	767bd17077	Fix "open tutorial button" not working in templates (#12511) ... The "open tutorial" button only existed in the DOM when the template card as actively hovered. For reasons I can not comprehend (probably overzealous pointer handlers somewhere), the act of clicking on the button would fire a mouseleave event. This caused the button to disappear for the exact moment it was clicked alike to a mischievous dondurma vendor. This is resolved by keeping the button always in DOM, but making it invisible when the card isn't hovered. The PR also removes a deeply nested `v-bind='$attrs'`. I'm assuming it must be a mistake that attributes applied to the entire template selector dialogue would be bound to every deeply nested tutorial button on individual workflow cards.	2026-05-28 23:38:15 +00:00
jaeone94	dc8471c6d3	fix: show workflow refresh loading state (#12509) ... ## Summary Adds visible loading feedback to the Workflows sidebar refresh button so users can tell when a workflow sync request is in flight. ## Changes - **What**: Exposes `isSyncLoading` from the workflow store and binds the Workflows sidebar refresh button to disabled, `aria-busy`, and spinning icon states while sync is pending. - **What**: Adds stable E2E selectors for the workflows refresh button and covers the loading state with unit and browser tests. - **Dependencies**: None. ## Review Focus Please verify the refresh control behavior while `/api/userdata?dir=workflows` is pending, especially that the button is disabled, exposes busy state, and returns to idle after sync completes. ## Validation - `pnpm format` - `pnpm test:unit src/components/sidebar/tabs/BaseWorkflowsSidebarTab.test.ts` - `pnpm test:browser:local browser_tests/tests/sidebar/workflows.spec.ts -g "Shows loading state while refreshing workflows"` - `pnpm lint` - Commit hooks: `oxfmt`, `oxlint`, `eslint`, `typecheck`, `typecheck:browser` ## Screenshots (if applicable) https://github.com/user-attachments/assets/e8b893ae-a91d-45c9-81ea-adaf164de227	2026-05-28 17:31:42 +00:00
Christian Byrne	8206022982	fix(subgraph): validate URL hash and redirect to root when subgraph missing (#12169) ... *PR Created by the Glary-Bot Agent* --- ## Summary Fix FE-559: browser forward/back to a deleted subgraph used to leave the canvas on stale state (and sometimes triggered unrelated tab navigation) because the subgraph id in the URL hash was looked up with no validation or fallback. ## Changes - **What**: - Added `src/schemas/subgraphIdSchema.ts` — `zSubgraphId = z.string().uuid()` + `isValidSubgraphId(value)` type guard, matching how subgraph ids are persisted in `workflowSchema.ts` and generated by `createUuidv4()`. - `subgraphNavigationStore.navigateToHash()` now (a) validates the hash with `isValidSubgraphId` before any lookup, (b) redirects to the root graph (`router.replace('#' + root.id)` + `canvas.setGraph(root)`) when the locator is malformed, missing from `root.subgraphs`, or still unresolved after a workflow-load attempt. - Replaced the `console.error('subgraph poofed after load?')` dead-end with the same redirect helper. - Re-ordered the "already on this graph" short-circuit so a stale canvas reference to a now-deleted subgraph doesn't suppress the redirect. ## Review Focus - TDD: 6 new tests in `subgraphNavigationStore.navigateToHash.test.ts` cover valid navigation, deleted-subgraph hash, malformed (non-UUID) hash, no-op when target equals current, empty-hash root case, and stale-canvas recovery. 15 new tests in `subgraphIdSchema.test.ts` lock down the validator. - `redirectToRoot()` toggles `blockHashUpdate` while calling `router.replace`, so the new redirect doesn't re-trigger `updateHash()` and clobber the canvas state. - Generalized validation: the new schema lives in `src/schemas/` and can be reused anywhere a subgraph id crosses an untrusted boundary (URL, IPC, etc.). ## Manual Verification Ran ComfyUI backend (`--cpu --port 8188`) + frontend dev server, then drove Playwright through three scenarios: | Input hash | Result | Console | |---|---|---| | `#11111111-2222-4333-8444-555555555555` (UUID-shaped, non-existent) | URL replaced with `#<root-id>` | `[subgraphNavigation] subgraph not found: 11111111-…; redirecting to root graph` | | `#not-a-valid-uuid` (malformed) | URL replaced with `#<root-id>` | `[subgraphNavigation] invalid subgraph id in hash: not-a-valid-uuid; redirecting to root graph` | | `#aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee` (UUID-shaped, non-existent) | URL replaced with `#<root-id>` | (same redirect message) | Screenshot below shows the redirected viewport. Fixes FE-559 ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12169-fix-subgraph-validate-URL-hash-and-redirect-to-root-when-subgraph-missing-35e6d73d3650819f840af1475b9f44d4) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>	2026-05-28 00:34:42 +00:00
Dante	a931acadd3	feat(dialog): migrate Settings dialog to Reka-UI (Phase 3) (#12182) ... ## Summary Phase 3 of the dialog migration. Closes the parity gaps in the Reka renderer (maximize affordance, headless layout mode, overlay-class plumbing), then flips `useSettingsDialog` onto the Reka path. Public API of `useDialogService` / `dialogStore` is unchanged. Parent: [FE-571](https://linear.app/comfyorg/issue/FE-571/dialog-system-migration-primevue-reka-ui-parent) This phase: [FE-575](https://linear.app/comfyorg/issue/FE-575/phase-3-migrate-settings-dialog-workspace-non-workspace-designer) Predecessors: #11719 (Phase 0, merged), #12041 (Phase 1, merged), #12109 (Phase 2, **stacked PR base**) > **Stacked on Phase 2**: this PR targets `jaewon/dialog-reka-migration-phase-2`. Rebase onto `main` after #12109 lands. ## Changes ### Reka primitives — parity gaps closed | File | Change | | --- | --- | | `src/components/ui/dialog/dialog.variants.ts` | New `maximized` variant. `false` keeps the centered/sized layout; `true` switches to `inset-2 top-2 left-2 size-auto max-h-none max-w-none sm:max-w-none` for full-screen mode | | `src/components/ui/dialog/DialogContent.vue` | Accepts `maximized` prop, forwards to variants | | `src/components/ui/dialog/DialogMaximize.vue` **(new)** | Icon-only button toggling `lucide--maximize-2` / `lucide--minimize-2`; emits `toggle`; uses `g.maximizeDialog` / `g.restoreDialog` i18n | | `src/stores/dialogStore.ts` | Adds `overlayClass?: HTMLAttributes['class']` to `CustomDialogComponentProps` (Reka-only; PrimeVue path uses `pt.mask`) | | `src/components/dialog/GlobalDialog.vue` | (a) Forwards `overlayClass` to `DialogOverlay`; (b) passes `:maximized` to `DialogContent`; (c) renders `DialogMaximize` in the header when `maximizable`, wired to a local `toggleMaximize`; (d) when `headless: true`, skips the inner `flex-1 overflow-auto px-4 py-2` wrapper so layout dialogs control their own chrome | ### Settings flip | File | Change | | --- | --- | | `src/platform/settings/composables/useSettingsDialog.ts` | Adds `dialogComponentProps: { renderer: 'reka', size: 'full', contentClass: '\<...\>', overlayClass }`. `contentClass` is `w-[90vw] max-w-[960px] sm:max-w-[960px] h-[80vh] max-h-none rounded-2xl overflow-hidden` — matches the previous `BaseModalLayout size="sm"` (960px × 80vh). `overlayClass: 'p-8'` only when `isCloud && teamWorkspacesEnabled` (preserves the workspace breathing-room contract) | | `src/components/dialog/GlobalDialog.vue` | Drops the now-dead `getDialogPt` workspace special case and the orphan `.settings-dialog-workspace` CSS. Removes unused imports (`merge`, `computed`, `useFeatureFlags`, `isCloud`, `DialogPassThroughOptions`) | ### Tests - `src/platform/settings/composables/useSettingsDialog.test.ts` **(new)** — 5 tests: renderer flip + sizing, workspace `overlayClass` toggle, panel forwarding, `showAbout()` ## Quality gates - [x] `pnpm typecheck` — clean - [x] `pnpm lint` — 0 errors (3 pre-existing warnings unrelated to this PR) - [x] `pnpm format` — applied - [x] `pnpm test:unit` (touched + adjacent areas): - `useSettingsDialog.test.ts` — 5/5 - `dialogService.renderer.test.ts` — 5/5 - `GlobalDialog.test.ts` — 9/9 - All `src/components/dialog/` — 73/73 - All `src/platform/settings/` — 75/75 - `CustomizationDialog.test.ts` — 4/4 - [ ] CI Playwright matrix - [ ] Manual verification on a backend ## Screenshots End-to-end verification of the Reka flip on a local dev server: | | | | --- | --- | | Settings dialog rendered via Reka (non-modal, focus stays in dialog body) |  | | Keybinding panel inside the Reka Settings dialog |  | | Nested PrimeVue **Modify keybinding** dialog stacked on top — `document.activeElement` is the `<input autofocus>`, proving the focus-trap fix |  | ## Public API impact None. `useSettingsDialog().show()` keeps the same signature. Reka primitives gain optional `maximized` prop and `overlayClass` field — additive, non-breaking. ## Out of scope (later phases) - Manager dialog — Phase 4 (FE-576) — will consume the new `maximizable` affordance - `ConfirmDialog` callers — Phase 5 (FE-577) - Removing PrimeVue `Dialog`/`<style>` overrides in `GlobalDialog.vue` — Phase 6 (FE-578) ## Review focus 1. **Sizing strategy** — `contentClass` overrides Reka's default content sizing (matching the existing `BaseModalLayout size="sm"` of 960 × 80vh). Worth a designer pass per FE-575's acceptance criteria. 2. **`overlayClass: 'p-8'` workspace mode** — Reka's `DialogContent` is positioned with viewport coordinates, so overlay padding does not constrain it the way the old PrimeVue `mask.p-8` did. Cosmetic gutter only. If designer flags missing breathing room, follow-up by shrinking `contentClass` in workspace mode. 3. **`headless: true` semantics for Reka** — now skips the inner padding wrapper. Existing migrated dialogs (Phases 1–2) all set a header, so no visible impact. The Reka-headless path is new with this PR. 4. **Maximize wiring** — `toggleMaximize` mutates `item.dialogComponentProps.maximized` directly (Pinia deep-reactive proxy). The store's `onMaximize` / `onUnmaximize` callbacks are still wired for the PrimeVue path; not double-fired. ## Test plan - [x] Unit: 102/102 across touched + adjacent areas - [ ] CI: full Vitest + Playwright matrix - [ ] Manual on a backend: - Open Settings via gear icon / keyboard shortcut → renders through Reka, search works, panel navigation works, ESC closes - Open Settings → trigger a reset confirmation (stacked confirm) → confirm renders above Settings, ESC closes only the confirm - Cloud workspace mode: Settings opens with workspace panel; `overlayClass` applied - Cloud non-workspace mode: Settings opens without workspace panel; no `overlayClass` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12182-feat-dialog-migrate-Settings-dialog-to-Reka-UI-Phase-3-35e6d73d36508144bb4af88f83c5ab20) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com>	2026-05-27 22:08:30 +00:00
AustinMroz	b89940134f	Better preview grid tiling (#12463) ... The previous image preview tiling code was less than ideal. It had fixed breakpoints based on the number of images. Outputs with many images would become comically long. This PR instead tiles images to fill the available space. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/e793ce65-8efc-44ca-b049-98f066a65b7d" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/ca891ce2-335f-42ce-aeec-a99579f669c8" />|	2026-05-27 20:26:44 +00:00
Christian Byrne	7ac1cbbd53	test: add E2E coverage for NE, SW, NW corner node resizing (#11408) ... *PR Created by the Glary-Bot Agent* --- ## Summary - Adds parameterized Playwright E2E tests covering all non-SE resize corners (NE, SW, NW), closing the coverage gap in the `useNodeResize.ts` switch statement - Adds `resizeFromCorner()` and `getResizeHandle()` to `VueNodeFixture` for reuse across tests - Test cases are derived from the production `RESIZE_HANDLES` config so they stay in sync with the actual handle definitions ## Test Groups (8 new tests) | Group | Tests | Coverage | |-------|-------|----------| | Corner resize directions | NE, SW, NW — size increases and correct edges shift | Lines 110-124, 184 | | Opposite edge anchoring | NE, SW, NW — opposite corner stays fixed | Position compensation end-to-end | | Minimum size enforcement | SW width clamp (≥ MIN_NODE_WIDTH), NE height clamp | Lines 162-176 | ## Design Decisions **Locator-based handle discovery**: `resizeFromCorner()` finds handles via `getByRole('button', { name: ariaLabel })` instead of coordinate offsets. The resize handles have `opacity-0 pointer-events-auto`, meaning they're always interactive even when visually transparent — Playwright considers elements with `opacity: 0` as visible (it only gates on `visibility: hidden` / `display: none` / zero-size bounding box). If this approach turns out to be flaky in CI due to handle discoverability, we can fall back to coordinate-based targeting (computing offsets from the node's bounding box corners), which is what the original SE-corner test uses. **Parameterization from production config**: Tests import `RESIZE_HANDLES` from `resizeHandleConfig.ts` and derive test case data (drag direction, which axes move) from the corner name. An upfront guard throws if any expected corner is missing from the config, preventing silent coverage loss. **Aria-label coupling**: `RESIZE_HANDLE_LABELS` in `VueNodeFixture` hardcodes the English aria-label strings. This is intentional — tests run in English locale, and aria-labels are the accessibility interface contract. If a more stable hook is needed (e.g., `data-testid` per handle), that can be added to `LGraphNode.vue` in a follow-up. **Frame settlement**: `resizeFromCorner()` calls `nextFrame()` after the mouse-up to ensure layout settles before assertions run, per `FLAKE_PREVENTION_RULES.md`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11408-test-add-E2E-coverage-for-NE-SW-NW-corner-node-resizing-3476d73d3650818d8a5ce5d6d535b38c) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com> Co-authored-by: Connor Byrne <c.byrne@comfy.org>	2026-05-27 17:26:59 +00:00
Alexander Brown	0157b47024	feat(subgraph): Subgraph Link Only Promotion (ADR 0009) + migration/store hygiene (#12197) ... ## Summary Introduces **Subgraph Link Only Promotion** (ADR 0009) — a new model for surfacing inner subgraph widgets on the parent SubgraphNode by *promoting through links* rather than by duplicating widget state on the host. Ships with the hygiene/refactor pass on the migration, store, and event layers that the new model depends on. ## What changes ### Subgraph Link Only Promotion (ADR 0009) Promoted widgets are defined by the link from a SubgraphNode input to the interior node, not by a duplicated widget instance on the host. Consequences: - A SubgraphNode renders inner widgets purely as a **projection** of the interior widgets and links — no host-side state to drift. - **Per-host independence**: multiple instances of the same SubgraphNode render and edit their own values without cross-talk. - **Reversible promote/demote**: structural link operation, so demote preserves host slots and external connections (#12278). ### Supporting refactors - **Migration** — Planner/classifier/repair/quarantine helpers collapsed into a single `proxyWidgetMigration` entry point with black-box round-trip coverage. Honors the source-node-id disambiguator on `proxyWidgets`, so deduplicated names (e.g. `text`, `text_1`) resolve to the right interior widget. - **Widget identity** — `appMode` unified on `WidgetEntityId`; promoted widget state is keyed by entityId across the store, DOM, and migration paths. - **SubgraphNode** — 3-key promoted-view cache replaced with a single version counter + explicit `invalidatePromotedViews()` at mutation sites; `id === -1` sentinel removed. - **Events** — `LGraph.trigger()` now dispatches node trigger payloads through `this.events`, replacing a leaky `onTrigger` monkey-patch. `SubgraphEditor` reactivity is driven from subgraph events instead of imperative refresh. - **Stores** — `appModeStore` migration helpers collapsed into `upgradeAndValidateInput`; `nodeOutputStore.*ByExecutionId` derived from the locator index; `previewExposureStore` cleanup and cycle-detection double-warn fix. - **Misc** — `Outcome` types consolidated; mutable accumulators replaced with `flatMap`; new ESLint rule forbids litegraph imports under `src/world/`. ### Tests - Browser tests for promoted widgets retagged `@vue-nodes` and rewritten to assert against the rendered Vue node DOM (via `getNodeLocator` / `getByRole('textbox')` / `enterSubgraph`) instead of `page.evaluate` graph introspection. - Per-host widget independence asserted via DOM. - Migration coverage moved to black-box round-trip tests. - Added coverage for duplicate-named promoted widget identity (ADR 0009) and the per-parent demote branch in `WidgetActions`. ## Review focus - ADR 0009 conformance of the link-only promotion model. - Disambiguator resolution path in `proxyWidgetMigration`. - Single-version-counter promoted-view cache and its `invalidatePromotedViews()` call sites. - `LGraph.trigger()` event dispatch and the `AppModeWidgetList.vue` migration off `onTrigger` (FE-667 tracks the remaining `useGraphNodeManager` conversion). ## Breaking changes None for users. Internal subgraph promotion APIs changed — see ADR 0009. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12197-feat-subgraph-link-only-widget-promotion-migration-store-hygiene-35e6d73d365081fd882cf3a69bc09956) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: AustinMroz <austin@comfy.org>	2026-05-27 00:29:11 -07:00
AustinMroz	876ed502c9	Mock sign-in request in test (#12482) ... Mock the sign in request in e2e tests to ensure tests success isn't tied to external variables.	2026-05-27 05:05:26 +00:00
AustinMroz	c638ad194b	Fix restoring values to dynamic combos (#12211) ... `DynamicCombo`s redefined `widget.value` without going through the store. This would result in desync of state. Most noticeably, swapping to and from a workflow would break vue reactivity and cause the default option to display visually ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12211-Fix-restoring-values-to-dynamic-combos-35f6d73d3650814ba12ccda42615239a) by [Unito](https://www.unito.io)	2026-05-26 21:19:59 +00:00
AustinMroz	bbaaa82125	Fix missing value control on 'Primitive Int' (#12431) ... #8505 added support for specifying default values for `control_after_generate`. Unbeknown to me, this exact same format of assigning `control_after_generate` to a string in the schema already served a function of renaming the control widget. As a result, control widgets with a default value set would use a different internal name, but due to other overlapping systems, would either have a label of `control_after_generate` or `control_before_generate`. The fix here, is incredibly simple and low scope. Instead of trying to filter control widgets by name, the dedicated `IS_CONTROL_WIDGET` symbol is used. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/5917e093-124a-4923-80ff-321fc0a94ef3" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/c6d95b5a-2764-4e71-a09f-dcae5ddcfdbb" />|	2026-05-26 01:49:44 +00:00
Alexander Brown	8d1a170136	feat: remove ability to create Group Nodes (#12347) ... *PR Created by the Glary-Bot Agent* --- Group Nodes are a legacy feature superseded by Subgraphs. This PR removes every UI entry point for *creating* a new Group Node while keeping the loading, ungrouping, and management code intact so existing workflows that contain Group Nodes continue to load and can still be unpacked or managed. ## Removed creation entry points - `Comfy.GroupNode.ConvertSelectedNodesToGroupNode` command - `Alt+G` keybinding - "Convert to Group Node (Deprecated)" canvas and node right-click menu items (`groupNode.ts` `getCanvasMenuItems` / `getNodeMenuItems`) - "Convert to Group Node" entry in the Vue selection menu (`useSelectionMenuOptions.ts`) - Associated `MENU_ORDER` entry in `contextMenuConverter.ts` - `convertSelectedNodesToGroupNode` / `convertDisabled` helpers in `groupNode.ts` - `BadgeVariant.DEPRECATED` enum member (no remaining consumers; knip-clean) - Matching `en` locale strings in `main.json` (`contextMenu.Convert to Group Node`, `commands.Convert selected nodes to group node`) and `commands.json` (`Comfy_GroupNode_ConvertSelectedNodesToGroupNode`) - Browser-test helpers `convertToGroupNode` / `convertAllNodesToGroupNode` and the three tests that exercised the creation flow ## Preserved (intentionally) - `GroupNodeHandler`, `GroupNodeConfig`, `GroupNodeBuilder`, `ManageGroupDialog` - `beforeConfigureGraph` / `nodeCreated` hooks that load and initialize Group Nodes from saved workflows - "Manage Group Nodes" canvas menu item, the `Comfy.GroupNode.ManageGroupNodes` command, and the per-node "Manage Group Node" / "Convert to nodes" options on existing group node instances - "Ungroup selected group nodes" command + `Alt+Shift+G` keybinding so users can disassemble existing group nodes in legacy workflows - Reduced `browser_tests/tests/groupNode.spec.ts` covering surviving behaviors: workflow loading (legacy `/` separator, hidden-input config, v1.3.3 fixture), copy/paste of already-loaded group nodes across workflows, and opening the Manage Group Node dialog ## Verification - `pnpm typecheck` clean - `pnpm typecheck:browser` clean - `pnpm format` clean - `pnpm knip` clean (no new findings; pre-existing flac.ts tag warning unchanged) - `pnpm test:unit` — 796 files, 10,789 tests pass (8 pre-existing skipped); includes a regression test in `useSelectionMenuOptions.test.ts` asserting the Vue selection menu no longer offers a Convert to Group Node option - Pre-commit hooks (oxfmt, oxlint, eslint, typecheck, typecheck:browser) passed - Manual verification against a live dev server: programmatically inspecting the GroupNode extension showed `getCanvasMenuItems` returns only `[Manage Group Nodes]`, `getNodeMenuItems` returns `[]`, and the `ConvertSelectedNodesToGroupNode` command + Alt+G keybinding are absent from the registries. Visually captured the node right-click menu (attached screenshot) — "Convert to Subgraph" remains, no "Convert to Group Node" entry - Browser E2E suite not executed locally (sandbox has no GPU and Playwright requires a full backend; the reduced spec will run in CI) - Non-English locales not modified — per `src/locales/CONTRIBUTING.md` they are regenerated by CI ## Notes for reviewers - This is a surgical removal of creation only; loading any older workflow that already contains group nodes will continue to work. - If you'd like to also remove the management UI (`Manage Group Nodes` command/menu/dialog) or the ungroup command in a follow-up, happy to open a separate PR. ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12347-feat-remove-ability-to-create-Group-Nodes-3656d73d365081d488bfd98ffd7545c0) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Amp <amp@ampcode.com>	2026-05-26 00:47:30 +00:00
AustinMroz	fb5b4a62ba	Fix mask editor sometimes showing wrong image (#12413) ... Mask editor checks `node.images` to determine the image which is edited. If the user generates an output image in litegraph mode, swaps to vue mode, then generates a new image, the mask editor will incorrectly display the image last shown in litegraph mode. This is resolved by having `syncLegacyNodeImgs` also synchronize node outputs to `node.images`.	2026-05-25 18:37:10 +00:00
Dante	d405002127	fix(widgets): collapse duplicate COLOR widget rendering on Color to RGB Int (FE-842) (#12447) ... ## Summary Fix the duplicate \`<WidgetColorPicker>\` rendering on the \`Color to RGB Int\` node (and any other COLOR-using V3 node that the runtime double-registers a widget for). <img width="480" alt="after-fix-dedupe-proof" src="https://github.com/user-attachments/assets/5c801806-ed5d-493f-92b6-e0b99dd8e408" /> ## Changes - **What**: - \`useProcessedWidgets.getWidgetIdentity\`: fall back to the host \`nodeId\` parameter for the dedupe identity root when neither \`storeNodeId/widget.nodeId\` nor \`sourceExecutionId\` is set. Normal root-graph widgets now dedupe identically to promoted/execution-scoped widgets, so any duplicate same-name+same-type widget collapses to one render. \`sourceExecutionId\` precedence is preserved. - \`useColorWidget\`: read top-level \`default\` from the V2 spec (falls back to nested \`options.default\` for hand-authored V2 specs), and short-circuit if a same-name color widget already exists on \`node.widgets\` so a second \`addWidget('color', …)\` call from upstream hooks (or a \`configure\` round-trip) no longer duplicates the row. - **Tests**: - New \`useColorWidget.test.ts\` covers top-level default, nested-options fallback, no-default fallback, and the idempotency guard. - \`useProcessedWidgets.test.ts\` gets a regression case for two identical color widgets on the same node collapsing to one render, plus an updated \`getWidgetIdentity\` case for the host-nodeId fallback. ## Review Focus - \`getWidgetIdentity\` precedence change. The fallback only fires when none of \`storeNodeId\`, \`widget.nodeId\`, or \`sourceExecutionId\` are present, so promoted/exec-scoped widgets (incl. the \"unresolved same-name promoted entries distinct by source execution identity\" \`NodeWidgets\` test) are unaffected. - \`useColorWidget\` idempotency guard is defensive — the root cause of the second \`addWidget\` call (cloud-only hook or persisted \`info.widgets\` configure round-trip) is not in this diff; that's tracked separately. Fixes [FE-842](https://linear.app/comfyorg/issue/FE-842/color-to-rgb-int-node-shows-duplicate-color-widgets)	2026-05-25 11:33:58 +00:00
pythongosssss	abd233d10d	feat: default search to essentials when graph is empty (#12377) ... ## Summary Currently, when opening node search on an empty graph, the default view shows "Most Relevant" nodes, which includes nodes like CLIP and VAE. For users building from scratch, these nodes are not necessarily the most helpful starting point. ## Changes - **What**: - Update default mode to Essentials when graph is empty ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12377-feat-default-search-to-essentials-when-graph-is-empty-3666d73d3650816d9d5ae3ed602a30ec) by [Unito](https://www.unito.io)	2026-05-25 09:35:52 +00:00
jaeone94	91d2df45a1	Fix V2 draft lifecycle persistence (#12269) ... ## Summary This PR fixes the remaining FE-367 workflow persistence gap by moving the workflow draft lifecycle callers from the legacy V1 draft store to `workflowDraftStoreV2`, following the core design from #10367 while omitting unrelated changes. It keeps the change focused on saved workflow tab restore and V2 draft lifecycle behavior: - save active workflow drafts through V2 before loading a new graph - load, save, save-as, close, rename, and delete workflows against V2 draft storage - prefer a fresh V2 draft when loading a saved workflow, and discard stale drafts when the remote workflow is newer - restore saved open tabs from persisted tab state instead of letting stale active-path state win - preserve V2 draft payload timestamps when moving or refreshing draft recency - remove the now-unused V1 draft store/cache implementation instead of suppressing knip; the raw V1 on-disk migration path remains for existing users Co-authored-by: xmarre <xmarre@users.noreply.github.com> ## Test coverage Added unit coverage for V2 draft load, stale draft discard, rename/close lifecycle cleanup, tab restore ordering, metadata-load waiting/fallback, draft recency updates, quota eviction retry, and persistence-disabled reset behavior. Updated the workflow persistence composable tests to use a real `vue-i18n` plugin host instead of mocking `vue-i18n`. Added an E2E regression test that saves two workflows, edits an inactive saved tab draft, makes the active-path pointer stale, reloads, and verifies the saved tab order, active tab, and inactive draft restoration. ## Validation - `pnpm format` - `pnpm lint` - `pnpm typecheck` - `pnpm test:unit` - pre-push `pnpm knip` (passes with the existing flac tag hint) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12269-Fix-V2-draft-lifecycle-persistence-3606d73d365081b4a84feb1696ed88bb) by [Unito](https://www.unito.io) --------- Co-authored-by: xmarre <xmarre@users.noreply.github.com>	2026-05-22 15:24:31 +00:00
AustinMroz	ee286291d4	Fix reactivity on matchType output slots (#12397) ... Relevant: #9935. A PR claimed to solve the same issue (and was approved by me), but the issue persists. Even when checking out that exact commit, the issue does not appear affected. This PR is somewhat heavier. It converts the outputs into shallowReactive. Since there is no individual moment of registration for outputs, this conversion happens on type change and leverages that calling `shallowReactive` on a shallow reactive is low cost and reflexive. It also adds a test to ensure that regression can not happen in the future. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/3e4f4a0a-906f-4539-95b6-b2e80de7ceff" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/1a29ac66-ed5e-4874-82dc-ce9f6135dea5" />|	2026-05-22 02:56:02 +00:00
pythongosssss	b3ba6c9344	fix: select node after adding from library (#12404) ... ## Summary When adding a node from the library sidebar, the node was not correctly selected upon placing it. This was due to the canvas capturing the node under the cursor on mouse down, however the node had not yet been comitted to the graph at that point, and so selection was then cleared on mouse up. ## Changes - **What**: - add `blockCommitPointerDown` so if the cursor is over the canvas stop propagation to prevent LiteGraph adding the mouse handler to clear the selection ## Review Focus Alternative approaches considered were blocking the event in endDrag however this then required manual cleanup of LiteGraph handlers or overriding the `pointer.onClick` function to force selection of our node, both felt worse than this approach. ## Screenshots (if applicable) https://github.com/user-attachments/assets/a2eb154e-5178-4a1e-b5c7-884efd7a10c6	2026-05-21 19:52:56 +00:00
AustinMroz	a50b3d16da	Persist splash until graph load completes (#12387) ... When an app mode workflow is opened on fresh page load, either from a template url, or a persisted in browser cache, the UI would briefly display the graph view prior to swapping to app mode. This is fixed by continuing to display the splash screen until workflow state has loaded. Share by url brings unique difficulties. The function call does not return until a user has responded to a dialogue. If the splash screen were blocked by this, the user would never be able to see the dialogue. Consequentially, this change is not applied to shared workflow urls and the (very unlikely) url including both a template url and a share url will now prioritize the template url. A best effort e2e test is included, but is a little clunky. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12387-Persist-splash-until-graph-load-completes-3666d73d3650813495e4ccad6052c1e4) by [Unito](https://www.unito.io)	2026-05-21 19:44:12 +00:00
AustinMroz	3ce0c07af2	Use utility function to add node with V2 search (#12382) ... Default search box settings are a little inconvenient to work with. This PR introduces a new `addNode` utility function to the V2 search fixture that handles all the steps of opening search and adding a node that a user would perform. It then migrates several PRs I have recently written to use this new fixture. See also #12029 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12382-Use-utility-function-to-add-node-with-V2-search-3666d73d3650817c8c73c9104b1113bf) by [Unito](https://www.unito.io)	2026-05-21 19:25:26 +00:00
jaeone94	f1f65cff61	feat: select top asset widget FormDropdown result on Enter (#12209) ... ## Summary Allow asset/media FormDropdown searches to select the top filtered result when the user presses Enter. This covers image, video, audio, mesh, model-like asset selects, and other `WidgetSelectDropdown`-backed media widgets. ## Implementation Scope This PR implements a **top-result Enter shortcut** for the custom asset/media dropdown path only: - In scope: `WidgetSelectDropdown` -> `FormDropdown` asset/media widgets. - In scope: while the dropdown is open, single-select, and the search text is non-empty, the first current search result becomes the Enter candidate. - In scope: pressing Enter in the search input selects that candidate/top result through the existing selection path. - In scope: candidate feedback for this shortcut, including visual candidate styling and a polite screen-reader announcement for the current top result. - In scope: stale async search protection, empty-query/no-result no-op behavior, multi-select guard behavior, and focus return to the trigger after Enter selection closes the menu. - Out of scope: plain combo widgets (`WidgetSelectDefault` / `SelectPlus`). That path is PrimeVue-based and should be handled separately from this focused asset-widget PR. - Out of scope: full combobox/listbox keyboard navigation, including Tab-to-list focus, ArrowUp/ArrowDown candidate movement, Home/End behavior, scroll-to-active-item behavior, and a full ARIA combobox/listbox refactor. Follow-up arrow-key navigation should validate the interaction model separately. This PR keeps the candidate state narrow and localized so that future work can either extend it into movable active-item state or replace it as part of a fuller combobox/listbox implementation. ## Changes - **What**: Added an explicit Enter event from `FormSearchInput`, routed it through the FormDropdown menu actions, and selected the current top search result in `FormDropdown`. - **What**: Kept the existing `computedAsync` + debounced filtering path for normal typing, while Enter performs a one-off search against the latest input before selecting. Stale async Enter results are ignored if the query or item source changes before resolution. - **What**: Prevented closed FormDropdown state from treating the full unfiltered list as current search results, limited Enter-to-select to single-select dropdowns, and made empty search Enter a no-op. - **What**: Returned focus to the dropdown trigger after single-select selection closes the menu. - **What**: Added candidate styling for the first current FormDropdown result while a search query is active so the Enter target is visible to users. - **What**: Added a polite screen-reader announcement for the current top result candidate. - **What**: Fixed the FormDropdownMenuActions `baseModelSelected` model default to use a `Set` factory instead of a shared instance. - **What**: Added unit coverage for the search Enter event, FormDropdown selection behavior, focus return, debounce/Enter behavior, stale async Enter protection, empty-query no-op behavior, closed-state stale result protection, multi-select guard behavior, and candidate announcement behavior. Added App Mode E2E coverage for asset FormDropdown Enter selection. - **What**: Extracted reusable app-mode dropdown fixture helpers and updated the existing FormDropdown clipping test to use the shared helper. ## Review Focus Please focus review on the asset/media FormDropdown path, especially `getTopSearchResult()`, the single-select/empty-query guards, stale async search protection, trigger focus return after selection, and candidate feedback in grid/list layouts. The plain combo path and full arrow-key navigation are intentionally left for separate follow-up work. ## Screenshots (if applicable) https://github.com/user-attachments/assets/3eb3456d-93a3-4959-91a3-188f8116ccc9 Validation performed: - Latest final-commit validation: - `pnpm test:unit src/renderer/extensions/vueNodes/widgets/components/form/FormSearchInput.test.ts src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuActions.test.ts src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.test.ts` - Commit hook: `pnpm exec stylelint ...`, `pnpm exec oxfmt --write ...`, `pnpm exec oxlint --type-aware --fix ...`, `pnpm exec eslint --cache --fix ...`, `pnpm typecheck` - Push hook: `pnpm knip --cache` - `git diff --check` - Earlier branch validation for this flow: - `pnpm install` - `pnpm typecheck:browser` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser -- --project=chromium browser_tests/tests/appMode.spec.ts -g "Drag and Drop|FormDropdown search Enter selects the top filtered item" --reporter=list` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser -- --project=chromium browser_tests/tests/appMode.spec.ts -g "FormDropdown search Enter selects the top filtered item" --reporter=list` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser -- --project=chromium browser_tests/tests/appModeDropdownClipping.spec.ts -g "FormDropdown popup is not clipped" --reporter=list`	2026-05-21 02:26:26 +00:00
Benjamin Lu	d472ca783b	test: cover FE-130 assets sidebar route mocks (#12332) ... ## Summary Adds a focused FE-130 assets sidebar browser-test slice without extending the stateful asset helper path. ## Changes - **What**: Extends `jobsRouteFixture` with job-detail and history-delete helpers for generated asset flows. - **What**: Adds an assets sidebar tab spec covering generated/imported rendering, preview opening, generated selection footer actions, and explicit delete refresh behavior. ## Review Focus The delete test keeps backend state explicit: it captures the `/api/history` request, then replaces the `/api/jobs` history mock with the post-delete response. The imported-file and `/api/view` mocks stay local to this focused spec instead of growing `AssetHelper` or adding a sidebar-specific fixture.	2026-05-20 17:01:51 -07:00
AustinMroz	2717d59451	Fix reactivity of vue subgraph price badges (#12029) ... When a subgraph contains partner nodes with price badges, those badges are also displayed on the subgraphNode. The reactivity here was spotty: The price badges would fail to display unless the user had navigated into the subgraph on the current page load. Fixing this is performed in 2 steps: - Firing a `node:property:changed` event when the badges contained in a subgraph are updated - Extending the reactivity updates so that badges update in vue mode despite using the litegraph badge getter. This PR also includes a minor styling tweak to fix text alignment on price badges | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/56a95cbe-12c9-43b0-8664-34e52b6415ac" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/bf4a0d81-21e4-4afc-946e-eba5967f1715" />| Resolves FE-346 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12029-Fix-reactivity-of-vue-subgraph-price-badges-3586d73d3650813cb12fe265090940e4) by [Unito](https://www.unito.io)	2026-05-20 11:22:42 -07:00
AustinMroz	d63b0f05bf	Subgraph io fixes (#12281) ... Fixes 3 different bugs when making links to and from subgraph IO from vue nodes - When dragging a link from a node to a subgraph IO, there is no feedback if a slot is not a valid connection target or if a slot is actively hovered - When a link is made from a subgraph IO to a node, the reactivity is not triggered on the node to indicate a change of link state. - When dragging a link from a subgraph IO to a node, the link would not snap to the valid connection targets on nodes - The fix for this one is not as thorough as I would like. It only allows connections to the slot, not connections to the hovered widget. We have two deeply disconnected linking systems and properly reconciling them would be a multi-week project. Resolves FE-561 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12281-Subgraph-io-fixes-3606d73d365081089f7ef19331c6d70a) by [Unito](https://www.unito.io)	2026-05-20 10:26:47 -07:00
AustinMroz	a95e53bf6d	On subgraph conversion, always unpack group nodes (#12356) ... This is a targeted small scope change to improve the availability for converting group nodes into a subgraph. The prior implementation would only apply on the litegraph context menu option for converting a node to a subgraph. It failed to apply on any of the other more common methods. The code for unpacking group nodes has been moved directly into the setup for converting a group of nodes into a subgraph and drastically simplified. Of note, several other long lived bugs were found while working on this fix, but they are out of scope for this targeted PR. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12356-On-subgraph-conversion-always-unpack-group-nodes-3666d73d365081d09774c00a851b8198) by [Unito](https://www.unito.io)	2026-05-20 05:23:46 +00:00
jaeone94	98a8a614e8	fix: avoid false missing media errors after importing shared workflow assets (#12333) ... ## Summary Import published media assets for shared workflows before loading the graph so the first missing-media scan sees the user's newly imported references instead of surfacing a false missing asset error. cc FE-773 ## Changes - **What**: Moves the shared workflow import step ahead of `loadGraphData` for the copy-and-open flow, while still allowing the workflow to open with a warning path if asset import fails. - **What**: Clears the shared workflow URL intent consistently on failure paths, including graph load failure after an import attempt, so reloads do not repeatedly replay the same shared workflow side effects. - **What**: Invalidates the input asset cache after published asset import so graph loading and missing-media resolution can observe the refreshed media state. - **What**: Adds a global loading spinner while shared workflow asset import and graph load are in progress, with `role="status"`, `aria-live`, reduced-motion-safe animation, and body teleporting so it stays visible above blocking UI. - **What**: Adds stable TestIds for the shared workflow dialog and updates existing shared workflow E2E selectors away from copy-dependent role text. - **What**: Adds a cloud E2E regression fixture and spec covering the critical flow: shared URL opens the dialog, the user confirms asset import, published media is imported before the public-inclusive input asset scan, the workflow loads, the share query is removed, and missing media UI is not surfaced. - **Breaking**: None. - **Dependencies**: None. ## Root Cause Shared workflow graph loading triggered the missing-media pipeline before the user-selected published media import had completed. Because `include_public=true` does not include published assets, the pre-import scan could classify shared media as missing even when the user was about to import those assets into their own library. ## Review Focus - The ordering in `useSharedWorkflowUrlLoader`: import published assets first, then load the graph, while keeping import failure non-fatal for workflow opening. - The failure cleanup behavior: the shared URL/preserved query intent is now cleared for graph load failures too, avoiding repeated reload-triggered imports. - The spinner behavior in `App.vue`: it uses the existing `workspaceStore.spinner` boolean and intentionally keeps broader ref-counted spinner ownership as follow-up work. - The E2E sentinel in `sharedWorkflowMissingMedia.spec.ts`: it asserts no public-inclusive input asset scan occurs before `/api/assets/import`, then waits for a settling window to ensure the missing-media overlay does not appear. ## Validation - `pnpm format` - `pnpm lint` (passed with existing unrelated warnings only) - `pnpm typecheck` - `pnpm test:unit` - Commit hook: lint-staged formatting/linting, `pnpm typecheck`, `pnpm typecheck:browser` - Push hook: `pnpm knip --cache` (passed with existing tag hint only) ## Follow-Up - Consider a ref-counted or scoped global spinner API so long-running flows do not directly toggle `workspaceStore.spinner`. - Consider separating shared workflow load status into orthogonal result fields instead of encoding partial success in a single string union. - Consider moving published asset import/cache invalidation behind an asset-service-owned API boundary. - Backend follow-up remains needed for `include_public=true` not including published assets; this PR only removes the frontend false positive when the user explicitly imports the shared media. ## Screenshots Before https://github.com/user-attachments/assets/dc790046-237c-4dd8-b773-2507f9a66650 After https://github.com/user-attachments/assets/6517cd38-2c3d-4bfe-a990-35892b7e50ae https://github.com/user-attachments/assets/d89dc3d3-75d9-4251-998b-0c354414e25b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12333-fix-avoid-false-missing-media-errors-after-importing-shared-workflow-assets-3656d73d365081b38634dcb7625cfc32) by [Unito](https://www.unito.io)	2026-05-20 02:59:44 +00:00
Benjamin Lu	64c75bfce5	test: avoid job history double setup (#12324) ... ## Summary Avoid a second `comfyPage.setup()` in the job history browser tests by registering the initial jobs route mocks before the normal page boot. ## Changes - **What**: Adds an `initialJobsScenario` Playwright option with an auto fixture for the job-history spec, moves QPOV2 setup into `initialSettings`, and keeps the sidebar helper focused on UI navigation. - **Dependencies**: None ## Review Focus Confirm the auto fixture ordering matches the intended browser-test setup: initial `/api/jobs` mocks should be installed before the `comfyPage` fixture performs its normal setup. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12324-test-avoid-job-history-double-setup-3656d73d365081778e24c11d3b65cbef) by [Unito](https://www.unito.io)	2026-05-19 20:09:01 +00:00
Dante	3b37488eee	fix: keep node context menu overflow visible when content fits (#12035) ... ## Summary Stops the Shape submenu (and any other PrimeVue nested submenu) from being clipped behind the node context menu when the menu fits in the viewport. ## Changes - **What**: `constrainMenuHeight` in `NodeContextMenu.vue` now applies `max-height` + `overflow-y: auto` to the root `<ul>` only when `scrollHeight > availableHeight`. The common case keeps `overflow: visible`. - Added `browser_tests/tests/nodeContextMenuShapeSubmenu.spec.ts` regression spec. ## Review Focus Root cause: setting only `overflow-y: auto` on a `<ul>` coerces `overflow-x` to a non-visible value per CSS spec (`If one of overflow-x/overflow-y is visible and the other isn't, the visible value is computed as auto`). PrimeVue `ContextMenuSub` renders submenus in-tree as a nested `<ul>` with `position: absolute; left: 100%`, so the implicit horizontal clip hides them entirely. The pre-existing overflow scenario (#10824 / #10854) is unchanged — when the menu actually overflows, the clamp still applies and `nodeContextMenuOverflow.spec.ts` continues to verify scroll. Submenu clipping in that overflow case is a known limitation, not introduced by this PR. Fixes FE-570 ## screenshot ### AS IS <img width="788" height="505" alt="Screenshot 2026-05-07 at 12 43 26 PM" src="https://github.com/user-attachments/assets/36d34070-0c57-4385-a130-0394f22f282e" /> ### TO BE <img width="779" height="627" alt="Screenshot 2026-05-07 at 12 42 44 PM" src="https://github.com/user-attachments/assets/00956729-763b-4787-822f-209e8ea42331" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12035-fix-keep-node-context-menu-overflow-visible-when-content-fits-3586d73d365081ad9aaec82f220d401c) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com>	2026-05-19 10:56:41 +00:00
jaeone94	fc7e6a0935	fix(terminal): resync logs console on backend reconnect (#12270) ... ## Summary When the built-in logs terminal stayed open during a backend restart, the buffer froze on pre-restart entries and live log streaming silently stopped — only closing and reopening the panel resynced. Listen for the api `reconnected` event and rebuild the terminal contents the same way a fresh open would. ## Changes - **What**: - Extract `useLogsTerminal` composable. The SFC is now a thin shell holding `terminal: shallowRef<Terminal>` and forwarding to the composable, so `onMounted`/`onScopeDispose` no longer rely on the child's emit callback timing. - Subscribe to `api`'s `reconnected` event via `useEventListener`, registered synchronously before any awaits. On reconnect: `terminal.reset()` → refetch raw logs → `scrollToBottom()` → `subscribeLogs(true)` (the backend loses the per-client subscription on restart, so re-subscribe is required for live streaming to resume). - Wrap in-flight resync/mount fetches in AbortControllers. Overlapping reconnects abort the prior resync, and unmount mid-fetch suppresses writes to the disposed xterm. - Hide BaseTerminal whenever `errorMessage` is set so the error layout doesn't expose an empty xterm container behind the message; `loading=false` after both load failure and resync success so a later successful reconnect can clear a stuck spinner. - Migrate the load/resync error strings to vue-i18n (`logsTerminal.loadError`, `logsTerminal.resyncError`). ## Review Focus - **Re-subscribe is the non-obvious half of the fix** — without it, even after the WebSocket reconnects the backend never resumes streaming logs to this client because its subscription state was wiped on restart. The visible "stale buffer" is only one symptom; the silent "no new logs" symptom needed the explicit `subscribeLogs(true)` re-call in resync. - `terminal.reset()` lives after a successful raw-logs fetch (not before) so a failed resync leaves the prior buffer visible instead of blanking it; resync errors surface via the same inline error message the mount path uses. - 8 unit tests around the composable: mount + subscribe, resync ordering (reset → write → scroll → subscribe via `invocationCallOrder`), in-flight resync abort on double reconnect, resync error surfacing, mount-failure-then-recovery, unmount-mid-fetch terminal-write suppression, listener cleanup on unmount. - 2 E2E tests using `ws.close()` on the proxied WebSocket as the reconnect trigger and `subscribeLogs` HTTP fetch count as the sync point (same pattern as `wsReconnectStaleJob.spec.ts`). Red-checked: disabling the `reconnected` listener fails exactly the two new tests, all 8 pre-existing tests stay green. Fixes FE-712 ## Screenshots Before - (After rebooting, the console window does not update from its state before the reboot must remount the console window for it to resync.) https://github.com/user-attachments/assets/b1e49c2c-89a4-4a4a-82b4-064412acee12 After - (The console window syncs automatically after a reboot.) https://github.com/user-attachments/assets/54b582c5-ad42-41c0-9886-18f4495859da ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12270-fix-terminal-resync-logs-console-on-backend-reconnect-3606d73d3650812fb13fd1934c632344) by [Unito](https://www.unito.io)	2026-05-19 01:55:11 +00:00
Benjamin Lu	a97f46b497	test: cover job history sidebar with typed route mocks (#12272) ... ## Summary Add the first product-area browser coverage on top of the merged typed route mock foundation: the docked job history sidebar. ## Changes - **What**: Adds `browser_tests/tests/sidebar/jobHistory.spec.ts` using `jobsRouteFixture`. - **What**: Covers direct sidebar entry, docked QPO history entry, terminal history jobs, active queue jobs, tab filtering, search, clear queue, and clear history. - **What**: Adds typed `POST /api/queue` and `POST /api/history` route helpers that validate request bodies with generated zod schemas. - **What**: Adds stable test ids for the job history sidebar and queue progress overlay so tests avoid structural CSS selectors. - **Dependencies**: Builds on the typed route mock foundation merged in #12267. ## Review Focus Review the product assertions and whether this is the right first coverage slice on top of the typed route mock foundation. This PR intentionally avoids asset sidebar and floating QPO lifecycle coverage; those should remain follow-up PRs. ## Screenshots (if applicable) Not applicable. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12272-test-cover-job-history-sidebar-with-typed-route-mocks-3606d73d3650817481d5f9fac4bfc93c) by [Unito](https://www.unito.io)	2026-05-18 18:34:56 -07:00
Benjamin Lu	3e31de5bbb	test: migrate MaxHistoryItems browser coverage (#12298) ... ## Summary Migrates the MaxHistoryItems browser coverage to the accepted jobs route fixture pattern. ## Changes - **What**: Composes `jobsRouteFixture` into the queue settings spec and removes the old `AssetsHelper` route setup. - **What**: Adds a `responseLimit` option to `jobsRouteFixture` so tests can match a requested history limit while intentionally returning more jobs. - **Dependencies**: None. ## Review Focus The key behavior is preserving both FE-501 acceptance cases: `/api/jobs` still receives the configured `limit`, and the queue panel still caps rendered history even if the mocked backend returns more rows than requested. Fixes FE-501 ## Screenshots (if applicable) Not applicable. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12298-test-migrate-MaxHistoryItems-browser-coverage-3616d73d365081d6bf77fb205fcd51d4) by [Unito](https://www.unito.io)	2026-05-18 18:59:20 +00:00
jaeone94	0558740c78	refactor: migrate default combo widget select to Reka (#12288) ... ## Summary Migrate the default combo widget select from the PrimeVue `SelectPlus` wrapper to a Reka `Combobox` implementation while preserving the existing Comfy combo widget contract and the node-canvas dropdown behavior. ## Changes - **What**: Rewrites `WidgetSelectDefault.vue` on top of Reka `ComboboxRoot`, `ComboboxTrigger`, `ComboboxInput`, `ComboboxContent`, and `ComboboxItem`. - **What**: Preserves the default combo widget surface: `v-model`, `widget` prop, `aria-label` from the widget name/label, `data-capture-wheel`, disabled state, placeholder/filter placeholder, default slot controls, invalid current value display, array values, dynamic/factory values, and `getOptionLabel` fallback behavior. - **What**: Keeps dynamic `values` compatibility by refreshing function-backed options when the dropdown opens, without re-evaluating the factory on every search keystroke. - **What**: Deletes the now-unused PrimeVue `SelectPlus.vue` wrapper and removes the PrimeVue test plugin/stub path from the default widget select tests. - **What**: Updates App Mode dropdown clipping coverage and combo-widget browser coverage to target the new Reka overlay/viewport structure. - **Breaking**: No breaking change is intended for the documented Comfy combo widget contract. This migration does not preserve incidental PrimeVue `Select` prop pass-through from `widget.options`; that was a side effect of wrapping PrimeVue rather than a stable widget API. - **Dependencies**: No new dependencies. ## Review Focus ### Compatibility choices The goal of this PR is a migration PR, not a broad behavior redesign. The new implementation keeps the Comfy-specific combo contract rather than attempting to emulate PrimeVue internals. In particular: - `values` still accepts arrays and functions, and function values are re-read on open to support dynamic/custom node option sources. - `getOptionLabel(value) || value` is intentionally preserved to match the sibling dropdown path and avoid turning an empty-string label into a blank rendered option. - Invalid/current values that are not present in the option list are still rendered in the trigger instead of disappearing. - `WidgetWithControl` continues to render its default slot in the control area, with trigger text truncation preserved. - App Mode `OverlayAppendToKey='body'` continues to map to a body portal to avoid panel clipping. ### Visual alignment and screenshot updates The previous PrimeVue implementation passed `size="small"`, which injected internal `.p-select-sm .p-select-label` styling. That internal PrimeVue style used its own small-select font size and padding, overriding the surrounding widget sizing intent and making the select trigger subtly taller with slightly larger text than nearby inline node widget controls. The Reka implementation intentionally keeps the normal widget styling path instead of recreating that PrimeVue-specific internal override. This means the trigger follows the same inline widget sizing direction as neighboring controls rather than preserving the incidental PrimeVue height/text-size delta. Because this is an expected visual difference from the migration, the affected E2E screenshots should be recaptured instead of treating the old PrimeVue select height as the target. ### Scrollbar and focus behavior Reka provides the combobox/listbox semantics we want, including search, arrow navigation, highlighted items, and Enter selection. The tricky part is the canvas dropdown scrollbar behavior. The native Reka viewport path hides/owns scrollbar behavior in a way that made it hard to preserve the previous widget dropdown affordances, especially visible scrollbars and mouse wheel capture over the node canvas. To keep the previous behavior, this PR renders a dedicated scrollable viewport inside `ComboboxContent` with the project scrollbar utilities (`scrollbar-thin`, stable gutter, transparent track). That preserves visible scroll affordance and allows wheel events over the dropdown to scroll the list instead of zooming the canvas. There was one Reka interaction to account for: pressing the native scrollbar can be treated as a focus-outside event from the search input, which previously closed the dropdown on mouse down or caused subsequent wheel events to leak back to the canvas. The new `useRestoreFocusOnViewportPointer` composable handles only that short pointer gesture: - viewport pointerdown marks a short-lived scrollbar/viewport interaction, - the next focus-outside event is prevented only if the search input can be restored, - the guard is cleared by `pointerup`, `pointercancel`, and a timeout so normal outside clicks still close the dropdown. ### Tests and regression coverage Unit coverage was updated around the new Reka implementation: - option sources from arrays and functions, - dynamic values refreshed on open but not on each search keystroke, - selection updates and blank/undefined Reka emissions being ignored, - search filtering and Reka keyboard selection behavior, - disabled state, invalid current values, `getOptionLabel`, empty results status, and WidgetWithControl slot preservation, - composable coverage for pointerup, pointercancel, repeated pointerdown listener cleanup, and no-input/no-op behavior. Browser regression coverage now checks the canvas-specific interaction surface: - opening and selecting default combo widget options, - wheel over the dropdown scrolls the list instead of zooming the canvas, - pressing the scrollbar does not close the dropdown, - wheel capture still works after pressing the scrollbar, - opening another node widget closes the previous dropdown, - switching between node widgets preserves dropdown scroll capture, - serialize/reload retains selected combo values. ## Screenshots (if applicable) New <img width="527" height="753" alt="스크린샷 2026-05-18 오전 1 36 27" src="https://github.com/user-attachments/assets/2293d510-6965-4b84-9b12-b8528f8a734f" /> Old <img width="496" height="473" alt="스크린샷 2026-05-18 오전 1 35 57" src="https://github.com/user-attachments/assets/47c0e28a-27df-44a6-81a8-14fcc1f3bd8f" /> Reka Supports Auto highlight top item on search (Search -> Enter -> Select 👍) https://github.com/user-attachments/assets/9d633dfc-c23a-4e7a-8d39-b044c219f1f3 The default combo widget trigger has a small intentional visual delta from the old PrimeVue path because the Reka implementation does not recreate PrimeVue's internal `size="small"` label override. https://github.com/user-attachments/assets/a9053a14-e39e-4d5e-a846-dcf9aeb0caed ## Validation - `pnpm format` - `pnpm lint` (passes; existing warning-only lint output remains in unrelated tests) - `pnpm typecheck` - `pnpm typecheck:browser` - `pnpm test:unit` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://127.0.0.1:5174 pnpm exec playwright test browser_tests/tests/vueNodes/widgets/combo/comboWidget.spec.ts --project=chromium` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12288-refactor-migrate-default-combo-widget-select-to-Reka-3616d73d365081fd8742c038a7dc7851) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com>	2026-05-18 02:58:37 +00:00
Terry Jia	058acfe592	test: add basic E2E tests for CurveEditor widget (#10730) ... ## Summary Add Playwright tests covering default rendering, interpolation selector, and click-to-add-point interaction. Includes test workflow asset. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10730-test-add-basic-E2E-tests-for-CurveEditor-widget-3336d73d365081f5a403df837737bc12) by [Unito](https://www.unito.io)	2026-05-17 20:54:07 -04:00
Csongor Czezar	7160a9ee3f	fix: QPO progress bar now shows node name in subgraphs (#7688) ... ## Summary Resolve the queue progress node label from queued prompt metadata so subgraph execution IDs show the correct node name without depending on the live canvas. ## Changes - **What**: Store a prompt-scoped `executionId -> { title, type }` lookup from `p.output` when queueing a job, and use that lookup for the active job's executing node label. - **What**: Reuse the same job-scoped node info for the browser tab title so it stays aligned with the queue overlay. - **What**: Add unit coverage for root and subgraph execution IDs, and merge the branch forward to current `main`. ## Review Focus This keeps the fix scoped to the existing singular `activeJobId` path. It fixes subgraph labels and avoids the workflow-switching regression from resolving against `app.rootGraph`, but it does not redesign concurrent multi-job selection yet. Longer term, the cleaner solution is still prompt-scoped execution metadata from the backend rather than frontend reconstruction. ## Screenshots (if applicable) N/A --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>	2026-05-15 14:00:33 -07:00
Alexander Brown	1ab63807e9	fix: reserve scrollbar gutter on textareas to prevent hover reflow (#12280) ... *PR Created by the Glary-Bot Agent* --- Adds the Tailwind 4.3 `scrollbar-gutter-stable` utility to the shared shadcn `Textarea` wrapper and the `ModelInfoPanel` description textarea so the layout no longer shifts when a vertical scrollbar appears on hover or focus. The wrapper edit propagates to all four consumers (`WidgetTextarea`, `WidgetMarkdown`, `ComfyHubDescribeStep`, `ComfyHubCreateProfileForm`). The `ModelInfoPanel` site uses a native `<textarea>` that bypasses the wrapper, so it is fixed inline. The `overflow-hidden hover:overflow-auto` performance pattern from #10804 is intentionally preserved. ## Verification - Typecheck, eslint, oxlint, oxfmt, stylelint clean on changed files - `Textarea.test.ts`, `WidgetTextarea.test.ts`, `WidgetMarkdown.test.ts`, `ComfyHubDescribeStep.test.ts`, `ModelInfoPanel.test.ts` — all 68 tests pass - `pnpm build` succeeds; the production CSS bundle contains the `scrollbar-gutter:stable` rule - Storybook (`UI/Textarea`): confirmed `getComputedStyle(textarea).scrollbarGutter === 'stable'` on Default, WithLabel, and overflowing-content scenarios Fixes FE-697 ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12280-fix-reserve-scrollbar-gutter-on-textareas-to-prevent-hover-reflow-3606d73d3650816f9947e20134abf59e) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com>	2026-05-15 01:48:35 +00:00
Christian Byrne	e35bea51d6	refactor(browser-tests): centralize template mocking via TemplateHelper (#11837) ... ## Summary Centralize template route mocking by mirroring the existing `AssetHelper` fixture pattern, so tests no longer hand-roll `page.route('**/templates/index.json', ...)` and `page.route('**/templates/**.webp', ...)` blocks. ## Changes - **What**: - Expand `browser_tests/fixtures/data/templateFixtures.ts` with stable distribution exports (`STABLE_CLOUD_TEMPLATE`, `STABLE_DESKTOP_TEMPLATE`, `STABLE_LOCAL_TEMPLATE`, `STABLE_UNRESTRICTED_TEMPLATE`), an `ALL_DISTRIBUTION_TEMPLATES` set, and a `generateTemplates(count, distribution?)` bulk generator. `makeTemplate` and `mockTemplateIndex` are preserved. - Add `browser_tests/fixtures/helpers/TemplateHelper.ts` modeled on `AssetHelper`: an immutable `TemplateConfig` driven by composable operators (`withTemplates`, `withTemplate`, `withCloudTemplates`, `withDesktopTemplates`, `withLocalTemplates`, `withUnrestrictedTemplates`, `withRawIndex`), a `TemplateHelper` class with `mock()` / `mockIndex()` / `mockThumbnails()` / `clearMocks()`, and a `createTemplateHelper(page, ...operators)` factory. - Add `browser_tests/fixtures/templateApiFixture.ts` exposing `templateApi: TemplateHelper` as a Playwright fixture with automatic `clearMocks()` teardown (mirrors `assetApiFixture`). - Migrate `browser_tests/tests/templateFilteringCount.spec.ts` to `mergeTests(comfyPageFixture, templateApiFixture)` and `templateApi.configure(withTemplates(...))` + `templateApi.mockIndex()`. The webp thumbnail mock moves into the helper. - **Breaking**: None. `makeTemplate` and `mockTemplateIndex` exports remain. ## Review Focus - `TemplateHelper.mock()` is split into `mockIndex()` and `mockThumbnails()` so the spec can install the thumbnail handler in `beforeEach` (where it has no per-test data) and the index handler per-test (after `configure(...)`). Both still register through `routeHandlers` so `clearMocks()` unroutes everything. - Operators are pure (`(config) => config`) and the helper deep-copies the resulting `templates` array, matching the AssetHelper immutability pattern. - The thumbnail route is `**/templates/**.webp` and serves `browser_tests/assets/example.webp` — identical to the previous inline behavior. Fixes #11431 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11837-refactor-browser-tests-centralize-template-mocking-via-TemplateHelper-3546d73d365081beac00d80c2ab5ea1c) by [Unito](https://www.unito.io)	2026-05-14 15:54:40 -07:00
Benjamin Lu	06e09df673	test: replace jobs mock fixture with typed route mocks (#12267) ... ## Summary Replace the merged stateful jobs API browser mock fixture with a small declarative typed route-mock foundation. ## Changes - **What**: Removes `JobsApiMock`, `jobsApiMockFixture`, and the old shared `jobFixtures` helper. - **What**: Adds a generic `RouteMocker` primitive for explicit typed JSON route responses. - **What**: Adds `jobsRouteFixture`, which registers explicit `/api/jobs` list/detail responses without filtering, mutation handling, or hidden in-memory backend behavior. - **What**: Migrates the current queue overlay and missing-media runtime specs onto the new jobs route fixture. - **What**: Keeps `./browser_tests/tsconfig.json` in the ESLint TypeScript resolver config. - **Dependencies**: None. ## Review Focus This is intended to be the foundation PR for the test-strategy reset: old stateful helper out, typed declarative route mocks in. It intentionally does not add the full asset sidebar, job history sidebar, or floating QPO coverage suite; those should stack on top after this fixture shape is accepted. The boundary this PR is trying to preserve: route mocks may describe frontend-visible API responses, but should not implement Core queue/history mutation semantics. Context: https://www.notion.so/comfy-org/E2E-Test-Strategy-for-Assets-Job-History-and-Queue-Progress-Overlay-35f6d73d365081209bc5f10e6c7eb8de ## Screenshots (if applicable) Not applicable.	2026-05-14 20:09:48 +00:00
Christian Byrne	e972d658d3	feat(settings): lower Comfy.Pointer.ClickBufferTime default from 150ms to 32ms (#12032) ... *PR Created by the Glary-Bot Agent* --- ## Summary Click vs drag is disambiguated by two thresholds: distance (`Comfy.Pointer.ClickDrift`, 6px) and time (`Comfy.Pointer.ClickBufferTime`). Distance does the real work — any intentional drag immediately exceeds 6px on the first move. The time threshold only matters in the corner case where the pointer is held still then released without crossing the distance threshold. The previous 150ms default forces every pointerdown to wait up to 150ms before drag begins, even when the user is clearly dragging. This is visible as lag when click+dragging an unselected node, where the wait stacks on top of selection-state work that runs at drag start ([FE-558](https://linear.app/comfyorg/issue/FE-558/bug-delay-when-clickdragging-an-unselected-node)). 32ms (~2 frames at 60fps) is well below human-perceptible click latency and still safely above incidental jitter on pointerdown. ## Changes - `src/platform/settings/constants/coreSettings.ts` — `Comfy.Pointer.ClickBufferTime`: - `defaultValue: 150` → `32` - `versionAdded: '1.4.3'` retained, add `versionModified: '1.44.19'` - Slider `step: 25` → `1` (the old step made `32` unrepresentable on the slider) - Tooltip extended with rationale (why distance is the real disambiguator, why this should be small) - `src/lib/litegraph/src/CanvasPointer.ts` — Static `bufferTime = 150` → `32` for consistency with the user-setting default; the runtime `watchEffect` in `useLitegraphSettings.ts` continues to override at boot. JSDoc explains the rationale and that the user setting overrides at runtime. ## Verification - `pnpm typecheck` clean. - Litegraph + settings test suites: 998/998 pass. - `pnpm format` clean (lint-staged enforced on commit). ## Why not just leave it user-overridable? It already is. But the default is what every user — including ones who never open settings — experiences. 150ms was never a justified value (no comment explains it, no benchmark, and 6px distance covers the disambiguation). Defaults should reflect best UX. Refs FE-558. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12032-feat-settings-lower-Comfy-Pointer-ClickBufferTime-default-from-150ms-to-32ms-3586d73d365081f5a29cdc84b4b736e3) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com>	2026-05-14 20:06:51 +00:00
Christian Byrne	f090ea3d28	fix: preserve app builder inputs through graph reconfiguration (#11422) ... *PR Created by the Glary-Bot Agent* --- ## Summary - Fixes app mode bug where custom combo inputs (and other widget inputs) unselect and show "Widget not visible" after refreshing or reopening a saved app workflow - Root cause: `resetSelectedToWorkflow()` loads from `changeTracker.activeState` which may not have linearData yet after refresh, and `pruneLinearData()` prunes valid entries during graph loading when nodes aren't yet in the graph - Two defensive guards: fallback to `initialState` for authoritative data, and skip pruning during graph loading ## Changes - `appModeStore.ts`: `resetSelectedToWorkflow()` now falls back to `initialState.extra.linearData` when `activeState` has none - `appModeStore.ts`: `pruneLinearData()` skips node-existence checks when `ChangeTracker.isLoadingGraph` is true - Unit tests: 4 new tests covering both fix paths (pruning during loading, fallback to initialState) - E2E test: Save-as → close → reopen → verify all inputs persist with no "Widget not visible" ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11422-fix-preserve-app-builder-inputs-through-graph-reconfiguration-3476d73d36508166a563f7df3967665c) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-05-14 12:28:21 -07:00
Christian Byrne	b7fe0365af	fix: remove "400 Free Credits Monthly" line from cloud paywall modal (#12251) ... *PR Created by the Glary-Bot Agent* --- ## Summary The in-app cloud paywall modal shown on first install of ComfyUI Desktop 1.0 advertised "400 Free Credits Monthly" as a benefit, but the free tier is currently disabled, making that copy misleading. Per the thread direction, this skips the feature-flag plumbing (which isn't available on Desktop anyway) and simply removes the line. ## Changes - `CloudNotificationContent.vue`: change the feature loop from `n in 4` to `n in [2, 3, 4]` so feature1 is skipped. - `src/locales/*/main.json` (12 locales): remove the now-unused `cloudNotification.feature1Title` key. - `browser_tests/tests/dialog.spec.ts`: add a regression assertion that "400" / "Free Credits" copy is no longer present in the modal. ## Test plan - `pnpm typecheck` — passes - `pnpm typecheck:browser` — passes - `pnpm exec vitest run src/platform/cloud/notification/components/DesktopCloudNotificationController.test.ts` — 3/3 pass - Pre-commit hooks (oxfmt, oxlint, eslint, stylelint, typecheck) — all pass - Manual: triggered `showCloudNotification()` against the dev server and confirmed only 3 benefit rows render (screenshot attached); the "400 Free Credits Monthly" line is gone. - Fixes DESK2-90 ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12251-fix-remove-400-Free-Credits-Monthly-line-from-cloud-paywall-modal-3606d73d365081eaa572e6cd995278d8) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>	2026-05-13 22:39:32 -07:00
jaeone94	a3106c4d53	fix: open node info panel from context menu (#12205) ... ## Summary Replaces #12164. Right-clicking a Vue node, using the selection toolbox More Options menu, or clicking the selection toolbox Node Info button now opens the right-side Info tab only when the new-menu UI makes that panel available. Legacy-menu contexts hide the no-op action even when the legacy node library design is selected; node-library help remains isolated to the node library itself. The existing `selection_toolbox_node_info_opened` telemetry fires only after the toolbox button successfully opens node info. No new context-menu telemetry event is added in this PR. ## Changes - **What**: Share the node-info availability/action path across the context menu and selection toolbox, keep legacy-menu state out of the right-side panel public store API, tighten node-info settings tests, and add unit plus E2E regression coverage for new-menu and legacy-menu modes. - **Dependencies**: None ## Review Focus Confirm the node context menu, selection toolbox direct Info button, and selection toolbox More Options entry all respect right-side panel availability, including legacy menu + legacy node library mode, while node-library help behavior remains isolated to the node library. ## Validation - Self-review: checked production path, unit mocks, and Playwright coverage; only gap found was weak E2E coverage for the toolbox direct Info path, now strengthened. - `pnpm test:unit -- src/composables/graph/useSelectionState.test.ts src/components/graph/SelectionToolbox.test.ts src/components/graph/selectionToolbox/InfoButton.test.ts` - `pnpm test:browser:local -- --project=chromium browser_tests/tests/selectionToolboxActions.spec.ts browser_tests/tests/selectionToolboxSubmenus.spec.ts browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts --grep "info button opens the right-side info tab|info button is hidden|hides Node Info|should open node info"` - `pnpm typecheck:browser` - `pnpm exec oxlint --type-aware browser_tests/tests/selectionToolboxActions.spec.ts` - `pnpm exec eslint --cache --no-warn-ignored browser_tests/tests/selectionToolboxActions.spec.ts` - `pnpm exec oxfmt --check browser_tests/tests/selectionToolboxActions.spec.ts` - `git diff --check` - Commit hooks: lint-staged + `pnpm typecheck` + `pnpm typecheck:browser` - Push hook: `knip --cache` (existing tag hint only) ## Screenshots (if applicable) Before https://github.com/user-attachments/assets/4b1f6ddb-a01c-4958-81ab-36167f434e59 https://github.com/user-attachments/assets/83433f0d-24f1-46b7-a81d-f0f065812496 After https://github.com/user-attachments/assets/30bd61e5-f8d4-48b7-97e0-26c93e3cb362 https://github.com/user-attachments/assets/afce9f51-a43d-434f-a006-6b357a61ac8f --------- Co-authored-by: github-actions <github-actions@github.com>	2026-05-14 02:26:11 +00:00
jaeone94	95e616b894	fix: clear media upload errors via widget change (#12212) ... ## Summary Clear missing media validation errors after paste/drop media uploads by emitting the existing widget-change event path. ## Changes - **What**: Emit `node.onWidgetChanged` after image/video upload completion updates the file combo widget. - **What**: Emit the same widget-change path after Load Audio upload completion. - **What**: Add unit coverage for upload completion emitting `onWidgetChanged` and for missing media clearing through that existing hook path. - **What**: Add E2E coverage for Load Image drag/drop and paste clearing validation rings, with red/green verified from a fresh `main` base. - **Dependencies**: None. ## Review Focus Please check that paste/drop upload paths now reuse the existing widget-change error-clearing path instead of expanding `widget.callback` patching. Also check the Load Image E2E helper path for synthetic paste/drop behavior. Supersedes #12207. Ref: FE-687 ## Screenshots Before https://github.com/user-attachments/assets/2cee52bc-b1c8-4dff-8a02-5b18a69ae639 After https://github.com/user-attachments/assets/e1ecd147-1d8a-470e-b77d-13345d473ef3 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12212-fix-clear-media-upload-errors-via-widget-change-35f6d73d365081bcb1a0dfc042d417eb) by [Unito](https://www.unito.io)	2026-05-14 01:28:41 +00:00
jaeone94	ad63f7cb9b	test: cover missing media runtime sources (#12126) ... ## Summary Adds browser coverage for the missing-media runtime paths introduced by #12069 and #12111: - OSS: annotated `[output]` media is resolved from job history. - Cloud: compact `[output]` media is resolved from output assets. - OSS and Cloud: dropped video uploads do not surface a missing-media error while the upload is still in progress. This PR is now rebased directly onto `main`; the parent fix PRs have been squash-merged, so this branch only contains the E2E coverage commit. ## Test Fixtures - Adds workflow fixtures for OSS spaced output annotations and Cloud compact output annotations. - Adds a small plain MP4 fixture for video drag/drop upload coverage. ## Validation - `pnpm exec oxfmt --check browser_tests/tests/propertiesPanel/errorsTabMissingMediaRuntime.spec.ts browser_tests/assets/missing/missing_media_cloud_output_annotation.json browser_tests/assets/missing/missing_media_output_annotations.json` - `pnpm typecheck:browser` - `pnpm exec oxlint browser_tests/tests/propertiesPanel/errorsTabMissingMediaRuntime.spec.ts --type-aware` - `git diff --check origin/main..HEAD` Note: before the rebase, the Cloud project target for this spec passed locally. Local OSS project execution against the currently running Cloud dist did not reach the test body because `ComfyPage.waitForAppReady` timed out in `beforeEach`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12126-test-cover-missing-media-runtime-sources-35d6d73d365081f0a981c02f33c0ff84) by [Unito](https://www.unito.io)	2026-05-13 22:46:47 +00:00
AustinMroz	9cc09cd46c	Add additional subgraph test fixtures and tests (#11806) ... - Adds functions to SubgraphHelper to perform widget promotion by standard user means - Right Click -> Promote - Properties Panel - Adds new slot fixture code that works with simple `locator.dragTo` operations. - Adds multiple subgraph tests with a focus on historically difficult operations. - Fixes a bug where the litegraph `node.selected` state would not be unset when switching graphs. This made it so 'Selecting a node -> leaving subgraph -> re-enter subgraph -> right click on node' would fail to select the node because it is marked as already selected. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11806-Add-helper-functions-for-widget-promotion-3536d73d365081f58dd9cd730c1a91a9) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>	2026-05-13 20:35:57 +00:00
pythongosssss	de1c1ee1f2	fix: add support for parsing python generated json with NaN/infinite (#12217) ... ## Summary API and other legacy JSON generated by python `json.dumps` can contain `NaN` and `Infinity` which cannot be parsed with JS `JSON.parse`. This adds regex to replace these invalid tokens with `null`. ## Changes - **What**: - add regex replace on bare NaN/infinity tokens after JSON.parse fails - update call sites - tests ## Review Focus - The regex should only rewrite bare NaN/-Infinity/Infinity and not touch string values or other invalid tokens. - A small regex was chosen over JSON5 due to package size (30.3kB Minified, 9kB Minified + Gzipped) or a manual parser due to the unnecessarily complexity vs a single regex replace. - The happy path is run first, the safe parse is only executed if that failed, meaning no overhead the vast majority of the time and no possiblity of corrupting valid workflows due to a bug in the fallback parser - Multiple call sites had to be updated due to pre-existing architecture of the various parsers, an issue for unifying these is logged for future cleanup - New binary fixtures added for validating e2e import using real files ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12217-fix-add-support-for-parsing-python-generated-json-with-NaN-infinite-35f6d73d365081889fc7f4af823f29c1) by [Unito](https://www.unito.io)	2026-05-13 20:33:19 +00:00