# PR Description
#11106
**This PR only focus on `slotCalculations.ts`.**
Add unit tests for `slotCalculations.ts` — the centralized utility that
calculates input/output slot positions in graph coordinates. This file
had zero test coverage despite containing non-trivial branching logic
used by both the litegraph adapter and the Vue renderer layout system.
## What's covered
### `calculateInputSlotPosFromSlot`
- [x] **Collapsed nodes**: Returns the node origin shifted up by half
the title height.
- [x] **Hard-coded offsets**: Slots with specific `pos` offsets return
`nodeOrigin + pos` directly.
- [x] **Default vertical layout**: Covers first slot x/y, multi-slot
vertical ordering, `slotStartY` offset, exclusion of widget input slots,
and exclusion of fixed-position slots from index ordering.
### `getSlotPosition` (Legacy fallback path, `vueNodesMode` disabled)
- [x] **Coordinate derivation**: Input and output slot positions derived
correctly from `node.pos`.
- [x] **Collapsed state**: Collapsed input and output nodes use
`title-height` and `NODE_COLLAPSED_WIDTH` offsets.
- [x] **Boundary handling**: Out-of-range slot index falls back to node
origin.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Test-only changes that don’t affect runtime behavior; risk is limited
to potential brittleness if slot layout constants or expectations
change.
>
> **Overview**
> Adds a new `slotCalculations.test.ts` suite covering
`calculateInputSlotPosFromSlot` and the legacy (`LiteGraph.vueNodesMode`
disabled) path of `getSlotPosition`.
>
> Tests exercise key branches for collapsed nodes, hard-coded slot `pos`
overrides, default vertical slot ordering (including `slotStartY`), and
filtering of widget/fixed-position inputs, plus boundary behavior for
out-of-range slot indices.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
51c5318695. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11302-test-add-unit-tests-for-slotCalculations-3446d73d36508181a0ade81be05bd25f)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
Closes coverage gaps in \`src/composables/\` as part of the unit-test
backfill.
## Testing focus
Three composables, each a different kind of test challenge: reactive
pagination state, DOM-track drag math, and SVG pointer interaction. No
third-party library is mocked.
### \`useLazyPagination\` (10 tests)
- Accepts both \`Ref<T[]>\` and plain \`T[]\` inputs.
- \`currentPage\` ceiling at \`totalPages\` (clamp behavior).
- Source-array replacement resets internal page state.
- \`loadedPages\` (Set) accumulates across navigation.
- **Observed source issue.** \`loadNextPage\` is declared \`async\` but
contains no \`await\` (the artificial delay is commented out).
Consequence: \`isLoading\` is never externally observable as \`true\`,
and the concurrent-call dedup in the design doesn't actually fire in
practice. Tests cover **observable** behavior only; the finding is noted
here as a candidate follow-up fix.
### \`useRangeEditor\` (11 tests)
- Drags each of \`min\` / \`max\` / \`midpoint\` handles; respects the
\`showMidpoint\` toggle (events on the midpoint are ignored when
hidden).
- Value clamping within \`[valueMin, valueMax]\`.
- \`denormalize\` receives the correct normalized position — verifies
the 0–1 mapping math, not just that it was called.
- \`trackRef.value === null\` → pointer events are no-ops (null-safety).
- **Real lifecycle.** Mounts a tiny \`defineComponent\` via
\`@testing-library/vue\`'s \`render\` and exercises cleanup through
\`unmount()\`. \`onBeforeUnmount\` only fires inside a component
instance — \`effectScope.stop()\` alone is insufficient.
### \`useCurveEditor\` (14 tests)
- \`curvePath\` empty when fewer than 2 points.
- Linear interpolation: \`M\` + \`L\` command sequence, points sorted by
x before drawing.
- Non-linear uses \`createInterpolator\` (our module → OK to mock and
assert call shape).
- Drag: dispatching \`pointermove\` updates \`modelValue\`; after
\`pointerup\`, a follow-up \`pointermove\` is a no-op.
- **happy-dom gaps polyfilled.** \`Element.setPointerCapture\` is
stubbed per-element and \`DOMPoint.prototype.matrixTransform\` is added
in \`beforeEach\`. Since the SVG has no CTM, \`DOMMatrix.inverse()\`
returns identity — so \`svgCoords\` maps \`clientX\`/\`clientY\`
directly into curve space, giving deterministic assertions without
brittle coordinate math.
## Principles applied
- No mocks of \`vue\`, \`@vueuse/core\`, or \`es-toolkit\`.
- Behavioral assertions only — no return-shape checks.
- All 35 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
## Summary
Show the sign-in button in the frontend when the `show_signin_button`
server feature flag is set, without requiring a special desktop
distribution build.
## Changes
- Add `SHOW_SIGNIN_BUTTON` to `ServerFeatureFlag` enum
- Add `showSignInButton` getter in `useFeatureFlags` composable (returns
`boolean | undefined`)
- Update `WorkflowTabs.vue` to use `flags.showSignInButton ?? isDesktop`
- the server flag takes precedence when set, falls back to compile-time
`isDesktop` for legacy desktop support
## Related
- Comfy-Org/ComfyUI-Desktop-2.0-Beta#415
- Backend: Comfy-Org/ComfyUI `feature/generic-feature-flag-cli`
- Launcher: Comfy-Org/ComfyUI-Desktop-2.0-Beta#418
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Vercel's auto-deploy triggers on every PR because files outside
workspace packages (e.g. `browser_tests/`, `src/`) are treated as global
changes by the monorepo skip logic.
## Changes
- **What**: Replace Vercel's GitHub integration with a GitHub Action
(`ci-vercel-website-preview.yaml`) that uses `paths:` filtering to only
deploy when `apps/website/`, `packages/design-system/`, or
`packages/tailwind-utils/` change. Add `vercel.json` with
`github.enabled: false` to disable Vercel's automatic GitHub
integration.
## Setup required after merge
Three GitHub repo secrets are needed. All secrets are scoped per-project
using the `VERCEL_WEBSITE_*` prefix. Future Vercel projects would follow
the same convention (e.g. `VERCEL_DOCS_*`).
### Step 1: Create a Vercel API Token
1. Go to [vercel.com/account/tokens](https://vercel.com/account/tokens)
2. Click **Create Token**
3. Fill in the form:
- **Token Name**: `github-actions-website`
- **Scope**: Select the **Comfy-Org** team (not "Full Account" — scope
it to the team that owns the project)
- **Expiration**: Choose **No Expiration** (or set a long expiration
like 1 year — if it expires the workflow will silently fail)
4. Click **Create**
5. **Copy the token immediately** — it is only shown once
### Step 2: Get Vercel Org ID and Project ID
1. Go to
[vercel.com/comfyui/website-frontend/settings](https://vercel.com/comfyui/website-frontend/settings)
2. Scroll down to the **Project ID** field — copy this value
3. Go to
[vercel.com/teams/comfyui/settings](https://vercel.com/teams/comfyui/settings)
(Team Settings → General)
4. Find the **Vercel ID** field (also called Team ID / Org ID) — copy
this value
### Step 3: Add secrets to GitHub
1. Go to
[github.com/Comfy-Org/ComfyUI_frontend/settings/secrets/actions](https://github.com/Comfy-Org/ComfyUI_frontend/settings/secrets/actions)
2. Click **New repository secret** and add each of the three secrets:
| Secret name | Value |
|---|---|
| `VERCEL_WEBSITE_TOKEN` | The token from Step 1 |
| `VERCEL_WEBSITE_ORG_ID` | The team/org ID from Step 2 |
| `VERCEL_WEBSITE_PROJECT_ID` | The project ID from Step 2 |
> **Note:** The `vercel.json` added by this PR (`github.enabled: false`)
automatically disables Vercel's built-in auto-deploy — no dashboard
changes needed.
## Review Focus
- Verify the `paths:` filter covers all dependencies of `apps/website`
- Confirm the PR comment logic is sound (creates once, updates on
subsequent pushes)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: DrJKL <DrJKL0424@gmail.com>
Co-authored-by: Amp <amp@ampcode.com>
Closes coverage gaps in `src/platform/cloud/subscription/` as part of
the unit-test backfill.
## Testing focus
`useBillingPlans` holds **module-scoped refs** (`plans`,
`currentPlanSlug`, `isLoading`, `error`). If state leaks between tests,
failures get masked as false-green. The suite uses `vi.resetModules()` +
dynamic `import()` in every test to get a fresh instance — state
isolation is the primary design constraint here.
### `useBillingPlans` (12 tests)
- **Concurrent-call dedup.** The \`isLoading\` guard is validated by
creating a pending promise, firing a second \`fetchPlans()\` while the
first is in-flight, and asserting the mock is called **exactly once**.
- **Error branching.** \`Error\` instance → \`.message\` captured.
Non-Error rejection → fallback string (\`'Failed to fetch plans'\`).
Both paths also verify \`console.error\` logging via a spy.
- **Error-reset invariant.** After a failure, a subsequent success must
null out \`error.value\` — order-dependent and easy to regress.
- **Shared-state invariant.** Two separate \`useBillingPlans()\` calls
return refs pointing at the same module-level state.
- **Computed filtering.** \`monthlyPlans\` / \`annualPlans\` partition
by duration — assertions on distinct output, not input re-assertion.
### \`tierBenefits\` (7 tests)
- Table-driven across all \`TierKey\` values for \`maxDuration\`,
\`addCredits\`, \`customLoRAs\` branches.
- \`monthlyCredits\` free-tier path including the
\`remoteConfig.free_tier_credits\` null fallback.
- Translator/formatter forwarding verified by spy.
## Principles applied
- No mocks of \`vue\`, \`pinia\`, or \`@vueuse/core\` — only our own
\`workspaceApi\`.
- Behavioral assertions only — no return-shape checks.
- All 19 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
## Summary
Follow-up to PR #10816. Fixes a telemetry semantic bug in
`saveWorkflowAs` that emitted two conflicting events for a single user
action.
### What changed
- `saveWorkflowAs` self-overwrite branch now calls
`workflowStore.saveWorkflow` directly instead of delegating to the
`saveWorkflow()` wrapper. The wrapper would run `prepareForSave` a
second time and emit `trackWorkflowSaved({ is_new: false })`, which then
conflicted with the outer `saveWorkflowAs`'s `trackWorkflowSaved({
is_new: true })` for the same user action.
- Added regression tests asserting a single `trackWorkflowSaved` call
with `{ is_new: true }` and a single `prepareForSave` invocation on both
the self-overwrite and copy paths.
### Issues fixed
- Fixes#10819
### Why no E2E test
The bug and fix are entirely about observability (how many telemetry
events are emitted and with what payload). There is no user-visible
change — the file is saved correctly in both pre- and post-fix cases,
and `is_new` values are never rendered in the UI. Playwright tests
cannot directly verify `trackWorkflowSaved` call counts/payloads without
intercepting outbound analytics traffic, which is not a pattern used
elsewhere in `browser_tests/`. Unit tests at the service boundary are
the appropriate level for this contract: they mock `useTelemetry` and
can assert exact call count and payload deterministically.
### Test plan
- [x] `pnpm test:unit
src/platform/workflow/core/services/workflowService.test.ts` — 56 tests
pass (including 2 new regression tests + 1 expanded assertion)
- [x] `pnpm typecheck`
- [x] `pnpm lint`
- [x] `pnpm format`
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11329-fix-prevent-duplicate-prepareForSave-and-conflicting-is_new-telemetry-on-self-overwrite-3456d73d36508192875ed5e70ab9c359)
by [Unito](https://www.unito.io)
## 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)
