mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-11 08:20:53 +00:00
fefb09cd9cf70573613f20d2b08a84a94674f899
7834 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
bymyself
|
fefb09cd9c |
test: trim RoleBadge tests to user-visible label behavior
Removes assertions on the root tag name, Vue rerender mechanics, and class-fallthrough utility forwarding — these locked down implementation details rather than user-visible behavior. The meaningful regression coverage for role labels lives in MemberListItem.test.ts. Addresses review feedback: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11383#discussion_r3176431334 |
||
|
bymyself
|
b29ad1f9fe |
fix: rely on Vue class fallthrough in RoleBadge
Removes `inheritAttrs: false` and the manual `cn()` class merge so all caller-provided attributes (aria-*, data-*, title, style, id, listeners) fall through to the root span. Tailwind class merging is handled by Vue's default class fallthrough. Addresses review feedback: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11383#discussion_r3176431333 |
||
|
bymyself
|
2d036e1dc1 |
fix: update cn import after tailwindUtil shim removal
Rebased onto main where PR #11453 removed the @/utils/tailwindUtil re-export shim. Switch RoleBadge to import cn directly from @comfyorg/tailwind-utils. |
||
|
bymyself
|
50d8b7a98a |
test: add MemberListItem unit tests for role badge label coverage
Covers getRoleBadgeLabel function to fix codecov/patch failure. |
||
|
bymyself
|
2cbeaae36b |
fix: rename attr passthrough test to clarify behavioral intent
Addresses review feedback: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11383#discussion_r3106189426 |
||
|
bymyself
|
18d448c740 |
refactor: extract RoleBadge component and reuse across workspace UI
Replace 4 inline badge spans with the new RoleBadge component in WorkspaceSwitcherPopover, MembersPanelContent, and TeamWorkspacesDialogContent. The component accepts a label prop and supports class merging via inheritAttrs: false + cn(). Fixes #10971 |
||
|
Christian Byrne
|
e831daae59 |
feat(website): point robots.txt at /sitemap-index.xml + AI crawler rules (#11823)
## Summary Once [comfy-router#22](https://github.com/Comfy-Org/comfy-router/pull/22) ships, `comfy.org/sitemap-index.xml` will return a unified index aggregating both the website (38 URLs) and workflow-templates sitemaps. This PR: 1. Reverts `Sitemap:` back to `/sitemap-index.xml` (was `/sitemap-0.xml` in #11802 as a workaround for the 404). 2. Adds explicit allow records for 21 search and AI/LLM crawlers (GPTBot, ChatGPT-User, OAI-SearchBot, Google-Extended, ClaudeBot, Claude-Web, anthropic-ai, PerplexityBot, Perplexity-User, Applebot-Extended, Bytespider, Amazonbot, CCBot, Meta-ExternalAgent, Meta-ExternalFetcher, Diffbot, etc.). 3. Adds `Disallow:` for `/_astro/`, `/_website/`, `/_vercel/` — Vercel build artifacts that aren't useful to crawl. ## Why granular UAs Stacked `User-agent:` records (per [RFC 9309 §2.2](https://datatracker.ietf.org/doc/html/rfc9309#section-2.2)) share one rule block. Listing each bot explicitly: - Signals intent to AI bots that look for their UA in robots.txt before crawling more aggressively. - Surfaces our crawl policy clearly to anyone inspecting the file. - Lets us add per-bot Disallows in future without restructuring. ## Merge order ⚠️ **Do NOT merge until comfy-router#22 is deployed to production.** Until then, `/sitemap-index.xml` returns 404 and this PR would re-break the issue PR #11802 patched. Verification: ```bash curl -sI https://comfy.org/sitemap-index.xml # expect: HTTP/2 200, x-served-by: worker-sitemap-index ``` Once that returns 200, this is safe to merge. ## Verification (after merge + deploy) ```bash # robots.txt is served and points at the unified index curl -s https://comfy.org/robots.txt | grep '^Sitemap:' # → Sitemap: https://comfy.org/sitemap-index.xml # Each AI crawler can fetch it for ua in 'GPTBot/1.0' 'ClaudeBot/1.0' 'PerplexityBot/1.0' 'Google-Extended' 'Applebot-Extended'; do curl -s -o /dev/null -w "$ua → %{http_code}\n" -A "$ua" https://comfy.org/robots.txt done # Sitemap is reachable from robots.txt SITEMAP=$(curl -s https://comfy.org/robots.txt | awk -F': ' '/^Sitemap:/ {print $2}') curl -s "$SITEMAP" | xmllint --noout - && echo "valid XML" ``` ## Linear / closes - Closes FE-437 (AI crawler rules) - Updates FE-432 — the robots.txt change in #11802 was a workaround that's no longer needed once #22 ships ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11823-feat-website-point-robots-txt-at-sitemap-index-xml-AI-crawler-rules-3546d73d3650811dbceedd06c00db444) by [Unito](https://www.unito.io) |
||
|
Robin Huang
|
96575fcec9 |
feat: redesign cloud onboarding survey for ICP and persona signal (#11628)
## Summary Replaces the 4-step Cloud onboarding survey with a 7-step flow that captures both ICP attributes and user persona dimensions. The survey questions are now populated dynamically from remoteConfig. ## Changes - **What**: New survey questions — Usage, Familiarity, Role, Team size, Industry, Making, Source. Role / Team size / Industry are gated to "Work" usage; Education users see a Student / Educator short list for Role. Most option lists are randomized per visit (familiarity and team size stay ordered as ordinals). \`SurveyResponses\` extended with optional \`usage\`, \`role\`, \`teamSize\`, \`source\` fields. - **Breaking**: None — \`useCase\` and \`workflowRelationship\` remain optional in the type and existing telemetry normalization keeps working unchanged. ## Review Focus - The \`role\` step has a function-form \`options\` so the list can swap based on \`usage\`. \`steps\` is a computed that filters by \`showWhen()\` and resolves the option function — verify reactivity when \`usage\` changes. - Changing \`usage\` clears the previously-picked \`role\` via a watcher to prevent a stale value from carrying over between Work / Education modes. - Per-visit shuffle is stable: option lists are passed through \`randomize()\` once at module load, not on every render. ## Screenshots https://github.com/user-attachments/assets/3602a388-50dc-401e-ada9-ea9016c5052d ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11628-feat-redesign-cloud-onboarding-survey-for-ICP-and-persona-signal-34d6d73d365081f4a792cfe76a987ffb) by [Unito](https://www.unito.io) --------- Co-authored-by: Dante <bunggl@naver.com> |
||
|
Kelly Yang
|
e7e1ae25a6 |
fix(load3d): suppress error toast on 404 when loading output model file (#11807)
## Summary
- Adds `silentOnNotFound` option to `LoadModelOptions` interface,
threaded through `Load3d.loadModel` → `LoaderManager.loadModel`
- 404 errors (detected via message text or `response.status`) are
silently swallowed when `silentOnNotFound: true`; all other errors still
surface a toast
- Sets `silentOnNotFound: true` for output-folder loads in `load3d.ts`
and `saveMesh.ts` — covers shared workflows opened on a machine that
never ran them
## Test plan
- [x] `LoaderManager.test.ts` — 40 unit tests covering 404 suppression,
non-404 still toasts, stale load handling
- [x] `Load3DConfiguration.test.ts` — 4 unit tests verifying
`silentOnNotFound` propagates correctly through `configureForSaveMesh`
and `configure`
- [x] `load3d.spec.ts` — 2 E2E tests: 404 → no toast, 500 → toast
appears
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes error-handling behavior in the 3D model loading pipeline and
extends method signatures/options; risk is mainly missed call sites or
incorrectly classifying non-404 errors as 404 and hiding real failures.
>
> **Overview**
> Prevents noisy user-facing toasts when an *output* 3D model referenced
by `Preview3D`/`SaveGLB` is missing locally by adding a
`silentOnNotFound` flag and suppressing the "Error loading model" toast
specifically for HTTP 404 failures.
>
> Threads the new `LoadModelOptions` through `Load3d.loadModel` →
`LoaderManager.loadModel` and updates `Load3DConfiguration`/callers to
opt in for output-folder loads, with new unit + Playwright coverage (404
stays silent, non-404 still toasts).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Yourz
|
4ed00cec08 |
update: robots.txt to point to /sitemap-0.xml (#11802)
## Summary <!-- One sentence describing what changed and why. --> robots.txt at [comfy.org](https://comfy.org/) references /sitemap-index.xml which returns 404. The actual working sitemap is at /sitemap-0.xml (200, 38 URLs). This blocks search engines from discovering the sitemap. ## Changes - **What**: <!-- Core functionality added/modified --> - Update robots.txt to point to /sitemap-0.xml, OR ## 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://app.notion.com/p/PR-11802-update-robots-txt-to-point-to-sitemap-0-xml-3536d73d365081bb9545eb96dd1e8025) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
f566abdd6e |
ci: extract changes-filter composite action; fix docs-only PR stall (#11785)
## Summary Extract a `.github/actions/changes-filter` composite action and adopt it across path-gated CI workflows, fixing the docs-only PR stall and removing duplicated `paths:` / `paths-ignore:` filtering across 8 workflows. ## Background Docs-only PRs stalled on required status checks because workflows using `paths-ignore: ['**/*.md']` never created a check run, while branch protection still required it. Observed on #11776 (the `test` check from `ci-tests-unit.yaml` never appeared). The fix pattern: keep the workflow triggered, gate downstream jobs on a `changes` job whose outputs are computed from a path filter. Skipped jobs count as passing under branch protection. ## What the action emits | Output | Meaning | |---|---| | `should-run` | Any file outside `apps/`, `docs/`, `.storybook/`, `**/*.md` changed. | | `app-website-changes` | Shared deps or `apps/website/**` changed. | | `app-desktop-changes` | Shared deps or `apps/desktop-ui/**` changed. | | `app-frontend-changes` | Shared deps or `src/**` changed. | | `packages-changes` | Shared deps or `packages/**` changed. | | `storybook-changes` | Shared deps or `.storybook/**` changed. | | `docs-changes` | `docs/**` or any `**/*.md` changed (deps NOT folded in). | | `dependency-changes` | Root `package.json`, `pnpm-lock.yaml`, or `pnpm-workspace.yaml` changed. | Shared deps are folded into every `app-*`, `packages-changes`, and `storybook-changes` output so a lockfile bump correctly invalidates each granular gate. Outputs default to `'true'` for non-`pull_request` events to avoid the silent-skip footgun on push / merge_group. ## Workflows migrated | Workflow | Gate | Notes | |---|---|---| | `ci-tests-unit.yaml` | `should-run` | Required check (`test`). Fixes the original stall. | | `ci-tests-e2e.yaml` | `should-run` | Required check (`e2e-status`). Replaces inline filter. | | `ci-perf-report.yaml` | `should-run` | Removes `paths-ignore`. | | `ci-website-build.yaml` | `app-website-changes \|\| packages-changes` | Refactor — not a required check, but moves to job-level gating. Filter scope broadens from `packages/{design-system}` to all `packages/**` (strictly safer). | | `ci-website-e2e.yaml` | `app-website-changes \|\| packages-changes` | Same restructure; `post-starting-comment` also gated to avoid spurious "tests are running" when E2E is skipped. | | `ci-dist-telemetry-scan.yaml` | `should-run` | New gate; was previously running on every PR including docs-only. | | `ci-oss-assets-validation.yaml` | `should-run` | Same. | | `ci-size-data.yaml` | `should-run` | Preserves existing repository guard on the new `changes` job. | | `ci-tests-storybook.yaml` | `storybook-changes \|\| app-frontend-changes \|\| packages-changes` | Gates 4 of 6 jobs. `deploy-production` (push to main) left ungated; `update-comment-with-chromatic` cascades naturally. | ## Branch protection (verified) Required status checks on `main` and `core/**`/`cloud/**`: `test`, `lint-and-format`, `e2e-status`. Only `test` and `e2e-status` use the composite — `lint-and-format` correctly stays unfiltered (must run on docs/apps too). The other 6 migrations are refactor wins (less wasted CI on docs/apps-only PRs), not stall fixes. ## Changes - **What**: New `.github/actions/changes-filter` composite + 8 workflow migrations to consume it. - **Breaking**: None. - **Dependencies**: New pin on `dorny/paths-filter@de90cc6` — already covered by `ci-validate-action-pins`. ## Review Focus - The `should-run` filter excludes `.storybook/**` (granular `storybook-changes` covers it instead). Storybook's gate combines all three: `storybook-changes || app-frontend-changes || packages-changes`. - Two `dorny/paths-filter` steps inside the composite — `predicate-quantifier=every` is required for the negated globs in `should-run` but breaks the multi-pattern OR filters. - The website filter scope intentionally broadens from `packages/{design-system,tailwind-utils}/**` to all `packages/**` for consistency and safety. Fixes #11776 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11785-ci-extract-changes-filter-composite-action-fix-docs-only-PR-stall-3526d73d36508172a1d7fe8c30fa6453) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
3c5695fd42 |
test: add Vue node error/validation ring e2e coverage (#11727)
## Summary Add additional test coverage for vue node errors ## Changes - **What**: - add tests for showing error on missing node, execution error, validation failure & resolved on fix - move ErrorsTabHelper to fixtures dir & update refs - add SLOW_MO env var for headed local tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11727-test-add-Vue-node-error-validation-ring-e2e-coverage-3506d73d365081069ff8f70f7970dd55) by [Unito](https://www.unito.io) |
||
|
Dante
|
4fff0c4b49 |
fix: report total file count, not job count, in ZIP export toast (#11737)
## Summary ZIP export toast now reports the total number of files instead of the number of selected jobs when any selected job has multiple outputs. ## Changes - **What**: In `downloadMultipleAssetsAsZip` (`src/platform/assets/composables/useMediaAssetActions.ts`), compute `fileCount` by summing each asset's `outputCount` metadata (fallback 1) and pass it to `mediaAsset.selection.exportStarted` instead of `assets.length`. The existing i18n string already handles `file`/`files` plural. - **Tests**: 3 new unit tests in `useMediaAssetActions.test.ts` covering multi-output, single-output fallback, and mixed selections. The `useToast` and `useI18n` mocks were lifted to hoisted refs so toast call args are assertable. ## Review Focus - Reduce uses `count > 1 ? count : 1`, mirroring the `hasMultiOutputJobs` gate above so a known `outputCount === 1` is still counted as 1 file (no double-counting). - Only `downloadMultipleAssetsAsZip` is touched; OSS individual-download path and direct-download path are unchanged. Fixes #11736 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11737-fix-report-total-file-count-not-job-count-in-ZIP-export-toast-3516d73d3650811ba78dfdb0a0ae8ea1) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
69dca2d600 |
test: add test coverage for workflow save settings (#11763)
## Summary Add tests for autosave/delay/node sort on save ## Changes - **What**: - add tests ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11763-test-add-test-coverage-for-workflow-save-settings-3516d73d365081d1b57bc1cf4b2e2ece) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
004530b23a |
fix: search bar layout and autocomplete clipping on Desktop at small sizes (#11713)
## Summary
Fixes two visual bugs in the Desktop app at small window sizes: the
search bar getting pushed/clipped in modal headers, and autocomplete
suggestion dropdowns being cut off by `overflow-hidden` ancestors.
## Changes
- **`SearchAutocomplete.vue`**: Wrap `ComboboxContent` in
`ComboboxPortal` so the suggestions dropdown teleports to `<body>`,
escaping `overflow-hidden` ancestors (fixes z-index clipping in Manager
dialog and other modals using `BaseModalLayout`)
- **`BaseModalLayout.vue`**: Replace `shrink-0` with `min-w-0` on the
header content container so the search bar can shrink at narrow window
sizes instead of overflowing and being clipped by the modal root's
`overflow-hidden`
- **`GraphCanvas.vue`**: Fix dead code where the native drag
(`app-drag`) div was nested inside a `v-if="workflowTabsPosition ===
'Topbar'"` block with its own mutually exclusive condition — move it
before the block and add `pointer-events-auto` so Desktop window
dragging works when tabs are in Sidebar position
## Why no E2E tests
- **`SearchAutocomplete` portal**: The fix is structural (teleport to
`<body>`). A meaningful regression test would require opening the
Manager dialog with a real or mocked extension list — that is a
substantial standalone effort tracked in #11714.
- **`BaseModalLayout` header shrink**: A viewport-resize assertion would
test CSS layout behaviour, not application logic; it would be fragile
and low-value.
- **`GraphCanvas` app-drag**: Desktop/Electron-only.
`-webkit-app-region: drag` cannot be exercised in headless Chromium.
Unit tests for `SearchAutocomplete` cover the new code paths (portal
rendering, suggestion display, item selection).
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk UI-only changes: adjusts layout CSS and combobox rendering
via `ComboboxPortal`, plus adds unit tests; no business logic or data
flow changes.
>
> **Overview**
> Fixes small-window Desktop UI issues where modal-header search inputs
could be clipped and autocomplete dropdowns could be cut off by
`overflow-hidden` ancestors.
>
> `SearchAutocomplete` now renders its suggestions list inside a
`ComboboxPortal` (teleporting the popper content outside clipping
containers) and adds a focused unit test suite covering empty/non-empty
suggestions, selection behavior, and `optionLabel` handling.
>
> `BaseModalLayout` tweaks header flexbox constraints (`min-w-0` on the
header content container) to allow the search bar to shrink instead of
overflowing.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Alexander Brown
|
73d4e24ffa |
revert: roll back #10849 + #11697 (per-instance promoted widget values) (#11790)
## Summary Reverts #10849 (per-instance promoted widget value storage) and its companion test-pinning PR #11697. The fix in #10849 caused regressions in promoted-widget serialization (notably the Z-Image-Turbo template, see #10146 follow-up). A replacement fix is being developed on `fix/subgraph-promoted-widget-inline-state` and will land separately. ## Changes - **Revert #11697** — drops the `it.fails`-marked tests that pin the #10849 corruption symptom. With #10849 reverted, those markers would falsely flip to passing. - **Revert #10849** — removes per-instance `_instanceWidgetValues` map, `_pendingWidgetsValues` configure-time hydration, the `widgets_values` write path in `SubgraphNode.serialize()`, the `sourceSerialize` field on `PromotedWidgetView`, the multi-instance Vitest suite, and the multi-instance E2E test + asset. - **Conflict resolution** in `browser_tests/tests/subgraph/subgraphSerialization.spec.ts`: kept the restored test coverage from #11579 (which is post-#10849 on main) and removed only the now-unreachable multi-instance test, its helper, and its workflow constant. Auto-merge with #11698 (`incrementVersion`) and #11699 (ID type aliases) was clean. ## Review Focus - Confirm no other on-main code path has come to depend on `PromotedWidgetView.sourceSerialize` or `SubgraphNode._instanceWidgetValues` since #10849 (grep is clean locally). - Confirm we want to land this revert before the replacement fix on `fix/subgraph-promoted-widget-inline-state` is ready — this leaves the original #10146 (multi-instance widget value collision) unfixed in the meantime. - The retained #11579 test coverage now exercises pre-#10849 behavior; some of those assertions were written expecting the #10849 code path. CI will surface any that need adjustment. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11790-revert-roll-back-10849-11697-per-instance-promoted-widget-values-3536d73d3650814094abd58b6b712d8d) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
09790bd7f3 |
test: add unit tests for useLayoutMutations (#11313)
## Summary
A follow-up PR of
https://github.com/Comfy-Org/ComfyUI_frontend/issues/11106.
This PR only focus on `layoutMutations.ts`.
`layoutMutations.ts` is the central API for all node layout mutations in
the Vue renderer. It previously had zero test coverage despite
containing non-trivial logic such as guard clauses, ID normalization,
and z-index scanning. This PR addresses issue #11106 to prevent silent
regressions in node positioning and lifecycle.
## What was tested and how
All tests use the real `layoutStore` singleton (no mocks).
`initializeFromLiteGraph` resets node state before each test, and
results are verified via `getNodeLayoutRef().value`.
| Method | Tests | Logic covered |
| :--- | :--- | :--- |
| `moveNode` | 3 | **Guard** (missing node -> no-op); position written
to store; numeric ID coerced to string |
| `resizeNode` | 2 | **Guard**; size written to store |
| `setNodeZIndex` | 2 | **Guard**; zIndex written to store |
| `createNode` | 1 | Node becomes readable with the provided position
and size |
| `deleteNode` | 2 | **Guard**; node removed from store |
| `batchMoveNodes` | 4 | Empty array -> no-op; multiple nodes updated
**atomically**; existing size preserved; missing nodes skipped while
valid ones still update |
| `bringNodeToFront` | 1 | Target node ends up with a **higher zIndex**
than all other nodes |
## What was not tested and why
| Method | Reason skipped |
| :--- | :--- |
| `createLink` / `deleteLink` | `layoutStore` exposes no public API to
query link existence by ID; methods contain no logic beyond a straight
`applyOperation` call. |
| `createReroute` / `deleteReroute` / `moveReroute` | Same reason as
above. |
| `setSource` / `setActor` | Single-line delegation to `layoutStore`; no
logic to test. |
| Default value behavior in `createNode` | Avoids "change-detector"
tests; asserting hardcoded defaults adds no regression value and would
block valid product changes. |
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Test-only changes; no production logic is modified. Main risk is
potential flakiness due to reliance on singleton store state across
tests.
>
> **Overview**
> Adds a new `layoutMutations.test.ts` Vitest suite that exercises
`useLayoutMutations` against the real `layoutStore` singleton
initialized from LiteGraph data.
>
> Tests cover no-op guard clauses for missing nodes, node ID
normalization, position/size/z-index updates, node create/delete
behavior, `batchMoveNodes` semantics (empty input, skipping missing
nodes, preserving size), and `bringNodeToFront` z-index promotion
relative to other nodes.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
dafb944c3b |
refactor: replace PrimeVue InputText/Textarea with project UI components (#11324)
## Summary
Fix part of the
#https://github.com/Comfy-Org/ComfyUI_frontend/issues/11092
A total of 4 `// @ts-expect-error` directives were removed across 3
files — all caused by PrimeVue's legacy `$el` access pattern (`const
inputElement = inputRef.value.$el`) — by replacing PrimeVue with
Reka-based UI components. 1 corresponding unit test file was added.
## Changes
### `src/components/ui/input/Input.vue`
- **Exposed APIs**: Extended `defineExpose` to include `blur()` and
`setSelectionRange()`. This allows parent components to programmatically
control input behavior without direct DOM manipulation.
### `src/components/ui/textarea/Textarea.vue`
- **Exposed APIs**: Added `focus()` via `defineExpose`.
- **Cleanup**: Removed redundant attribute spreading (`...restAttrs`) to
lean on Vue’s default `$attrs` inheritance, making the component more
predictable.
---
## Refactored Feature Components
### `WidgetMarkdown.vue` (Note/Markdown Widgets)
- **Dependency Swap**: Replaced `primevue/textarea` with local
`Textarea.vue`.
- **Logic Simplification**: Simplified focus logic from
`textareaRef.value?.$el?.focus()` to a typed
`textareaRef.value?.focus()`.
- **Code Style**: Converted arrow functions to function declarations and
removed redundant section comments.
### `PromptDialogContent.vue` (Generic Prompt Dialogs)
- **Component Update**: Replaced PrimeVue `FloatLabel` and `InputText`
with a native `<label>` and local `Input.vue`.
- **Vue 3.5 Adoption**: Implemented **Reactive Destructuring** for
props.
- **Conflict Resolution**: Renamed internal `onConfirm` handler to
`handleConfirm` to prevent collision with destructured props.
### `EditableText.vue` (Node Titles & Sidebar Items)
- **Style Modernization**: Removed `<style scoped>` block in favor of
**Tailwind CSS** utility classes (e.g., `inline`, `w-full`).
- **Clean Implementation**: Replaced PrimeVue PassThrough (`:pt`) logic
with standard `@blur` and `v-bind` attributes.
---
## Testing & Quality Assurance
### Updated Tests
- **Redundancy Removal**: Cleaned up `EditableText.test.ts` and
`WidgetMarkdown.test.ts` by removing unused PrimeVue global
registrations. All 34 existing behavioral tests remain passing.
### New Coverage
- **`PromptDialogContent.test.ts`**: Added 3 new tests to verify:
1. Correct initialization with `defaultValue`.
2. Value persistence when clicking the Confirm button.
3. Form submission via the `Enter` key.
---
## Manual Test Screenshot
All functions have passed testing.
<img width="594" height="530" alt="test5"
src="https://github.com/user-attachments/assets/46a6b3b2-1855-414e-ac78-65668052ce50"
/>
<img width="1190" height="1074" alt="test4"
src="https://github.com/user-attachments/assets/89aa61ab-9401-44c2-9eae-9ca8761df675"
/>
<img width="1154" height="1028" alt="test3"
src="https://github.com/user-attachments/assets/3f63cfdf-8fbd-4dd3-9e42-dbebe4d8d421"
/>
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Moderate risk because it swaps underlying input/textarea components
and ref handling (focus/blur/selection) in interactive UI paths
(editable labels, prompt dialogs, markdown editor), which could subtly
change keyboard/blur behavior.
>
> **Overview**
> Refactors several Vue components to stop using PrimeVue
`InputText`/`Textarea` (and `$el` access) in favor of the project’s
`Input`/`Textarea` components, updating bindings/events and Tailwind
classes accordingly.
>
> Extends the shared `Input` to expose `blur`, `setSelectionRange`, and
`selectAll`, and updates `Textarea` to expose `focus`, enabling callers
to manage focus/selection without DOM internals.
>
> Adds a new unit test suite for `PromptDialogContent` and simplifies
existing tests by removing PrimeVue plugin/component setup; the groups
e2e test replaces a screenshot assertion with a functional visibility
check for the new title input.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Christian Byrne
|
d429d481e8 |
test: use real vue-i18n plugin in useReconnectingNotification tests (#11386)
## Summary
Replace `vi.mock('vue-i18n')` stub with a real `createI18n` plugin
instance in `useReconnectingNotification` tests.
## Changes
- **What**: Add `setupComposable()` helper that renders a wrapper
component via `@testing-library/vue` with a real `createI18n` plugin.
Assertions now check translated values
(`'Reconnecting'`/`'Reconnected'`) instead of raw i18n keys. Removes the
brittle `vi.mock('vue-i18n')` stub.
## Review Focus
Straightforward test-only change — the composable requires a component
setup context for `useI18n()`, so we render a thin wrapper via
`@testing-library/vue` with the i18n plugin installed.
Fixes #11153
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11386-test-use-real-vue-i18n-plugin-in-useReconnectingNotification-tests-3476d73d3650814ba70eea6df91c8bbe)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
|
||
|
Comfy Org PR Bot
|
a9aae6af4a |
1.44.15 (#11787)
Patch version increment to 1.44.15 **Base branch:** `main` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11787-1-44-15-3536d73d3650814da3a6dfe300ff559c) by [Unito](https://www.unito.io) --------- Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com>v1.44.15 |
||
|
jaeone94
|
46ba65e25c |
fix: hide advanced footer button on collapsed Vue nodes (#11778)
Before https://github.com/user-attachments/assets/82e323d7-0c9b-4303-81eb-87cb7b62f1c1 After https://github.com/user-attachments/assets/56f1db45-1b5d-47a9-9960-9a73deb68e78 Fixes the Vue node footer so the Advanced button only appears while a node is expanded. Root cause: `showAdvancedInputsButton` only checked for advanced widgets and the global setting, so collapsed nodes with advanced widgets could still render the Advanced tab, including the combined error + advanced footer state. Changes: - Return `false` for advanced footer visibility when the Vue node is collapsed. - Add regression coverage for collapsed advanced nodes and collapsed error + advanced nodes. Validation: - `pnpm test:unit -- src/renderer/extensions/vueNodes/components/LGraphNode.test.ts` - Commit hook: format, stylelint, oxlint, eslint, `pnpm typecheck` - Push hook: `pnpm knip` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11778-codex-Hide-advanced-footer-on-collapsed-Vue-nodes-3526d73d365081d79399ddda40f2a7f7) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
11432f7d0e |
refactor: extract missing model refresh pipeline (#11751)
## Summary Extracts the missing-model pipeline orchestration out of `ComfyApp` and into an app-independent platform module, while tightening the workflow-flattening type boundary that refresh needs when rescanning the live LiteGraph graph. This PR is intentionally refactor-heavy. It is the follow-up to the earlier missing-model refresh work: instead of keeping refresh-specific candidate recheck logic beside the UI, this change makes the refresh path reuse the existing missing-model pipeline and removes the direct dependency on private `ComfyApp` pipeline methods. Linear: FE-499 Issues covered by this PR: - Fixes #11678 - Fixes #11680 - Partially addresses #11679 by removing the missing-model refresh path's unsafe `graph.serialize() as unknown as ComfyWorkflowJSON` cast and replacing it with the narrower flattenable workflow contract. Broader workflow serialization/type-boundary cleanup outside this missing-model refresh path remains deferred. ## Changes - **What**: - Added `src/platform/missingModel/missingModelPipeline.ts` as the orchestration module for missing-model detection/verification. - `runMissingModelPipeline(...)` now owns the pipeline previously embedded in `ComfyApp`: - candidate scan and enrichment - active ancestor filtering for muted/bypassed subgraph containers - pending warning cache updates - OSS folder path and file-size follow-up work - cloud asset verification follow-up work - surfaced missing-model errors via the existing execution error store - `refreshMissingModelPipeline(...)` handles the refresh-specific flow: - calls the injected `reloadNodeDefs()` first - serializes the current live graph - preserves model metadata by preferring active workflow `models`, then falling back to current missing-model candidate metadata - delegates back into the same pipeline used during workflow load - Kept `ComfyApp` as the compatibility caller instead of the owner of the pipeline. - `loadGraphData(...)` now calls `runMissingModelPipeline(...)` with `graph`, `graphData`, `missingNodeTypes`, and `silent` options. - `refreshMissingModels(...)` is now a thin wrapper around `refreshMissingModelPipeline(...)` and keeps the existing default `silent: true` refresh behavior. - The new pipeline module does not import `@/scripts/app`; app-owned data/actions are passed in as inputs. - Moved the workflow node-flattening helpers out of `workflowSchema.ts` and into `src/platform/workflow/core/utils/workflowFlattening.ts`. - This includes `flattenWorkflowNodes`, `buildSubgraphExecutionPaths`, and `isSubgraphDefinition`. - The move is intentional: these helpers are not zod schema definitions or workflow validation logic. They are core workflow traversal utilities used to flatten root workflow nodes plus nested subgraph definition nodes into the execution-shaped node list needed by missing-model scanning. - The refresh path receives data from `LGraph.serialize()`, whose return type is serialized LiteGraph data rather than validated `ComfyWorkflowJSON`. Previously this forced unsafe typing like `graph.serialize() as unknown as ComfyWorkflowJSON`. - The new `FlattenableWorkflowGraph` / `FlattenableWorkflowNode` structural contract describes only what flattening actually needs: `nodes`, `definitions.subgraphs`, node `id`, `type`, `mode`, `widgets_values`, and `properties`. - This lets both normal workflow-load data (`ComfyWorkflowJSON`) and refresh-time live graph serialization (`LGraph.serialize()`) flow into the same scan/enrichment path without pretending serialized LiteGraph output is a fully validated workflow schema document. - Updated `missingModelScan.ts` to consume that minimal flattenable workflow shape via `MissingModelWorkflowData`. - `MissingModelWorkflowData` extends the flattenable workflow contract with optional workflow-level `models` metadata. - Removed now-unnecessary casts around execution IDs, flattened nodes, and `widgets_values` object access. - Updated `getSelectedModelsMetadata(...)` to accept readonly widget value arrays so flattened workflow data can stay read-only. - Reduced the exported surface of the new pipeline module after `knip` flagged unused exported internal option/store interfaces. - Kept `workflowSchema.ts` focused on validation schemas. The flattening helpers are not re-exported from the schema module because they are internal workflow core utilities, not public schema API. - **Breaking**: None intended. - Internal imports were updated to the new core utility path. - This repo is not exposing these flattening helpers as a public package API, so the old schema-local helper location is treated as an internal implementation detail. - **Dependencies**: None. ## Review Focus - **Pipeline extraction / dependency direction**: - Please verify that `missingModelPipeline.ts` stays independent from `@/scripts/app`. - `ComfyApp` should remain the caller/adapter, not the owner of missing-model pipeline orchestration. - **Workflow flattening type boundary**: - The main type-cleanup goal is removing the refresh-time `graph.serialize() as unknown as ComfyWorkflowJSON` lie. - `LGraph.serialize()` and validated workflow JSON are not the same contract. The new flattenable workflow contract is deliberately smaller and structural because the missing-model enrichment path only needs enough data to flatten nodes and read embedded model metadata. - This is why the flattening helpers moved from `workflowSchema.ts` to `workflow/core/utils`: the logic is reusable workflow traversal, not validation schema. - **Behavior preservation**: - The PR is intended to preserve existing user-facing missing-model behavior while moving ownership out of `app.ts`. - Existing async follow-up behavior remains intentionally fire-and-forget: - cloud asset verification still surfaces after verification completes - OSS folder paths still update asynchronously before surfacing confirmed missing models - file-size metadata fetching remains asynchronous - More invasive behavior changes, such as adding non-cloud post-fetch `isMissingCandidateActive(...)` re-verification or redesigning the fire-and-forget result contract, are intentionally left for follow-up work because they are not pure extraction. - **Downloadable model metadata**: - `missingModels` returned for download metadata now requires both `url` and `directory`. - Candidates without a directory still remain in `confirmedCandidates`, but they are not exposed as downloadable model metadata. This keeps the returned downloadable list aligned with what the download flow can actually use. - **Test ownership**: - Complex missing-model pipeline behavior tests moved out of `src/scripts/app.test.ts` and into `src/platform/missingModel/missingModelPipeline.test.ts`. - `app.test.ts` now only covers thin delegation for `app.refreshMissingModels(...)`. - Workflow flattening tests moved with the helper from schema tests into `src/platform/workflow/core/utils/workflowFlattening.test.ts`. - **Deferred follow-ups**: - Broader function decomposition for cognitive complexity. - Wider dependency-injection/port cleanup for stores and services beyond the app boundary. - Cloud-specific pipeline unit tests, which need a separate `isCloud` mocking strategy. - Additional E2E coverage expansion beyond the existing OSS refresh path. - More general workflow serialization/type-boundary cleanup outside the missing-model refresh path. ## Validation - `pnpm format` - `pnpm lint` - Passed. Existing lint output included a pre-existing `no-misused-spread` warning and icon-name logs, but the command exited successfully. - `pnpm typecheck` - `pnpm test:unit` - `714 passed`, `9514 passed | 8 skipped` - Pre-push `pnpm knip` - Passed after reducing the exported surface of the new pipeline module. ## Screenshots (if applicable) Not applicable. This PR is a pipeline/type-boundary refactor with no UI changes. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11751-refactor-extract-missing-model-refresh-pipeline-3516d73d3650816d9245d4b1324b71c9) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
9384beaec6 |
test: Add tests for bounding box widget (#11343)
## Summary Adds coverage for the bounding box widget ## Changes - **What**: - Validates user interactions and functionality on the widget ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11343-test-Add-tests-for-bounding-box-widget-3456d73d365081eb8d03f2220a837816) by [Unito](https://www.unito.io) --------- Co-authored-by: bymyself <cbyrne@comfy.org> Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
4a05d89fdb |
fix: detach DOM widget event listeners on widget removal (#11724)
## Summary Fixes leaked event listeners ## Changes - **What**: - update all listeners to use AbortController to signal removal on widget remove ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11724-fix-detach-DOM-widget-event-listeners-on-widget-removal-3506d73d3650811dae81c034c1098759) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
ef98ba0e8f |
feat: add plum/ink color primitives and standardize design tokens (#11139)
*PR Created by the Glary-Bot Agent* --- ## Summary Adds new `plum` and `ink` color scales for Comfy Hub branding and standardizes existing tokens to align with current Figma design system. ### Changes **Phase 1 — New primitives** (`_palette.css`) - Added `plum-300/400/500/600` and `ink-100` through `ink-900` **Phase 2 — Token cleanup** (`style.css`) - Removed deprecated `slate-100/200/300` primitives (cool blue-grey, removed from Figma) - Removed duplicate `graphite-400` (identical hex to slate-100) - Dark mode: migrated 6 slate/graphite references to muted-foreground, smoke-700, smoke-800, charcoal-200 - Light mode: replaced 3 `ash-500` references with `smoke-800` per designer alignment ### Token migration detail | Dark mode token | Old value | New value | Rationale | |---|---|---|---| | `--node-component-header-icon` | slate-300 (#5b5e7d) | muted-foreground (smoke-800) | Figma `node/foreground-secondary` | | `--node-component-slot-text` | slate-200 (#9fa2bd) | smoke-700 (#a0a0a0) | Lighter neutral for text contrast | | `--node-component-surface-highlight` | slate-100 (#9c9eab) | smoke-800 (#8a8a8a) | Neutral grey highlight | | `--node-component-tooltip-border` | slate-300 (#5b5e7d) | charcoal-200 (#494a50) | Consistent with dark border tokens | | `--text-secondary` | slate-100 (#9c9eab) | smoke-700 (#a0a0a0) | Adequate contrast on dark surfaces | | `--widget-background-highlighted` | graphite-400 (#9c9eab) | smoke-800 (#8a8a8a) | Removed duplicate, neutral replacement | ### Visual note These changes shift some dark mode colors from cool blue-grey to neutral grey. This is intentional per the design team. The `--node-component-surface-highlight` and `--node-component-tooltip-border` tokens should be QA'd as the designer noted. ### Not included (Phase 3) Hub Dark overlay theme will ship separately once the Hub UI work is ready to validate against. ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11139-feat-add-plum-ink-color-primitives-and-standardize-design-tokens-33e6d73d365081418e13e0efe6161fb5) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Amp <amp@ampcode.com> |
||
|
Christian Byrne
|
019c1787a5 |
[chore] Update Comfy Registry API types from comfy-api@e993818 (#11783)
## Automated API Type Update This PR updates the Comfy Registry API types from the latest comfy-api OpenAPI specification. - API commit: e993818 - Generated on: 2026-04-30T17:57:05Z These types are automatically generated using openapi-typescript. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11783-chore-Update-Comfy-Registry-API-types-from-comfy-api-e993818-3526d73d3650810e9e14f55da44714b2) by [Unito](https://www.unito.io) Co-authored-by: coderfromthenorth93 <213232275+coderfromthenorth93@users.noreply.github.com> |
||
|
Alexander Brown
|
87fab87d84 |
docs: add reviewing-unit-tests skill (#11777)
## Summary Adds `.claude/skills/reviewing-unit-tests/SKILL.md`, a project-scoped agent skill that gives reviewing agents a concrete rubric for evaluating Vitest unit-test diffs. ## Changes - **What**: New skill at `.claude/skills/reviewing-unit-tests/SKILL.md`. Mirrors the structural template of the peer skill `.claude/skills/writing-playwright-tests/SKILL.md` (frontmatter shape, table-driven sections, terse prescriptive prose). Description triggers on PR review of `*.test.ts` files. Codifies: alias-by-renaming detection, `vi.mocked()` scope rule, unnecessary-cast detection, divergence checks against `docs/testing/unit-testing.md` / `store-testing.md` / `component-testing.md` / `vitest-patterns.md`, redundant `mockClear()` flag, `vue-i18n`-must-not-be-mocked rule (with `testI18n` reference), bugfix regression-validity check, leading-close framing flag, testing-library requirement for new component tests. ## Review Focus - **Retrospective validation against PR #11737** (head SHA `4573d62450fd12ac6f06e5e491f8af84ccbd27de`, file `src/platform/assets/composables/useMediaAssetActions.test.ts`). Mentally running the skill's checklist against that diff flags: 1. ✅ `getToastAddMock` / `getI18nTMock` module-level helpers — caught by the "Renaming ≠ Restructuring" section + Mocking Smells row "Module-level helper functions wrapping mocked composable returns". 2. ✅ `as ReturnType<typeof vi.fn>` casts in those helpers — caught by the "`vi.mocked()` Scope" section + Mocking Smells row on stray casts. 3. ✅ `vi.mock('vue-i18n', ...)` — caught by Mocking Smells row directing to mount real `createI18n` per `docs/testing/vitest-patterns.md` and the shared `testI18n` in `src/components/searchbox/v2/__test__/testUtils.ts`. 4. ✅ `vi.mock('primevue/usetoast', ...)` — explicitly **not** flagged. The "Distinguish" paragraph after the Mocking Smells table marks trivially-shaped third-party hooks as acceptable to mock; "Don't Mock What You Don't Own" applies to behavior-rich APIs, not single-method composables. - **Reference style**: links to `docs/testing/*.md` and `docs/guidance/typescript.md` instead of restating their content, per the FE-511 acceptance criterion. - **Open question**: scope. The repo-local skill is the FE-511 deliverable; equivalent prescriptive rules already live in the user's global `~/.config/amp/AGENTS.md` "Code Review Rigor" section as a separate artifact. No duplication intended — the skill is the project-scoped surface a reviewing agent loads when entering this repo. Closes FE-511 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11777-docs-add-reviewing-unit-tests-skill-3526d73d365081848759e6a8fab942f0) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Alexander Brown
|
f2a99adaa3 |
docs: prefer real createI18n over mocking vue-i18n in tests (#11776)
## Summary Document that `vue-i18n` should not be mocked in tests — mount with a real `createI18n` plugin instance instead. ## Changes - **What**: Expanded `docs/testing/vitest-patterns.md` "Don't Mock `vue-i18n`" section with a concrete code example (covering both component and composable tests), guidance for asserting on translation keys with empty messages, and a real-example link to `src/components/searchbox/v2/__test__/testUtils.ts`. Added a callout at the top of `docs/testing/unit-testing.md` "Mocking Composables with Reactive State" cross-linking the new section, since that section applies to *owned* composables. ## Review Focus - The previous `vitest-patterns.md` paragraph pointed at a non-existent `SearchBox.test.ts`; the new link points to the actual shared `testI18n` helper. - The "Mocking Composables with Reactive State" pattern should not be applied to third-party composables like `useI18n` — the callout makes that explicit. Surfaced during review of #11737, where the test file mocked `vue-i18n` and then accumulated structural workarounds (hoisted aliases, helper functions, type casts) to interact with the mocked `t`. A real `createI18n` instance avoids the entire ceremony. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11776-docs-prefer-real-createI18n-over-mocking-vue-i18n-in-tests-3526d73d365081d4bc39fbf3c2502e49) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> |
||
|
Robin Huang
|
a934056246 |
feat(website): add PostHog analytics (#11735)
## Summary Adds PostHog page analytics to the marketing website (`apps/website/`). ## Changes - **What**: New `posthog.ts` script with `initPostHog`/`capturePageview`. Wired into `BaseLayout.astro` behind `import.meta.env.PROD` (mirroring the GTM gate). Pageviews are captured on every `astro:page-load` so ClientRouter view-transition navigations are tracked, not just hard reloads. - **Dependencies**: `posthog-js` (already in the workspace catalog at `^1.358.1`; previously used by the workbench telemetry provider). ## Review Focus - API host is set to `https://t.comfy.org` to match `src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` — confirm that proxy is OK to share with the website surface. - Project token is hard-coded inline. It is a public `phc_…` frontend token (designed to ship to clients); this matches the pattern used for `gtmId` in the same file. Happy to switch to a `PUBLIC_POSTHOG_KEY` env var if preferred. - `person_profiles: 'identified_only'` to avoid creating profiles for every anonymous visitor — flag if you want full anonymous tracking instead. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11735-feat-website-add-PostHog-analytics-3516d73d3650811189c6d64c3af4ded9) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
b8dfbfc0bb |
fix: ensure escape key/graph navigation cancels ghost node placement (#11779)
## Summary When inside a subgraph, the parent key handler intercepts the escape key before `processKey` is called, causing a stale ghost node to be added to the inner subgraph when the graph is changed to the parent. ## Changes - **What**: - move escape key handler to document, prevent propagating the event - add saftey net cancel in setGraph - tests ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11779-fix-ensure-escape-key-graph-navigation-cancels-ghost-node-placement-3526d73d3650812292e4ca10d384f783) by [Unito](https://www.unito.io) |
||
|
Comfy Org PR Bot
|
036c79259b |
1.44.14 (#11769)
Patch version increment to 1.44.14 **Base branch:** `main` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11769-1-44-14-3526d73d3650816ba3a9cd87afeef035) by [Unito](https://www.unito.io) --------- Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: github-actions <github-actions@github.com>v1.44.14 |
||
|
Benjamin Lu
|
17c18b0707 |
fix: embed HubSpot contact form (#11723)
## Summary Embed the HubSpot-hosted contact sales form on `/contact` and `/zh-CN/contact` so HubSpot owns submission handling, validation, anti-spam/security updates, tracking context, and form configuration. ## Changes - **What**: Replaces the custom local contact form stub with a HubSpot embed component using the HubSpot-hosted developer/raw HTML script and `hs-form-html` container. - **Localization**: Uses the existing English form ID by default and switches to the zh-CN form ID for `/zh-CN/contact`. - **Styling**: Applies HubSpot form CSS custom properties to match the Comfy contact page colors and `PP Formula` font more closely. - **Docs**: Updates the website README with the developer embed snippet and the zh-CN form ID. - **Dependencies**: None. ## Why Embedded Form - HubSpot docs say forms should be loaded with the HubSpot-hosted JavaScript file, and that security, anti-spam, accessibility, and performance improvements will not propagate if the embed runtime is copied or self-hosted: https://developers.hubspot.com/docs/cms/start-building/building-blocks/modules/forms - The direct form submission endpoint is documented under `v3 legacy`: https://developers.hubspot.com/docs/api-reference/legacy/marketing/forms/v3-legacy/submit-data-unauthenticated - HubSpot’s legacy API overview says numeric-versioned APIs are legacy and will be deprecated in a future release: https://developers.hubspot.com/docs/api-reference/legacy/overview ## Review Focus - Confirm the portal ID and form IDs are correct: - `en`: `94e05eab-1373-47f7-ab5e-d84f9e6aa262` - `zh-CN`: `6885750c-02ef-4aa2-ba0d-213be9cccf93` - Check visual fit on `/contact` and `/zh-CN/contact`, especially font, background, inputs, radio controls, and submit button. - Confirm the developer/raw HTML embed remains the preferred integration over a custom Forms API POST. ## Local Checks - `pnpm exec oxfmt --check apps/website/src/components/contact/HubspotFormEmbed.vue apps/website/README.md` - `pnpm exec eslint apps/website/src/components/contact/HubspotFormEmbed.vue` - `pnpm --filter @comfyorg/website typecheck` - `pnpm --filter @comfyorg/website test:unit` - `pnpm --filter @comfyorg/website build` - Commit hooks: stylelint, oxfmt, oxlint, eslint, `pnpm typecheck`, `pnpm typecheck:website` - Push hook: `pnpm knip --cache` Build completed with existing non-fatal environment warnings: Node `v25.8.1` vs requested `24.x`, unresolved website font paths deferred to runtime, `transformWithEsbuild` deprecation, and missing Ashby env falling back to the committed snapshot. Incredibly, during the taking of these screenshots, I discovered a bug in macos, where despite the snapshot/record bar not existing, from me esc'ing out, some of the tooltips persist. Closing and reopening the lid did not fix this. I didn't see the process in activity monitor. <img width="1512" height="862" alt="Screenshot 2026-04-29 at 7 09 55 PM" src="https://github.com/user-attachments/assets/92518795-6845-4b34-8da3-54048b440eb1" /> <img width="1512" height="862" alt="Screenshot 2026-04-29 at 7 13 18 PM" src="https://github.com/user-attachments/assets/f7609e58-898d-413c-975c-b02b70b84e73" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11723-fix-embed-HubSpot-contact-form-3506d73d365081528bfbe4b024c2a21f) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Alexander Brown
|
ca8407218b |
chore(husky): skip pre-push knip hook in CI (#11772)
*PR Created by the Glary-Bot Agent* --- ## Summary Fixes a false-positive knip failure in the `i18n: Update Core` workflow that's been blocking version-bump PRs (e.g. #11769, run [25141091787](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/25141091787/job/73690747401)). ## Root cause 1. The `update-locales` job uses `setup-comfyui-server`, which copies `tools/devtools/*` → `ComfyUI/custom_nodes/ComfyUI_devtools/*` at the workspace root. 2. The job's final `git push` triggers the husky `.husky/pre-push` hook, which runs `pnpm knip`. 3. Knip's `project` glob (`**/*.{js,ts,vue}` in `knip.config.ts`) sweeps the runtime `ComfyUI/` directory; the existing `tools/devtools/web/**` ignore doesn't cover the copied path, so `ComfyUI/custom_nodes/ComfyUI_devtools/web/legacyWidget.js` (added in #11574) is reported as unused → exit 1 → push aborted. ## Why fix the hook (not knip config or workflows) - **`knip.config.ts` ignore** (`'ComfyUI/**'`): tried first; `treatConfigHintsAsErrors: true` makes the unused-pattern hint fatal in normal CI runs where `ComfyUI/` doesn't exist, breaking `ci-lint-format` everywhere. - **`HUSKY=0` per workflow step**: works, but bots can't push workflow file changes without `workflows` scope, and it would need to be repeated on every CI workflow that pushes. - **Skip the hook in CI**: the canonical `pnpm knip` check runs in `.github/actions/lint-format-verify/action.yml` on every PR, so the pre-push duplicate is pure dev-side guardrail. Gating it on `$CI` (set to `true` by all GitHub Actions runners) cleanly removes it from every CI push without affecting local developer workflow. ## Verification Reproduced the failure locally by mirroring the CI sequence (`cp -r tools/devtools/* ComfyUI/custom_nodes/ComfyUI_devtools/` then `pnpm knip:no-cache`): - Without fix: `[log] Unused files (1) ComfyUI/custom_nodes/ComfyUI_devtools/web/legacyWidget.js` → exit 1 (matches CI log) - With `CI=true`, the hook short-circuits at `exit 0` before invoking knip - `pnpm knip` on a clean checkout (no `ComfyUI/`) still passes ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11772-chore-husky-skip-pre-push-knip-hook-in-CI-3526d73d3650819f9a01d549d122b69c) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Benjamin Lu
|
26ac1eece1 |
Allow website screenshot workflow to remove its label (#11725)
## Summary Allows the website screenshot update workflow to remove its own trigger label when a label-triggered run completes. ## Changes - **What**: Grants the screenshot update job `issues: write`, which is required for `issues.removeLabel`, and keeps the cleanup scoped to `Update Website Screenshots`. - **Dependencies**: None. ## Review Focus Confirm the workflow permission scope is appropriate and that unexpected label cleanup failures should fail the workflow instead of being silently swallowed. ## Validation - `/Users/ben/go/bin/actionlint .github/workflows/pr-update-website-screenshots.yaml` - `/Users/ben/Library/Python/3.9/bin/yamllint --config-file .yamllint .github/workflows/pr-update-website-screenshots.yaml` - `git diff --check HEAD~1 HEAD` No browser e2e regression was added because this change only adjusts GitHub Actions token permissions and label cleanup behavior; it does not change shipped app/runtime behavior. Fixes FE-487 |
||
|
Benjamin Lu
|
810381ab63 |
Stabilize website GitHub stars in visual snapshots (#11771)
## Summary Stabilize the website nav GitHub star count in visual-test builds so snapshot comparisons do not drift as the live GitHub count changes. ## Changes - **What**: Added `WEBSITE_GITHUB_STARS_OVERRIDE` for build-time star-count overrides and set it to `111000` in the website E2E and screenshot-update workflows. - **Dependencies**: None. ## Review Focus Confirm the deterministic build-time override is preferable to screenshot masking, since Playwright masks draw a colored rectangle whose geometry can also drift when masked content changes size. ## Screenshots (if applicable) Not included; this keeps visual-test input data stable rather than changing the UI. |
||
|
Christian Byrne
|
cc1a737291 |
test: add unit tests for useImagePreviewWidget (#11394)
## Summary Add 23 unit tests for `useImagePreviewWidget` composable, improving coverage from ~52% to significantly higher. ## Test Coverage - Widget construction (factory return, name, type, serialization, options) - `computeLayoutSize` returns correct dimensions - `createCopyForNode` creates properly bound copy - Upload spinner rendering (animation, stroke color) - Single image rendering (draws image, handles missing node.size) - Image size text overlay (enabled/disabled via setting) - Multi-image thumbnail grid (compact vs non-compact mode, cell borders) - Pointer interaction (pointerDown cleanup, overIndex reset) - `previewImages` override from DrawWidgetOptions - `onPointerDown` drag handler setup - `onClick` no-op behavior ## Testing ```bash pnpm test:unit -- src/renderer/extensions/vueNodes/widgets/composables/useImagePreviewWidget.test.ts ``` All 23 tests pass. No source changes. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11394-test-add-unit-tests-for-useImagePreviewWidget-3476d73d365081a69c78e87329e75d9f) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Amp <amp@ampcode.com> |
||
|
Kelly Yang
|
c74e08e244 |
refactor: extract useBrushAdjustment from useBrushDrawing (#11544)
## Summary
Part of the `useBrushDrawing` decomposition plan (PR C). Extracts brush
size/hardness adjustment logic (Alt+drag interaction) into a dedicated
`useBrushAdjustment` composable. No runtime behavior is changed — pure
structural refactor.
## Changes
- **New** `src/composables/maskeditor/useBrushAdjustment.ts` —
encapsulates `startBrushAdjustment` and `handleBrushAdjustment`,
including dead zone filtering, dominant axis suppression, and
size/hardness clamping
- **New** `src/composables/maskeditor/useBrushAdjustment.test.ts` — unit
tests covering: no-op before start, dead zone suppression, size increase
on drag, size/hardness clamping, dominant axis lock
- **Updated** `src/composables/maskeditor/useBrushDrawing.ts` — removes
inlined adjustment state and functions, delegates to
`useBrushAdjustment(initialSettings)`
## Test Functionality
Open ComfyUI and enter the MaskEditor of any image node. On the canvas,
Alt + Right-click Drag:
- Drag Right → Increase brush size - pass
- Drag Left → Decrease brush size - pass
- Drag Up → Increase hardness - pass
- Drag Down → Decrease hardness - pass
https://github.com/user-attachments/assets/273e8383-dab5-4c82-ac7b-0a1534dfd770
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches core pointer-interaction logic for brush tuning and changes
adjustment behavior (removes delta saturation and uses initial values),
which could subtly affect UX even though scope is localized to the mask
editor.
>
> **Overview**
> Extracts the Alt-drag brush size/hardness adjustment logic out of
`useBrushDrawing` into a new `useBrushAdjustment` composable, and wires
`useBrushDrawing` to delegate to it.
>
> The extracted logic now bases adjustments off the captured initial
brush size/hardness and removes prior delta capping (no ±100px
saturation), which changes how large/continuous drags affect the final
values. Adds a Vitest suite covering dead-zone behavior, dominant-axis
suppression, clamping, and the no-op-before-start contract.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Robin Huang
|
8f9f452c86 |
fix: enable Chrome password autofill on signup form (#11636)
## Summary Add `name` attributes to the signup form's email, password, and confirm-password inputs so Chrome's password manager recognizes the form and offers autofill/save. ## Changes - **What**: Pass `name` through to the underlying `<input>` on the email field (via `pt:root:name`) and on both password fields (via `pt:pc-input-text:root:name`). Without `name`, Chrome can't pair the email with the password and won't surface the save-password / suggest-strong-password prompts. ## Review Focus - The PrimeVue passthrough syntax (`pt:root:*` for `InputText`, `pt:pc-input-text:root:*` for `Password`) lands the attribute on the actual `<input>` element — verified in DevTools. - `confirm-password` is not a standard `autocomplete` token; we keep `autocomplete="new-password"` on both password fields and only differentiate via `name`. ## Screenshots (if applicable) https://github.com/user-attachments/assets/e1e25ab5-8496-4c84-b5f1-630f31956c80 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11636-fix-enable-Chrome-password-autofill-on-signup-form-34e6d73d36508180882cc9ebafb58417) by [Unito](https://www.unito.io) |
||
|
Benjamin Lu
|
9e16390c33 |
test: assert core command help urls (#11768)
## Summary - Tighten the new `useCoreCommands` help command tests to assert the exact external URL opened for GitHub issues and Discord. ## Testing ```bash pnpm test:unit -- src/composables/useCoreCommands.test.ts pnpm format:check src/composables/useCoreCommands.test.ts ``` Also passed pre-commit `pnpm typecheck` and push hook `pnpm knip`. Stacked on #11748. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11768-test-assert-core-command-help-urls-3516d73d365081de99d0c71f707d0fb4) by [Unito](https://www.unito.io) --------- Co-authored-by: dante01yoon <bunggl@naver.com> |
||
|
Terry Jia
|
c88275b2a4 |
test(load3d): add unit tests for SceneManager (#11762)
## Summary Add 35 unit tests for `SceneManager`, the largest remaining gap in the load3d core (452 LOC). Targets only the logic-bearing methods (background mode dispatch, render-mode switching, aspect-ratio scaling, capture pipeline, dispose). Renderer-passthrough internals are intentionally left to E2E. Follow-up to Tier 1 (#11733), Tier 2, Tier 3a, and Tier 3b. ## Changes - **What**: 35 new tests covering construction (main scene + background scene + grid + tiled mesh + default color mode), `toggleGrid`, `setBackgroundColor` (color update, scene-bg cleanup, panorama-demote, prior-texture dispose), `setBackgroundImage` (empty-path fallback, loading-start emit, temp/output subfolder rewrite, /api prefix, tiled-mesh material swap, panorama scene-background promotion, prior-texture dispose, error-path fallback), `removeBackgroundImage`, `setBackgroundRenderMode` (no-op same-mode, color-only emit, image panorama-promote, image tiled-demote), `updateBackgroundSize` (no-texture/no-mesh/no-map guards, wide vs. tall image scaling), `handleResize` (image-bg active vs. color-only), `getCurrentBackgroundInfo`, `captureScene` (returns 3 data URLs + restores renderer state, restores grid visibility, propagates errors), and `dispose` (resource cleanup + scene-background null). ## Review Focus - **Coverage**: `SceneManager.ts` 89.5% lines / 74.2% branches / 89.5% funcs. Uncovered lines are concentrated in `renderBackground` and the deep mesh-traversal loop inside `captureScene` — exactly the renderer-passthrough territory deferred per the Tier 3c plan. - **`THREE.Material.needsUpdate` is a write-only setter** in THREE.js — reading returns `undefined`. The "demote panorama → tiled" test asserts `mat.map === texture` instead of `mat.needsUpdate === true`, with a comment explaining why. - **happy-dom canvas `clientWidth`/`clientHeight` default to 0** — `makeRenderer()` overrides them via `Object.defineProperty` so production code reading `renderer.domElement.clientWidth` gets the test value. - **`THREE.TextureLoader` is mocked via `vi.mock('three', ...)` with `importOriginal`**, matching the pattern in `RecordingManager.test.ts` and `HDRIManager.test.ts`. `mockTextureLoad` is hoisted so each test can resolve/reject the load callback independently. - **`vi.spyOn(manager, 'setBackgroundColor')` in three places** to assert internal delegation (empty-path fallback, error fallback, `removeBackgroundImage`). Defensible because the delegation IS the documented contract for these methods. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11762-test-load3d-add-unit-tests-for-SceneManager-3516d73d365081628ff4c146defebac1) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
23e48b2140 |
feat: Node search - Improve category tree on mobile with collapse (#11687)
## Summary Improves the search experience on mobile by collapsing the category menu & reogranises the filer buttons ## Changes - **What**: - add toggle button to collapse category selection - auto collapse on mobile - floating panel on mobile - re-order filter buttons - tests ## Screenshots (if applicable) Closed: <img width="415" height="373" alt="image" src="https://github.com/user-attachments/assets/c99cd6cd-eb92-4ce3-9844-591dd1e80769" /> Desktop open: <img width="455" height="328" alt="image" src="https://github.com/user-attachments/assets/df15bdda-f77a-4c12-90e1-8608d67c55b4" /> Mobile open: <img width="427" height="600" alt="image" src="https://github.com/user-attachments/assets/a2b115ad-bce0-4ed1-9d30-126a35263259" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11687-feat-Node-search-Improve-category-tree-on-mobile-with-collapse-34f6d73d365081729075e8b0071a3bc1) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
af43619ae1 |
test(load3d): add unit tests for copyLoad3dState in load3dService (#11761)
## Summary Add 19 unit tests for `Load3dService.copyLoad3dState` (the one method intentionally deferred from Tier 1). Brings `load3dService.ts` from 54.5% to 100% line coverage. Follow-up to Tier 1 (#11733), Tier 2, and Tier 3a. ## Changes - **What**: 19 new tests covering every branch of `copyLoad3dState`: no-source-model fast path, splat fast path (with and without `originalURL`), mesh path (existing-target-model removal, SkeletonUtils clone, originalModel/material/upDirection/texture copy, initial transform on clone, gizmo transform application, gizmo enable/disable across both source and target prior states, animation copy when present/absent), background-image vs. background-color dispatch, light-intensity falsy fallback, perspective-vs-orthographic FOV gating, and the always-detach + setupForModel gizmo contract. ## Review Focus - **Coverage**: `load3dService.ts` lines 54.5% → **100%**, branches 50% → **90.9%**, funcs 88.9% → **100%**. Remaining uncovered lines are minor (`loadSkeletonUtils` cache-hit path, a couple of null-map early returns). - **Test fixtures use real `THREE.Object3D` and `THREE.Scene`** so production code's `.position.set(...)`, `.rotation.set(...)`, `scene.add/remove` calls work without further stubbing. - **`makeTarget` memoizes the gizmo manager** (`getGizmoManager: () => gizmoManager` rather than returning a fresh literal each call). Production code calls `getGizmoManager()` multiple times; without memoization, the `detach` and `setupForModel` mocks would be unobservable from tests. - **`state` return on `makeTarget`** exposes mutable `modelManager`, captured `gizmoManager`/`animationManager`, and `sceneAdded`/`sceneRemoved` arrays so tests can assert post-state directly without casts through the production-typed `Load3d` interface. - **Background-image test uses `createMockLGraphNode({ id, properties })` overrides** rather than mid-test property mutation. - **Destructuring-default gotcha**: `const { lightsIntensity = 0.8 } = overrides` applies the default even when `undefined` is passed explicitly. The "fallback to setLightIntensity(1)" test passes `0` instead — production code's `intensity || 1` short-circuits the same way. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11761-test-load3d-add-unit-tests-for-copyLoad3dState-in-load3dService-3516d73d36508142bc72d97b27b0a36b) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
d2e88011aa |
test(load3d): add unit tests for EventManager, ViewHelperManager, and load3dLazy (#11760)
## Summary Add unit tests for the three remaining small/wrapper modules in the load3d domain (`EventManager` pub/sub, `ViewHelperManager` ViewHelper wrapper, and `load3dLazy` lazy-extension loader). Follow-up to Tier 1 (#11733) and Tier 2. ## Changes - **What**: 28 new unit tests across 3 files covering EventManager add/remove/emit semantics, ViewHelperManager container DOM creation + pointer-event interception + animation-finished camera-state emission + perspective/orthographic zoom snapshotting + dispose ordering, and load3dLazy extension registration + 3D-node-type recognition + Load3D-specific `mesh_upload` injection + `beforeRegisterNodeDef` hook replay for newly registered extensions. ## Review Focus - **Coverage** (lines/branches/funcs): EventManager 100% / 100% / 100%, ViewHelperManager 100% / 83.3% / 72.7%, load3dLazy 95.8% / 80% / 100%. Aggregate: **98.6% / 85.3% / 85.7%**. - **`vi.mock` factory side-effects only fire once per test file** — `load3dLazy.test.ts` originally tried to count dynamic imports of `./load3d` and `./saveMesh` via spies inside the mock factory, but factories aren't re-invoked across `vi.resetModules()`. Switched to verifying observable side effects (`enabledExtensions` getter call counts, `beforeRegisterNodeDef` replay invocations). - **Snapshot-vs-diff `enabledExtensions` queue** in `load3dLazy.test.ts`: production code does `before = new Set(enabledExtensions); await imports; diff = enabledExtensions.filter(!before.has)`. To exercise the replay branch, the mock returns `[]` first (for `before`) and `[newExtension]` second (for the post-import snapshot) via `mockReturnValueOnce` queueing. - **`MockViewHelper` is defined inside the `vi.mock()` factory** rather than `vi.hoisted()`, matching the `GizmoManager.test.ts:15-41` convention (hoisted handles only, classes inside the factory). - **PointerEvent propagation tests** require the production code's `event.stopPropagation()` to actually keep events from bubbling to the parent in happy-dom; the parent listener gets attached and asserted-not-called. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11760-test-load3d-add-unit-tests-for-EventManager-ViewHelperManager-and-load3dLazy-3516d73d3650814bb2dac3360ab0d2a1) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
180a0001e8 |
test(load3d): add unit tests for LightingManager, ControlsManager, exportMenuHelper, and ModelExporter (#11758)
## Summary Add unit tests for the four Tier 2 untested logic modules in the load3d domain (`LightingManager`, `ControlsManager`, `exportMenuHelper`, and `ModelExporter`). Follow-up to the Tier 1 PR (#11733). ## Changes - **What**: 43 new unit tests across 4 files covering light setup/intensity scaling/HDRI mode/disposal, OrbitControls construction (including DOM-parent fallback) and camera-state event emission, the export submenu builder (item structure, submenu opening, format dispatch, success/error toasts), and the static `ModelExporter` (URL parsing, direct-URL fast paths for matching extensions, GLB/OBJ/STL serialization branches, error toast paths). ## Review Focus - **Coverage** (lines/branches/funcs): LightingManager 100% / 50% / 90.9%, ControlsManager 100% / 100% / 87.5%, exportMenuHelper 100% / 100% / 100%, ModelExporter 98.4% / 95.7% / 100%. Aggregate: **99.2% / 93.5% / 95.1%**. - **`vi.mock(import('@/lib/litegraph/src/litegraph'), ...)` in `exportMenuHelper.test.ts`** uses the dynamic-import form so `importOriginal()` is auto-typed. Required because `apiSchema.ts` transitively imports `LinkMarkerShape` from the same module — replacing the whole module breaks the build. The mock replaces only `LiteGraph.ContextMenu` in-place on the real singleton. - **`MockContextMenu` must be a class**, not an arrow function — production code does `new LiteGraph.ContextMenu(...)`. Initial arrow-function mock failed with "is not a constructor". - **Fake-timer rejection pattern in `ModelExporter.test.ts`**: rejection assertions are attached *before* `vi.runAllTimersAsync()` (`const assertion = expect(p).rejects.toThrow(...); await drain; await assertion`) to avoid unhandled-rejection warnings. - **Surprising `detectFormatFromURL` behavior**: `detectFormatFromURL('?filename=cube')` returns `'cube'`, not `null`, because `'cube'.split('.').pop()` returns the whole basename when no dot is present. Test documents this rather than asserting an incorrect expectation. - **Two unreachable lines left uncovered**: `LightingManager:65` (`?? 1` fallback in the `setLightIntensity` multiplier lookup — every light is registered in the map at construction, so the fallback is dead) and `ModelExporter:21` (a `try/catch` around `URLSearchParams` whose constructor cannot throw on the inputs the production code passes). ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11758-test-load3d-add-unit-tests-for-LightingManager-ControlsManager-exportMenuHelper-and-3516d73d365081cb96fff33672503822) by [Unito](https://www.unito.io) |
||
|
Dante
|
8f011225bf |
test: add unit tests for useCoreCommands canvas/help commands (#11748)
## Summary Adds 8 tests across three new describe blocks for \`src/composables/useCoreCommands.ts\`: - **Canvas view**: \`Comfy.Canvas.ResetView\`, \`Comfy.Canvas.ZoomIn\`, \`Comfy.Canvas.ZoomOut\`. - **Workflow lifecycle**: \`Comfy.OpenClipspace\`, \`Comfy.RefreshNodeDefinitions\`. - **Help**: \`Comfy.Help.OpenComfyUIIssues\`, \`Comfy.Help.OpenComfyOrgDiscord\`, \`Comfy.Help.AboutComfyUI\`. Adds \`vi.hoisted\` mocks for \`useTelemetry\`, \`useSettingsDialog\`, and \`useLitegraphService.resetView\` so they remain isolated from the existing 15-test suite. ## Why this slice \`useCoreCommands.ts\` exports 118 distinct command callbacks (1356 LOC). A single coverage-backfill PR for the whole file would be unwieldy and risk merge conflicts (this file is touched frequently). This PR covers a coherent slice — view/lifecycle/help commands — and follow-up PRs can pick off remaining clusters. ## Testing \`\`\`bash pnpm vitest run src/composables/useCoreCommands.test.ts \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11748-test-add-unit-tests-for-useCoreCommands-canvas-help-commands-3516d73d365081c384ffcc72c15dfd47) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
3c50487c18 |
test: add test for MaxHistoryItems setting (#11750)
## Summary Adds test to ensure MaxHistoryItems limits both API query & client rendering ## Changes - **What**: - ensure limit is added to url - ensure virtual grid is capped to limit ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11750-test-add-test-for-MaxHistoryItems-setting-3516d73d365081daa973ccfd8a7f479e) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
818e549e8e |
fix: hide blueprint node id in search (#11759)
There is a setting that enables Node IDs to display on the search results. Subgraphs have long non-user friendly IDs which cause this to render badly for the built in blueprints (and user published). This update hides the IDs for blueprint nodes. ## Changes - **What**: - hide if blueprint - add test ## Screenshots (if applicable) Before: <img width="910" height="504" alt="image" src="https://github.com/user-attachments/assets/9eea9fd7-8f72-4e1b-9522-46efba0ef71a" /> After: <img width="797" height="552" alt="image" src="https://github.com/user-attachments/assets/43d6fc62-4102-41c3-b9bb-a3efd244580d" /> ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11759-fix-hide-blueprint-node-id-in-search-3516d73d365081baa055d12c6a31fadd) by [Unito](https://www.unito.io) |
||
|
AustinMroz
|
fd1a8e9432 |
Fix legacy widget width in app mode (#11574)
When in app mode, widgets can be drawn with size different from the size of the parent node. Mouse events on legacy canvas widgets require that the client code query the current state of the node and widget to determine if any elements are being interacted with. This PR sets the `widget.width` property when a legacy canvas widget draw operation occurs so that custom nodes can properly resolve subsequent mouse events. At current, no core nodes exist that utilize legacy widgets. As a result the setup code to test this bug fix is slightly involved. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11574-Fix-legacy-widget-width-in-app-mode-34b6d73d365081caaa34c6204f8361f6) by [Unito](https://www.unito.io) |
||
|
Dante
|
8f61ecd82e |
test: add unit tests for executionStore WebSocket handlers (#11746)
## Summary Adds 22 new tests for \`src/stores/executionStore.ts\`, raising coverage from **48.7% → 82.9%** lines (functions 47% → 72.5%). Tests drive each WebSocket handler by capturing handlers registered through the mocked \`api.addEventListener\` and dispatching CustomEvents at them. ## Test Coverage WebSocket handlers (driven through \`bindExecutionEvents\`): - \`execution_start\` — sets activeJobId, seeds queued job entry, clears initializing state for the starting job. - \`execution_cached\` — marks listed nodes; no-op when no active job. - \`execution_interrupted\` — clears active job state. - \`executed\` — marks executed node; no-op when no active job. - \`execution_success\` — clears active job and progress state. - \`executing\` — clears \`_executingNodeProgress\` and activeJobId on null detail. - \`progress\` — sets \`_executingNodeProgress\`. - \`status\` — reads clientId once and stops listening. - \`execution_error\` — service-level (no node_id) routes to \`lastPromptError\`; runtime errors route to \`lastExecutionError\`. - \`notification\` — marks job as initializing on "Waiting for a machine"; ignores empty id and unrelated text. Other: - \`unbindExecutionEvents\` removes every listener registered. - \`storeJob\` populates queuedJobs, jobIdToWorkflowId, jobIdToSessionWorkflowPath. - \`registerJobWorkflowIdMapping\` ignores empty inputs. - \`ensureSessionWorkflowPath\` is idempotent and updates on change. ## Testing \`\`\`bash pnpm vitest run src/stores/executionStore.test.ts pnpm vitest run src/stores/executionStore.test.ts --coverage --coverage.include='src/stores/executionStore.ts' \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11746-test-add-unit-tests-for-executionStore-WebSocket-handlers-3516d73d3650810aa910f5a022fdc17b) by [Unito](https://www.unito.io) |
||
|
Dante
|
8fe0385a57 |
test: add unit tests for pnginfo wrappers and getLatentMetadata (#11745)
## Summary Extends \`src/scripts/pnginfo.test.ts\` with 5 new tests covering the format-specific delegating wrappers and the safetensors metadata reader. Lifts pnginfo.ts coverage from **17.6% → 23.2%** lines (the remaining gap is \`importA1111\`, which needs a refactor before it can be tested cleanly — left to a follow-up). ## Test Coverage - \`getPngMetadata\`, \`getFlacMetadata\`, \`getAvifMetadata\` delegate to their respective \`metadata/*\` modules (mocked). - \`getLatentMetadata\` returns the \`__metadata__\` object from a hand-built safetensors header. - \`getLatentMetadata\` resolves \`undefined\` when the header has no \`__metadata__\` entry. ## Out of scope \`importA1111\` (lines 176-542) is a 270-line A1111-prompt → ComfyUI-graph builder. Testing it requires either heavy LiteGraph mocks or a refactor that extracts pure parsing helpers from the graph-mutation code. Tracking separately. ## Testing \`\`\`bash pnpm vitest run src/scripts/pnginfo.test.ts pnpm vitest run src/scripts/pnginfo.test.ts --coverage --coverage.include='src/scripts/pnginfo.ts' \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11745-test-add-unit-tests-for-pnginfo-wrappers-and-getLatentMetadata-3516d73d365081c080a6c8146aa1bee8) by [Unito](https://www.unito.io) |
