Commit Graph

5474 Commits

Author SHA1 Message Date
dante01yoon
d577c0d048 Merge remote-tracking branch 'origin/main' into fix/secrets-date-invalid
# Conflicts:
#	src/utils/dateTimeUtil.test.ts
2026-04-21 09:03:20 +09:00
pythongosssss
9ed7a7bd87 test: Add tests for help center (#11475)
## Summary

Test coverage for help center & associated popups

## Changes

- **What**: 
- Adds HelpCenterHelper for mocking endpoints and locators
- Tests for popup, menu items & positioning

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11475-test-Add-tests-for-help-center-3486d73d365081af91a2eb7465e503fe)
by [Unito](https://www.unito.io)
2026-04-20 22:43:28 +00:00
dante01yoon
db786e2d2b fix: normalize short ISO fractional seconds 2026-04-21 06:44:04 +09:00
Dante
78630f5485 test: add WidgetRange unit tests (#11471)
## Summary

Splits the WidgetRange test coverage out of #11446 so this widget can be
reviewed independently.

## Changes

- **What**: Adds WidgetRange unit tests covering value pass-through,
display propagation, disabled-state handling, upstream overrides, and
histogram derivation.

## Review Focus

Focused test-only PR extracted from #11446.
Validated with `pnpm test:unit -- --run
src/components/range/WidgetRange.test.ts`.

## Screenshots (if applicable)

N/A

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11471-test-add-WidgetRange-unit-tests-3486d73d365081d7a684ca3ff02320d6)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-04-20 20:38:10 +00:00
Christian Byrne
4b5c15fc7d fix: show credits in legacy user popover on non-cloud distributions (#11463)
*PR Created by the Glary-Bot Agent*

---

## Summary

Credits no longer showed in the current user popover on local/desktop
builds. Root cause: the credits row in `CurrentUserPopoverLegacy.vue`
was gated behind `isCloud && isActiveSubscription`, and `isCloud` is a
compile-time constant that resolves to `false` on local
(`DISTRIBUTION='localhost'`) — so the element never rendered and
`fetchBalance()` never fired (no network request, no console logs).

This fix decouples the credits balance row from the `isCloud` gate.
Subscription-specific UI (subscribe button, partner nodes, plans &
pricing, manage plan, upgrade-to-add-credits) remains gated by `isCloud`
as intended by PR #9958.

## Changes

- `CurrentUserPopoverLegacy.vue`: credits row `v-if` changed from
`isCloud && isActiveSubscription` → `isActiveSubscription`. On
non-cloud, `isActiveSubscription` resolves to `true` via
`isSubscribedOrIsNotCloud` in `useSubscription.ts`, so credits display
for logged-in users.
- `CurrentUserPopoverLegacy.vue`: `upgrade-to-add-credits` button now
requires `isCloud && isFreeTier` (subscription-tier concept only
meaningful on cloud). The `add-credits` top-up button remains available
everywhere.
- `CurrentUserPopoverLegacy.test.ts`: updated non-cloud tests to assert
credits balance is visible and add-credits button renders, while
upgrade-to-add-credits and other subscription UI stay hidden.

Mirrors the behavior of `CurrentUserPopoverWorkspace.vue`, which never
had the `isCloud` gate on its credits row.

## Verification

- `pnpm vitest run
src/components/topbar/CurrentUserPopoverLegacy.test.ts`: **21/21
passing**, including new non-cloud assertions
- `pnpm typecheck`: clean
- `pnpm lint` / `pnpm format:check`: clean
- Live frontend dev server renders on localhost with
`__DISTRIBUTION__='localhost'` (the previously-failing scenario).
Attached screenshot shows the app running on local distribution; the
popover itself only appears for logged-in users, so its contents are
exercised by the unit tests.

Fixes FE-219

## Screenshots

![Frontend running on localhost distribution
(__DISTRIBUTION__='localhost'), the previously-failing
scenario](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/7e49ca118370224f2a9be2db5b71b2ed78e095b999031b2cd040af1cf7a208f0/pr-images/1776661075381-a367eb49-a8f9-4737-be58-28b63a27f931.png)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11463-fix-show-credits-in-legacy-user-popover-on-non-cloud-distributions-3486d73d365081c587d8ee7eae9a5c3d)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-20 20:17:48 +00:00
Christian Byrne
2f4116fa81 test: add unit tests for numberUtil and dateTimeUtil (#11253)
## Summary

Adds unit tests for two untested utility modules to improve coverage:

- **`numberUtil.ts`** — `clampPercentInt`, `formatPercent0` (clamping,
rounding, locale formatting)
- **`dateTimeUtil.ts`** — `dateKey`, `isToday`, `isYesterday`,
`formatShortMonthDay`, `formatClockTime`

20 new tests total. This PR also serves as an E2E validation of the
coverage Slack notification workflow (#10977) — merging should trigger a
Slack notification showing the coverage improvement.

## Test Plan

- `pnpm test:unit -- src/utils/numberUtil.test.ts
src/utils/dateTimeUtil.test.ts`
- All 20 tests pass locally

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11253-test-add-unit-tests-for-numberUtil-and-dateTimeUtil-3436d73d365081aab388fd1f1fcac7d7)
by [Unito](https://www.unito.io)
2026-04-20 18:53:11 +00:00
pythongosssss
35bfe509b3 test: add/update terminal tests (#11239)
## Summary

Adds test coverage for the integrated terminal

## Changes

- **What**: 
- refactor and simplify existing tests
- add new tests for xterm integration

┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11239-test-add-update-terminal-tests-3426d73d365081c99445c35d8808afb4)
by [Unito](https://www.unito.io)
2026-04-20 10:11:37 +00:00
Christian Byrne
5d98e11ba1 feat: enable queue panel v2 by default on nightly builds (#11376)
*PR Created by the Glary-Bot Agent*

---

## Summary
- Changes the `Comfy.Queue.QPOV2` setting's `defaultValue` from `false`
to `isNightly`
- On nightly builds, users get the docked job history/queue panel (v2)
by default
- On stable builds, behavior is unchanged (v1 floating overlay remains
default)
- Users can still toggle the setting manually regardless of build type

## Pattern
Follows the existing pattern used by `Comfy.VueNodes.Enabled` which uses
`isCloud || isDesktop` as its version-conditional default. This is a
compile-time constant from `@/platform/distribution/types`.

## Context
Part of a dual-variant audit to graduate experimental features. QPO v2
has 0 extension ecosystem dependencies (confirmed via GitHub
codesearch), making nightly default-on safe for gathering feedback
before promoting to all users.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11376-feat-enable-queue-panel-v2-by-default-on-nightly-builds-3466d73d36508140b814d1d684acacba)
by [Unito](https://www.unito.io)

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-20 10:01:48 +00:00
Christian Byrne
60c7471818 feat: enable node replacement by default (#11439)
*PR Created by the Glary-Bot Agent*

---

## Summary

Enable node replacement suggestions by default so users see Quick Fix
options for deprecated/renamed nodes without toggling an experimental
setting.

- Change `Comfy.NodeReplacement.Enabled` default from `false` to `true`
and remove `experimental` flag
- Add `versionModified` metadata for release tracking
- No breaking change — users who previously disabled this setting keep
their preference

## Safety gates

This is an intentional global rollout, gated by two additional
server-side checks:

1. Server must provide `node_replacements` feature flag as true (PostHog
controlled)
2. `GET /api/node_replacements` must return data (cloud PR
Comfy-Org/cloud#2686)

Without both, changing this default alone has no effect. The three gates
ensure safe rollout.

## Companion PRs

- Comfy-Org/cloud#2686 — backend `GET /api/node_replacements` endpoint +
server-side validation bypass

Replicate of #11246, retargeted to `main` for backport automation.

Labels: `needs-backport`, `cloud/1.42`, `cloud/1.43`, `core/1.42`,
`core/1.43`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11439-feat-enable-node-replacement-by-default-3486d73d36508192b77aea9640986106)
by [Unito](https://www.unito.io)

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-20 02:16:54 +00:00
Dante
feafdc0b4a fix: chain Load3D node lifecycle callbacks to preserve widget cleanup (#11359)
## Summary

Undo on a workflow with an interactive 3D/camera node (e.g. Qwen
MultiAngle Camera) broke the interactive UI: it disappeared for Vue
Nodes 2.0 and desynced for LiteGraph.

Root cause: `initializeLoad3d` in `useLoad3d.ts` assigned
`node.onRemoved`, `node.onResize`, and the other node lifecycle handlers
by direct assignment, overwriting the cleanup chain that `addWidget()`
had already appended during node construction (line `node.onRemoved =
useChainCallback(node.onRemoved, () => widget.onRemove?.())` in
`domWidget.ts`). When undo cleared the graph, `widget.onRemove` never
ran, so the component widget stayed in `domWidgetStore` pointing at a
detached element while new nodes registered fresh widgets at the same
UUID keys.

Fix: wrap all of those assignments with `useChainCallback` so earlier
subscribers (widget registration, badge composables, extension
nodeCreated hooks) continue to fire.

- Fixes FE-214
(<https://linear.app/comfyorg/issue/FE-214/undo-breaks-and-desyncs-qwen-multiangle-camera-ui>)

## Red-Green Verification

| Commit | CI Status | Purpose |
|--------|-----------|---------|
| `test: add failing test for FE-214 undo losing Load3D widget callback
chain` | 🔴 Red | Proves the test catches the bug |
| `fix: chain Load3D node lifecycle callbacks to preserve widget
cleanup` | 🟢 Green | Proves the fix resolves the bug |

## Test Plan

- [ ] CI red on test-only commit
- [ ] CI green on fix commit
- [ ] Manual: load Qwen MultiAngle Camera workflow, mutate camera, press
Ctrl+Z, confirm interactive UI stays mounted and value reflects restored
state (Vue Nodes 2.0 and LiteGraph)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11359-fix-chain-Load3D-node-lifecycle-callbacks-to-preserve-widget-cleanup-3466d73d365081e2b64de65c26ee6abf)
by [Unito](https://www.unito.io)
2026-04-20 01:55:44 +00:00
Christian Byrne
2fea0aa538 fix: trigger Vue reactivity on output slot type changes in matchType (#9935)
## Summary

Fix VHS unbatch output slot color not updating when slot types change
via matchType resolution in Vue renderer.

## Changes

- **What**: After `changeOutputType` mutates `output.type` on objects
inside a `shallowReactive` array, spread-copy `this.outputs` to trigger
the shallowReactive setter so `SlotConnectionDot` re-evaluates the slot
color.

## Review Focus

The fix adds `this.outputs = [...this.outputs]` after the matchType
resolution loop in `withComfyMatchType`. This forces Vue's
shallowReactive proxy to fire, since mutating a property on an object
inside the array doesn't trigger the setter. The spread is placed after
all outputs are updated to batch the reactivity trigger.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9935-fix-trigger-Vue-reactivity-on-output-slot-type-changes-in-matchType-3246d73d365081c4a293f57931892c61)
by [Unito](https://www.unito.io)
2026-04-20 01:51:08 +00:00
Christian Byrne
0638e8e993 test: add unit tests for SceneModelManager (#11392)
## Summary

Add 44 unit tests for `SceneModelManager` in the 3D viewer
(`src/extensions/core/load3d/`).

## Changes

- **What**: New test file `SceneModelManager.test.ts` covering
constructor, dispose, createSTLMaterial, addModelToScene, setupModel,
setOriginalModel, clearModel, reset, setMaterialMode (all 5 modes),
setupModelMaterials, setUpDirection (all 7 directions), hasSkeleton,
setShowSkeleton, containsSplatMesh, and PLY mode switching (point cloud,
wireframe, vertex colors, cleanup).

## Review Focus

- Test coverage of PLY mode switching edge cases (vertex colors, old
model cleanup)
- Mock strategy for WebGLRenderer (happy-dom cannot instantiate it)
- SplatMesh mock leverages the existing global mock in `vitest.setup.ts`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11392-test-add-unit-tests-for-SceneModelManager-3476d73d3650819097f3f6d73d8fbe02)
by [Unito](https://www.unito.io)
2026-04-19 20:48:34 -04:00
Dante
07ce7123c8 test: cover useErrorActions and useErrorReport (#11320)
Closes coverage gaps in \`src/components/rightSidePanel/errors/\` as
part of the unit-test backfill.

## Testing focus

\`useErrorActions\` is thin (telemetry + command + \`window.open\`), but
\`useErrorReport\` is a real async watcher with multiple store
dependencies, \`@vueuse/core\`'s \`until(...)\`, and a cancellation
guard. The tricky part is keeping \`until\` reactive without mocking
\`@vueuse/core\`.

### \`useErrorActions\` (8 tests)

- Three functions × telemetry-fired × command/window invocation × the
\`telemetry?.\` null-safe branch.
- \`findOnGitHub\` encoding: verifies \`encodeURIComponent\` runs on the
error message and \` is:issue\` is appended.
- \`window.open\` stubbed via \`vi.spyOn\`, restored in \`afterEach\`.

### \`useErrorReport\` (9 tests)

- **Reactive \`until()\`.** \`@vueuse/core\` is **not** mocked. The
\`useSystemStatsStore\` mock creates real Vue \`ref\`s and exposes them
via getter/setter so \`until(() => isLoading).toBe(false)\` resolves
through actual reactivity.
- **\`__setSystemStats\` / \`__setIsLoading\` helpers** on the mocked
store let tests mutate state from the outside without leaking global
mutable state beyond \`vi.hoisted\`.
- **Cancellation guard.** Manually-resolvable deferred \`getLogs\`
promise — while it's pending, the \`cardSource\` ref is swapped. The
previous run's results must **not** mutate \`enrichedDetails\`.
Regressions here would cause race-dependent UI state when users switch
between error cards quickly.
- **Fallback paths.** Missing \`exceptionType\` →
\`FALLBACK_EXCEPTION_TYPE\` ('Runtime Error'). \`serialize()\` throws →
early return. \`generateErrorReport\` throws → \`displayedDetailsMap\`
falls back to the raw \`error.details\`.
- **Watcher cleanup.** Swapping the card ref clears stale
\`enrichedDetails\` before re-enrichment.
- \`console.warn\` spy suppresses noise; restored in \`afterEach\`.

## Principles applied

- No mocks of \`vue\` or \`@vueuse/core\` — only our own modules
(\`api\`, \`app\`, \`systemStatsStore\`, \`errorReportUtil\`).
- \`@vue/test-utils\` isn't installed; a local \`flushPromises\` helper
is used (matches the existing pattern in
\`useNodeHelpContent.test.ts\`).
- All 17 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
2026-04-20 08:49:36 +09:00
Dante
799ffcf4b6 test: cover useWorkspaceUI and useWorkspaceBilling (#11319)
Closes coverage gaps in \`src/platform/workspace/composables/\` as part
of the unit-test backfill.

## Testing focus

\`useWorkspaceUI\` is wrapped in \`createSharedComposable\` (shared
instance across all callers). \`useWorkspaceBilling\` is a large
stateful composable: parallel API calls, exponential-backoff polling,
computed mappers, lifecycle cleanup. Both need careful state isolation
and real lifecycle behavior — not faked hooks.

### \`useWorkspaceUI\` (8 tests)

- **Permission / UI-config matrix.** Three role/type combinations —
(personal × any), (team × owner), (team × member) — plus the
no-active-workspace default. Assertions target concrete flags that
differ per role (the table itself is the contract), not the return
shape.
- **\`createSharedComposable\` identity invariant.** Multiple calls
return the same instance.
- **Isolation.** Each test uses \`vi.resetModules()\` to get a fresh
shared instance so the memoization doesn't leak between cases.

### \`useWorkspaceBilling\` (23 tests)

- **Parallel init.** \`initialize\` runs \`Promise.all([status, balance,
plans])\` concurrently, then re-fetches balance when free-tier shows a
zero amount (lazy credit grant path).
- **Polling with fake timers.** \`cancelSubscription\`'s exponential
backoff (\`1000 * 2^attempt\`, max 5000ms) driven by
\`vi.useFakeTimers()\` + \`advanceTimersByTimeAsync()\`. Covers success,
failure, and the unmount-stops-polling case.
- **Real lifecycle.** \`onBeforeUnmount\` only fires inside a component
instance — not inside a raw \`effectScope\`. The unmount test mounts a
minimal Vue app via \`createApp\` / \`app.unmount\` so the production
cleanup path actually runs.
- **Computed getter mapping.** \`subscription\`, \`balance\`,
\`isActiveSubscription\`, \`isFreeTier\` assert the snake_case API shape
is remapped to the camelCase UI shape correctly.
- **\`window\` effects.** \`window.open\` stubbed via \`vi.spyOn\`,
\`window.location.href\` via \`vi.stubGlobal\`. Restored in
\`afterEach\`.

## Principles applied

- No mocks of \`vue\` or \`@vueuse/core\` — only our own workspace API,
stores, and sibling composables.
- Behavioral assertions only.
- All 31 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
2026-04-20 08:23:23 +09:00
Comfy Org PR Bot
1020e8cf32 1.44.5 (#11213)
Patch version increment to 1.44.5

**Base branch:** `main`

---------

Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
2026-04-19 12:37:51 -07:00
jaeone94
b157182a20 refactor: inline node footer layout to fix selection bounding box (#10741)
## Summary

Refactor node footer from absolute overlay to inline flow layout, fixing
the selection bounding box not encompassing footer buttons and collapsed
node dimensions.

## Background

The node footer (Enter Subgraph, Advanced, Error buttons) was rendered
as an absolute overlay (`absolute top-full`) outside the node body. This
caused:

1. **Selection bounding box** did not include footer height — the dashed
multi-select border cut through footer buttons
2. **Footer offset compensation** required 3 hardcoded computed classes
(`footerStateOutlineBottomClass`, `footerRootBorderBottomClass`,
`footerResizeHandleBottomClass`) with magic pixel values (31px, 35px,
etc.) that had to stay in sync with CSS

## Solution: Inline Footer with `isolate -z-1`

The footer is moved into normal document flow (no longer `absolute
top-full`). The key challenge was keeping the footer visually behind the
body's rounded bottom edge (the "tuck under" effect) without adding
`z-index` to the body — because adding `z-index` to the body creates a
stacking context that traps slot connection dots, making them appear
behind overlay borders.

The solution uses CSS `isolation: isolate` combined with `-z-1` on the
footer wrapper:

- **`isolate`** creates an independent stacking context for the footer,
so internal z-index (Error button `z-10` above Enter button) does not
leak to the parent
- **`-z-1`** places the entire footer behind the body (`z-index: auto`),
achieving the visual overlap without touching the body's stacking
behavior
- **Slot dots remain free** — the body has no explicit z-index, so slots
participate in the root stacking context and are never trapped behind
overlay borders

This eliminates all 3 footer offset computed classes and their hardcoded
pixel values.

## Selection Box: `min-height` on root + unified size path

Moving `min-h-(--node-height)` from the body (`node-inner-wrapper`) to
the root element makes the footer height naturally included in
`node.size` via ResizeObserver → layoutStore → litegraph sync. This
means `boundingRect` is automatically correct for expanded nodes — no
callbacks or overrides needed.

For collapsed nodes, a pre-existing issue (since v1.40) caused
`_collapsed_width` to fall back to `NODE_COLLAPSED_WIDTH = 80px` because
Vue nodes lack a canvas context for text measurement.

The fix lets collapsed dimensions flow through the **same**
`batchUpdateNodeBounds` path as expanded nodes — no parallel data
structure, no separate accessor, no cache:

1. ResizeObserver writes the collapsed DOM dimensions to
`layoutStore.size` via `batchUpdateNodeBounds`
2. `useLayoutSync` syncs `layoutStore.size` → `liteNode.size` as it does
for any other size change
3. The expanded size survives the collapse→expand round trip via CSS
custom properties — the `isCollapsed` watcher in `LGraphNode.vue` swaps
`--node-width` to `--node-width-x` on collapse and restores it on expand
4. `measure()` reads `this.size` directly for Vue collapsed nodes via a
one-line gate: `if (!this.flags?.collapsed || LiteGraph.vueNodesMode)`.
Legacy behavior is unchanged.

## Changes

- **NodeFooter.vue**: `absolute top-full` overlay → inline flow with
`isolate -z-1` wrappers, Error/Enter button layering via `-mr-5` + DOM
order, reactive props destructuring, static `RADIUS_CLASS` lookup for
Tailwind scanning, Vue 3.3+ `defineEmits` property syntax
- **LGraphNode.vue**: Move `min-h-(--node-height)` from body to root;
remove `footerStateOutlineBottomClass`, `footerRootBorderBottomClass`,
`footerResizeHandleBottomClass`, `hasFooter` computed; replace dynamic
`beforeShapeClass` interpolation with static
`bypassOverlayClass`/`mutedOverlayClass` computeds for Tailwind scanning
- **LGraphNode.ts**: `measure()` collapsed branch gated by `||
LiteGraph.vueNodesMode` — Vue mode defers to `this.size`; legacy path
unchanged
- **useVueNodeResizeTracking.ts**: Collapsed and expanded nodes both
flow through `batchUpdateNodeBounds`; narrowed `useVueElementTracking`
parameter from `MaybeRefOrGetter<string>` to `string`;
`deferredElements.delete(element)` on unmount to prevent memory
retention
- **selectionBorder.ts**: Unchanged — `createBounds` just works because
`boundingRect` is now correct
- **12 parameterized E2E tests**: Vue mode (subgraph/regular ×
expanded/collapsed × bottom-left/bottom-right) + legacy mode
(expanded/collapsed × bottom-left/bottom-right), driven by
`keyboard.collapse()` (Alt+C)
- **Unit tests**: `measure()` branching (legacy fallback, Vue
`this.size` usage, expanded parity)
- **Shared test helpers**: `repositionNodes`, `KeyboardHelper.collapse`,
`measureSelectionBounds`, `assertSelectionEncompassesNodes`

## Review Focus

- `isolate -z-1` CSS layering pattern — is this acceptable long-term?
- `measure()` collapsed branch gated on `LiteGraph.vueNodesMode` —
one-line gate to avoid the canvas-ctx-less fallback in Vue mode
- Footer button overlap design (`-mr-5` with DOM order for painting)

## Screenshots
<img width="1392" height="800" alt="image"
src="https://github.com/user-attachments/assets/abaebff5-bb8c-4b5b-8734-8d44fdee4cb9"
/>
<img width="1493" height="872" alt="image"
src="https://github.com/user-attachments/assets/6b9c77f9-e3ae-4d4e-81dc-acfa9a24c768"
/>
<img width="813" height="515" alt="image"
src="https://github.com/user-attachments/assets/ce15bafb-e157-408c-971b-a650088f316a"
/>
<img width="1031" height="669" alt="image"
src="https://github.com/user-attachments/assets/20fdc336-4bc2-4d47-ab7e-c0cbcee0d150"
/>
<img width="753" height="525" alt="image"
src="https://github.com/user-attachments/assets/2dccbe31-7d18-49bc-9ed4-158b1659fddf"
/>
<img width="730" height="370" alt="image"
src="https://github.com/user-attachments/assets/ab87edfa-a4b4-46f7-86ae-4965a4509b42"
/>
<img width="1132" height="465" alt="image"
src="https://github.com/user-attachments/assets/54643f5b-4a31-4c3d-9475-c433f87aedb0"
/>
<img width="1102" height="449" alt="image"
src="https://github.com/user-attachments/assets/9c045df3-e1f5-481e-b1cb-ead1db1626f5"
/>

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-04-19 04:58:34 +00:00
Hunter
4c35add5bc feat: add civitai.red hostname support (#11349)
*PR Created by the Glary-Bot Agent*

---

## Summary

Civitai split its domain — NSFW content moved to `civitai.red` while
`civitai.com` stays SFW. The frontend only recognized `civitai.com`
URLs, causing the import button to silently reject `.red` links. This
was the root cause of 8+ support tickets in 3 days.

Companion to backend PR: https://github.com/Comfy-Org/cloud/pull/3259

## Changes

### Import source recognition
- **`civitaiImportSource.ts`**: Added `'civitai.red'` to `hostnames`
array — this is the primary fix for "button doesn't recognize the links"

### Missing model auto-download
- **`missingModelDownload.ts`**: Added `'https://civitai.red/'` to
`ALLOWED_SOURCES`

### URL detection utilities
- **`formatUtil.ts`**: `isCivitaiModelUrl()` now accepts `civitai.red`
URLs with proper hostname validation
- **`assetMetadataUtils.ts`**: `getSourceName()` returns "Civitai" for
`.red` URLs

### Tests (4 files)
- `useUploadModelWizard.test.ts`: Added civitai.red hostnames and URL
test case
- `missingModelDownload.test.ts`: Added civitai.red cases for
`toBrowsableUrl` and `isModelDownloadable`
- `assetMetadataUtils.test.ts`: Added civitai.red case for
`getSourceName`
- `useMissingModelInteractions.test.ts`: Updated mock hostnames
- `formatUtil.test.ts`: Added civitai.red cases for `isCivitaiModelUrl`

## Not changed (intentionally)
- `getAssetSourceUrl()` ARN fallback (line 88) — ARNs don't carry domain
info, `civitai.com` is correct default
- `fetchCivitaiMetadata()` API URL (line 109) — REST API works on both
domains, keeping `civitai.com`

Resolves BE-353

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11349-feat-add-civitai-red-hostname-support-3456d73d3650810d9c62ef4ad95ae031)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Terry Jia <terryjia88@gmail.com>
2026-04-19 04:51:01 +00:00
Christian Byrne
a3893a593d refactor: move select components from input/ to ui/ component library (#11378)
*PR Created by the Glary-Bot Agent*

---

## Summary

Reconciles `src/components/input/` (older select components) into
`src/components/ui/` (internal component library), eliminating the
separate `input/` directory entirely.

## Changes

- **Move MultiSelect** →
`src/components/ui/multi-select/MultiSelect.vue`
- **Move SingleSelect** →
`src/components/ui/single-select/SingleSelect.vue`
- **Extract shared resources** → `src/components/ui/select/types.ts`
(SelectOption type) and `src/components/ui/select/select.variants.ts`
(CVA styling variants)
- **Update 7 consuming files** to use new import paths
- **Update 1 test file** (AssetFilterBar.test.ts mock paths)
- **Move stories and tests** alongside their components
- **Delete `src/components/input/`** directory

## Context

The `input/` directory contained only MultiSelect and SingleSelect — two
well-built components that already used the same stack as `ui/` (Reka
UI, CVA, Tailwind 4, Composition API). MultiSelect even imported
`ui/button/Button.vue`. Moving them into `ui/` removes the split and
consolidates all reusable components in one place.

No API changes — all component props, slots, events, and behavior are
preserved exactly.

## Verification

- `pnpm typecheck` 
- `pnpm build` 
- `pnpm lint` (stylelint + oxlint + eslint) 
- All 15 relevant tests pass (MultiSelect: 5, SingleSelect: 2,
AssetFilterBar: 8) 
- `pnpm knip` — no dead exports 
- No stale `@/components/input/` references remain 
- Pre-commit hooks pass 
- Git detected all moves as renames (97-100% similarity)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11378-refactor-move-select-components-from-input-to-ui-component-library-3476d73d3650810e99b4c3e0842e67f3)
by [Unito](https://www.unito.io)

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-18 20:00:34 -07:00
Terry Jia
deba72e7a0 gizmo controls (#11274)
## Summary
Add Gizmo transform controls to load3d

- Remove automatic model normalization (scale + center) on load; models
now appear at their original transform. The previous auto-normalization
conflicted with gizmo controls — applying scale/position on load made it
impossible to track and reset the user's intentional transform edits vs.
the system's normalization
- Add a manual Fit to Viewer button that performs the same normalization
on demand, giving users explicit control
- Add Gizmo Controls (translate/rotate) for interactive model
manipulation with full state persistence across node properties, viewer
dialog, and model reloads
- Gizmo transform state is excluded from scene capture and recording to
keep outputs clean

## Motivation
The gizmo system is a prerequisite for these potential features:
- Custom cameras — user-placed cameras in the scene need transform
gizmos for precise positioning and orientation
- Custom lights — scene lighting setup requires the ability to
interactively position and aim light sources
- Multi-object scene composition — positioning multiple models relative
to each other requires per-object transform controls
- Pose editor — skeletal pose editing depends on the same transform
infrastructure to manipulate individual bones/joints

Auto-normalization was removed because it silently mutated model
transforms on load, making it impossible to distinguish between the
original model pose and user edits. This broke gizmo reset (which needs
to know the "clean" state) and would corrupt round-trip transform
persistence.

## Screenshots (if applicable)

https://github.com/user-attachments/assets/621ea559-d7c8-4c5a-a727-98e6a4130b66

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11274-gizmo-controls-3436d73d365081c38357c2d58e49c558)
by [Unito](https://www.unito.io)
2026-04-18 22:45:06 -04:00
Rizumu Ayaka
3db0eac353 perf: textarea widget layer composition (#10804)
## Summary

I noticed that nodes using textarea for user input, which contain long
user-entered text, require scrolling within a single node.

Having 40 such textarea nodes in a test canvas is enough to cause lag
(20fps). In contrast, a control group using regular nodes can handle up
to 500 nodes without lag (60fps).

the numerous scrolling text widgets in test workflows are the main
source of performance pressure. Each scrolling text input box imposes
independent layout and layering pressure.

I initially tried more complex solutions to fix this issue, like virtual
scrolling. However, I found that a simple CSS modification was
sufficient and effective. Even when I quadrupled the problematic number
of nodes on my M5 MacBook Air, it remained smooth.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10804-perf-textarea-widget-layer-composition-3356d73d3650814da75adec266d7cad9)
by [Unito](https://www.unito.io)

---------

Co-authored-by: bymyself <cbyrne@comfy.org>
2026-04-19 02:29:00 +00:00
Dante
4c7729ee0b fix: remove hover dimming overlay on image nodes (#11296)
## Summary

Remove the black opacity/dimming overlay on image node hover and add
shadows to action buttons for visibility against light backgrounds.

## Changes

- **What**: Remove `opacity-50` dimming on hover in
`DisplayCarousel.vue`, remove `transition-opacity hover:opacity-80` from
grid thumbnails in `ImagePreview.vue`, add `shadow-md` to action buttons
in `ImagePreview.vue`. Applies to Save Image, Load Image, Preview Image,
and all nodes using these shared image components.

## Review Focus

Button shadows (`shadow-md`) should provide sufficient contrast against
light image backgrounds without needing the dimming overlay.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11296-fix-remove-hover-dimming-overlay-on-image-nodes-3446d73d36508193bb5cc27d431014fd)
by [Unito](https://www.unito.io)
2026-04-18 22:40:11 +00:00
Dante
40083d593b test: cover Button, Textarea, Slider components (#11325)
Closes coverage gaps in \`src/components/ui/\` as part of the unit-test
backfill. Uses \`@testing-library/vue\` +
\`@testing-library/user-event\` for user-centric, behavioral assertions.

## Testing focus

Three Reka-UI primitives. The challenge is testing the contract — not
the library internals — given happy-dom's gaps and Reka's
\`useMounted()\`-based async initialization.

### \`Button\` (7 tests)

- Slot rendering + click event propagation.
- \`loading=true\`: three invariants hold **simultaneously** — slot
hidden, \`pi-spin\` spinner present, button is \`toBeDisabled()\`.
- \`disabled=true\` alone: button disabled, no spinner.
- \`as="a"\`: polymorphic root tag (Reka \`Primitive\`'s \`as\` prop
switches the rendered element).
- Variant class pass-through: **one** deliberate style assertion because
the variant-system wiring is part of the component's public contract. No
other styling/class checks (AGENTS.md bans class-based tests).

### \`Textarea\` (6 tests)

- \`v-model\` two-way binding: \`user.type()\` updates the bound ref;
initial value populates the textarea.
- \`disabled\` asserted **behaviorally** — typing is blocked when
disabled, not just the attribute presence.
- Pass-through: \`placeholder\`, \`rows\`, \`class\`.

### \`Slider\` (8 tests)

- Thumb count matches \`modelValue.length\` (range support).
- ARIA: \`aria-valuemin\` / \`aria-valuemax\` / \`aria-valuenow\`.
**Caveat:** Reka's \`SliderRoot\` uses \`useMounted()\`, so
\`aria-valuenow\` is absent on the first render tick. The tests use a
two-tick \`flush()\` helper (\`await nextTick()\` twice) to wait it out
— no mocking of Reka required.
- Keyboard drag: \`user.keyboard('{ArrowRight}')\` / \`'{ArrowLeft}'\`
moves the value; with \`step: 10\` starting from 50, ArrowRight produces
exactly \`[60]\`.
- \`disabled\` → no emit on keyboard events.

### Reka integration limit

Pointer-driven \`slide-start\` / \`slide-end\` gestures in happy-dom
would require faking \`getBoundingClientRect\` and \`setPointerCapture\`
— that crosses into mocking Reka internals. Keyboard-drag paths are
covered instead (the user-facing contract); the \`pressed\` CSS state is
exercised implicitly by surviving a full mount + update cycle.

## Principles applied

- No mocks of Vue, Reka, or \`@vueuse/core\`.
- Queries via \`getByRole\` / \`getByLabelText\`; **no** class-name or
Tailwind-token queries (per AGENTS.md).
- All 21 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
2026-04-18 22:36:16 +00:00
Dante
7089a7d1a0 fix: show asset display names in bulk delete confirmation (#11321)
## Summary
Bulk-delete confirmation on Comfy Cloud listed raw SHA-256 filenames,
making the modal impossible to use to verify what would be deleted.

## Changes
- **What**: `useMediaAssetActions.deleteAssets` now maps each asset
through `getAssetDisplayName`, so the confirmation's `itemList` matches
the user-assigned names shown in the left media panel
(`MediaAssetCard`).
- **Tests**: Added two regression tests covering `user_metadata.name` /
`display_name` resolution and the `asset.name` fallback.

## Review Focus
- Parity with `MediaAssetCard` display: we reuse the same
`getAssetDisplayName` helper; extension stripping (via
`getFilenameDetails`) is not applied in the modal since file extensions
are useful context when confirming deletions.

Reported in Slack:
https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1776383570015289

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11321-fix-show-asset-display-names-in-bulk-delete-confirmation-3456d73d36508108a3d5f2290ca39e18)
by [Unito](https://www.unito.io)
2026-04-18 22:35:39 +00:00
jaeone94
b756545f59 refactor: clean up ChangeTracker logging, guards, and redundant widget wrapper (#11328)
## Summary

Follow-ups to PR #10816. Bundles four review items left open after that
PR merged — three inside `ChangeTracker` itself and one in the widget
composable that wraps it.

### What changed

- **Removed all `loglevel` logging from `src/scripts/changeTracker.ts`**
— the logger was set to `info`, so every `logger.debug` call was dead
code at runtime. `logger.warn` calls were replaced with direct
reporting. The only-downstream dead code (`graphDiff` helper) and its
sole dependency (`jsondiffpatch`) are also removed.
- **Named the `captureCanvasState()` guard conditions** —
`isUndoRedoing` and `isInsideChangeTransaction` now carry the intent
that the inline `_restoringState` / `changeCount > 0` expressions used
to obscure.
- **Surfaced lifecycle violations through a single reporting helper** —
`reportInactiveTrackerCall()` logs `console.warn` once per method per
session and, on Desktop, emits a `Sentry.addBreadcrumb` with the
offending workflow path. `deactivate()` and `captureCanvasState()` share
this path so the same invariant is reported consistently.
- **Inlined `captureWorkflowState` wrapper in `useWidgetSelectActions`**
— the private helper forwarded to `changeTracker.captureCanvasState()`
with no added logic. Both call sites now invoke the change tracker
directly.

### Issues fixed

- Fixes #11249
- Fixes #11259
- Fixes #11258
- Fixes #11248

### Test plan

- [x] `pnpm test:unit src/scripts/changeTracker.test.ts` — 16 tests pass
- [x] `pnpm test:unit
src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectActions.test.ts`
— 6 tests pass
- [x] `pnpm typecheck`
- [x] `pnpm lint`
- [x] `pnpm format`
2026-04-18 22:28:05 +00:00
Alexander Brown
da91bdc957 fix: persist middle-click reroute node setting across reloads (#11362)
*PR Created by the Glary-Bot Agent*

---

## Summary

- Remove hardcoded `LiteGraph.middle_click_slot_add_default_node = true`
from `slotDefaults` extension `init()` that unconditionally overrode the
user's persisted preference on every page load
- Add E2E regression test verifying both the setting store value and the
LiteGraph runtime flag persist through page reload

## Root Cause

The `Comfy.SlotDefaults` extension's `init()` method (in
`slotDefaults.ts`) contained a hardcoded
`LiteGraph.middle_click_slot_add_default_node = true` from the original
JS→TS conversion (July 2024). When `Comfy.Node.MiddleClickRerouteNode`
was later made configurable in v1.3.42, this line was never removed.
Since extension `init()` runs **after** `useLitegraphSettings()` syncs
the stored value, the hardcoded assignment overwrote the user's
preference on every reload.

## Changes

| File | Change |
|------|--------|
| `src/extensions/core/slotDefaults.ts` | Remove line 21
(`LiteGraph.middle_click_slot_add_default_node = true`) |
| `browser_tests/tests/dialogs/settingsDialog.spec.ts` | Add reload
persistence test asserting both store value and LiteGraph global |

The setting default (`true`) is already properly managed by
`coreSettings.ts` and reactively synced via `useLitegraphSettings.ts`,
so removing the hardcoded line preserves existing default behavior while
allowing user overrides to persist.

## Screenshots

![Setting shown as enabled (default
state)](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/a820b6dee65aa3491b51c6e86d1e803bdf53309234e9591bd78b5a7c83d4684c/pr-images/1776528970358-dcd6bd51-00c8-4ed4-86ce-0f1a89576f52.png)

![Setting toggled off by
user](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/a820b6dee65aa3491b51c6e86d1e803bdf53309234e9591bd78b5a7c83d4684c/pr-images/1776528970719-fb1f587f-964d-4e6c-954e-3145812badaf.png)

![Setting correctly persists as off after page reload (with fix
applied)](https://pub-1fd11710d4c8405b948c9edc4287a3f2.r2.dev/sessions/a820b6dee65aa3491b51c6e86d1e803bdf53309234e9591bd78b5a7c83d4684c/pr-images/1776528971113-36b577cb-5fd1-445d-8c8f-3ea8f6f46326.png)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11362-fix-persist-middle-click-reroute-node-setting-across-reloads-3466d73d365081ef8692dbd0619c8594)
by [Unito](https://www.unito.io)

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-18 21:29:44 +00:00
pythongosssss
be2d757c47 test: add regression test for getCanvasCenter null guard (#8399) (#11271)
## Summary

Add a regression test for #8399 (null check in `getCanvasCenter` to
prevent crash on asset insert). The fix in
`src/services/litegraphService.ts` added optional chaining around
`app.canvas?.ds?.visible_area` with a `[0, 0]` fallback so inserting an
asset before the canvas finishes initializing no longer crashes. There
was no existing unit test for `litegraphService`, so this regression
could silently return.

## Changes

- **What**: New unit test file `src/services/litegraphService.test.ts`
covering `useLitegraphService().getCanvasCenter`.
- Mocks `@/scripts/app` so `app.canvas` can be swapped per test via
`Reflect.set`.
- Null-canvas case (regression for #8399): returns `[0, 0]` instead of
throwing.
- Missing `ds.visible_area` case: also returns `[0, 0]`.
- Initialised case: returns the centre of the visible area.
- Verified RED→GREEN locally.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11271-test-add-regression-test-for-getCanvasCenter-null-guard-8399-3436d73d3650815c9925c8fdf9ec4bd3)
by [Unito](https://www.unito.io)
2026-04-18 16:32:03 +00:00
dante01yoon
3277805d4e test: cover full fractional-second range and invalid last_used_at 2026-04-18 19:38:27 +09:00
dante01yoon
dc198fb844 fix: handle ISO timestamps with >3 fractional-second digits in Secrets panel
The backend emits RFC 3339 timestamps via Go's time.RFC3339Nano, which
produces variable fractional-second precision (e.g. "…55.6513Z"). The
ECMA-262 Date grammar only accepts exactly 3 digits, so stricter
parsers (older Safari, some WebViews) return Invalid Date, which
toLocaleDateString() stringifies as literal "Invalid Date".

Add parseIsoDateSafe() that trims the fractional portion to millisecond
precision before parsing and returns null for unparseable input. Use
it in SecretListItem and hide the Created / Last Used line when the
value is missing or invalid instead of rendering "Invalid Date".
2026-04-18 19:32:12 +09:00
Kelly Yang
6dba67da6b refactor: remove @ts-expect-error suppressions in sidebar components (#11338)
…(issue #11092 phase 4a)

## Summary
Part of #11092 — Phase 4a: remove 10 @ts-expect-error suppressions from
three sidebar component files.
## Changes
3 files in the sidebar had `@ts-expect-error` suppressions that all
traced back to the same root cause: **optional properties on generic
interfaces that TypeScript cannot narrow through indirect conditions.**

`TreeExplorerNode<T>` declares `data?: T` — optional by design, since
folder nodes may carry no payload. Every `handleClick`, `handleDrop`,
and `handleDelete` method that accessed `this.data` was relying on the
runtime invariant that leaf nodes always have data, but TypeScript has
no way to derive `data !== undefined` from `this.leaf === true`. The fix
was to make the invariant explicit in the condition (`if (this.leaf &&
this.data)`) or add an early-return guard (`if (!nodeDefToAdd) return`).

The same pattern appeared in a closure in `ModelLibrarySidebarTab.vue`:
`model` was `ComfyModelDef | null` from an outer const, and `if
(this.leaf)` inside a method cannot narrow a captured variable. Widening
the condition to `if (this.leaf && model)` resolved it. Two additional
suppressions in that file covered `addNodeOnGraph`'s nullable return and
its optional `widgets` property, both fixed with optional chaining.

The remaining suppression was an unannotated function parameter inferred
as `any`; adding the explicit type from the `filters` ref removed it.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: changes are TypeScript-safety refactors (extra
null/undefined guards) plus new unit tests; runtime behavior should only
differ in edge cases where `data`/`model`/`widgets` are unexpectedly
missing.
> 
> **Overview**
> Removes several `@ts-expect-error` suppressions in sidebar library
tabs by making leaf-node invariants explicit (`if (this.leaf &&
this.data/model)`), adding early returns for missing drag-drop payloads,
and using optional chaining for nullable `addNodeOnGraph`/`widgets`
access.
> 
> Adds new Vitest coverage for `ModelLibrarySidebarTab`,
`NodeLibrarySidebarTab`, and `NodeBookmarkTreeExplorer` to validate
click-to-add-node behavior, folder expansion toggling, filter add/remove
flow, bookmark drag/drop, and safe no-op paths when required data is
absent.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
acd2855151. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11338-refactor-remove-ts-expect-error-suppressions-in-sidebar-components-3456d73d365081e2858af020b88d7f05)
by [Unito](https://www.unito.io)
2026-04-17 20:28:25 -07:00
Johnpaul Chiwetelu
beaa269a63 feat: polish settings dialog layout and keybinding display (#11241)
Polish keybinding display. Based on #11212 with adjustments:
left-aligned content (no centering), key uppercase moved to UI layer.

- Reduce settings content font size to 14px
- Increase spacing between setting sections with cleaner dividers
- Consistent min-height for form items (toggle, slider, dropdown)
- Capitalize keybinding badges via CSS `uppercase` instead of mutating
data model
- Remove '+' separator between keybinding badges
- Unbold keybinding badges with fixed min-width

Supersedes #11212

┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11241-feat-polish-settings-dialog-layout-and-keybinding-display-3426d73d3650812a97e4d941a76a4fe9)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alex <alex@Mac.localdomain>
Co-authored-by: github-actions <github-actions@github.com>
2026-04-17 20:22:39 -07:00
Kelly Yang
cf98013c18 test: expand Image Crop E2E and fix loading overlay deadlock (#11193)
## Summary

Expands Playwright coverage for the **ImageCropV2** widget (Levels 1–3
from the image crop E2E plan), fixes **loading / image mount** behavior
when `imageUrl` changes, adds **stable resize-handle selectors**, and
adds a **small Vitest** for URL→loading transitions.

## Changes

- [x] **Level 1 (E2E)** — Empty state: assert resize handle hidden;
screenshot baseline `image-crop-empty-state.png`; pointer drag on empty
state does not change widget bounds.
- [x] **Level 1 (E2E)** — After run: assert **8 visible** resize handles
with `data-testid` + `filter({ visible: true })`; broken `img.src`
returns to empty state (`crop-empty-state`, no overlay).
- [x] **Level 1 (E2E)** — **Slow `/api/view`** route (delay only
`example.png`) to assert **“Loading…”** then hidden after image loads;
comment clarifies delay is in the route handler, not
`page.waitForTimeout`.
- [x] **Level 2 (E2E)** — Drag clamps to **right/bottom** and
**top-left** image bounds via `setCropBounds` + `expect.poll` on natural
bounds.
- [x] **Level 3 (E2E)** — Free resize: right / left / bottom / top
edges; SE and NW corners; `MIN_CROP_SIZE` (16px); right-edge boundary
clamp; **8 handles** screenshot `image-crop-eight-handles.png`; SE/NW
screenshots (`image-crop-resize-se.png`, `image-crop-resize-nw.png`).
- [x] **E2E helpers** — Shared `getCropValue`, `setCropBounds`,
`dragOnLocator`, `POINTER_OPTS`; drag regression uses **`expect.poll`**
instead of `toPass` where appropriate.
- [x] **`WidgetImageCrop.vue`** — When `imageUrl` is set, **always
render `<img>`**; show loading as an **absolute overlay** (fixes
deadlock where `isLoading` blocked `<img>` so `@load` never ran); add
**`data-testid="crop-resize-{direction}"`** on resize handles.
- [x] **`useImageCrop.ts`** — Watch `imageUrl` and drive `isLoading`;
extract **`imageCropLoadingAfterUrlChange`** (`boolean | null`) for
clear semantics and tests.
- [x] **`useImageCrop.test.ts`** — Vitest coverage for
`imageCropLoadingAfterUrlChange` (null URL, URL change, first URL,
unchanged URL).

## Screenshot / CI notes

- [ ] **Linux screenshot expectations** for new/updated
`toHaveScreenshot(...)` names must be produced on **CI (Linux)** — add
the **`New Browser Test Expectation`** label (or equivalent workflow);
**do not** commit local **Darwin** golden files.
- [x] Existing Linux baselines under `imageCrop.spec.ts-snapshots/` for
prior tests are unchanged where applicable; new baselines are expected
from CI after merge workflow.

## Files

- [x] `browser_tests/tests/vueNodes/widgets/imageCrop.spec.ts`
- [x] `src/components/imagecrop/WidgetImageCrop.vue`
- [x] `src/composables/useImageCrop.ts`
- [x] `src/composables/useImageCrop.test.ts` (new)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches interactive crop UI rendering and `isLoading` state
transitions, which can affect user-visible behavior and input handling;
changes are mitigated by extensive new E2E and unit tests.
> 
> **Overview**
> Improves the `WidgetImageCrop` loading behavior by always rendering
the preview `<img>` when `imageUrl` is set and showing “Loading…” as an
absolute overlay, preventing a deadlock where `isLoading` could block
the `@load` event. Adds stable `data-testid="crop-resize-{direction}"`
selectors for resize handles and hardens pointer-capture handling in
`useImageCrop`.
> 
> Greatly expands automated coverage: the Playwright spec now tests
empty-state rendering/screenshot, drag/resize interactions (edge/corner,
min size, and clamping to image bounds), aspect-ratio lock handle
visibility, slow `/api/view` loading overlay behavior, and broken image
fetch recovery. Adds a new Vitest suite for `useImageCrop` (including
`imageCropLoadingAfterUrlChange`) to unit-test URL→loading transitions
and core drag/resize/aspect-ratio logic.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c4f88a42b5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11193-test-expand-Image-Crop-E2E-and-fix-loading-overlay-deadlock-3416d73d365081eb99dae577c939baa9)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-04-17 20:22:05 -07:00
Benjamin Lu
ecb7fd4796 feat: add frontend subscription success recovery (#11286)
Improving our subscription detection system. Optimal will have to come
after BE team brings personal billing to cloud repo off of comfy api.

## Summary
- replace the dialog-local focus poller with a frontend checkout tracker
stored in `localStorage`
- recover pending subscription checkouts from app boot plus global page
lifecycle (`pageshow`, `visibilitychange`) with bounded retries only
while an attempt is pending
- emit `subscription_success` through GTM with frontend-derived metadata
once subscription state reaches the expected target tier/cycle

## Why
This is the frontend-only 80/20 path. It fixes the brittle "old tab must
regain focus" behavior without adding new backend endpoints or backend
event storage. The browser records one pending checkout attempt when
checkout is opened, and any returning cloud tab can recover it later by
comparing current subscription state against the expected target plan.

## Tradeoffs
- browser-scoped, not backend-authoritative
- no server transaction id
- scheduled downgrades through the billing portal are intentionally not
inferred as immediate success events
- still best-effort compared with the backend outbox/WebSocket approach

## Validation
- `pnpm exec vitest run
src/platform/cloud/subscription/composables/useSubscription.test.ts
src/platform/telemetry/providers/cloud/GtmTelemetryProvider.test.ts`
- `pnpm typecheck`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11286-feat-add-frontend-subscription-success-recovery-3436d73d3650814d9f74c89e6926aa84)
by [Unito](https://www.unito.io)
2026-04-17 22:49:49 +00:00
AustinMroz
e28c1e7e43 Show multitype slices of shared color (#11250)
A tiny update requested by the backend team.

Previously, multitype slot indicators would have inputs that resolve to
the same connection color display be combined into a single slice. For
example, both `INT` and `FLOAT` have the same color, so an `INT,FLOAT`
slot displays as a solid color instead of 2 semi-circles. This was a
conscientious decision to improve readability on slots that allow many
types, but meant that the more common cases (like `INT,FLOAT`) would
have no indicator at all. Since priority is given to types based on
order of listing, node authors can still control which types are elided
on a slot accepting many types.

<img width="430" height="320" alt="image"
src="https://github.com/user-attachments/assets/1fc7fb1c-a634-487c-bc03-711637aeef13"
/>

- I do not believe there are any core nodes affected by this change.
- The vue implementation of merging slot colors never functioned
properly, but is still removed.
- Vue was bugged to incorrectly pass slot types for widgets. This is
also fixed.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11250-Show-multitype-slices-of-shared-color-3436d73d365081b6b484ea74423435a1)
by [Unito](https://www.unito.io)
2026-04-17 22:19:59 +00:00
Kelly Yang
f6f267b46d test: add unit tests for slotCalculations (#11302)
# PR Description

#11106 
**This PR only focus on `slotCalculations.ts`.**

Add unit tests for `slotCalculations.ts` — the centralized utility that
calculates input/output slot positions in graph coordinates. This file
had zero test coverage despite containing non-trivial branching logic
used by both the litegraph adapter and the Vue renderer layout system.

## What's covered

### `calculateInputSlotPosFromSlot`
- [x] **Collapsed nodes**: Returns the node origin shifted up by half
the title height.
- [x] **Hard-coded offsets**: Slots with specific `pos` offsets return
`nodeOrigin + pos` directly.
- [x] **Default vertical layout**: Covers first slot x/y, multi-slot
vertical ordering, `slotStartY` offset, exclusion of widget input slots,
and exclusion of fixed-position slots from index ordering.

### `getSlotPosition` (Legacy fallback path, `vueNodesMode` disabled)
- [x] **Coordinate derivation**: Input and output slot positions derived
correctly from `node.pos`.
- [x] **Collapsed state**: Collapsed input and output nodes use
`title-height` and `NODE_COLLAPSED_WIDTH` offsets.
- [x] **Boundary handling**: Out-of-range slot index falls back to node
origin.



<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Test-only changes that don’t affect runtime behavior; risk is limited
to potential brittleness if slot layout constants or expectations
change.
> 
> **Overview**
> Adds a new `slotCalculations.test.ts` suite covering
`calculateInputSlotPosFromSlot` and the legacy (`LiteGraph.vueNodesMode`
disabled) path of `getSlotPosition`.
> 
> Tests exercise key branches for collapsed nodes, hard-coded slot `pos`
overrides, default vertical slot ordering (including `slotStartY`), and
filtering of widget/fixed-position inputs, plus boundary behavior for
out-of-range slot indices.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
51c5318695. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11302-test-add-unit-tests-for-slotCalculations-3446d73d36508181a0ade81be05bd25f)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-04-17 21:45:26 +00:00
Dante
e8d833bc54 test: cover useLazyPagination, useRangeEditor, useCurveEditor (#11326)
Closes coverage gaps in \`src/composables/\` as part of the unit-test
backfill.

## Testing focus

Three composables, each a different kind of test challenge: reactive
pagination state, DOM-track drag math, and SVG pointer interaction. No
third-party library is mocked.

### \`useLazyPagination\` (10 tests)

- Accepts both \`Ref<T[]>\` and plain \`T[]\` inputs.
- \`currentPage\` ceiling at \`totalPages\` (clamp behavior).
- Source-array replacement resets internal page state.
- \`loadedPages\` (Set) accumulates across navigation.
- **Observed source issue.** \`loadNextPage\` is declared \`async\` but
contains no \`await\` (the artificial delay is commented out).
Consequence: \`isLoading\` is never externally observable as \`true\`,
and the concurrent-call dedup in the design doesn't actually fire in
practice. Tests cover **observable** behavior only; the finding is noted
here as a candidate follow-up fix.

### \`useRangeEditor\` (11 tests)

- Drags each of \`min\` / \`max\` / \`midpoint\` handles; respects the
\`showMidpoint\` toggle (events on the midpoint are ignored when
hidden).
- Value clamping within \`[valueMin, valueMax]\`.
- \`denormalize\` receives the correct normalized position — verifies
the 0–1 mapping math, not just that it was called.
- \`trackRef.value === null\` → pointer events are no-ops (null-safety).
- **Real lifecycle.** Mounts a tiny \`defineComponent\` via
\`@testing-library/vue\`'s \`render\` and exercises cleanup through
\`unmount()\`. \`onBeforeUnmount\` only fires inside a component
instance — \`effectScope.stop()\` alone is insufficient.

### \`useCurveEditor\` (14 tests)

- \`curvePath\` empty when fewer than 2 points.
- Linear interpolation: \`M\` + \`L\` command sequence, points sorted by
x before drawing.
- Non-linear uses \`createInterpolator\` (our module → OK to mock and
assert call shape).
- Drag: dispatching \`pointermove\` updates \`modelValue\`; after
\`pointerup\`, a follow-up \`pointermove\` is a no-op.
- **happy-dom gaps polyfilled.** \`Element.setPointerCapture\` is
stubbed per-element and \`DOMPoint.prototype.matrixTransform\` is added
in \`beforeEach\`. Since the SVG has no CTM, \`DOMMatrix.inverse()\`
returns identity — so \`svgCoords\` maps \`clientX\`/\`clientY\`
directly into curve space, giving deterministic assertions without
brittle coordinate math.

## Principles applied

- No mocks of \`vue\`, \`@vueuse/core\`, or \`es-toolkit\`.
- Behavioral assertions only — no return-shape checks.
- All 35 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
2026-04-17 21:41:09 +00:00
AustinMroz
3fd3c565ae Fix dropdown chevron color (#11335)
Updates the the color of the chevron on dropdown widgets to only have
the disabled color when the widget is disabled.

| Before | After |
| ------ | ----- |
| <img width="360" alt="before"
src="https://github.com/user-attachments/assets/25d35e78-9147-4397-be19-df9d6f87ac72"
/> | <img width="360" alt="after"
src="https://github.com/user-attachments/assets/3bf3640d-fa14-42cb-afb4-7109eb878d1a"
/>|

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11335-Fix-dropdown-chevron-color-3456d73d3650819e99c7d15173f11319)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-04-17 20:37:22 +00:00
Jedrzej Kosinski
ff4c812d08 feat: show sign-in button via server feature flag (#11298)
## Summary

Show the sign-in button in the frontend when the `show_signin_button`
server feature flag is set, without requiring a special desktop
distribution build.

## Changes

- Add `SHOW_SIGNIN_BUTTON` to `ServerFeatureFlag` enum
- Add `showSignInButton` getter in `useFeatureFlags` composable (returns
`boolean | undefined`)
- Update `WorkflowTabs.vue` to use `flags.showSignInButton ?? isDesktop`
- the server flag takes precedence when set, falls back to compile-time
`isDesktop` for legacy desktop support

## Related

- Comfy-Org/ComfyUI-Desktop-2.0-Beta#415
- Backend: Comfy-Org/ComfyUI `feature/generic-feature-flag-cli`
- Launcher: Comfy-Org/ComfyUI-Desktop-2.0-Beta#418

Co-authored-by: Amp <amp@ampcode.com>
2026-04-17 13:45:41 -07:00
Dante
7ffaff7e1b test: cover useBillingPlans and tierBenefits (#11318)
Closes coverage gaps in `src/platform/cloud/subscription/` as part of
the unit-test backfill.

## Testing focus

`useBillingPlans` holds **module-scoped refs** (`plans`,
`currentPlanSlug`, `isLoading`, `error`). If state leaks between tests,
failures get masked as false-green. The suite uses `vi.resetModules()` +
dynamic `import()` in every test to get a fresh instance — state
isolation is the primary design constraint here.

### `useBillingPlans` (12 tests)

- **Concurrent-call dedup.** The \`isLoading\` guard is validated by
creating a pending promise, firing a second \`fetchPlans()\` while the
first is in-flight, and asserting the mock is called **exactly once**.
- **Error branching.** \`Error\` instance → \`.message\` captured.
Non-Error rejection → fallback string (\`'Failed to fetch plans'\`).
Both paths also verify \`console.error\` logging via a spy.
- **Error-reset invariant.** After a failure, a subsequent success must
null out \`error.value\` — order-dependent and easy to regress.
- **Shared-state invariant.** Two separate \`useBillingPlans()\` calls
return refs pointing at the same module-level state.
- **Computed filtering.** \`monthlyPlans\` / \`annualPlans\` partition
by duration — assertions on distinct output, not input re-assertion.

### \`tierBenefits\` (7 tests)

- Table-driven across all \`TierKey\` values for \`maxDuration\`,
\`addCredits\`, \`customLoRAs\` branches.
- \`monthlyCredits\` free-tier path including the
\`remoteConfig.free_tier_credits\` null fallback.
- Translator/formatter forwarding verified by spy.

## Principles applied

- No mocks of \`vue\`, \`pinia\`, or \`@vueuse/core\` — only our own
\`workspaceApi\`.
- Behavioral assertions only — no return-shape checks.
- All 19 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
2026-04-17 13:55:49 +00:00
jaeone94
5d04df7b2c fix: prevent duplicate prepareForSave and conflicting is_new telemetry on self-overwrite Save As (#11329)
## Summary

Follow-up to PR #10816. Fixes a telemetry semantic bug in
`saveWorkflowAs` that emitted two conflicting events for a single user
action.

### What changed

- `saveWorkflowAs` self-overwrite branch now calls
`workflowStore.saveWorkflow` directly instead of delegating to the
`saveWorkflow()` wrapper. The wrapper would run `prepareForSave` a
second time and emit `trackWorkflowSaved({ is_new: false })`, which then
conflicted with the outer `saveWorkflowAs`'s `trackWorkflowSaved({
is_new: true })` for the same user action.
- Added regression tests asserting a single `trackWorkflowSaved` call
with `{ is_new: true }` and a single `prepareForSave` invocation on both
the self-overwrite and copy paths.

### Issues fixed

- Fixes #10819

### Why no E2E test

The bug and fix are entirely about observability (how many telemetry
events are emitted and with what payload). There is no user-visible
change — the file is saved correctly in both pre- and post-fix cases,
and `is_new` values are never rendered in the UI. Playwright tests
cannot directly verify `trackWorkflowSaved` call counts/payloads without
intercepting outbound analytics traffic, which is not a pattern used
elsewhere in `browser_tests/`. Unit tests at the service boundary are
the appropriate level for this contract: they mock `useTelemetry` and
can assert exact call count and payload deterministically.

### Test plan

- [x] `pnpm test:unit
src/platform/workflow/core/services/workflowService.test.ts` — 56 tests
pass (including 2 new regression tests + 1 expanded assertion)
- [x] `pnpm typecheck`
- [x] `pnpm lint`
- [x] `pnpm format`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11329-fix-prevent-duplicate-prepareForSave-and-conflicting-is_new-telemetry-on-self-overwrite-3456d73d36508192875ed5e70ab9c359)
by [Unito](https://www.unito.io)
2026-04-17 09:29:03 +00:00
Dante
2d50cc2d76 feat: show success toast after ComfyHub publish (#11316)
## Summary

Adds a success toast in the ComfyHub publish flow so users get explicit
confirmation that the workflow was published before the dialog closes.

## Changes

- **What**: `ComfyHubPublishDialog.handlePublish()` calls `toast.add({
severity: 'success', ... })` after `submitToComfyHub()` resolves and
before `onClose()` runs. Adds two i18n keys (`publishSuccessTitle`,
`publishSuccessDescription`) and an assertion in the existing
success-path test.

## Review Focus

- This is the lightweight stop-gap discussed in [Slack
thread](https://comfy-organization.slack.com/archives/C0AEPRS8N74/p1776370871654139?thread_ts=1776362591.237159&cid=C0AEPRS8N74)
while the larger published-state design is still pending phase-2 work.
Symmetric with the existing `publishFailedTitle/Description` error
toast.
- `submitToComfyHub` is synchronous (asset uploads happen inside it), so
a successful resolve means the workflow is live.
- `<Toast>` is mounted in `GlobalToast.vue`, so it persists after
`onClose()` destroys the dialog.

## Screenshots (if applicable)
<img width="1135" height="634" alt="Screenshot 2026-04-17 at 8 11 34 AM"
src="https://github.com/user-attachments/assets/a71400a7-2055-4c2a-a761-9298cfa24e9a"
/>

n/a — toast text only.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11316-feat-show-success-toast-after-ComfyHub-publish-3446d73d365081a7bbb3ca29ca3bb618)
by [Unito](https://www.unito.io)
2026-04-16 23:32:36 +00:00
jaeone94
a1e6fb36d2 refactor: harden ChangeTracker lifecycle with self-defending API (#10816)
## Summary

Harden the `ChangeTracker` lifecycle to eliminate the class of bugs
where an inactive workflow's tracker silently captures the wrong graph
state. Renames `checkState()` to `captureCanvasState()` with a
self-defending assertion, introduces `deactivate()` and
`prepareForSave()` lifecycle methods, and closes a latent undo-history
corruption bug discovered during code review.

## Background

ComfyUI supports multiple workflows open as tabs, but only one canvas
(`app.rootGraph`) exists at a time. When the user switches tabs, the old
workflow's graph is unloaded and the new one is loaded into this shared
canvas.

The old `checkState()` method serialized `app.rootGraph` into
`activeState` to track changes for undo/redo. It had no awareness of
*which* workflow it belonged to -- if called on an inactive tab's
tracker, it would capture the active tab's graph data and silently
overwrite the inactive workflow's state. This caused permanent data loss
(fixed in PR #10745 with caller-side `isActive` guards).

The caller-side guards were fragile: every new call site had to remember
to add the guard, and forgetting would reintroduce the same silent data
corruption. Additionally, `beforeLoadNewGraph` only called `store()`
(viewport/outputs) without `checkState()`, meaning canvas state could be
stale if a tab switch happened without a preceding mouseup event.

### Before (fragile)

```
saveWorkflow(workflow):
  if (isActive(workflow))              <-- caller must remember this guard
    workflow.changeTracker.checkState()      <-- name implies "read", actually writes
  ...

beforeLoadNewGraph():
  activeWorkflow.changeTracker.store()      <-- only saves viewport, NOT graph state
```

### After (self-defending)

```
saveWorkflow(workflow):
  workflow.changeTracker.prepareForSave()   <-- handles active/inactive internally
  ...

beforeLoadNewGraph():
  activeWorkflow.changeTracker.deactivate() <-- captures graph + viewport together
```

## Changes

- Rename `checkState` to `captureCanvasState` with active-tracker
assertion
- Add `deactivate()` and `prepareForSave()` lifecycle methods
- Fix undo-history corruption: `captureCanvasState()` guarded by
`_restoringState`
- Fix viewport regression during undo: `deactivate()` skips
`captureCanvasState()` during undo/redo but always calls `store()` to
preserve viewport (regression from PR #10247)
- Log inactive tracker warnings unconditionally at warn level (not
DEV-only)
- Deprecated `checkState()` wrapper for extension compatibility
- Rename `checkState` to `captureCanvasState` in
`useWidgetSelectActions` composable
- Add `appModeStore.ts` to manual call sites documentation
- Add `checkState()` deprecation note to architecture docs
- Add 16 unit tests covering all guard conditions, lifecycle methods,
and undo behavior
- Add E2E test: "Undo preserves viewport offset"

## New ChangeTracker Public API

| Method | Caller | Purpose |
|--------|--------|---------|
| `captureCanvasState()` | Event handlers, UI interactions | Snapshots
canvas into activeState, pushes undo. Asserts active tracker. |
| `deactivate()` | `beforeLoadNewGraph` only | `captureCanvasState()`
(skipped during undo/redo) + `store()`. Freezes state for tab switch. |
| `prepareForSave()` | Save paths only | Active: `captureCanvasState()`.
Inactive: no-op. |
| `checkState()` | **Deprecated** -- extensions only | Wrapper that
delegates to `captureCanvasState()` with deprecation warning. |
| `store()` | Internal to `deactivate()` | Saves viewport, outputs,
subgraph navigation. |
| `restore()` | `afterLoadNewGraph` | Restores viewport, outputs,
subgraph navigation. |
| `reset()` | `afterLoadNewGraph`, save | Resets initial state (marks as
"clean"). |

## Test plan

- [x] Unit tests: 16 tests covering all guard conditions, state capture,
undo queue behavior
- [x] E2E test: "Undo preserves viewport offset" verifies no viewport
drift on undo
- [x] E2E test: "Prevents captureCanvasState from corrupting workflow
state during tab switch"
- [x] Existing E2E: "Closing an inactive tab with save preserves its own
content"
- [ ] Manual: rapidly switch tabs during undo/redo, verify no viewport
drift
- [ ] Manual: verify extensions calling `checkState()` see deprecation
warning in console
2026-04-16 12:54:12 +00:00
jaeone94
394e36984f fix: re-sync collapsed node slot positions after subgraph fitView (#11240)
## Summary

Fix collapsed node connection links rendering at wrong positions when
entering a subgraph for the first time. `fitView()` (added in #10995)
changes canvas scale/offset, invalidating cached slot positions for
collapsed nodes.

## Changes

- **What**: Schedule `requestSlotLayoutSyncForAllNodes()` on the next
frame after `fitView()` in `restoreViewport()` so collapsed node slot
positions are re-measured against the updated transform. Inner RAF
guarded against mid-frame graph changes.
- **Test coverage**:
- Unit tests in `subgraphNavigationStore.viewport.test.ts` verify the
RAF chain calls `requestSlotLayoutSyncForAllNodes` after `fitView`, and
skip the re-sync when the active graph changes between frames.
- E2E screenshot test (`@screenshot` tag) validates correct link
rendering on first subgraph entry using a new fixture with a
pre-collapsed inner node.

## Review Focus

The nested `requestAnimationFrame` is intentional: the outer RAF runs
`fitView()`, which updates `ds.scale`/`ds.offset` and triggers a CSS
transform update on `TransformPane`. The inner RAF ensures the DOM has
reflowed with the new transform before
`requestSlotLayoutSyncForAllNodes()` measures `getBoundingClientRect()`
on slot elements.

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-04-16 12:38:01 +00:00
Dante
e5c81488e4 fix: include focusMode in splitter refresh key to prevent panel resize (#11295)
## Summary

When the properties panel is open, toggling focus mode on then off
causes the panel to resize unexpectedly. The root cause is that
`splitterRefreshKey` in `LiteGraphCanvasSplitterOverlay.vue` does not
include `focusMode`, so the PrimeVue Splitter component instance is
reused across focus mode transitions and restores stale panel sizes from
localStorage.

Fix: add `focusMode` to `splitterRefreshKey` so the Splitter is
recreated when focus mode toggles.

## Red-Green Verification

| Commit | CI Status | Purpose |
|--------|-----------|---------|
| `test: add failing test for focus mode toggle resizing properties
panel` | 🔴 Red | Proves the test catches the bug |
| `fix: include focusMode in splitter refresh key to prevent panel
resize` | 🟢 Green | Proves the fix resolves the bug |

## demo

### AS IS


https://github.com/user-attachments/assets/95f6a9e3-e4c7-4aba-8e17-0eee11f70491


### TO BE


https://github.com/user-attachments/assets/595eafcd-6a80-443d-a6f3-bb7605ed0758



## Test Plan

- [ ] CI red on test-only commit
- [ ] CI green on fix commit
- [ ] E2E regression test added in
`browser_tests/tests/focusMode.spec.ts`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11295-fix-include-focusMode-in-splitter-refresh-key-to-prevent-panel-resize-3446d73d365081b7bc3ac65338e17a8f)
by [Unito](https://www.unito.io)
2026-04-16 13:43:02 +09:00
Terry Jia
6fb90b224d fix(load3d): restore missed hover state when viewer init is async (#11265)
## Summary
followup https://github.com/Comfy-Org/ComfyUI_frontend/pull/9520
mouseenter fires before load3d is created during async init
(getLoad3dAsync), so the STATUS_MOUSE_ON_VIEWER flag is never set.
This causes isActive() to return false after INITIAL_RENDER_DONE,
stopping the animation loop from calling controlsManager.update() and
making OrbitControls unresponsive on first open.

Track hover state in the composable and sync it to load3d after
creation.
2026-04-15 22:34:57 -04:00
pythongosssss
a8e1fa8bef test: add regression test for WEBP RIFF padding (#8527) (#11267)
## Summary

Add a regression test for #8527 (handle RIFF padding for odd-sized WEBP
chunks). The fix added + (chunk_length % 2) to the chunk-stride
calculation in getWebpMetadata so EXIF chunks following an odd-sized
chunk are still located correctly. There was no existing unit test
covering getWebpMetadata, so without a regression test the fix could
silently break in a future
  refactor. 

## Changes

- **What**: 
- New unit test file src/scripts/pnginfo.test.ts covering
getWebpMetadata's RIFF chunk traversal.
- Helpers build a minimal in-memory WEBP with one VP8 chunk of
configurable length followed by an EXIF chunk encoding workflow:<json>.
- Odd-length case (regression for #8527): without the % 2 padding
adjustment, the parser walks one byte short and returns {}.
- Even-length case: guards against an over-correction that always adds
1.
- Verified RED→GREEN locally.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11267-test-add-regression-test-for-WEBP-RIFF-padding-8527-3436d73d36508117a66edf3cb108ded0)
by [Unito](https://www.unito.io)
2026-04-15 18:14:49 +00:00
pythongosssss
83ceef8cb3 test: add regression test for non-string serverLogs (#8460) (#11268)
## Summary

Add a regression test for #8460 (handle non-string `serverLogs` in error
report). The fix added `typeof error.serverLogs === 'string' ? ... :
JSON.stringify(...)` in `errorReportUtil.ts` so object-shaped logs no
longer render as `[object Object]`. There was no existing unit test for
`generateErrorReport`, so this regression could silently return.

## Changes

- **What**: New unit test file `src/utils/errorReportUtil.test.ts`
covering `generateErrorReport`'s `serverLogs` rendering.
- String case: verifies plain-string logs still appear verbatim and
`[object Object]` is absent.
- Object case (regression for #8460): verifies object logs are
JSON-stringified instead of coerced to `[object Object]`.
- Verified RED→GREEN locally.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11268-test-add-regression-test-for-non-string-serverLogs-8460-3436d73d36508195a32fc559ab7ce5bb)
by [Unito](https://www.unito.io)
2026-04-15 18:14:17 +00:00
Christian Byrne
873a75d607 test: add unit tests for usePainter composable (#11137)
## Summary

Add 25 behavioral unit tests for `usePainter` composable, bringing
coverage from 0% to ~35% lines / ~57% functions.

## Changes

- **What**: New test file `src/composables/painter/usePainter.test.ts`
covering widget sync, settings persistence, canvas sizing, brush display
scaling, serialization, restore, pointer event guards, and cursor
visibility.

## Review Focus

- Mock patterns: singleton factory mocks for stores, wrapper component
for lifecycle hooks
- Test coverage prioritization: focused on mount-time sync, reactive
watchers, and computed behavior rather than canvas pixel output

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11137-test-add-unit-tests-for-usePainter-composable-33e6d73d36508147bde7e9c349c743ca)
by [Unito](https://www.unito.io)
2026-04-15 11:13:31 -07:00
pythongosssss
06686a1f50 test: App mode - additional app mode coverage (#11194)
## Summary

Adds additional test coverage for empty state/welcome screen/connect
outputs/vue nodes auto switch

## Changes

- **What**: 
- add tests

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11194-test-App-mode-additional-app-mode-coverage-3416d73d365081ca91d0ed61de19f840)
by [Unito](https://www.unito.io)
2026-04-15 11:42:22 +00:00
jaeone94
693b8383d6 fix: missing-asset correctness follow-ups from #10856 (#11233)
Follow-up to #10856. Four correctness issues and their regression tests.

## Bugs fixed

### 1. ErrorOverlay model count reflected node selection

`useErrorGroups` exposed `filteredMissingModelGroups` under the public
name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute
its model count label, so selecting a node shrank the overlay total. The
overlay must always show the whole workflow's errors.

Exposed both shapes explicitly: `missingModelGroups` /
`missingMediaGroups` (unfiltered totals) and
`filteredMissingModelGroups` / `filteredMissingMediaGroups`
(selection-scoped). `TabErrors.vue` destructures the filtered variant
with an alias.


Before 


https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003

After 


https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120



### 2. Bypass → un-bypass dropped url/hash metadata

Realtime `scanNodeModelCandidates` only reads widget values, so
un-bypass produced a fresh candidate without the url that
`enrichWithEmbeddedMetadata` had previously attached from
`graphData.models`. `MissingModelRow`'s download/copy-url buttons
disappeared after a bypass/un-bypass cycle.

Added `enrichCandidateFromNodeProperties` that copies
`url`/`hash`/`directory` from the node's own `properties.models` — which
persists across mode toggles — into each scanned candidate. Applied to
every call site of the per-node scan. A later fix in the same branch
also enforces directory agreement to prevent a same-name /
different-directory collision from stamping the wrong metadata.

Before 


https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b

After 


https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833



### 3. Initial full scan surfaced interior errors of a muted/bypassed
subgraph container

`scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based
missing-node scan only check each node's own mode. Interior nodes whose
parent container was bypassed passed the filter.

Added `isAncestorPathActive(rootGraph, executionId)` to
`graphTraversalUtil` and post-filter the three pipelines in `app.ts`
after the live rootGraph is configured. The filter uses the execution-ID
path (`"65:63"` → check node 65's mode) so it handles both
live-scan-produced and JSON-enrichment-produced candidates.

Before


https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602

After 


https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8




### 4. Bypassed subgraph entry re-surfaced interior errors

`useGraphNodeManager` replays `graph.onNodeAdded` for each existing
interior node when the Vue node manager initializes on subgraph entry.
That chain reached `scanSingleNodeErrors` via
`installErrorClearingHooks`' `onNodeAdded` override. Each interior
node's own mode was active, so the caller guards passed and the scan
re-introduced the error that the initial pipeline had correctly
suppressed.

Added an ancestor-activity gate at the top of `scanSingleNodeErrors`,
the single entry point shared by paste, un-bypass, subgraph entry, and
subgraph container activation. A later commit also hardens this guard
against detached nodes (null execution ID → skip) and applies the same
ancestor check to `isCandidateStillActive` in the realtime verification
callback.

Before


https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441

After


https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4



## Tests

- 6 unit tests for `isAncestorPathActive` (root, active,
immediate-bypass, deep-nested mute, unresolvable ancestor, null
rootGraph)
- 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment,
no-overwrite, name mismatch, directory mismatch)
- 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry
replaying onNodeAdded)
- 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract
- 4 E2E tests:
- ErrorOverlay model count stays constant when a node is selected (new
fixture `missing_models_distinct.json`)
- Bypass/un-bypass cycle preserves Copy URL button (uses
`missing_models_from_node_properties`)
- Loading a workflow with bypassed subgraph suppresses interior missing
model error (new fixture `missing_models_in_bypassed_subgraph.json`)
- Entering a bypassed subgraph does not resurface interior missing model
error (shares the above fixture)

`pnpm typecheck`, `pnpm lint`, 206 related unit tests passing.

## Follow-up

Several items raised by code review are deferred as pre-existing tech
debt or scope-avoided refactors. Tracked via comments on #11215 and
#11216.

---
Follows up on #10856.
2026-04-15 10:58:24 +00:00
Kelly Yang
66e8d570e7 test: expand Image Compare E2E and stabilize widget selectors (#11196)
## Summary

This PR extends **Image Compare** browser tests, adds **stable
`data-testid` hooks** on `WidgetImageCompare.vue`, and aligns slider
interactions with the **same viewport element** used by
`useMouseInElement`, with **layout polling** to reduce flakes.

## What this PR does

- [x] Adds **`data-testid="image-compare-viewport"`** on the compare
area root (the `containerRef` div) so E2E targets the real slider hit
region instead of a long Tailwind class chain or the `<img>` box alone.
- [x] Adds **`data-testid="image-compare-empty"`** on the no-images
branch so the empty state can be asserted **without hard-coded English**
UI text.
- [x] Adds a **smoke** test that the widget **shows both images and the
drag handle** after value injection, with **`waitForImagesLoaded`** (no
extra full-node screenshot to avoid duplicating the default-50 golden).
- [x] Extends slider coverage to **clamp at both edges** (**0%** and
**~100%**) using the viewport locator and **`expect.poll`** until
**`boundingBox()`** is valid before reading coordinates.
- [x] Updates **`moveToPercentage`** to **`expect.poll`** until the
target locator has a **non-empty layout box** before moving the mouse.
- [x] Routes **hover**, **preserve position**, and **25% / 75%
screenshot** mouse moves through **`image-compare-viewport`**
(consistent with `containerRef`).
- [x] Adds an E2E assertion that the **workflow ImageCompare node** size
is **at least 400×350** (matches the widget workflow asset).
- [x] Hardens the **broken image** case: **`page.on('pageerror')`** /
**`page.off`** in **`finally`**, **`http://127.0.0.1:1/...`** URLs for
fast failure, **`expect.soft`** on key UI invariants, and a hard
assertion that **no page errors** were recorded.
- [x] Extends the **large batch (20×20)** test to **page to the last
index** and assert **counters `20 / 20`**, **previous enabled**, and
**next disabled** on both sides.
- [x] Renames the clamp test title to use an **ASCII hyphen** (`0-100%`)
for easier grepping.

## Out of scope (unchanged in this PR)

- [ ] Replacing **`setImageCompareValue`**’s **`page.evaluate`** setup
with a full UI-driven path (would be a larger follow-up).

## Suggested title

`test: expand Image Compare E2E and stabilize widget selectors`

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Primarily test and selector-hook changes; low production risk, with
only minor DOM attribute additions that could affect external test
tooling if relied upon.
> 
> **Overview**
> Improves Image Compare Playwright coverage and reduces flakiness by
driving slider interactions through a new stable
`data-testid="image-compare-viewport"` hook and polling for a valid
layout box before mouse moves.
> 
> Updates assertions to avoid localized text (new
`data-testid="image-compare-empty"`), adds smoke coverage that
images/handle render after value injection, validates slider clamping at
both 0% and ~100%, and extends screenshot tests to use the same viewport
target.
> 
> Hardens edge-case tests by ensuring broken image loads don’t raise
uncaught `pageerror`s, adds a minimum node size assertion, and extends
large-batch navigation checks through the final index and
disabled/enabled nav states.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
ce3f7fbf8c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11196-test-expand-Image-Compare-E2E-and-stabilize-widget-selectors-3416d73d3650814d8d2bf207943d6205)
by [Unito](https://www.unito.io)
2026-04-14 21:53:13 -04:00