1012 Commits

Author SHA1 Message Date
Christian Byrne
3a73ce72bb refactor: replace inline getByRole('dialog') with page objects in E2E tests (#10724)
## Summary

Replace remaining inline `getByRole('dialog')` calls in E2E tests with
page object accessors, following the pattern from PR #10586.

**Note:** The original scope covered 9 calls across 5 files. Three of
those refactors (ConfirmDialog, MediaLightbox, TemplatesDialog) were
already merged into main via other PRs, so this PR now covers only the
remaining 3 files.

## Changes

- **ComfyNodeSearchBox.ts**: Extract a `root` locator on
`ComfyNodeSearchFilterSelectionPanel` so `header` getter uses
`this.root` instead of an inline `page.getByRole('dialog')`
- **AppModeHelper.ts**: Add `imagePickerPopover` getter that locates the
PrimeVue Popover (`role="dialog"`) filtered by a button named "All"
- **appModeDropdownClipping.spec.ts**: Replace 4-line inline
`getByRole('dialog')` chain with `comfyPage.appMode.imagePickerPopover`

## Review Focus

Pure test infrastructure refactor — no production code changes. Each
page object follows existing conventions.

Fixes #10723
2026-04-06 16:42:09 -07:00
Dante
64f75f0727 test(assets): add E2E tests for delete confirmation flow (#10785)
## Summary

Add E2E Playwright tests for the asset delete confirmation dialog flow.

## Changes

- **What**: New `Assets sidebar - delete confirmation` describe block in
`assets.spec.ts` covering right-click delete showing confirmation
dialog, confirming delete removes asset with success toast, and
cancelling delete preserves asset. Added `mockDeleteHistory()` to
`AssetsHelper` to intercept POST `/api/history` delete payloads and
update mock state.

## Review Focus

Tests use existing `ConfirmDialog` page object and `AssetsHelper` mock
infrastructure. The `mockDeleteHistory` handler removes jobs from the
in-memory mock array so subsequent `/api/jobs` fetches reflect the
deletion.

Fixes #10781

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10785-test-assets-add-E2E-tests-for-delete-confirmation-flow-3356d73d365081fb90c8e2a69de3a666)
by [Unito](https://www.unito.io)
2026-04-06 10:17:23 -07:00
Dante
0d535631a5 refactor(test): extract dialog page objects from inline getByRole usage (#10822)
## Summary

Extract inline `getByRole('dialog')` calls across E2E tests into
reusable page objects.

## Changes

- **What**: Extract `ConfirmDialog` class from `ComfyPage.ts` into
`browser_tests/fixtures/components/ConfirmDialog.ts` with new `save`
button locator. Add `MediaLightbox` and `TemplatesDialog` page objects.
Refactor 4 test files to use these page objects instead of raw dialog
locators.
- **Skipped**: `appModeDropdownClipping.spec.ts` uses
`getByRole('dialog')` for a PrimeVue Popover (not a true dialog), left
as-is.

## Review Focus

The `ConfirmDialog.click()` method now supports a `save` action used by
`workflowPersistence.spec.ts`, which also waits for the dialog mask to
disappear and workflow service to settle.

Fixes #10723

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10822-refactor-test-extract-dialog-page-objects-from-inline-getByRole-usage-3366d73d365081b3bc0ee7ef0ddce658)
by [Unito](https://www.unito.io)
2026-04-06 09:49:32 -07:00
Benjamin Lu
b61e15293c test: address review comments on new browser tests (#10852) 2026-04-04 19:26:55 -07:00
Dante
899660b135 test: add queue overlay and workflow search E2E tests (#10802)
## Summary
- Add queue overlay E2E tests: toggle, filter tabs, completed filter,
close (5 tests)
- Add workflow sidebar search E2E tests: search input, filter by name,
clear, no matches (4 tests)
- Fix AssetsHelper mock timestamps from seconds to milliseconds
(matching backend's `int(time.time() * 1000)`)
- Type AssetsHelper response pagination with `JobsListResponse` from
`@comfyorg/ingest-types`

## Test plan
- [ ] CI passes all Playwright shards
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm lint` passes

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10802-test-add-queue-overlay-and-workflow-search-E2E-tests-3356d73d365081018df8c7061c0854ee)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
2026-04-04 13:15:17 -07:00
Johnpaul Chiwetelu
f4fb7a458e test: add browser tests for selection toolbox button actions (#10764) 2026-04-04 14:03:50 +01:00
Dante
17d2870ef4 test(modelLibrary): add E2E tests for model library sidebar tab (#10789)
## Summary
- Add `ModelLibraryHelper` mock helper for `/experiment/models` and
`/view_metadata` endpoints
- Add `ModelLibrarySidebarTab` page object fixture with search, folder,
and leaf locators
- Add 11 E2E test scenarios covering tab open/close, folder display,
folder expansion, search with debounce, refresh, load all folders, and
empty state

## Test plan
- [ ] CI passes all Playwright shards
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm lint` passes

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10789-test-modelLibrary-add-E2E-tests-for-model-library-sidebar-tab-3356d73d365081b49a7ed752512164da)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 08:04:46 +09:00
Dante
7a68943839 test(assets): strengthen pagination E2E assertions (#10773)
## Summary

The existing pagination smoke test only asserts `count >= 1`, which
passes even if the sidebar eagerly loads all items or ignores page
boundaries entirely.

### What changed

**Before:**
- Created 30 mock jobs (less than BATCH_SIZE of 200) — all loaded in one
request, `has_more: false`
- Asserted `count >= 1` — redundant with the grid-render smoke test

**After — two targeted assertions:**

1. **Initial batch < total**: Mock 250 jobs (> BATCH_SIZE 200). First
`/api/jobs?limit=200&offset=0` returns 200 items with `has_more: true`.
Assert `initialCount < 250`.

2. **Scroll triggers second fetch**: Scroll `VirtualGrid` container to
bottom → `approach-end` event → `handleApproachEnd()` →
`assetsStore.loadMoreHistory()` → `/api/jobs?limit=200&offset=200`
fetches remaining 50. Assert `finalCount > initialCount` via
`expect.poll()`.

### Types
Mock data uses `RawJobListItem` from
`src/platform/remote/comfyui/jobs/jobTypes.ts` (Zod-inferred). This is
the correct source-of-truth per `docs/guidance/playwright.md` —
`/api/jobs` is a Python backend endpoint not covered by
`@comfyorg/ingest-types`.

## Test plan
- [ ] CI E2E tests pass
- [ ] `initial batch is smaller than total job count` validates
pagination boundary
- [ ] `scrolling to the end loads additional items` triggers actual
second API call

Fixes #10649

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 08:00:50 +09:00
Dante
8912f4159a test: add E2E tests for workflow tab operations (#10796)
## Summary

Add Playwright E2E tests for workflow tab interactions in the topbar.

## Changes

- **What**: New test file
`browser_tests/tests/topbar/workflowTabs.spec.ts` with 5 tests covering
default tab visibility, tab creation, switching, closing, and context
menu. Added `newWorkflowButton`, `getTab()`, and `getActiveTab()`
locators to `Topbar.ts` fixture.

## Review Focus

Tests are focused on tab UI interactions only (sidebar workflow
operations are already covered in `workflows.spec.ts`). Context menu
assertion uses Reka UI's `data-reka-context-menu-content` attribute.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10796-test-add-E2E-tests-for-workflow-tab-operations-3356d73d36508170a657ef816e23b71c)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 07:07:31 +09:00
Dante
794b986954 test: add E2E tests for Node Library V2 sidebar (#10798)
## Summary

- Adds Playwright E2E tests for the Node Library V2 sidebar tab
(`Comfy.NodeLibrary.NewDesign: true`)
- Adds `NodeLibrarySidebarTabV2` fixture class with V2-specific locators
(search input, tab buttons, node cards)
- Exposes `menu.nodeLibraryTabV2` on `ComfyPage` for test access
- Tests cover: tab visibility, default tab selection, tab switching,
folder expansion, search filtering, and sort button presence

## Test plan

- [ ] Run `pnpm test:browser:local -- --grep "Node library sidebar V2"`
against a running ComfyUI server with the V2 node library
- [ ] Verify tests pass in CI

Fixes #9079

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10798-test-add-E2E-tests-for-Node-Library-V2-sidebar-3356d73d36508185a11feaf95e32225b)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 07:04:56 +09:00
Johnpaul Chiwetelu
26f3f11a3e test: replace raw CSS selectors with TestIds in context menu spec (#10760)
## Summary
- Replace raw CSS selectors (`.lg-node-header`, `.p-contextmenu`,
`.node-title-editor input`, `.image-preview img`) with centralized
`TestIds` constants and existing fixtures in the context menu E2E spec
- Add `data-testid="title-editor-input"` to TitleEditor overlay for
stable selector targeting
- Use `NodeLibrarySidebarTab` fixture for node library sidebar
interaction

## Changes
- `browser_tests/fixtures/selectors.ts`: add `pinIndicator`,
`innerWrapper`, `titleEditorInput`, `mainImage` to `TestIds.node`
- `browser_tests/fixtures/utils/vueNodeFixtures.ts`: add `pinIndicator`
getter
- `src/components/graph/TitleEditor.vue`: add `data-testid` via
`input-attrs`
- `browser_tests/.../contextMenu.spec.ts`: replace all raw selectors
with TestIds/fixtures

## Test plan
- [x] All 23 context menu E2E tests pass locally
- [x] Typecheck passes
- [x] Lint passes

Fixes #10750
Fixes #10749

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10760-test-replace-raw-CSS-selectors-with-TestIds-in-context-menu-spec-3336d73d3650818790c0e32e0b6f1e98)
by [Unito](https://www.unito.io)
2026-04-02 21:04:50 +01:00
jaeone94
d9466947b2 feat: detect and resolve missing media inputs in error tab (#10309)
## Summary

Add detection and resolution UI for missing image/video/audio inputs
(LoadImage, LoadVideo, LoadAudio nodes) in the Errors tab, mirroring the
existing missing model pipeline.

## Changes

- **What**: New `src/platform/missingMedia/` module — scan pipeline
detects missing media files on workflow load (sync for OSS, async for
cloud), surfaces them in the error tab with upload dropzone, thumbnail
library select, and 2-step confirm flow
- **Detection**: `scanAllMediaCandidates()` checks combo widget values
against options; cloud path defers to `verifyCloudMediaCandidates()` via
`assetsStore.updateInputs()`
- **UI**: `MissingMediaCard` groups by media type; `MissingMediaRow`
shows node name (single) or filename+count (multiple), upload dropzone
with drag & drop, `MissingMediaLibrarySelect` with image/video
thumbnails
- **Resolution**: Upload via `/upload/image` API or select from library
→ status card → checkmark confirm → widget value applied, item removed
from error list
- **Integration**: `executionErrorStore` aggregates into
`hasAnyError`/`totalErrorCount`; `useNodeErrorFlagSync` flags nodes on
canvas; `useErrorGroups` renders in error tab
- **Shared**: Extract `ACCEPTED_IMAGE_TYPES`/`ACCEPTED_VIDEO_TYPES` to
`src/utils/mediaUploadUtil.ts`; extract `resolveComboValues` to
`src/utils/litegraphUtil.ts` (shared across missingMedia + missingModel
scan)
- **Reverse clearing**: Widget value changes on nodes auto-remove
corresponding missing media errors (via `clearWidgetRelatedErrors`)

## Testing

### Unit tests (22 tests)
- `missingMediaScan.test.ts` (12): groupCandidatesByName,
groupCandidatesByMediaType (ordering, multi-name),
verifyCloudMediaCandidates (missing/present, abort before/after
updateInputs, already resolved true/false, no-pending skip, updateInputs
spy)
- `missingMediaStore.test.ts` (10): setMissingMedia, clearMissingMedia
(full lifecycle with interaction state), missingMediaNodeIds,
hasMissingMediaOnNode, removeMissingMediaByWidget
(match/no-match/last-entry), createVerificationAbortController

### E2E tests (10 scenarios in `missingMedia.spec.ts`)
- Detection: error overlay shown, Missing Inputs group in errors tab,
correct row count, dropzone + library select visibility, no false
positive for valid media
- Upload flow: file picker → uploading status card → confirm → row
removed
- Library select: dropdown → selected status card → confirm → row
removed
- Cancel: pending selection → returns to upload/library UI
- All resolved: Missing Inputs group disappears
- Locate node: canvas pans to missing media node

## Review Focus

- Cloud verification path: `verifyCloudMediaCandidates` compares widget
value against `asset_hash` — implicit contract
- 2-step confirm mirrors missing model pattern (`pendingSelection` →
confirm/cancel)
- Event propagation guard on dropzone (`@drop.prevent.stop`) to prevent
canvas LoadImage node creation
- `clearAllErrors()` intentionally does NOT clear missing media (same as
missing models — preserves pending repairs)
- `runMissingMediaPipeline` is now `async` and `await`-ed, matching
model pipeline

## Test plan

- [x] OSS: load workflow with LoadImage referencing non-existent file →
error tab shows it
- [x] Upload file via dropzone → status card shows "Uploaded" → confirm
→ widget updated, error removed
- [x] Select from library with thumbnail preview → confirm → widget
updated, error removed
- [x] Cancel pending selection → returns to upload/library UI
- [x] Load workflow with valid images → no false positives
- [x] Click locate-node → canvas navigates to the node
- [x] Multiple nodes referencing different missing files → correct row
count
- [x] Widget value change on node → missing media error auto-removed

## Screenshots


https://github.com/user-attachments/assets/631c0cb0-9706-4db2-8615-f24a4c3fe27d
2026-04-01 17:59:02 +09:00
jaeone94
bb96e3c95c fix: resolve subgraph promoted widget panel regressions (#10648)
## Summary

Fix four bugs in the subgraph promoted widget panel where linked
promotions were not distinguished from independent ones, causing
incorrect UI state in both the SubgraphEditor (Settings) panel and the
Parameters tab WidgetActions menu.

## Changes

- **What**: Add `isLinkedPromotion` helper to correctly identify widgets
driven by subgraph input connections. Fix `disambiguatingSourceNodeId`
lookup mismatch that broke `isWidgetShownOnParents` and
`handleHideInput` for non-nested promoted widgets. Replace fragile CSS
icon selectors with `data-testid` attributes.

## Bugs fixed

Companion fix PR for #10502 (red-green test PR). All 4 E2E tests from
#10502 now pass:

| Bug | Root cause | Fix |
|-----|-----------|-----|
| Linked promoted widgets have hide toggle enabled | `SubgraphEditor`
only checked `node.id === -1` (physical) — linked promotions from
subgraph input connections were not detected | Added `isLinkedPromotion`
helper that checks `input._widget` bindings; `SubgraphNodeWidget`
`:is-physical` prop now covers both physical and linked cases |
| Linked promoted widgets show eye icon instead of link icon | Same root
cause as above — `isPhysical` prop was only true for `node.id === -1` |
Extended the `:is-physical` condition to include `isLinkedPromotion`
check |
| Widget labels show raw names instead of renamed values |
`SubgraphEditor` passed `widget.name` instead of `widget.label \|\|
widget.name` | Changed `:widget-name` binding to prefer `widget.label` |
| WidgetActions menu shows Hide/Show for linked promotions |
`v-if="hasParents"` didn't exclude linked promotions | Added
`canToggleVisibility` computed that combines `hasParents` with
`!isLinked` check via `isLinkedPromotion` |

### Additional bugs discovered and fixed

| Bug | Root cause | Fix |
|-----|-----------|-----|
| "Show input" always displayed instead of "Hide input" for promoted
widgets | `SectionWidgets.isWidgetShownOnParents` used
`getSourceNodeId(widget)` which falls back to `widget.sourceNodeId` when
`disambiguatingSourceNodeId` is undefined — this mismatches the
promotion store key (`undefined`) | Changed to
`widget.disambiguatingSourceNodeId` directly |
| "Hide input" click does nothing | `WidgetActions.handleHideInput` used
`getSourceNodeId(widget)` for the same reason — `demote()` couldn't find
the entry to remove | Same fix — use `widget.disambiguatingSourceNodeId`
directly |

## Tests added

### E2E (Playwright) —
`browser_tests/tests/subgraphPromotedWidgetPanel.spec.ts`

| Test | What it verifies |
|------|-----------------|
| linked promoted widgets have hide toggle disabled | All toggle buttons
in SubgraphEditor shown section are disabled for linked widgets (covers
1-level and 2-level nested promotions via `subgraph-nested-promotion`
fixture) |
| linked promoted widgets show link icon instead of eye icon | Link
icons appear for linked widgets, no eye icons present |
| widget labels display renamed values instead of raw names |
`widget.label` is displayed when set, not `widget.name` |
| linked promoted widget menu should not show Hide/Show input |
Three-dot menu on Parameters tab omits Hide/Show options for linked
promotions, Rename is still available |

### Unit (Vitest) — `src/core/graph/subgraph/promotionUtils.test.ts`

7 tests covering `isLinkedPromotion`: basic matching, negative cases,
nested subgraph with `disambiguatingSourceNodeId`, multiple inputs, and
mixed linked/independent state.

### Unit (Vitest) —
`src/components/rightSidePanel/parameters/WidgetActions.test.ts`

- Added `isSubgraphNode: () => false` to mock nodes to prevent crash
from new `isLinked` computed

## Review Focus

- `isLinkedPromotion` reads `input._widget` (WeakRef-backed,
non-reactive) directly in the template. This is intentional — `_widget`
bindings are set during subgraph initialization before the user opens
the panel, so stale reads don't occur in practice. A computed-based
approach was attempted but reverted because `_widget` changes cannot
trigger Vue reactivity.
- `getSourceNodeId` removal in `SectionWidgets` and `WidgetActions` is
intentional — the old fallback (`?? widget.sourceNodeId`) caused key
mismatches with the promotion store for non-nested widgets.

## Screenshots
Before
<img width="723" height="935" alt="image"
src="https://github.com/user-attachments/assets/09862578-a0d1-45b4-929c-f22f7494ebe2"
/>

After
<img width="999" height="952" alt="image"
src="https://github.com/user-attachments/assets/ed8fe604-6b44-46b9-a315-6da31d6b405a"
/>
2026-04-01 17:10:30 +09:00
jaeone94
df42b7a2a8 fix: collapsed node connection link positions (#10641)
## Summary

Fix connection links rendering at wrong positions when nodes are
collapsed in Vue nodes mode.

## Changes

- **What**: Fall back to `clientPosToCanvasPos` for collapsed node slot
positioning since DOM-relative scale derivation is invalid when layout
store preserves expanded size. Clear stale `cachedOffset` on collapse
and defer sync when canvas is not yet initialized.
- 3 unit tests for collapsed node slot sync fallback
(clientPosToCanvasPos, cachedOffset clearing, canvas-not-initialized
deferral)
- 3 E2E tests for collapsed node link positions (within bounds, after
position change, after expand recovery)

## Review Focus

- `clientPosToCanvasPos` fallback is safe for collapsed nodes because
collapse is user-initiated (no loading-time transform desync risk that
#9121 originally fixed)
- `cachedOffset` clearing prevents stale expanded-state offsets during
collapsed node drag
- Regression from #9121 (DOM-relative scale) combined with #9680
(collapsed node ResizeObserver skip)

## Screenshots 
Before
<img width="1030" height="434" alt="image"
src="https://github.com/user-attachments/assets/2f8b8a1f-ed22-4588-ab62-72b89880e53f"
/>

After
<img width="1029" height="476" alt="image"
src="https://github.com/user-attachments/assets/52dbbf7c-61ed-465b-ae19-a9781513e7e8"
/>


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10641-fix-collapsed-node-connection-link-positions-3316d73d365081f4aee3fecb92c83b91)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Alexander Brown <DrJKL0424@gmail.com>
2026-04-01 13:49:12 +09:00
Dante
380fae9a0d chore(test): remove dead QueueHelper from browser tests (#10771)
## Summary
- Remove unused `QueueHelper` class and its `comfyPage.queue` property
- `QueueHelper` mocks the legacy `/api/queue` tuple format which the app
no longer uses (now `/api/jobs` via `fetchQueue()`)
- `comfyPage.queue.*` is never called in any test

Fixes #10670

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10771-chore-test-remove-dead-QueueHelper-from-browser-tests-3346d73d36508117bb19db9492bcbed3)
by [Unito](https://www.unito.io)
2026-03-31 19:55:52 +09:00
pythongosssss
515f234143 fix: Ensure all save/save as buttons are the same width (#10681)
## Summary

Makes the save/save as buttons in the builder footer toolbar all a fixed
size so when switching states the elements dont jump

## Changes

- **What**: 
- Apply widths from design to the buttons
- Add tests that measure the sizes of the buttons

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10681-fix-Ensure-all-save-save-as-buttons-are-the-same-width-3316d73d36508187bb74c5a977ea876f)
by [Unito](https://www.unito.io)
2026-03-31 02:47:27 -07:00
Alexander Brown
1624750a02 fix(test): fix bulk context menu test using correct Playwright patterns (#10762)
*PR Created by the Glary-Bot Agent*

---

## Summary

Fixes the `Bulk context menu shows when multiple assets selected` test
that is failing on main.

**Root cause — two issues:**

1. `click({ modifiers: ['ControlOrMeta'] })` does not fire `keydown`
events that VueUse's `useKeyModifier('Control')` tracks (used in
`useAssetSelection.ts`). Multi-select silently fails because the
composable never sees the Control key pressed. Fix: use
`keyboard.down('Control')` / `keyboard.up('Control')` around the click.

2. `click({ button: 'right' })` can be intercepted by canvas overlays
(documented gotcha in `browser_tests/AGENTS.md`). Fix: use
`dispatchEvent('contextmenu', { bubbles: true, cancelable: true })`
which bypasses overlay interception.

Also removed the `toPass()` retry wrapper since the root causes are now
addressed directly.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10762-fix-test-fix-bulk-context-menu-test-using-correct-Playwright-patterns-3346d73d3650811c843ee4a39d3ab305)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-03-30 18:38:25 -07:00
Benjamin Lu
86a3938d11 test: add runtime-safe browser_tests alias (#10735)
## What changed

Added a runtime-safe `#e2e/*` alias for `browser_tests`, updated the
browser test docs, and migrated a representative fixture/spec import
path to the new convention.

## Why

`@/*` only covers `src/`, so browser test imports were falling back to
deep relative paths. `#e2e/*` resolves in both Node/Playwright runtime
and TypeScript.

## Validation

- `pnpm format`
- `pnpm typecheck:browser`
- `pnpm exec playwright test browser_tests/tests/actionbar.spec.ts
--list`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10735-test-add-runtime-safe-browser_tests-alias-3336d73d36508122b253cb36a4ead1c1)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-03-30 19:24:09 +00:00
jaeone94
e11a1776ed fix: prevent saving active workflow content to inactive tab on close (#10745)
## Summary

- Closing an inactive workflow tab and clicking "Save" overwrites that
workflow with the **active** tab's content, causing permanent data loss
- `saveWorkflow()` and `saveWorkflowAs()` call `checkState()` which
serializes `app.rootGraph` (the active canvas) into the inactive
workflow's `changeTracker.activeState`
- Guard `checkState()` to only run when the workflow being saved is the
active one — in both `saveWorkflow` and `saveWorkflowAs`

## Linked Issues

- Fixes https://github.com/Comfy-Org/ComfyUI/issues/13230

## Root Cause

PR #9137 (commit `9fb93a5b0`, v1.41.7) added
`workflow.changeTracker?.checkState()` inside `saveWorkflow()` and
`saveWorkflowAs()`. `checkState()` always serializes `app.rootGraph` —
the graph on the canvas. When called on an inactive tab's change
tracker, it captures the active tab's data instead.

## Test plan

- [x] E2E: "Closing an inactive tab with save preserves its own content"
— persisted workflow B with added node, close while A is active, re-open
and verify
- [x] E2E: "Closing an inactive unsaved tab with save preserves its own
content" — temporary workflow B with added node, close while A is
active, save-as with filename, re-open and verify
- [x] Manual: open A and B, edit B, switch to A, close B tab, click
Save, re-open B — content should be B's not A's
2026-03-30 12:12:38 -07:00
Johnpaul Chiwetelu
61144ea1d5 test: add 23 E2E tests for Vue node context menu actions (#10603)
## Summary
- Add 23 Playwright E2E tests for all right-click context menu actions
on Vue nodes
- **Single node (7 tests)**: rename, copy/paste, duplicate, pin/unpin,
bypass/remove bypass, minimize/expand, convert to subgraph
- **Image node (4 tests)**: copy image to clipboard, paste image from
clipboard, open image in new tab, download via save image
- **Subgraph (3 tests)**: convert + unpack roundtrip, edit subgraph
widgets opens properties panel, add to library and find in node library
search
- **Multi-node (9 tests)**: batch rename, copy/paste, duplicate,
pin/unpin, bypass/remove bypass, minimize/expand, frame nodes, convert
to group node, convert to subgraph
- Uses `ControlOrMeta` modifier for multi-node selection

## Test plan
- [x] All 23 tests pass locally (`pnpm test:browser:local`)
- [x] TypeScript type check passes (`pnpm typecheck:browser`)
- [x] ESLint passes
- [x] CodeRabbit review: no findings

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10603-test-add-23-E2E-tests-for-Vue-node-context-menu-actions-3306d73d3650818a932fc62205ac6fa8)
by [Unito](https://www.unito.io)
2026-03-30 19:31:51 +01:00
Dante
3ac08fd1da test(assets-sidebar): add comprehensive E2E tests for Assets browser panel (#10616)
## Summary
- Extend `AssetsSidebarTab` page object with selectors for search, view
mode, asset cards, selection footer, context menu, and folder view
navigation
- Add mock data factories (`createMockJob`, `createMockJobs`,
`createMockImportedFiles`) to `AssetsHelper` for generating realistic
test fixtures
- Write 30 E2E test cases across 10 categories covering the Assets
browser sidebar panel

## Test coverage added

| Category | Tests | Details |
|----------|-------|---------|
| Empty states | 3 | Generated/Imported empty copy, zero cards |
| Tab navigation | 3 | Default tab, switching, search reset on tab
change |
| Grid view display | 2 | Generated card rendering, Imported tab assets
|
| View mode toggle | 2 | Grid↔List switching via settings menu |
| Search | 4 | Input visibility, filtering, clearing, no-match state |
| Selection | 5 | Click select, Ctrl+click multi, footer, deselect all,
tab-switch clear |
| Context menu | 7 | Right-click menu,
Download/Inspect/Delete/CopyJobID/Workflow actions, bulk menu |
| Bulk actions | 3 | Download/Delete buttons, selection count display |
| Pagination | 1 | Large job set initial load |
| Settings menu | 1 | View mode options visibility |

## Context
Part of [FixIt
Burndown](https://www.notion.so/comfy-org/FixIt-Burndown-32e6d73d365080609a81cdc9bc884460)
— "Untested Side Panels: Assets browser" assigned to @dante01yoon.

## Test plan
- [ ] Run `npx playwright test
browser_tests/tests/sidebar/assets.spec.ts` against local ComfyUI
backend
- [ ] Verify all 30 tests pass
- [ ] CI green

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10616-test-assets-sidebar-add-comprehensive-E2E-tests-for-Assets-browser-panel-3306d73d365081eeb237e559f56689bf)
by [Unito](https://www.unito.io)
2026-03-30 21:15:14 +09:00
Christian Byrne
81d3ef22b0 refactor: extract comfyExpect and makeMatcher from ComfyPage (#10652)
## Summary

Extract `makeMatcher` and `comfyExpect` from `ComfyPage.ts` into the
standalone `browser_tests/fixtures/utils/customMatchers.ts` module,
reducing the page-object file by ~50 lines.

## Changes

- **What**: Removed duplicate `makeMatcher`/`comfyExpect` definitions
from `ComfyPage.ts`; the canonical implementation now lives in
`customMatchers.ts`. A backward-compatible re-export keeps all existing
imports working.

## Review Focus

- The re-export ensures `import { comfyExpect } from
'../fixtures/ComfyPage'` continues to resolve for all ~25 spec files.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10652-refactor-extract-comfyExpect-and-makeMatcher-from-ComfyPage-3316d73d365081bf8e7cd7fa324bf9a6)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: GitHub Action <action@github.com>
2026-03-29 16:05:10 -07:00
Christian Byrne
2d99fb446c test: add QueueClearHistoryDialog E2E tests (DLG-02) (#10586)
## Summary
Adds Playwright E2E tests for the QueueClearHistoryDialog component.

## Tests added
- Dialog opens from queue panel history actions menu
- Dialog shows confirmation message with title, description, and assets
note
- Cancel button closes dialog without clearing history
- Close (X) button closes dialog without clearing history
- Confirm clear action triggers queue history clear API call
- Dialog state resets properly after close/reopen

## Task
Part of Test Coverage Q2 Overhaul (DLG-02).

## Conventions
- Uses Vue nodes with new menu enabled (`Comfy.UseNewMenu: 'Top'`)
- Tests read as user stories
- No full-page screenshots
- Proper waits, no sleeps

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10586-test-add-QueueClearHistoryDialog-E2E-tests-DLG-02-3306d73d36508174a07bd9782340a0f7)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-29 16:02:19 -07:00
Christian Byrne
dee236cd60 test: comprehensive properties panel E2E tests (PNL-01) (#10548)
## Summary
Comprehensive Playwright E2E tests for the properties panel (right
sidebar).

Part of the **Test Coverage Q2 Overhaul** initiative (Phase 2: PNL-01).

## What's included
- **PropertiesPanelHelper** page object in `browser_tests/helpers/` —
locators + action methods for all panel elements
- **35 test cases** covering:
  - Open/close via actionbar toggle
- Workflow Overview (no selection): tabs, title, nodes list, global
settings
  - Single node selection: title, parameters, info tab, widgets display
  - Multi-node selection: item count, node listing, hidden Info tab
  - Title editing: pencil icon, edit mode, rename, visibility rules
  - Search filtering: query, clear, empty state
- Settings tab: Normal/Bypass/Mute state, color swatches, pinned toggle
  - Selection transitions: no-selection ↔ single ↔ multi
  - Nodes tab: list all, search filter
  - Tab label changes based on selection count
  - **Errors tab scaffold** (for @jaeone94 ADD-03)

## Testing
- All tests use Vue nodes with new menu enabled
- Zero flaky tests (proper waits, no sleeps)
- Screenshots scoped to panel elements

## Unblocks
- **ADD-03** (error systems by @jaeone94) — errors tab scaffold ready to
extend

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10548-test-comprehensive-properties-panel-E2E-tests-PNL-01-32f6d73d36508199a216fd8d953d8e18)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-29 15:57:42 -07:00
Christian Byrne
b12b20b5ab test: add 12 workflow persistence playwright tests (#10547)
## What

12 regression tests covering 10 workflow persistence bug gaps, including
the **critical data corruption fix in PR #9531** (pythongosssss) which
previously had ZERO test coverage.

## Why

Deep scan of 37 workflow persistence bugs found 12 E2E-testable gaps
with no regression tests. Workflow persistence is a core reliability
concern — data corruption bugs are the highest risk category.

## Tests

### 🔴 Critical
| Bug | PR | Tests | Description |
|-----|----|-------|-------------|
| Data corruption | #9531 | 2 | checkState during graph loading corrupts
workflow data |
| State desync | #9533 | 2 | Rapid tab switching desyncs workflow/graph
state |

### 🟡 Medium  
| Bug | PR/Commit | Tests | Description |
|-----|-----------|-------|-------------|
| Lost previews | #9380 | 1 | Node output previews lost on tab switch |
| Stale canvas | 44bb6f13 | 1 | Canvas not cleared before loading new
workflow |
| Widget loss | #7648 | 1 | Widget values lost on graph change |
| API format | #9694 | 1 | API format workflows fail with missing nodes
|
| Paste duplication | #8259 | 1 | Middle-click paste duplicates workflow
|
| Blob URLs | #8715 | 1 | Transient blob: URLs in serialization |

### 🟢 Low
| Bug | PR/Commit | Tests | Description |
|-----|-----------|-------|-------------|
| Locale break | #8963 | 1 | Locale change breaks workflows |
| Panel drift | — | 1 | Splitter panel size drift |

## Conventions
- All tests use Vue nodes + new menu enabled
- Each test documents which PR/commit it regresses
- Proper waits (no sleeps)
- Screenshots scoped to relevant elements
- Tests read like user stories

## 🎉 Shoutout
PR #9531 by @pythongosssss was a critical data corruption fix that now
has regression test coverage for the first time.

Part of: Test Coverage Q2 Overhaul (REG-01)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10547-test-12-workflow-persistence-regression-tests-incl-critical-PR-9531-32f6d73d3650818796c6c5950c77f6d1)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-29 15:57:13 -07:00
Christian Byrne
04f90b7a05 test: add mock data fixtures for backend API responses (#10662)
## Summary

Add deterministic mock data fixtures for browser tests so they can use
`page.route()` to intercept API calls without depending on a live
backend.

## Changes

- **`browser_tests/fixtures/data/nodeDefinitions.ts`** — Mock
`ComfyNodeDef` objects for KSampler, CheckpointLoaderSimple, and
CLIPTextEncode
- **`browser_tests/fixtures/data/systemStats.ts`** — Mock `SystemStats`
with realistic RTX 4090 GPU info
- **`browser_tests/fixtures/data/README.md`** — Usage guide for
`page.route()` interception

All fixtures are typed against the Zod schemas in `src/schemas/` and
pass `pnpm typecheck:browser`.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10662-test-add-mock-data-fixtures-for-backend-API-responses-3316d73d3650813ea5c8c1faa215db63)
by [Unito](https://www.unito.io)

---------

Co-authored-by: dante01yoon <bunggl@naver.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
2026-03-29 15:55:50 -07:00
Christian Byrne
1e7c8d9889 test: add SignIn dialog E2E tests (DLG-04) (#10587)
## Summary
Adds Playwright E2E tests for the SignIn dialog component and its
sub-forms.

## Tests added
- Dialog opens from login button in topbar
- Sign In form is the default view with email/password fields
- Toggle between Sign In and Sign Up forms
- API Key form navigation (forward and back)
- Terms of Service and Privacy Policy links present
- Form field presence verification
- Dialog close behavior (close button and Escape key)
- Forgot password link presence
- 'Or continue with' divider and API key button

## Notes
- Tests focus on UI navigation and element presence (no real Firebase
auth in test env)
- Dialog opened via `extensionManager.dialog.showSignInDialog()` API
- All selectors use stable IDs from the component source
(`#comfy-org-sign-in-email`, etc.)

## Task
Part of Test Coverage Q2 Overhaul (DLG-04).

## Conventions
- Uses Vue nodes with new menu enabled (`Comfy.UseNewMenu: 'Top'`)
- Tests read as user stories
- No full-page screenshots
- Proper waits, no sleeps

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10587-test-add-SignIn-dialog-E2E-tests-DLG-04-3306d73d3650815db171f8c5228e2cf3)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-29 15:37:37 -07:00
Kelly Yang
798f6de4a9 fix: image compare node displays wrong height with mismatched resolut… (#10714)
## Summary

Revert `object-cover` to `object-contain` so images are never cropped
when the container is short, and add imagecompare to `EXPANDING_TYPES`
so the widget row grows to fill the full node body instead of collapsing
to `min-content`.


## Screenshots
before
<img width="2674" height="2390" alt="image"
src="https://github.com/user-attachments/assets/8fa5cf41-f393-4a7d-a767-75ce944d00d4"
/>

after




https://github.com/user-attachments/assets/46e1fffc-5f65-4b69-9303-fe6255d9de79

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10714-fix-image-compare-node-displays-wrong-height-with-mismatched-resolut-3326d73d3650818293d3c716cb8fafb5)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-03-29 14:45:56 -07:00
Christian Byrne
af0f7cb945 refactor: extract assetPath as standalone pure function (#10651)
## Summary

Extract `assetPath` from a `ComfyPage` method to a standalone pure
function, removing unnecessary coupling to the page object.

## Changes

- **What**: Moved `assetPath` to
`browser_tests/fixtures/utils/paths.ts`. `DragDropHelper` and
`WorkflowHelper` import it directly instead of receiving it via
`ComfyPage`. `ComfyPage.assetPath` kept as thin delegate for backward
compat.

## Review Focus

Structural-only refactor — no behavioral changes. The function was
already pure (no `this`/`page` usage).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10651-refactor-extract-assetPath-as-standalone-pure-function-3316d73d365081c0b0e0ce6dde57ef8e)
by [Unito](https://www.unito.io)
2026-03-29 00:27:28 -07:00
Dante
bce7a168de fix: type API mock responses in browser tests (#10668)
## Motivation

Browser tests mock API responses with `route.fulfill()` using untyped
inline JSON. When the OpenAPI spec changes, these mocks silently drift —
mismatches aren't caught at compile time and only surface as test
failures at runtime.

We already have auto-generated types from OpenAPI and manual Zod
schemas. This PR makes those types the source of truth for test mock
data.

From Mar 27 PR review session action item: "instruct agents to use
schemas and types when writing browser tests."

## Type packages and their API coverage

The frontend has two OpenAPI-generated type packages, each targeting a
different backend API with a different code generation tool:

| Package | Target API | Generator | TS types | Zod schemas |
|---------|-----------|-----------|----------|-------------|
| `@comfyorg/registry-types` | Registry API (node packages, releases,
subscriptions, customers) | `openapi-typescript` | Yes | **No** |
| `@comfyorg/ingest-types` | Ingest API (hub workflows, asset uploads,
workspaces) | `@hey-api/openapi-ts` | Yes | Yes |

Additionally, Python backend endpoints (`/api/queue`, `/api/features`,
`/api/settings`, etc.) are typed via manual Zod schemas in
`src/schemas/apiSchema.ts`.

This PR applies **compile-time type checking** using these existing
types. Runtime validation via Zod `.parse()` is not yet possible for all
endpoints because `registry-types` does not generate Zod schemas — this
requires a separate migration of `registry-types` to
`@hey-api/openapi-ts` (#10674).

## Summary

- Add "Typed API Mocks" guideline to `docs/guidance/playwright.md` with
a sources-of-truth table mapping endpoint categories to their type
packages
- Add rule to `AGENTS.md` Playwright section requiring typed mock data
- Refactor `releaseNotifications.spec.ts` to use `ReleaseNote` type
(from `registry-types`) via `createMockRelease()` factory
- Annotate template mock in `templates.spec.ts` with
`WorkflowTemplates[]` type

Refs #10656

## Example workflow: writing a new typed E2E test mock

When adding a new `route.fulfill()` mock, follow these steps:

### 1. Identify the type source

Check which API the endpoint belongs to:

| Endpoint category | Type source | Zod available |
|---|---|---|
| Ingest API (hub, billing, workflows) | `@comfyorg/ingest-types` | Yes
— use `.parse()` |
| Registry API (releases, nodes, publishers) |
`@comfyorg/registry-types` | Not yet (#10674) — TS type only |
| Python backend (queue, history, settings) | `src/schemas/apiSchema.ts`
| Yes — use `z.infer` |
| Templates | `src/platform/workflow/templates/types/template.ts` | No —
TS type only |

### 2. Create a typed factory (with Zod when available)

**Ingest API endpoints** — Zod schemas exist, use `.parse()` for runtime
validation:

```typescript
import { zBillingStatusResponse } from '@comfyorg/ingest-types/zod'
import type { BillingStatusResponse } from '@comfyorg/ingest-types'

function createMockBillingStatus(
  overrides?: Partial<BillingStatusResponse>
): BillingStatusResponse {
  return zBillingStatusResponse.parse({
    plan: 'free',
    credits_remaining: 100,
    renewal_date: '2026-04-28T00:00:00Z',
    ...overrides
  })
}
```

**Registry API endpoints** — TS type only (Zod not yet generated):

```typescript
import type { ReleaseNote } from '../../src/platform/updates/common/releaseService'

function createMockRelease(
  overrides?: Partial<ReleaseNote>
): ReleaseNote {
  return {
    id: 1,
    project: 'comfyui',
    version: 'v0.3.44',
    attention: 'medium',
    content: '## New Features',
    published_at: new Date().toISOString(),
    ...overrides
  }
}
```

### 3. Use in test

```typescript
test('should show upgrade banner for free plan', async ({ comfyPage }) => {
  await comfyPage.page.route('**/billing/status', async (route) => {
    await route.fulfill({
      status: 200,
      contentType: 'application/json',
      body: JSON.stringify(createMockBillingStatus({ plan: 'free' }))
    })
  })

  await comfyPage.setup()
  await expect(comfyPage.page.getByText('Upgrade')).toBeVisible()
})
```

The factory pattern keeps test bodies focused on **what varies** (the
override) rather than the full response shape.

## Scope decisions

| File | Decision | Reason |
|------|----------|--------|
| `releaseNotifications.spec.ts` | Typed | `ReleaseNote` type available
from `registry-types` |
| `templates.spec.ts` | Typed | `WorkflowTemplates` type available in
`src/platform/workflow/templates/types/` |
| `QueueHelper.ts` | Skipped | Dead code — instantiated but never called
in any test |
| `FeatureFlagHelper.ts` | Skipped | Response type is inherently
`Record<string, unknown>`, no stronger type exists |
| Fixture factories | Deferred | Coordinate with Ben's fixture
restructuring work to avoid duplication |

## Follow-up work

Sub-issues of #10656:

- #10670 — Clean up dead `QueueHelper` or rewrite against `/api/jobs`
endpoint
- #10671 — Expand typed factory pattern to more endpoints
- #10672 — Evaluate OpenAPI generation for excluded Python backend
endpoints
- #10674 — Migrate `registry-types` from `openapi-typescript` to
`@hey-api/openapi-ts` to enable Zod schema generation

## Test plan

- [x] `pnpm typecheck:browser` passes
- [x] `pnpm lint` passes
- [ ] Existing `releaseNotifications` and `templates` tests pass in CI
2026-03-29 15:45:06 +09:00
Christian Byrne
e7c2cd04f4 perf: add FPS, p95 frame time, and target thresholds to CI perf report (#10516)
## Summary

Enhances the CI performance report with explicit FPS metrics, percentile
frame times, and milestone target thresholds.

### Changes

**PerformanceHelper** (data collection):
- `measureFrameDurations()` now returns individual frame durations
instead of just the average, enabling percentile computation
- Computes `p95FrameDurationMs` from sorted frame durations
- Strips `allFrameDurationsMs` from serialized JSON to avoid bloating
artifacts

**perf-report.ts** (report rendering):
- **Headline summary** at top of report with key metrics per test
scenario
- **FPS display**: derives avg FPS and P5 FPS from frame duration
metrics
- **Target thresholds**: shows P5 FPS ≥ 52 target with / pass/fail
indicator
- **p95 frame time**: added as a tracked metric in the comparison table
- Metrics reordered to show frame time/FPS first (what people look for)

### Target

From the Nodes 2.0 Perf milestone: **P5 ≥ 52 FPS** on 245-node workflow
(equivalent to P95 frame time ≤ 19.2ms).

### Example headline output

```
> **vue-large-graph-pan**: 60 avg FPS · 58 P5 FPS  (target: ≥52) · 12ms TBT · 45.2 MB heap
> **canvas-zoom-sweep**: 45 avg FPS · 38 P5 FPS  (target: ≥52) · 85ms TBT · 52.1 MB heap
```

Follow-up to #10477 (merged).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10516-perf-add-FPS-p95-frame-time-and-target-thresholds-to-CI-perf-report-32e6d73d365081a2a2a6ceae7d6e9be5)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-28 23:29:19 -07:00
Terry Jia
391a6db056 test: add minimap e2e tests for close button, viewport, and pan (#10596)
## Summary
add more basic tests for minimap

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10596-test-add-minimap-e2e-tests-for-close-button-viewport-and-pan-3306d73d365081b9bf64dc7a3951d65f)
by [Unito](https://www.unito.io)
2026-03-28 23:19:04 -07:00
Christian Byrne
4d4dca2a46 docs: document fixture/page-object separation in browser tests (#10645)
## Summary

Document the agreed-upon architectural separation for browser test
fixtures:

- `fixtures/data/` — Static test data (mock API responses, workflow
JSONs, node definitions)
- `fixtures/components/` — Page object components (locators, user
interactions)
- `fixtures/helpers/` — Focused helper classes (domain-specific actions)
- `fixtures/utils/` — Pure utility functions (no page dependency)

## Changes

- **`browser_tests/AGENTS.md`** — Added architectural separation section
with clear rules for each directory
- **`browser_tests/fixtures/data/README.md`** (new) — Explains the data
directory purpose and what belongs here vs `assets/`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10645-docs-document-fixture-page-object-separation-in-browser-tests-3316d73d365081febf52d165282c68f6)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-28 23:11:34 -07:00
Christian Byrne
ba9f3481fb test(infra): cloud Playwright project with @cloud/@oss tagging (#10546)
## What

Adds a `cloud` Playwright project so E2E tests can run against
`DISTRIBUTION=cloud` builds, with `@cloud` / `@oss` test tagging.

## Why

100+ usages of `isCloud` / `DISTRIBUTION` across 9 categories (API
routing, UI visibility, settings, auth). Zero cloud test infrastructure
existed — cloud-specific UI components (LoginButton, SubscribeButton,
etc.) had no E2E coverage path.

## Investigation: Runtime Toggle

Investigated whether `isCloud` could be made runtime-toggleable in
dev/test mode (via `window.__FORCE_CLOUD__`). **Not feasible** —
`__DISTRIBUTION__` is a Vite `define` compile-time constant used for
dead-code elimination. Runtime override would break tree-shaking in
production.

Full investigation:
`research/architecture/cloud-runtime-toggle-investigation.md`

## What's included

### Playwright Config
- New `cloud` project alongside existing `chromium`
- Cloud project: `grep: /@cloud/` — only runs `@cloud` tagged tests
- Chromium project: `grepInvert: /@cloud/` — excludes cloud tests

### Build Script
- `npm run build:cloud` → `DISTRIBUTION=cloud vite build`

### Test Tagging Convention
```typescript
test('works in both', async () => { ... });
test('subscription button visible @cloud', async () => { ... });
test('install manager prompt @oss', async () => { ... });
```

### Example Tests
- 2 cloud-only tests validating cloud UI visibility

## NOT included (future work)
- CI workflow job for cloud tests (separate PR)
- Cloud project is opt-in — not run by default locally

## Unblocks
- Cloud-specific E2E tests for entire team
- TB-03 LoginButton, TB-04 SubscribeButton (@Kaili Yang)
- DLG-04 SignIn, DLG-06 CancelSubscription

Part of: Test Coverage Q2 Overhaul

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10546-test-infra-cloud-Playwright-project-with-cloud-oss-tagging-32f6d73d3650810ebb59dea8ce4891e9)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-03-28 22:34:37 -07:00
Alexander Brown
0e7cab96b7 test: reorganize subgraph E2E tests into domain-organized directory (#10695)
## Summary

From the primordial entropy of 17 scattered spec files — a formless
sprawl of mixed concerns and inconsistent naming — emerges a clean,
domain-organized hierarchy. Order triumphs over chaos.

## Changes

- **What**: Reorganize all subgraph E2E tests from 17 flat files in
`browser_tests/tests/` into 10 domain-grouped files under
`browser_tests/tests/subgraph/`.

| File | Tests | Domain |
|------|-------|--------|
| `subgraphSlots` | 16 | I/O slot CRUD, rename, alignment, promoted slot
position |
| `subgraphPromotion` | 22 | Auto-promote, visibility, reactivity,
context menu, cleanup |
| `subgraphSerialization` | 16 | Hydration, round-trip, legacy formats,
ID remapping |
| `subgraphNavigation` | 10 | Breadcrumb, viewport, hotkeys, progress
state |
| `subgraphNested` | 9 | Configure order, duplicate names, pack values,
stale proxies |
| `subgraphLifecycle` | 7 | Source removal cleanup, pseudo-preview
lifecycle |
| `subgraphPromotionDom` | 6 | DOM widget persistence, cleanup,
positioning |
| `subgraphCrud` | 5 | Create, delete, copy, unpack |
| `subgraphSearch` | 3 | Search aliases, description, persistence |
| `subgraphOperations` | 2 | Copy/paste inside, undo/redo inside |

Where once the monolith `subgraph.spec.ts` (856 lines) mixed slot CRUD
with hotkeys, DOM widgets with navigation, and copy/paste with undo/redo
— now each behavioral domain has its sovereign territory.

Where once `subgraph-rename-dialog.spec.ts`,
`subgraphInputSlotRename.spec.ts`, and
`subgraph-promoted-slot-position.spec.ts` scattered rename concerns
across three kingdoms — now they answer to one crown:
`subgraphSlots.spec.ts`.

Where once `kebab-case` and `camelCase` warred for dominion — now a
single convention reigns.

All 96 test cases preserved. Zero test logic changes. Purely structural.

## Review Focus

- Verify no tests were lost in the consolidation
- Confirm import paths all resolve correctly at the new depth
(`../../fixtures/`)
- The `import.meta.dirname` asset path in `subgraphSlots.spec.ts` (slot
alignment test) updated for new directory depth

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10695-test-reorganize-subgraph-E2E-tests-into-domain-organized-directory-3326d73d36508197939be8825b69ea88)
by [Unito](https://www.unito.io)

Co-authored-by: Amp <amp@ampcode.com>
2026-03-28 21:06:00 -07:00
Christian Byrne
8eb1525171 feat: add assertHasItems and openFor to ContextMenu page object (#10659)
## Summary

Add composite assertion and scoped opening methods to the `ContextMenu`
Playwright page object.

## Changes

- **What**: Added `assertHasItems(items: string[])` using
`expect.soft()` per item, and `openFor(locator: Locator)` which
right-clicks and waits for menu visibility. Fully backward-compatible.

## Review Focus

Both methods reuse existing locators (`primeVueMenu`, `litegraphMenu`,
`getByRole("menuitem")`). `openFor` uses `.or()` to handle both menu
types.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10659-feat-add-assertHasItems-and-openFor-to-ContextMenu-page-object-3316d73d36508193af45da7d3af4f50c)
by [Unito](https://www.unito.io)
2026-03-28 18:58:16 -07:00
Dante
b8480f889e feat: add Tag component from design system and rename SquareChip (#10650)
## Summary
- Add `Tag` component based on Figma design system with CVA variants
  - `square` (rounded-sm) and `rounded` (pill) shapes
- `overlay` shape for tags on image thumbnails (pending Figma
confirmation)
  - `default`, `unselected`, `selected` states matching Figma
  - `removable` prop with X close button and `remove` event
  - Icon slot support
- Rename `SquareChip` → `Tag` across all consumers
(WorkflowTemplateSelectorDialog, SampleModelSelector)
- Update all Storybook stories (Tag, Card, BaseModalLayout)
- Delete old `SquareChip.vue` and `SquareChip.stories.ts`
- Add E2E screenshot test for template card overlay tags

Foundation for migrating PrimeVue `Chip` and `Tag` components in
follow-up PRs.

## Test plan
- [x] Unit tests pass (5 tests: rendering, removable, icon slot)
- [x] E2E screenshot test: template cards with overlay tags
- [x] Typecheck passes
- [x] Lint passes
- [ ] Verify Tag stories render correctly in Storybook
- [ ] Verify WorkflowTemplateSelectorDialog tags display correctly
- [ ] Verify SampleModelSelector chips display correctly

## Follow-up work
- **PR 4** (#10673): Migrate PrimeVue `Chip` → custom `Tag`
(SearchFilterChip, NodeSearchItem, DownloadItem)
- **PR 5** (planned): Migrate PrimeVue `Tag` → custom `Tag` (~14 files)
2026-03-29 08:27:53 +09:00
Christian Byrne
b49ea9fabd feat: add getNodesByTitle and getNodeByTitleNth helpers to VueNodeHelpers (#10666)
## Summary

Add helpers for safely interacting with nodes that share the same title
without hitting Playwright strict mode.

## Changes

- **What**: Added `getNodesByTitle(title)` and `getNodeByTitleNth(title,
index)` to `VueNodeHelpers`. Updated `docs/guidance/playwright.md` with
a gotcha note about duplicate node names.

## Review Focus

These are purely additive helpers — no existing behavior changes.
`getNodesByTitle` returns all matching nodes (callers use `.nth()` to
pick), and `getNodeByTitleNth` is a convenience wrapper. The existing
`selectNodes(nodeIds)` by-ID method is unchanged.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10666-feat-add-getNodesByTitle-and-getNodeByTitleNth-helpers-to-VueNodeHelpers-3316d73d3650812eabe6e56a768a34d2)
by [Unito](https://www.unito.io)
2026-03-28 16:09:18 -07:00
pythongosssss
54a00aac75 test/refactor: App mode - Refactor & Save As tests (#10680)
## Summary

Adds e2e Save As tests for #10679.
Refactors tests to remove getByX and other locators in tests to instead
be in fixtures.

## Changes

- **What**: 
- extract app mode fixtures
- add test ids where required
- add new tests

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10680-test-refactor-App-mode-Refactor-Save-As-tests-3316d73d3650815b9d49ccfbb6d54416)
by [Unito](https://www.unito.io)
2026-03-28 15:02:23 -07:00
Alexander Brown
d2358c83e8 test: extract shared subgraph E2E test utilities (#10629)
## Summary

Extract repeated patterns from 12 subgraph Playwright spec files into
shared test utilities, reducing duplication by ~142 lines.

## Changes

- **What**: New shared helpers for common subgraph test operations:
- `SubgraphHelper`: `getSlotCount()`, `getSlotLabel()`, `removeSlot()`,
`findSubgraphNodeId()`
  - `NodeReference`: `delete()`
- `subgraphTestUtils`: `serializeAndReload()`,
`convertDefaultKSamplerToSubgraph()`, `expectWidgetBelowHeader()`,
`collectConsoleWarnings()`, `packAllInteriorNodes()`
- Replaced ~72 inline `page.evaluate` blocks and multi-line sequences
with single helper calls across 12 spec files

## Review Focus

- Behavioral equivalence: every replacement is a mechanical extraction
with no test logic changes
- API surface of new helpers: naming, parameter types, placement in
existing utility classes
- Whether any remaining inline patterns in the spec files would benefit
from further extraction

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10629-test-extract-shared-subgraph-E2E-test-utilities-3306d73d365081b0b6b5db52ed0a4552)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-03-28 21:12:26 +00:00
Christian Byrne
b2f848893a test: add perf test for viewport pan sweep GC churn (#10479)
## Summary

Adds a `@perf` test to establish a baseline for viewport panning GC
churn on large graphs.

## Changes

- **What**: New `large graph viewport pan sweep` perf test that pans
aggressively back and forth across a 245-node graph, forcing many nodes
to cross the viewport boundary. Measures style recalcs, forced layouts,
task duration, heap delta, and DOM node count.

## Review Focus

This is **PR 1 of 2** (perf-fix-with-proof pattern). The fix (viewport
culling) will follow in a separate PR once this baseline is established
on main. CI will then show the delta proving the improvement.

The test uses 120 steps out + 120 steps back at 8px/step = ~960px total
displacement, enough to sweep across a significant portion of the large
graph layout.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10479-test-add-perf-test-for-viewport-pan-sweep-GC-churn-32d6d73d365081cc9f15fe3d5890675d)
by [Unito](https://www.unito.io)
2026-03-28 13:54:26 -07:00
Alexander Brown
5b4ebf4d99 test: audit skipped tests — prune stale, re-enable stable, remove dead code (#10312)
## Summary

Audit all skipped/fixme tests: delete stale tests whose underlying
features were removed, re-enable tests that pass with minimal fixes, and
remove orphaned production code that only the deleted tests exercised.
Net result: **−2,350 lines** across 50 files.

## Changes

- **Pruned stale skipped tests** (entire files deleted):
- `LGraph.configure.test.ts`, `LGraph.constructor.test.ts` — tested
removed LGraph constructor paths
- `LGraphCanvas.ghostAutoPan.test.ts`,
`LGraphCanvas.linkDragAutoPan.test.ts`, `useAutoPan.test.ts`,
`useSlotLinkInteraction.autoPan.test.ts` — tested removed auto-pan
feature
- `useNodePointerInteractions.test.ts` — single skipped test for removed
callback
  - `ImageLightbox.test.ts` — component replaced by `MediaLightbox`
- `appModeWidgetRename.spec.ts` (E2E) — feature removed; helper
`AppModeHelper.ts` also deleted
- `domWidget.spec.ts`, `widget.spec.ts` (E2E) — tested removed widget
behavior

- **Removed orphaned production code** surfaced by test pruning:
- `useAutoPan.ts` — composable + 93 lines of auto-pan logic in
`LGraphCanvas.ts`
  - `ImageLightbox.vue` — replaced by `MediaLightbox`
- Auto-pan integration in `useSlotLinkInteraction.ts` and
`useNodeDrag.ts`
- Dead settings (`LinkSnapping.AutoPanSpeed`,
`LinkSnapping.AutoPanMargin`) in `coreSettings.ts` and
`useLitegraphSettings.ts`
- Unused subgraph methods (`SubgraphNode.getExposedInput`,
`SubgraphInput.getParentInput`)
- Dead i18n key, dead API schema field, dead fixture exports
(`dirtyTest`, `basicSerialisableGraph`)
  - Dead test utility `litegraphTestUtils.ts`

- **Re-enabled skipped tests with minimal fixes**:
  - `useBrowserTabTitle.test.ts` — removed skip, test passes as-is
- `eventUtils.test.ts` — replaced MSW dependency with direct `fetch`
mock
- `SubscriptionPanel.test.ts` — stabilized button selectors,
timezone-safe date assertion
- `LinkConnector.test.ts` — removed stale describe blocks, kept passing
suite
- `widgetUtil.test.ts` — removed skipped tests for deleted functionality
- `comfyManagerStore.test.ts` — removed skipped `isPackInstalling` /
`action buttons` / `loading states` blocks

- **Re-enabled then re-skipped 3 flaky E2E tests** (fail in CI for
pre-existing reasons):
- `browserTabTitle.spec.ts` — canvas click timeout (element not visible)
  - `groupNode.spec.ts` — screenshot diff (stale golden image)
  - `nodeSearchBox.spec.ts` — `p-dialog-mask` intercepts pointer events

- **Simplified production code** alongside test cleanup:
- `useNodeDrag.ts` — removed auto-pan integration, simplified from
170→100 lines
- `DropZone.vue` — refactored URL-drop handling, removed unused code
path
- `ToInputFromIoNodeLink.ts`, `SubgraphInputEventMap.ts` — removed dead
subgraph wiring

- **Dependencies**: none
- **Breaking**: none (all removed code was internal/unused)

## Review Focus

- Confirm deleted production code (`useAutoPan`, `ImageLightbox`,
subgraph methods) has no remaining callers
- Validate that simplified `useNodeDrag.ts` preserves drag behavior
without auto-pan
- Check that re-skipped E2E tests have clear skip reasons for future
triage

## Screenshots (if applicable)

N/A

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-03-28 13:08:52 -07:00
pythongosssss
6836419e96 fix: App mode - Save as not using correct extension or persisting mode on change (#10679)
## Summary

With a previously saved workflow, selecting "Save as" in app mode would
not correctly change the file extension to the chosen mode, and would
require an additional save after to persist the actual mode change.

Recreation:
- Build app
- Save as worklow X, app mode
- Select Save as from builder footer [Save | v] chevron button
- Select node graph
- Save
- Check workflow on disk - it's still called X.app.json and doesn't have
linearMode: false <-- bug

## Changes

- **What**: 
- pass isApp to save workflow
- ensure active graph & initialMode are correctly set when calling
saveAs BEFORE the actual saveWorkflow call
- add linearMode to workflowShema to prevent casts
- tests

## Review Focus
e2e tests coming in a follow up PR along with some refactoring of the
browser tests (left this PR focused to the actual fix)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10679-fix-App-mode-Save-as-not-using-correct-extension-or-persisting-mode-on-change-3316d73d365081ef985cf57c91c34299)
by [Unito](https://www.unito.io)
2026-03-28 12:08:35 -07:00
Terry Jia
3eb7c29ea4 test: add image compare widget basic e2e tests (#10597)
## Summary
test: add image compare widget basic e2e tests

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10597-test-add-image-compare-widget-basic-e2e-tests-3306d73d365081699125e86b6caa7188)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-03-27 18:15:11 -07:00
Terry Jia
070a5f59fe add basic mask editor tests (#10574)
## Summary
add basic mask editor tests

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10574-add-basic-mask-editor-tests-32f6d73d36508170b8b2c684be56cd26)
by [Unito](https://www.unito.io)
2026-03-27 16:04:36 -04:00
pythongosssss
7864e780e7 feat: App mode - Rework save flow (#10439)
## Summary

Users were finding the final step of the builder flow
confusing/misleading, with the "choose default mode" not actually saving
the workflow and people losing changes. This updates it to remove
"save"/"set default" as a step in the builder, and changes it to a
distinct action.

## Changes

- **What**: 
- add mode selection tab on footer toolbar
- extract reusable radio group component
- remove setting default mode dialog
- add save/save as/saved dialogs

## Screenshots (if applicable)


https://github.com/user-attachments/assets/c7439c2e-a917-4f2b-b176-f8bb8c10026d

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10439-feat-App-mode-Rework-save-flow-32d6d73d3650814781b6c7bbea685a97)
by [Unito](https://www.unito.io)
2026-03-27 12:53:09 -07:00
Arthur R Longbottom
7e7e2d5647 fix: persist subgraph viewport across navigation and tab switches (#10247)
## Summary

Fix subgraph viewport (zoom + position) drifting when navigating in/out
of subgraphs and switching workflow tabs.

## Problem

Three root causes:
1. **First visit**: `restoreViewport()` silently returned on cache miss,
leaving canvas at stale position
2. **Cross-workflow leakage**: Cache keyed by bare `graphId` — two
workflows with the same subgraph or unsaved workflows shared cache
entries
3. **Stale save on tab switch**: `loadGraphData` and
`changeTracker.restore()` overwrite `canvas.ds` before the async watcher
could save the old viewport

## Solution

1. **Workflow-scoped cache keys**: `${path}#${instanceId}:${graphId}` —
WeakMap assigns unique IDs per workflow object, handling unsaved
workflows with identical paths
2. **`flush: 'sync'` on activeSubgraph watcher**: Fires immediately
during `setGraph()`, BEFORE `loadGraphData`/`changeTracker` can corrupt
`canvas.ds`
3. **Cache miss → rAF fitToBounds**: On first visit, computes bounds
from `graph._nodes` and calls `ds.fitToBounds()` after the browser has
rendered
4. **Workflow switch watcher** (`flush: 'sync'`): Pre-saves viewport
under old workflow identity, suppresses `onNavigated` saves during load
cycle

Key architectural insight: `setGraph()` never touches `canvas.ds`, but
`loadGraphData` and `changeTracker.restore()` both write to it. By using
`flush: 'sync'`, the save happens during `setGraph` (before the
overwrites).

## Review Focus

- `subgraphNavigationStore.ts` — the three fixes and their interaction
- `flush: 'sync'` watchers — critical for correct save timing
- `suppressNavigatedSave` flag — prevents stale saves during async
workflow load

## Breaking Changes

None. Viewport cache is session-only (in-memory LRU). Existing workflows
unaffected.

## Demo Video of Fix


https://github.com/user-attachments/assets/71dd4107-a030-4e68-aa11-47fe00101b25

## Test plan

- [x] Unit: save/restore with workflow-scoped keys
- [x] Unit: cache miss doesn't mutate canvas synchronously
- [x] Unit: navigation integration (enter/exit preserves viewport)
- [x] E2E: first subgraph visit has visible nodes
- [x] Manual: enter subgraph → zoom/pan → exit → re-enter → viewport
restored
- [x] Manual: tab with subgraph → different tab → back → viewport
restored
- [x] Manual: two unsaved workflows → switch between → viewports
isolated

- Fixes #10246
- Related: #8173
<!-- QA_REPORT_SECTION -->
---
## 🔍 Automated QA Report

| | |
|---|---|
| **Status** |  Complete |
| **Report** |
[sno-qa-10247.comfy-qa.pages.dev](https://sno-qa-10247.comfy-qa.pages.dev/)
|
| **CI Run** | [View
workflow](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/23373279990)
|

Before/after video recordings with **Behavior Changes** and **Timeline
Comparison** tables.
2026-03-27 19:32:57 +00:00
pythongosssss
dabfc6521e test: Add test to prevent regression of workflow corruption during graph loading (#10623)
## Summary

Adds regression test for
https://github.com/Comfy-Org/ComfyUI_frontend/pull/9531

## Changes

- **What**:  
- registers extension that triggers checkState during
afterConfigureGraph (at this point the workflow data and active graph
are not in sync), previously causing it to overwrite the workflow data
- switches between tabs
- ensures they are not corrupted

Line 35 can be uncommented to cause the test to fail by clearing the
isLoadingGraph flag

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10623-test-Add-test-to-prevent-regression-of-workflow-corruption-during-graph-loading-3306d73d3650815fbf02ef23dfdcddce)
by [Unito](https://www.unito.io)
2026-03-27 11:54:07 -07:00
Benjamin Lu
db6e5062f2 test: add assets sidebar empty-state coverage (#10595)
## Summary

Add the first user-centric Playwright coverage for the assets sidebar
empty state and introduce a small assets-specific test helper/page
object surface.

## Changes

- **What**: add `AssetsSidebarTab`, add `AssetsHelper`, and cover
generated/imported empty states in a dedicated browser spec

## Review Focus

This is intentionally a small first slice for assets-sidebar coverage.
The new helper still mocks the HTTP boundary in Playwright for now
because current OSS job history and input files are global backend
state, which makes true backend-seeded parallel coverage a separate
backend change.

Long-term recommendation: add backend-owned, user-scoped test seeding
for jobs/history and input assets so browser tests can hit the real
routes on a shared backend. Follow-up: COM-307.

Fixes COM-306

## Screenshots (if applicable)

Not applicable.

## Validation

- `pnpm typecheck:browser`
- `pnpm exec playwright test browser_tests/tests/sidebar/assets.spec.ts
--project=chromium` against an isolated preview env

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10595-test-add-assets-sidebar-empty-state-coverage-3306d73d365081d1b34fdd146ae6c5c6)
by [Unito](https://www.unito.io)
2026-03-26 21:19:38 -07:00
Terry Jia
6da5d26980 test: add painter widget e2e tests (#10599)
## Summary
add painter widget e2e tests

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10599-test-add-painter-widget-e2e-tests-3306d73d365081899b3ec3e1d7c6f57c)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-03-27 00:19:17 -04:00