- baseTooltip.spec.ts: use @e2e/ alias for ComfyPage import
- OutputSlot.test.ts: stub BaseTooltip and mock tooltipsEnabled/tooltipDelay
- NodeWidgets.test.ts: stub BaseTooltip so widget slot renders in tests
## 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)
## Summary
This is a follow-up PR of #11102
| Requirement | Status | Implementation |
| :--- | :--- | :--- |
| Add vitest configuration for desktop-ui workspace | ✅ Done | Added
`apps/desktop-ui/vitest.config.mts` with `happy-dom` environment, `@`
alias, and `setupFiles` pointing to `src/test/setup.ts` (registers
`@testing-library/jest-dom` matchers) |
| Add test:unit script to package.json | ✅ Done | Added `"test:unit":
"vitest run --config vitest.config.mts"` to
`apps/desktop-ui/package.json` |
| stores/maintenanceTaskStore.ts | ✅ Done | 34 tests covering task state
machine, IPC integration, executeTask flow, and error handling via
`@pinia/testing` |
| utils/electronMirrorCheck.ts | ✅ Done | 5 tests covering URL
validation, canAccessUrl delegation, and true/false return logic |
| utils/refUtil.ts (useMinLoadingDurationRef) | ✅ Done | 7 tests
covering initial state, timing behavior using `vi.useFakeTimers`, and
computed ref input |
| utils/envUtil.ts | ✅ Done | 7 tests covering electronAPI detection and
fallback behavior |
| constants/desktopDialogs.ts | ✅ Done | 8 tests covering dialog
structure and field contracts |
| constants/desktopMaintenanceTasks.ts | ✅ Done | 5 tests covering
`pythonPackages.execute` success/failure return values, and URL-opening
tasks calling `window.open` |
| composables/bottomPanelTabs/useTerminal.ts | ✅ Done | 7 tests covering
key event handler: Ctrl/Meta+C with/without selection, Ctrl/Meta+V,
non-keydown events, and unrelated keys — mocked xterm with Vitest
v4-compatible function constructors |
| composables/bottomPanelTabs/useTerminalBuffer.ts | ✅ Done | 2 tests
for `copyTo`: verifies serialized buffer content is written to
destination terminal |
| utils/validationUtil.ts | ⛔ Skipped | The current file contains only a
`ValidationState` enum with no logic. There is no behavior to test
without writing a change-detector test (asserting enum values), which
violates project testing guidelines |
**Additional config changes (not in issue but required to make tests
work):**
| Change | Reason |
| :--- | :--- |
| Added `"vitest.config.mts"` to `apps/desktop-ui/tsconfig.json` include
| Required for ESLint's TypeScript parser to process the config file
without a parsing error |
| Removed 6 redundant test devDependencies from
`apps/desktop-ui/package.json` | `vitest`, `@testing-library/*`,
`@pinia/testing`, `happy-dom` are already declared at the root and
hoisted by pnpm — re-declaring them in the sub-package is unnecessary |
## Changes
- Add vitest.config.mts with happy-dom environment and path aliases
- Add src/test/setup.ts to register @testing-library/jest-dom matchers
- Add test:unit script to package.json
- Add vitest.config.mts to tsconfig.json include for ESLint
compatibility
- Remove redundant test devDependencies already declared at root
- Add 132 tests across 16 files covering stores, composables, utils, and
constants
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Test- and config-only changes; main risk is CI/build instability from
new Vitest configuration or brittle mocks, with no runtime behavior
changes shipped to users.
>
> **Overview**
> Adds a dedicated Vitest setup for `apps/desktop-ui` (new
`vitest.config.mts` using `happy-dom`, aliases, and a `jest-dom` setup
file) and wires it into the workspace via a new `test:unit` script plus
`tsconfig.json` inclusion.
>
> Introduces a broad set of new unit tests for desktop UI components,
composables, constants, utilities, and the `maintenanceTaskStore`
(mocking Electron/PrimeVue/Xterm as needed) to validate state
transitions, validation flows, and key UI behaviors without changing
production logic.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
0a96ffb37c. 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-11275-test-add-unit-test-suite-for-apps-desktop-ui-3436d73d36508145ae1fe99ec7a3a4fa)
by [Unito](https://www.unito.io)
## Summary
Adds Playwright coverage for `Preview3D execution` and persistence :
real queue execution against a `Load3D → Preview3D` workflow, plus `save
/ full reload / reopen` from the sidebar.
## What these tests do
**Fixture** (every test)
Turns on Vue Nodes, uses the sidebar for workflows, loads a Load3D →
Preview3D workflow, waits for nodes, then clears saved workflows after
the test so runs stay isolated.
**Test 1 — execution updates Preview3D**
Uploads `cube.obj`(the existing test file in the merged version) to
Load3D, runs `Queue Prompt`, then checks that Preview3D’s model_file and
Last Time Model File match and the canvas has non-zero size. No 3D
screenshots (GPU flakiness).
**Test 2 — persistence after reload**
Same upload + queue, then saves the workflow, reloads the page,
re-applies the same UI settings, opens the saved workflow, and checks
the same model path and camera state (with a small numeric tolerance).
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Adds new slow, WebGL-dependent E2E tests and fixtures, which can
increase CI runtime and introduce flakiness due to timing/graphics
variability, but does not change production logic.
>
> **Overview**
> Adds a new `Load3D → Preview3D` workflow asset and a dedicated
Playwright fixture (`Preview3DPipelineFixture`) to drive real queue
execution, upload a 3D model, and interact with the 3D canvases (orbit
drags) while asserting `model_file`/`Last Time Model File` and camera
state via node properties.
>
> Introduces camera-state comparison helpers with explicit numeric
tolerances, and adds a new `preview3dExecution.spec.ts` suite that
validates (1) Preview3D updates from execution output and (2) model +
camera persistence across save, full page reload, and reopening the
workflow from the sidebar.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
5f54b0f650. 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-11014-test-add-Preview3D-execution-flow-E2E-tests-33e6d73d3650811fa298c364ae196606)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Terry Jia <terryjia88@gmail.com>
## 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
## 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>
## Summary
Replace two hardcoded blank-canvas click positions in
`copyPaste.spec.ts` with the existing
`comfyPage.canvasOps.clickEmptySpace()` helper.
## Changes
- **What**: Both `{ x: 50, y: 500 }` click literals in the `Copy paste
node, image paste onto LoadImage, image paste on empty canvas` test now
use `canvasOps.clickEmptySpace()` (which wraps
`DefaultGraphPositions.emptySpaceClick = { x: 35, y: 31 }`). Redundant
`await nextFrame()` calls dropped — the helper already awaits a frame
internally.
## Review Focus
Draft PR — need CI to confirm `(35, 31)` is a valid blank-canvas click
for the `load_image_with_ksampler` workflow used by this test. The
workflow places `LoadImage` at `[50, 50]` and `KSampler` at `[500, 50]`,
so `(35, 31)` should be clear of both. Locally the test was already
failing on `main` (pre-existing, unrelated), so CI is the source of
truth here. If CI fails, the fallback is to add a dedicated named
constant `emptyCanvasClick: { x: 50, y: 500 }` to
`DefaultGraphPositions` as originally proposed in the issue.
Fixes#10330
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10991-refactor-test-use-canvasOps-clickEmptySpace-in-copyPaste-spec-33d6d73d3650817aa3ccea44cb48c0ae)
by [Unito](https://www.unito.io)
## 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)
## Summary
Add a validation step after merging E2E coverage shards to detect data
loss and improve observability.
## Changes
- **What**: After `lcov -a` merges shard LCOVs, a new step parses merged
+ per-shard stats (source files, lines hit) and writes them to the
**GitHub Actions job summary** as a markdown table. If merged `LH`
(lines hit) is less than any single shard's `LH`, an error annotation is
emitted — this invariant should never be violated since merging should
only add coverage.
- Helps diagnose the 68% → 42% E2E coverage drop after sharding was
introduced.
## Review Focus
The step is informational — it emits `::error::` annotations but does
not `exit 1`, so it won't block the workflow. We can make it a hard
failure once we're confident the merge is stable.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11290-fix-add-validation-to-E2E-coverage-shard-merge-3446d73d365081c8a942e92deba92006)
by [Unito](https://www.unito.io)
## 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.
## 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)
## 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)
## 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)
## Summary
Internalize `nextFrame()` calls into fixture/helper methods so spec
authors don't need to remember to call it after common operations.
`nextFrame()` waits for one `requestAnimationFrame` (~16ms) — an extra
call is always safe, making this a low-risk refactor.
## Changes
### Phase 1: `SettingsHelper.setSetting()`
`setSetting()` now calls `nextFrame()` internally. Removed 15 redundant
calls across 7 files.
### Phase 2: `CommandHelper.executeCommand()`
`executeCommand()` now calls `nextFrame()` internally. Removed 15
redundant calls across 7 files, including the now-redundant call in
`AppModeHelper.toggleAppMode()`.
### Phase 3: `WorkflowHelper.loadGraphData()`
New helper wraps `page.evaluate(loadGraphData)` + `nextFrame()`.
Migrated `SubgraphHelper.serializeAndReload()` and `groupNode.spec.ts`.
### Phase 4: `NodeReference` cleanup
Removed redundant `nextFrame()` from `copy()`, `convertToGroupNode()`,
`resizeNode()`, `dragTextEncodeNode2()`, and
`convertDefaultKSamplerToSubgraph()`. Removed 6 spec-level calls after
`node.click('title')`.
### Phase 5: `KeyboardHelper.press()` and `delete()`
New convenience methods that press a key and wait one frame. Converted
40 `canvas.press(key)` + `nextFrame()` pairs across 13 spec files.
### Phase 6: `ComfyPage.expectScreenshot()`
New helper combines `nextFrame()` + `toHaveScreenshot()`. Converted 45
pairs across 12 spec files.
## Total impact
- **~130 redundant `nextFrame()` calls eliminated** across ~35
spec/helper files
- **3 new helper methods** added (`loadGraphData`, `press`/`delete`,
`expectScreenshot`)
- **2 existing methods** enhanced (`setSetting`, `executeCommand`)
## What was NOT changed
- `performance.spec.ts` frame-counting loops (intentional)
- `ComfyMouse.ts` / `CanvasHelper.ts` (already internalized)
- `SubgraphHelper.packAllInteriorNodes()` (deliberate orchestration)
- Builder helpers (already internalized)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11166-refactor-internalize-nextFrame-into-fixture-helper-methods-33f6d73d3650817bb5f6fb46e396085e)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
## Summary
A follow-up PR of #11196.
| # | Nit | Action | Reason |
| :--- | :--- | :--- | :--- |
| 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is**
| The `pageErrors` array is an accumulator checked at the end via
`expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken
image URLs don't surface as uncaught JS exceptions during the test run.
A request-wait can't substitute for that behavioral assertion, so the
listener pattern is intentional here. |
| 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass |
**Left as-is** | Helpers such as `setImageCompareValue` and
`moveToPercentage` are only used in this file, making local
encapsulation enough. Extracting them to a page object would increase
the file/interface surface area and violate YAGNI; additionally,
`AGENTS.md` clearly states to "minimize the exported values of each
module. |
| 3 | Use `TestIds` enum for test ID strings | **Fixed** – added
`imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8
inline string IDs in `imageCompare.spec.ts` with
`TestIds.imageCompare.*` references | The project already has a
`TestIds` convention for centralizing test IDs. Inline strings create
drift risk between the Vue component and the test file. |
| 4 | Move `expect.poll` bounding box check to helper/page object |
**Left as-is** | This logic already lives inside `moveToPercentage`,
which is a local helper. Moving it further to a page object is the same
refactor as #2 above. |
| 5 | Remove `// ---` style section header comments | **Fixed** –
removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent
with project guidelines and your explicit preference. Test names already
describe what each block does. |
| 6 | Name magic numbers `400` and `350` | **Fixed** – introduced
`minWidth = 400` and `minHeight = 350` constants in the test |
Descriptive names make the constraint self-documenting and easier to
update if the workflow asset changes. |
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to Playwright E2E test code and shared
selector constants, with no production logic impacted.
>
> **Overview**
> **E2E Image Compare tests now use centralized selectors.** Adds an
`imageCompare` section to `TestIds` and updates `imageCompare.spec.ts`
to reference `TestIds.imageCompare.*` instead of inline `data-testid`
strings.
>
> Cleans up the spec by removing divider comments and naming the minimum
size magic numbers (`minWidth`, `minHeight`) used in the node sizing
assertion.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
ece25be5cc. 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-11260-test-address-review-nits-for-image-compare-E2E-3436d73d365081a69cacc1fff390035a)
by [Unito](https://www.unito.io)
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.
## Summary
Adds a GitHub Actions workflow + TypeScript script that posts to Slack
when a merged PR improves unit or E2E test coverage.
## Changes
- **What**: New `coverage-slack-notify.yaml` workflow triggered on push
to main. Compares current coverage against previous baselines, generates
Slack Block Kit payload with progress bars and milestone celebrations,
posts to `#p-frontend-automated-testing`.
- **Script**: `scripts/coverage-slack-notify.ts` — parses lcov files,
computes deltas, detects milestone crossings (every 5%), builds Slack
payload. Pure functions exported for testability.
- **Tests**: 26 unit tests in `scripts/coverage-slack-notify.test.ts`
covering all pure functions including edge cases (malformed lcov, exact
boundaries, zero coverage).
### Security hardening
- All `${{ }}` expressions moved from `run:` blocks to `env:` variables
- `SLACK_BOT_TOKEN` passed via env var, not inline
- Unique heredoc delimiter (timestamp-based) prevents payload injection
- `parseInt` fallback (`|| 0`) guards against malformed lcov
- PR regex anchored to first line of commit message
### Robustness
- `continue-on-error: true` on Slack post step (outage does not fail the
job)
- Baseline save guarded by `steps.unit-tests.outcome == success`
(prevents corrupt baselines on test failure)
- Channel ID commented for maintainability
- Top-level `text` field added for Slack mobile push notifications
- Author linked to GitHub profile instead of bare `@username`
## Review Focus
- Workflow step ordering and conditional logic
- Security of expression handling and secret management
- Slack payload structure and Block Kit formatting
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10977-feat-add-Slack-notification-workflow-for-coverage-improvements-33d6d73d3650819c8950f483c83f297c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
## 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)
## Summary
- Add `browser_tests/tests/bottomPanel.spec.ts` with tests for behaviors
not covered by existing `bottomPanelLogs` and `bottomPanelShortcuts`
specs
- Tests cover: close button (X), tab persistence on re-open, resize
gutter visibility and drag, canvas interaction when panel is closed,
cross-panel switching (terminal <-> shortcuts), and registered tab
enumeration
- Extend `BottomPanel` fixture with `closeButton` and `resizeGutter`
locators
## Test plan
- [ ] `pnpm test:browser:local` passes all new tests in
`bottomPanel.spec.ts`
- [ ] Existing `bottomPanelLogs.spec.ts` and
`bottomPanelShortcuts.spec.ts` are unaffected
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm lint` passes
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10814-test-add-E2E-tests-for-bottom-panel-core-behaviors-3366d73d365081ea9b90c643897845fa)
by [Unito](https://www.unito.io)
---------
Co-authored-by: dante <dante@danteui-MacStudio.local>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary
- Adds `selectionToolboxMoreActions.spec.ts` with E2E coverage for
previously untested toolbox actions
- Covers: pin/unpin, minimize/expand, adjust size, copy, duplicate,
refresh button, align (top/left), distribute (horizontal/vertical),
alignment options hidden for single selection, multi-node bypass toggle
- Part of the FixIt Burndown test coverage initiative (toolbox actions)
## Test plan
- [ ] All new tests pass in CI
- [ ] No regressions in existing selectionToolbox tests
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10968-test-expand-E2E-coverage-for-toolbox-actions-33c6d73d3650811286cefdd0eb4f5242)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary
- Multiple SubgraphNode instances of the same blueprint share inner
nodes, causing promoted widget values to collide — the last configured
instance overwrites all previous values
- Add per-instance value storage (`_instanceWidgetValues`) on
SubgraphNode so each instance preserves its own promoted widget values
independently
- Restore `widgets_values` from serialized data into this per-instance
map after promoted views are created during configure
- Fixes#10146
## Root Cause
When loading a workflow with multiple SubgraphNode instances of the same
blueprint:
1. `LGraph.configure()` creates ONE shared Subgraph per blueprint (line
2625)
2. Each SubgraphNode instance calls `configure(instanceData)`
sequentially
3. `PromotedWidgetView.value` setter writes to the **shared inner node's
widget** (`promotedWidgetView.ts:199`)
4. The last instance's `configure()` overwrites all previous instances'
values
**Regression**: Introduced by PR #8594 (WidgetValueStore, v1.41.3) which
centralized widget state without per-instance scoping for shared
blueprints.
## Fix
- **SubgraphNode**: Add `_instanceWidgetValues` Map and
`_pendingWidgetsValues` for configure-time restoration
- **PromotedWidgetView getter**: Check instance map first before falling
back to widget store / inner node
- **PromotedWidgetView setter**: Write to instance map to avoid shared
inner node mutation
- **_internalConfigureAfterSlots**: Apply serialized `widgets_values` to
per-instance map after promoted views are created
## Red-Green Verification
| Commit | CI Status | Purpose |
|--------|-----------|---------|
| `test: add failing tests for multi-instance subgraph widget value
collision` | 🔴 Red | Proves widget values collide across
instances |
| `fix: store promoted widget values per SubgraphNode instance` |
🟢 Green | Per-instance storage prevents collision |
## Test Plan
- [x] CI red on test-only commit
- [x] CI green on fix commit
- [x] Unit test: `preserves promoted widget values after configure with
different widgets_values`
- [x] All 253 existing subgraph tests pass
- [ ] Manual: load workflow from issue image → verify 3 subgraph
instances produce different results
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10849-fix-store-promoted-widget-values-per-SubgraphNode-instance-3386d73d3650815a8544f54adcc0d504)
by [Unito](https://www.unito.io)
---------
Co-authored-by: dante <dante@danteui-MacStudio.local>
Leveraging the fancy coverage functionality of #10930, this PR aims to
add coverage to missing dialogue models.
This has proven quite constructive as many of the dialogues have since
been shown to be bugged.
- The APINodes sign in dialog that displays when attempting to run a
workflow containing Partner nodes while not logged in was intended to
display a list of nodes required to execute the workflow. The import for
this component was forgotten in the original commit (#3532) and the
backing component was later knipped
- Error dialogs resulting are intended to display the file responsible
for the error, but the prop was accidentally left out during the
refactoring of #3265
- ~~The node library migration (#8548) failed to include the 'Edit
Blueprint' button, and had incorrect sizing and color on the 'Delete
Blueprint' button.~~
- On request, the library button changes were spun out to a separate PR
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11133-Add-missing-dialog-tests-33e6d73d3650812cb142d610461adcd4)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
The node library migration (#8548) failed to include the 'Edit
Blueprint' button, and had incorrect sizing and color on the 'Delete
Blueprint' button.
- Re-add edit blueprint which was missed in the migration
- Fix incorrect sizing on delete blueprint
- Fix color (lucide uses background, not text)
- Migrate all action buttons use our capital 'B' Button component and
use standardized variants where possible
Spun out of #11133
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11232-Fix-node-library-action-buttons-3426d73d365081339cafc7457c0b5421)
by [Unito](https://www.unito.io)
## Summary
Add Claude Code PreToolUse hooks to block agents from bypassing
package.json scripts with raw tool invocations.
## Changes
- **What**: 15 PreToolUse hooks in `.claude/settings.json` that
intercept `npx`/`pnpx`/bare invocations of tsc, vitest, eslint,
prettier, oxlint, stylelint, and knip — redirecting agents to the
correct `pnpm` script (`pnpm typecheck`, `pnpm test:unit`, `pnpm lint`,
etc.)
- Also removes stale `permissions.allow` entries left over from a
debugging session
## Review Focus
- Pattern coverage: are there common agent invocations we're missing?
- The `if` field only supports simple `*` globs (no alternation), so
each pattern needs its own hook entry
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11201-feat-add-PreToolUse-hooks-to-enforce-pnpm-scripts-3416d73d365081a59a38c86ee4669aee)
by [Unito](https://www.unito.io)
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Add `release_type` input (`minor`/`patch`) to the release workflow so
patch releases can target the current production branch instead of
always preferring the next minor.
## Problem
When a patch release is needed for `core/1.42` but `core/1.43` already
exists, the resolver always prefers `1.43`. There was no way to do a
patch release with PyPI publish + ComfyUI PR for the current production
version.
## Changes
- Rename workflow from "Release: Bi-weekly ComfyUI" → "Release: ComfyUI"
(serves both cadences)
- Add `release_type` choice input: `minor` (default, bi-weekly) vs
`patch` (hotfix for current production version)
- Update `resolve-comfyui-release.ts` to read `RELEASE_TYPE` env var for
branch targeting
- Scheduled runs continue to work as before (default to `minor`)
## Usage
```bash
# Bi-weekly minor release (or just let the schedule run)
gh workflow run release-biweekly-comfyui.yaml --ref main
# Patch release for current production version
gh workflow run release-biweekly-comfyui.yaml --ref main --field release_type=patch
```
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11223-ops-add-patch-release-support-to-ComfyUI-release-workflow-3426d73d365081498c15ff978a7f1116)
by [Unito](https://www.unito.io)
## Problem
PR #10338 added `useTransformCompatOverlayProps()` to FormDropdown and
FormDropdownMenuActions, which sets `appendTo: 'self'` in graph mode.
This breaks PrimeVue Popover positioning inside CSS-transformed
containers — the dropdown appears at incorrect Y positions.
## Root Cause
PrimeVue Popover with `appendTo: 'self'` renders the overlay inside the
component's DOM, inheriting parent CSS transforms. This causes the
popover to miscalculate its position when the parent has `transform:
scale()` or `translate()`.
## Fix
Remove the `appendTo` override from both FormDropdown and
FormDropdownMenuActions. PrimeVue defaults to `appendTo: 'body'`, which
teleports the popover to `<body>` — correctly positioning it outside any
CSS transform context.
- **Graph mode**: restores pre-#10338 behavior (`appendTo: 'body'`
default)
- **App mode**: unaffected — `'body'` is exactly what app mode needs
(prevents sidebar overflow clipping)
## Testing
- Existing unit tests pass (5/5)
- Typecheck clean
- Lint clean
- **E2E test rationale**: No E2E test added — this is a pure removal of
a prop override (reverting to PrimeVue defaults). The positioning bug
requires CSS transforms at specific viewport scales which are
impractical to assert reliably in Playwright. The existing
`subgraph-dom-widget-clipping` perf test exercises dropdown rendering in
transformed contexts and shows no regression.
Fixes#10499
Supersedes #11001 (temporary hotfix for backport)
---------
Co-authored-by: GitHub Action <action@github.com>
## Summary
Prevent early `progress_text` websocket events from throwing before the
graph canvas is initialized.
## Changes
- **What**: Guard `handleProgressText()` until `canvasStore.canvas`
exists, and add a regression test for a startup-time `progress_text`
event arriving before `GraphCanvas` finishes initialization.
## Review Focus
Confirm this is the right guard point for the startup race between
`GraphView` websocket binding and `GraphCanvas` async setup, and that
progress text behavior is unchanged once the canvas is ready.
## Validation
- `pnpm exec eslint src/stores/executionStore.ts
src/stores/executionStore.test.ts`
- `pnpm exec vitest run src/stores/executionStore.test.ts -t "should
ignore progress_text before the canvas is initialized"`
- `pnpm test:unit -- --run src/stores/executionStore.test.ts` still
reports one unrelated isolated-file failure in
`nodeLocatorIdToExecutionId` on current `main`
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11174-fix-guard-progress_text-before-canvas-init-3406d73d3650813dad23d511fb51add5)
by [Unito](https://www.unito.io)
## Summary
Follow-up cleanup for #11184 — removes redundant test setup calls that
the `@vue-nodes` fixture now handles.
## Changes
- **What**: Remove 40 lines of redundant `setSetting`, `setup()`, and
`waitForNodes()` calls across 11 test files
- `UseNewMenu: 'Top'` calls (already fixture default)
- `setup()` + `waitForNodes()` on default workflow (fixture already does
this for `@vue-nodes`)
- Page reload in `subgraphZeroUuid` (fixture applies VueNodes.Enabled
server-side before navigation)
## Review Focus
Each removal was verified against the fixture's `setupSettings()`
defaults (ComfyPage.ts:420-442) and the `@vue-nodes` auto-setup (lines
454-456). Tests that call `setup()`/`waitForNodes()` after
`loadWorkflow()` or `page.evaluate()` were intentionally kept.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11195-test-remove-redundant-setup-settings-now-handled-by-vue-nodes-fixture-3416d73d36508154827df116a97e9130)
by [Unito](https://www.unito.io)
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Track GTM `subscription_success` when a workspace subscription completes
synchronously in the dialog. The async billing-operation path already
emitted telemetry; the missing gap was the immediate `subscribed`
response.
## Changes
- **What**: Add the missing GTM success emission to both synchronous
workspace subscribe success branches while preserving the existing
toast, billing refresh, and dialog close behavior.
## Review Focus
Verify the synchronous `response.status === "subscribed"` workspace
dialog paths are the only missing frontend success emissions, while the
async billing-operation telemetry path remains unchanged.
This PR intentionally stays minimal. It does not add new browser
coverage yet; the previous component-level unit test was more
implementation-coupled than this fix justified, and a better long-term
test would be a higher-level workspace billing flow test once we have a
cleaner harness.
## Problem
API node generation status text (sent via `progress_text` WebSocket
binary messages) was not showing on local ComfyUI, but worked on cloud.
## Root Cause
The binary decoder for `progress_text` messages (eventType 3) checked
`getClientFeatureFlags()?.supports_progress_text_metadata` — the
**client's own flags** — to decide whether to parse the new format with
`prompt_id`. Since the client always advertises
`supports_progress_text_metadata: true`, it always tried to parse the
new wire format:
```
[4B event_type][4B prompt_id_len][prompt_id][4B node_id_len][node_id][text]
```
But the backend PR that adds `prompt_id` to the binary message
([ComfyUI#12540](https://github.com/Comfy-Org/ComfyUI/pull/12540)) was
**closed without merging**, so local ComfyUI still sends the legacy
format:
```
[4B event_type][4B node_id_len][node_id][text]
```
The decoder misinterpreted the `node_id_len` as `prompt_id_len`,
consuming the actual node_id bytes as a prompt_id, then producing
garbled `nodeId` and `text` — silently dropping all progress text
updates via the catch handler.
Cloud worked because the cloud backend supports and echoes the feature
flag.
## Fix
One-line change: check `serverFeatureFlags.value` (what the server
echoed back) instead of `getClientFeatureFlags()` (what the client
advertises).
## Tests
Added 3 tests covering:
- Legacy format parsing when server doesn't support the flag
- New format parsing when server does support the flag
- Corruption regression test: client advertises support but server
doesn't
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10996-fix-check-server-feature-flags-for-progress_text-binary-format-33d6d73d365081449a0dc918358799de)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
## Summary
Fix tests failing lint
## Changes
- **What**:
- Fix relative imports
- Fix test not using comfyPage
## Review Focus
<!-- Critical design decisions or edge cases that need attention -->
<!-- If this PR fixes an issue, uncomment and update the line below -->
<!-- Fixes #ISSUE_NUMBER -->
## Screenshots (if applicable)
<!-- Add screenshots or video recording to help explain your changes -->
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11182-refactor-fix-lint-errors-in-tests-3416d73d3650812fbf1bc88554c57de2)
by [Unito](https://www.unito.io)
## Summary
Muted and bypassed nodes are excluded from execution but were still
triggering missing model/media/node warnings. This PR makes the error
system mode-aware: muted/bypassed nodes no longer produce missing asset
errors, and all error lifecycle events (mode toggle, deletion, paste,
undo, tab switch) are handled consistently.
- Fixes Comfy-Org/ComfyUI#13256
## Behavioral notes
- **Tab switch overlay suppression (intentional)**: Switching back to a
workflow with missing assets no longer re-shows the error overlay. This
reverses the behavior introduced in #10190. The error state is still
restored silently in the errors tab — users can access it via the
properties panel without being interrupted by the overlay on every tab
switch.
## Changes
### 1. Scan filtering
- `scanAllModelCandidates`, `scanAllMediaCandidates`,
`scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS`
- `collectMissingNodes` (serialized data): skip error reporting for
muted/bypassed nodes while still calling `sanitizeNodeName` for safe
`configure()`
- `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes;
workflow-level `graphData.models` only create candidates when active
nodes exist
- `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models
when all referencing nodes are inactive
### 2. Realtime mode change handling
- `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect
`node:property:changed` (mode)
- Deactivation (active → muted/bypassed): remove missing
model/media/node errors for the node
- Activation (muted/bypassed → active): scan the node and add confirmed
errors, show overlay
- Subgraph container deactivation: remove all interior node errors
(execution ID prefix match)
- Subgraph container activation: scan all active interior nodes
recursively
- Subgraph interior mode change: resolve node via
`localGraph.getNodeById()` then compute execution ID from root graph
### 3. Node deletion
- `graph.onNodeRemoved`: remove missing model/media/node errors for the
deleted node
- Handle `node.graph === null` at callback time by using
`String(node.id)` for root-level nodes
### 4. Node paste/duplicate
- `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after
`node.configure()` restores widget values)
- Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab
switch handled by pipeline)
- Guard: skip muted/bypassed nodes
### 5. Workflow tab switch optimization
- `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab
switch
- Cache missing model/media/node state per workflow via
`PendingWarnings`
- `beforeLoadNewGraph`: save current store state to outgoing workflow's
`pendingWarnings`
- `showPendingWarnings`: restore cached errors silently (no overlay),
always sync missing nodes store (even when null)
- Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using
`setMissingModels([])` instead of `clearMissingModels()`
- `MissingModelRow.vue`: fetch file size on mount via
`fetchModelMetadata` memory cache
### 6. Undo/redo overlay suppression
- `silentAssetErrors` option propagated through pipeline →
`surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option
- `showPendingWarnings` `{ silent }` option for missing nodes overlay
- `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo
### 7. Error tab node filtering
- Selected node filters missing model/media card contents (not just
group visibility)
- `isAssetErrorInSelection`: resolve execution ID → graph node for
selection matching
- Missing nodes intentionally unfiltered (pack-level scope)
- `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab
visibility
- Download All button: show only when 2+ downloadable models exist
### 8. New store functions
- `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId`
- `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId`
- `missingNodesErrorStore`: `removeMissingNodesByNodeId`
- `missingModelScan`: `scanNodeModelCandidates` (extracted single-node
scan)
- `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node
scan)
### 9. Test infrastructure improvements
- `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`)
- Error-related TestIds moved from `dialogs` to `errorsTab` namespace in
`selectors.ts`
- Removed unused `TestIdValue` type
- Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts`
- Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab`
- Added `aria-label` to pencil edit button and subgraph toggle button
## Test plan
### Unit tests (41 new)
- Store functions: `addMissing*`, `removeMissing*ByNodeId`
- `executionErrorStore`: `surfaceMissing*` silent option
- Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`,
`scanNodeMediaCandidates`
- `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph`
caching
### E2E tests (17 new in `errorsTabModeAware.spec.ts`)
**Missing nodes**
- [x] Deleting a missing node removes its error from the errors tab
- [x] Undo after bypass restores error without showing overlay
**Missing models**
- [x] Loading a workflow with all nodes bypassed shows no errors
- [x] Bypassing a node hides its error, un-bypassing restores it
- [x] Deleting a node with missing model removes its error
- [x] Undo after bypass restores error without showing overlay
- [x] Pasting a node with missing model increases referencing node count
- [x] Pasting a bypassed node does not add a new error
- [x] Selecting a node filters errors tab to only that node
**Missing media**
- [x] Loading a workflow with all nodes bypassed shows no errors
- [x] Bypassing a node hides its error, un-bypassing restores it
- [x] Pasting a bypassed node does not add a new error
- [x] Selecting a node filters errors tab to only that node
**Subgraph**
- [x] Bypassing a subgraph hides interior errors, un-bypassing restores
them
- [x] Bypassing a node inside a subgraph hides its error, un-bypassing
restores it
**Workflow switching**
- [x] Does not resurface error overlay when switching back to workflow
with missing nodes
- [x] Restores missing nodes in errors tab when switching back to
workflow
# Screenshots
https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5
## Follow-up work
- Extract error-detection computed properties from `RightSidePanel.vue`
into a composable (e.g. `useErrorsTabVisibility`)
---------
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: GitHub Action <action@github.com>
## Summary
Adds Playwright E2E tests verifying that
1. the minimap canvas renders node content
2. clears when the graph is empty
3. correctly navigates the main canvas on click
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10738-test-add-minimap-E2E-tests-for-graph-content-and-click-to-navigate-3336d73d365081eb955ce711b3efc57f)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to adding `data-testid` attributes to
the minimap UI and expanding Playwright E2E assertions, with no
production behavior changes expected.
>
> **Overview**
> Strengthens minimap E2E coverage by switching existing assertions from
CSS selectors to new `data-testid`-based selectors and adding helper
utilities for canvas/overlay interactions.
>
> Adds new Playwright tests that verify the minimap canvas renders
content when nodes exist, clears when the graph is emptied, and that
clicking the minimap pans the main canvas (including a
post-`fitViewToSelectionAnimated` tolerance check).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
06e7542af1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Alexander Brown <DrJKL0424@gmail.com>
Co-authored-by: GitHub Action <action@github.com>
### Summary of Improvements
* **Custom Test Coverage Extension**: Enhanced the Painter widget E2E
test suite by refactoring logic for better maintainability and
robustness.
* **Stable Component Targeting**: Introduced
`data-testid="painter-dimension-text"` to `WidgetPainter.vue`, providing
a reliable, non-CSS-dependent locator for canvas size verification.
* **Improved Test Organization**: Reorganized existing test scenarios
into logical categories using `test.describe` blocks (Drawing, Brush
Settings, Canvas Size Controls, etc.).
* **Asynchronous Helper Integration**: Converted `hasCanvasContent` to
an asynchronous helper and unified its usage across multiple test cases
to eliminate redundant pixel-checking logic.
* **Locator Resilience**: Updated Reka UI slider interaction logic to
use more precise targeting (`:not([data-slot])`), preventing ambiguity
and improving test stability.
* **Scenario Refinement**: Updated the `pointerup` test logic to
accurately reflect pointer capture behavior when interactions occur
outside the canvas boundaries.
* **Enhanced Verification Feedback**: Added descriptive error messages
to `expect.poll` assertions to provide clearer context on potential
failure points.
* **Standardized Tagging**: Restored the original tagging strategy
(including `@smoke` and `@screenshot` tags) to ensure tests are
categorized correctly for CI environments.
### Red-Green Verification
| Commit | CI Status | Purpose |
| :--- | :--- | :--- |
| `test: refactor painter widget e2e tests and address review findings`
| 🟢 Green | Addresses all E2E test quality and stability issues from
review findings. |
### Test Plan
- [x] **Quality Checks**: `pnpm format`, `pnpm lint`, and `pnpm
typecheck` verified as passing.
- [x] **Component Integration**: `WidgetPainter.vue` `data-testid`
correctly applied and used in tests.
- [x] **Helper Reliability**: `hasCanvasContent` correctly identifies
colored pixels and returns a promise for `expect.poll`.
- [x] **Locator Robustness**: Verified Reka slider locators correctly
exclude internal thumb spans.
- [x] **Boundary Interaction**: Verified `pointerup` correctly ends
strokes when triggered outside the viewport.
- [x] **Tagging Consistency**: Verified `@smoke` and `@screenshot` tags
are present in the final test suite.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10846-test-Painter-Widget-E2E-Test-Plan-3386d73d365081deb70fe4afbd417efb)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily adds/refactors Playwright E2E tests and stable `data-testid`
hooks, with no changes to Painter drawing logic. Risk is limited to
potential test brittleness or minor UI attribute changes.
>
> **Overview**
> Expands the Painter widget Playwright suite with new grouped scenarios
covering drawing/erasing behavior, tool switching, brush inputs, canvas
resizing (including preserving drawings), clear behavior, and
serialization/upload flows (including failure toast).
>
> Refactors the tests to use a shared `@e2e/helpers/painter` module
(`drawStroke`, `hasCanvasContent`, `triggerSerialization`), improves
stability via role/testid-based locators and clearer `expect.poll`
messaging, and adds `data-testid` attributes (e.g.,
`painter-clear-button`, `painter-*-row`, `painter-dimension-text`) to
`WidgetPainter.vue` to avoid CSS-dependent selectors.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
053a8e9ed2. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: GitHub Action <action@github.com>
## Summary
Add E2E tests for ImageCompare widget
Covers slider interaction, batch navigation, single-image modes, visual
regression screenshots, and edge cases for the ImageCompare Vue node
widget.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10767-test-add-E2E-tests-for-ImageCompare-widget-3346d73d365081c6bfc6fbd97fa04e4d)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Adds Playwright E2E coverage and screenshot assertions only; main risk
is increased CI runtime/flakiness due to additional image-loading and
hover/position polling.
>
> **Overview**
> Adds a new Playwright E2E suite for the ImageCompare Vue widget
(tagged `@widget`) that programmatically sets widget values and asserts
rendering for empty, single-image, and dual-image states.
>
> Expands coverage to **slider behavior** (default 50%, hover movement,
clamping, persistence) using polling on inline `clip-path`/handle
position, and adds **batch navigation** tests for multi-image
before/after sets.
>
> Introduces **visual regression screenshots** at default and specific
slider positions, plus edge-case tests for broken URLs, rapid updates
resetting batch index, legacy string values, and custom alt text.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
2c65440384. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: GitHub Action <action@github.com>
## Summary
Adds Playwright E2E tests for the ImageCropV2 widget covering
1. the empty state (no source image)
2. default control rendering
3. source image display with crop overlay
4. drag-to-reposition behavior.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10737-test-add-E2E-tests-for-ImageCropV2-widget-3336d73d365081b28ed9db63e5df383e)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: primarily adds Playwright E2E coverage and introduces
`data-testid` attributes for more stable selectors, with no changes to
core crop behavior.
>
> **Overview**
> Adds new Playwright E2E coverage for the `ImageCropV2` Vue-node
widget, including workflows/fixtures for a disconnected input and a
`LoadImage -> ImageCropV2 -> PreviewImage` pipeline.
>
> Tests validate the empty state and default controls, verify the crop
overlay renders after execution with screenshot assertions, and exercise
drag-to-reposition by dispatching pointer events and asserting the
widget’s crop value updates.
>
> Updates `WidgetImageCrop.vue` to add `data-testid` hooks (empty
state/icon and crop overlay) to make the E2E selectors stable.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
9f29272742. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
