mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-24 06:35:10 +00:00
764977aaf697d404f9db02f5791c69604de5a31d
640 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Dante
|
d6f632477f |
feat(dialog): migrate Error / NodeSearchBox / SecretForm / VideoHelp / Customization to Reka-UI (Phase 2) (#12109)
## Summary Phase 2 of the dialog migration kicked off in #11719 and continued in #12041. Migrates four medium-complexity dialogs to the Reka-UI primitives. Public API of `useDialogService` / `dialogStore` is unchanged. Parent: [FE-571](https://linear.app/comfyorg/issue/FE-571/dialog-system-migration-primevue-reka-ui-parent) This phase: [FE-574](https://linear.app/comfyorg/issue/FE-574/phase-2-migrate-error-nodesearchbox-secretform-videohelp-customization) Predecessors: #11719 (Phase 0, merged), #12041 (Phase 1, merged) > **NodeSearchBoxPopover deferred** — host of an inner PrimeVue Dialog (filter panel) that teleports to body and conflicts with Reka's DismissableLayer outside-pointer detection (CI dismissed the outer dialog mid-interaction). Tracking as a follow-up PR; FE-574 stays open for it. ## Changes ### `src/services/dialogService.ts` | Call site | Renderer | Size | | --- | --- | --- | | `showExecutionErrorDialog()` | `'reka'` | `lg` | | `showErrorDialog()` | `'reka'` | `lg` | ### `src/components/dialog/content/ErrorDialogContent.vue` - Drops `import Divider from 'primevue/divider'` and `import ScrollPanel from 'primevue/scrollpanel'` - Replaces with `<hr class="border-t border-border-subtle">` + `<div class="h-[400px] w-full max-w-[80vw] overflow-auto">` ### Direct PrimeVue → Reka swaps (no `dialogStore` involvement) | File | Notes | | --- | --- | | `src/components/common/CustomizationDialog.vue` | Reka primitives + DialogTitle/Header/Footer; drops PrimeVue Divider; `:modal="false"` and `pointer-down-outside` overlay guard so the PrimeVue ColorPicker overlay (teleported to body) does not auto-dismiss the dialog | | `src/platform/assets/components/VideoHelpDialog.vue` | Headless Reka content; preserves capture-phase ESC by stopping propagation on `escape-key-down`; `VisuallyHidden` title for a11y | | `src/platform/secrets/components/SecretFormDialog.vue` | Reka primitives, retains `v-model:visible`, autofocus on the provider trigger, form submit/validation | ### Tests - `src/services/dialogService.renderer.test.ts`: extends the regression net to cover both error-dialog call sites (renderer `'reka'`, size `'lg'`) - `src/components/common/CustomizationDialog.test.ts`: swaps PrimeVue Dialog stub for Reka primitive stubs ### screenshot <img width="1236" height="761" alt="Screenshot 2026-05-11 at 10 26 51 PM" src="https://github.com/user-attachments/assets/086cb73f-a98d-41f8-96ee-21922da8dd73" /> <img width="1161" height="786" alt="Screenshot 2026-05-12 at 1 26 39 PM" src="https://github.com/user-attachments/assets/db7383d8-f737-4472-91c0-dab5aa41547b" /> ## Quality gates - [x] `pnpm typecheck` — clean - [x] `pnpm lint` — 0 errors (3 pre-existing warnings unrelated to this PR) - [x] `pnpm format` — applied - [x] `pnpm test:unit` (touched + adjacent areas): - `dialogService.renderer.test.ts` — 5/5 - `CustomizationDialog.test.ts` — 4/4 - All `src/components/dialog` tests — 73/73 - `src/platform/secrets` tests — 39/39 - `NodeBookmarkTreeExplorer.test.ts` — 7/7 - [ ] CI Playwright matrix ## Public API impact None. `useDialogService` / `dialogStore` signatures unchanged. Custom-node extensions calling `app.extensionManager.dialog.*` continue to work. ## Out of scope (later phases) - NodeSearchBoxPopover — follow-up PR under FE-574 - Settings dialog — Phase 3 (FE-575) - Manager dialog — Phase 4 (FE-576) - `ConfirmDialog` callers (`SecretsPanel`, `BaseWorkflowsSidebarTab`) — Phase 5 (FE-577) - Removing PrimeVue `Dialog`/`<style>` overrides in `GlobalDialog.vue` — Phase 6 (FE-578) ## Test plan - [x] Unit: 73/73 dialog-area, 39/39 secrets - [ ] CI: full Vitest + Playwright matrix - [ ] Manual on a backend: - Trigger an execution error → error dialog opens through Reka, scroll body, copy-to-clipboard, close - Add/edit a secret → form submits, validation errors render, ESC and cancel close - Open VideoHelpDialog from `UploadModelFooter` while inside the asset modal → ESC closes only the help dialog - Customize a node bookmark color/icon → apply/reset, color picker overlay works |
||
|
Alexander Brown
|
1ab63807e9 |
fix: reserve scrollbar gutter on textareas to prevent hover reflow (#12280)
*PR Created by the Glary-Bot Agent* --- Adds the Tailwind 4.3 `scrollbar-gutter-stable` utility to the shared shadcn `Textarea` wrapper and the `ModelInfoPanel` description textarea so the layout no longer shifts when a vertical scrollbar appears on hover or focus. The wrapper edit propagates to all four consumers (`WidgetTextarea`, `WidgetMarkdown`, `ComfyHubDescribeStep`, `ComfyHubCreateProfileForm`). The `ModelInfoPanel` site uses a native `<textarea>` that bypasses the wrapper, so it is fixed inline. The `overflow-hidden hover:overflow-auto` performance pattern from #10804 is intentionally preserved. ## Verification - Typecheck, eslint, oxlint, oxfmt, stylelint clean on changed files - `Textarea.test.ts`, `WidgetTextarea.test.ts`, `WidgetMarkdown.test.ts`, `ComfyHubDescribeStep.test.ts`, `ModelInfoPanel.test.ts` — all 68 tests pass - `pnpm build` succeeds; the production CSS bundle contains the `scrollbar-gutter:stable` rule - Storybook (`UI/Textarea`): confirmed `getComputedStyle(textarea).scrollbarGutter === 'stable'` on Default, WithLabel, and overflowing-content scenarios Fixes FE-697 ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12280-fix-reserve-scrollbar-gutter-on-textareas-to-prevent-hover-reflow-3606d73d3650816f9947e20134abf59e) 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> |
||
|
Alexander Brown
|
d6b4137eec |
chore: upgrade tailwindcss to 4.3 (#12275)
*PR Created by the Glary-Bot Agent* --- ## Summary Bump `tailwindcss` and `@tailwindcss/vite` from `^4.2.0` to `^4.3.0` in the workspace catalog so the new first-class scrollbar utilities (notably `scrollbar-gutter-stable`) are available — the missing utility behind the FE-697 workaround. Release notes: https://tailwindcss.com/blog/tailwindcss-v4-3. ## Changes - **What**: - `pnpm-workspace.yaml` catalog: `tailwindcss` and `@tailwindcss/vite` → `^4.3.0` (lockfile resolves to `4.3.0` for `tailwindcss`, `@tailwindcss/vite`, `@tailwindcss/node`, and all `@tailwindcss/oxide-*` native packages). - Auto-fix 21 lint errors that `eslint-plugin-better-tailwindcss` now reports because the new utilities have canonical forms: - `VirtualGrid.vue`: `[scrollbar-gutter:stable]` → `scrollbar-gutter-stable` (the FE-697 case) - `VirtualGrid.vue` + `RightSidePanel.vue`: `scrollbar-thin` class-order fix - `TopBarHeader.vue`: `h-6.25 w-6.25` → `size-6.25` (6 button cells) - `Dialogue.vue`: `translate-x-[-50%] translate-y-[-50%]` → `translate-[-50%]` - Minor `-mx-[…]`, `-inset-[…]`, `-ml-[…]` arbitrary-value reorderings in `NodeSearchFilterBar.vue`, `LGraphNode.vue`, `CloudNotificationContent.vue` - All 21 are auto-fixes; all existed on `main` and would have started failing CI on next merge. - **Breaking**: None. v4.3 is purely additive; existing config (CSS-first `@theme`/`@utility`/`@plugin`, custom `lucideStrokePlugin`, `tailwindcss-primeui`, `@iconify/tailwind4`, `tw-animate-css`) is unchanged. ## Review Focus - Confirmed `scrollbar-gutter: stable` compiles into `dist/assets/index-*.css` from `VirtualGrid.vue` after the canonicalization. - Runtime probe via Playwright (preview build) confirmed `scrollbar-gutter-stable`, `scrollbar-thin`, `tab-4`, and `zoom-100` all apply. - `pnpm typecheck`, `pnpm lint`, `pnpm build`, `pnpm knip` all pass. - `pnpm test:unit`: 10687/10696 pass. 1 failure in `GraphView.test.ts` ("reconnect wiring") confirmed to fail identically on `main` (flaky `toHaveBeenCalledTimes(1)` getting `3`/`4`) — unrelated to this change. - Oracle code review: 0 findings. Refs FE-697. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12275-chore-upgrade-tailwindcss-to-4-3-3606d73d3650813185cece1c7315e1c2) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Christian Byrne
|
e972d658d3 |
feat(settings): lower Comfy.Pointer.ClickBufferTime default from 150ms to 32ms (#12032)
*PR Created by the Glary-Bot Agent* --- ## Summary Click vs drag is disambiguated by two thresholds: distance (`Comfy.Pointer.ClickDrift`, 6px) and time (`Comfy.Pointer.ClickBufferTime`). Distance does the real work — any intentional drag immediately exceeds 6px on the first move. The time threshold only matters in the corner case where the pointer is held still then released without crossing the distance threshold. The previous 150ms default forces every pointerdown to wait up to 150ms before drag begins, even when the user is clearly dragging. This is visible as lag when click+dragging an unselected node, where the wait stacks on top of selection-state work that runs at drag start ([FE-558](https://linear.app/comfyorg/issue/FE-558/bug-delay-when-clickdragging-an-unselected-node)). 32ms (~2 frames at 60fps) is well below human-perceptible click latency and still safely above incidental jitter on pointerdown. ## Changes - `src/platform/settings/constants/coreSettings.ts` — `Comfy.Pointer.ClickBufferTime`: - `defaultValue: 150` → `32` - `versionAdded: '1.4.3'` retained, add `versionModified: '1.44.19'` - Slider `step: 25` → `1` (the old step made `32` unrepresentable on the slider) - Tooltip extended with rationale (why distance is the real disambiguator, why this should be small) - `src/lib/litegraph/src/CanvasPointer.ts` — Static `bufferTime = 150` → `32` for consistency with the user-setting default; the runtime `watchEffect` in `useLitegraphSettings.ts` continues to override at boot. JSDoc explains the rationale and that the user setting overrides at runtime. ## Verification - `pnpm typecheck` clean. - Litegraph + settings test suites: 998/998 pass. - `pnpm format` clean (lint-staged enforced on commit). ## Why not just leave it user-overridable? It already is. But the default is what every user — including ones who never open settings — experiences. 150ms was never a justified value (no comment explains it, no benchmark, and 6px distance covers the disambiguation). Defaults should reflect best UX. Refs FE-558. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12032-feat-settings-lower-Comfy-Pointer-ClickBufferTime-default-from-150ms-to-32ms-3586d73d365081f5a29cdc84b4b736e3) 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> |
||
|
Christian Byrne
|
01742672bb |
feat: extend version warning to all comfy_package_versions entries (#12167)
*PR Created by the Glary-Bot Agent* --- ## Summary Extend the existing frontend version-mismatch warning UI to consume the new `comfy_package_versions` array now exposed by ComfyUI's `/system_stats` endpoint. The backend ships installed/required versions for every `comfy*` package pinned in `requirements.txt` (frontend, workflow-templates, embedded-docs, kitchen, aimdo, …); the frontend now surfaces one toast per outdated package, reusing the existing N=1 frontend-version warning shape. Backend PR: https://github.com/Comfy-Org/ComfyUI/pull/13875 ## Changes - `src/schemas/apiSchema.ts` — add `comfy_package_versions: Array<{name, installed, required}>` to the `SystemStats` schema and export a `ComfyPackageVersion` type. The field is `.optional()` so older backends remain compatible. - `src/platform/updates/common/versionCompatibilityStore.ts` — new `outdatedComfyPackages` / `packageWarningMessages` computeds that mirror the existing semver `gt` comparison (same `valid` guard). Skip `comfyui-frontend-package` because the dedicated frontend warning above already covers it (and uses the running bundle's version rather than the installed pip version). Outdated packages are sorted by `name`/`installed`/`required` before being folded into the dismissal storage key so the key is stable across response orderings — a fresh package bump re-shows the warning, but the same outdated set in a different order does not. - `src/platform/updates/common/useFrontendVersionMismatchWarning.ts` — emit one toast per outdated package in addition to the existing frontend toast, reusing the same i18n wrapper and the existing `hasShownWarning` once-per-session guard. - `src/locales/en/main.json` — new `g.comfyPackageOutdated` string. - Unit tests — added coverage for outdated/skip/invalid-version paths, dismissal-key inclusion, and stable ordering. Existing 21 store + 8 composable tests untouched and still passing. CI suppression is unchanged: warnings still gate on `versionCompatibilityStore.shouldShowWarning` (which respects the `Comfy.VersionCompatibility.DisableWarnings` setting and the 7-day dismissal cache), and unit tests continue to mock the store the same way. ## Verification - `pnpm vitest run` for the version-warning module: **31/31** passing. - Targeted sweep across `src/platform/updates`, `src/stores/systemStatsStore.test.ts`, `src/schemas`: **160/160** passing. - `pnpm typecheck`: clean. - `pnpm lint`: 0 errors (3 pre-existing warnings in unrelated 3D test files). - `pnpm format`: applied, no incidental changes. - **Manual Playwright run** against the real dev server with `/api/system_stats` intercepted to return outdated package data — produced exactly the expected toasts and correctly skipped `comfyui-frontend-package` and the up-to-date `comfy-kitchen` entry. Same run with all-up-to-date data produced zero toasts. ### Toasts produced (manual verification) The fixture used: `required_frontend_version=99.99.99`, plus `comfy_package_versions=[frontend-package (outdated, skipped), workflow-templates 0.9.0→0.9.5 (outdated), embedded-docs 0.4.0→0.5.0 (outdated), comfy-kitchen 0.2.8→0.2.8 (up to date)]`. ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12167-feat-extend-version-warning-to-all-comfy_package_versions-entries-35e6d73d365081e7b993d0f06c9e5c98) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Christian Byrne
|
b7fe0365af |
fix: remove "400 Free Credits Monthly" line from cloud paywall modal (#12251)
*PR Created by the Glary-Bot Agent* --- ## Summary The in-app cloud paywall modal shown on first install of ComfyUI Desktop 1.0 advertised "400 Free Credits Monthly" as a benefit, but the free tier is currently disabled, making that copy misleading. Per the thread direction, this skips the feature-flag plumbing (which isn't available on Desktop anyway) and simply removes the line. ## Changes - `CloudNotificationContent.vue`: change the feature loop from `n in 4` to `n in [2, 3, 4]` so feature1 is skipped. - `src/locales/*/main.json` (12 locales): remove the now-unused `cloudNotification.feature1Title` key. - `browser_tests/tests/dialog.spec.ts`: add a regression assertion that "400" / "Free Credits" copy is no longer present in the modal. ## Test plan - `pnpm typecheck` — passes - `pnpm typecheck:browser` — passes - `pnpm exec vitest run src/platform/cloud/notification/components/DesktopCloudNotificationController.test.ts` — 3/3 pass - Pre-commit hooks (oxfmt, oxlint, eslint, stylelint, typecheck) — all pass - Manual: triggered `showCloudNotification()` against the dev server and confirmed only 3 benefit rows render (screenshot attached); the "400 Free Credits Monthly" line is gone. - Fixes DESK2-90 ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12251-fix-remove-400-Free-Credits-Monthly-line-from-cloud-paywall-modal-3606d73d365081eaa572e6cd995278d8) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Dante
|
bdb92c845e |
fix: include share_id when importing published assets (FE-603) (#12055)
*PR Created by the Glary-Bot Agent* https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778150003248989 FE-603 --- ## Summary Send `share_id` alongside `published_asset_ids` from `workflowShareService.importPublishedAssets`, and type / zod-validate the body against `ImportPublishedAssetsRequest` + `zImportPublishedAssetsRequest` from `@comfyorg/ingest-types`. This is a **parameter-schema change**, not a live bug fix. The original `BAD_REQUEST: share_id is required` failure is **no longer reproducible in production** — the backend rolled back the required-field enforcement in [BE-855](https://linear.app/comfyorg/issue/BE-855) (cloud [#3587](https://github.com/Comfy-Org/cloud/pull/3587) / [#3588](https://github.com/Comfy-Org/cloud/pull/3588)). Today the import succeeds without `share_id`. - Closes FE-603 ## Why we still ship this Frontend goes first so the backend can later re-tighten the contract without breaking shared-workflow imports a second time. Agreed sequence in [Slack](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778535349236419) — BE-6 was split into two sub-issues to make the chain explicit: 1. **[BE-898](https://linear.app/comfyorg/issue/BE-898)** (blocks FE-603) — BE makes `share_id` **optional**, validating when present. [cloud PR #3633](https://github.com/Comfy-Org/cloud/pull/3633), In Review. 2. **FE-603** (this PR) — FE sends `share_id`. 3. **[BE-899](https://linear.app/comfyorg/issue/BE-899)** (blocked by FE-603) — BE flips `share_id` back to **required** after FE-603 deploys. Steps 1 and 2 ship in order: BE-898 → FE-603 → BE-899. ## Changes - `workflowShareService.importPublishedAssets` now takes `shareId: string` and types the request body with `ImportPublishedAssetsRequest`. The body is parsed through `zImportPublishedAssetsRequest` before the network call so future contract drift surfaces as a typecheck or zod parse failure rather than a silent runtime 400. - `useSharedWorkflowUrlLoader.ts` threads `payload.shareId` (already in scope from `SharedWorkflowPayload`) into the import call. - Unit tests assert `share_id` is sent and that an empty `share_id` is rejected before fetch. ## Verification - `pnpm typecheck` — clean - `pnpm lint` — 0 errors / 0 warnings on changed files (3 pre-existing warnings in unrelated files) - `pnpm format` — applied - `pnpm vitest run` on both affected files — 34/34 passing ### Live in-browser smoke test (Vite dev server) Loaded the built module in the browser, intercepted `fetch`, and exercised the new code path. The new `importPublishedAssets` produces the correct wire format and zod rejects bad input before fetch: ```json // Happy path — sent to /api/assets/import (POST) { "published_asset_ids": ["pa-1", "pa-2", "pa-3"], "share_id": "share-abc" } ``` ```text // Empty share_id — zod rejects, fetch is never called [ { code: "too_small", minimum: 1, path: ["share_id"], message: "String must contain at least 1 character(s)" } ] fetchCallsBetweenAttempts: 0 ``` ## Why typecheck didn't catch the original miss `api.fetchApi(route: string, options?: RequestInit)` accepts the standard DOM `RequestInit`, so `body` is just `BodyInit | null`. Once the call site does `JSON.stringify({ ... })`, the inline object is erased into a string and TS has no schema to enforce. There was no `@ts-ignore` or `as any` — the generated types were simply never imported. This PR plugs that one call site; the same pattern should be applied wherever the frontend hits ingest endpoints (the broader hey-zod migration gap mentioned in #bug-dump). Reported in #bug-dump. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12055-fix-include-share_id-when-importing-published-assets-3596d73d365081c0a1c7e69102f5cfcc) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
de1c1ee1f2 |
fix: add support for parsing python generated json with NaN/infinite (#12217)
## Summary API and other legacy JSON generated by python `json.dumps` can contain `NaN` and `Infinity` which cannot be parsed with JS `JSON.parse`. This adds regex to replace these invalid tokens with `null`. ## Changes - **What**: - add regex replace on bare NaN/infinity tokens after JSON.parse fails - update call sites - tests ## Review Focus - The regex should only rewrite bare NaN/-Infinity/Infinity and not touch string values or other invalid tokens. - A small regex was chosen over JSON5 due to package size (30.3kB Minified, 9kB Minified + Gzipped) or a manual parser due to the unnecessarily complexity vs a single regex replace. - The happy path is run first, the safe parse is only executed if that failed, meaning no overhead the vast majority of the time and no possiblity of corrupting valid workflows due to a bug in the fallback parser - Multiple call sites had to be updated due to pre-existing architecture of the various parsers, an issue for unifying these is logged for future cleanup - New binary fixtures added for validating e2e import using real files ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12217-fix-add-support-for-parsing-python-generated-json-with-NaN-infinite-35f6d73d365081889fc7f4af823f29c1) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
7ce0973386 |
fix: prevent first user template popup when following shared link (#12024)
## Summary When a user who has not used the app before first loads up, they are presented with the template selection dialog. This conflicts when the first-time user visits the app via a share link - both the share & template dialog are triggered. ## Changes - **What**: - Skip the templates browser when share param is in URL - Tests - Add `url` to `setup`/`goto` to allow specifying the `share` parameter ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12024-fix-prevent-first-user-template-popup-when-following-shared-link-3586d73d365081cbbcecdba45a1ad1ea) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
c92030b158 |
refactor: deduplicate Civitai hostname logic in getSourceName (#11822)
## Summary
- Extract `isCivitaiHost` private helper from `isCivitaiModelUrl` in
`formatUtil.ts` for DRY hostname checking
- Add `isCivitaiUrl` exported function for hostname-only Civitai URL
detection (distinct from `isCivitaiModelUrl` which also validates the
path format)
- Refactor `getSourceName` in `assetMetadataUtils.ts` to use the shared
`isCivitaiUrl` instead of inline duplicate hostname checks
- Add tests for `isCivitaiUrl` covering `.com`, `.red`, subdomain, and
invalid URL cases
## Changes
- `packages/shared-frontend-utils/src/formatUtil.ts` — add
`isCivitaiHost` private helper + export `isCivitaiUrl`; refactor
`isCivitaiModelUrl` to use helper
- `packages/shared-frontend-utils/src/formatUtil.test.ts` — add
`isCivitaiUrl` test suite
- `src/platform/assets/utils/assetMetadataUtils.ts` — import
`isCivitaiUrl` from `@/utils/formatUtil`; remove inline hostname logic
from `getSourceName`
## Testing
### Automated
- Added `isCivitaiUrl` test suite (6 cases: `.com`, `.red`, subdomains,
non-Civitai, invalid URL)
- All 71 existing `formatUtil` tests pass
- All 53 existing `assetMetadataUtils` tests pass (behavior preserved)
- TypeScript typecheck passes
### E2E Verification Steps
1. Run unit tests: `npx vitest run
packages/shared-frontend-utils/src/formatUtil.test.ts
src/platform/assets/utils/assetMetadataUtils.test.ts`
2. Expected: all tests pass
3. Verify `getSourceName('https://civitai.red/models/123')` returns
`'Civitai'`
4. Verify `isCivitaiUrl('https://civitai.com/models/any-path')` returns
`true`
5. Verify `isCivitaiModelUrl` still rejects non-API paths while
`isCivitaiUrl` accepts them
## Review Focus
`isCivitaiUrl` (new, hostname-only) vs `isCivitaiModelUrl` (existing,
hostname+path format): `getSourceName` needs to recognize ANY Civitai
URL as a source, so using `isCivitaiModelUrl` directly would incorrectly
reject valid browse URLs like `civitai.com/models/123`.
Closes #11357
┆Issue is synchronized with this [Notion
page](https://app.notion.com/p/PR-11822-refactor-deduplicate-Civitai-hostname-logic-in-getSourceName-3546d73d36508110974ccc3b7384d82b)
by [Unito](https://www.unito.io)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
Dante
|
9fe19a2afb |
fix(settings): unify settings item heights and use 14px label text (#12180)
## Summary Body text in the settings dialog was still rendering at the inherited 16px (browser default) instead of the 14px design spec, and rows with different control types (toggle, slider, dropdown, radio) collapsed to different heights — making the list look uneven and cramped. ## Changes - **What**: `FormItem` label now uses `text-sm` (14px) and the row enforces `min-h-8` (32px) so toggle/slider/dropdown/radio rows align. `SettingGroup` bumps inter-item margin from `mb-2` to `mb-3` for breathing room between settings. ## Review Focus `FormItem` is also used by `ServerConfigPanel`, so the 14px/32px row also applies there — consistent with the same settings-dialog visual language, but worth a glance. Fixes #FE-525 ## Screenshots Lite Graph panel (1280×900 viewport) showing toggle/slider/dropdown/radio rows side-by-side: | Before (`origin/main`) | After (this PR) | | --- | --- | | <img src="https://raw.githubusercontent.com/Comfy-Org/ComfyUI_frontend/pr-12180-screenshots/before-litegraph.png" width="480"> | <img src="https://raw.githubusercontent.com/Comfy-Org/ComfyUI_frontend/pr-12180-screenshots/after-litegraph.png" width="480"> | Before: label text inherits 16px from `<body>`; toggle-only rows (e.g. "Always snap to grid", "Live selection") shrink to ~24px while dropdown/slider rows stay ~32px, so the list looks uneven and cramped. After: labels are 14px; every row is at least 32px tall so toggles/sliders/dropdowns/radios line up; `mb-3` adds 4px of breathing room between rows. ## References - Linear: https://linear.app/comfyorg/issue/FE-525/verify-settings-text-size-and-item-heights - Figma: https://www.figma.com/design/vALUV83vIdBzEsTJAhQgXq/Comfy-Design-System?node-id=6290-75412 - Origin thread: https://comfy-organization.slack.com/archives/C075ANWQ8KS/p1777657610484679?thread_ts=1776808927.654249 --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com> |
||
|
pythongosssss
|
681915275e |
fix: remove failed to export toast when cancelling export workflow (#12134)
## Summary An incorrect error toast currently shows when cancelling the workflow export from an asset ## Changes - **What**: - skip toast on cancel - add e2e & unit tests - refactor asset tab open helper to wait by default & cleanup usage ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12134-fix-remove-failed-to-export-toast-when-cancelling-export-workflow-35d6d73d3650815b839ff26edd70a472) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
e765eb1bb2 |
fix: suppress missing media scan during uploads (#12111)
## Summary - Prevent missing media detection from scanning media loader nodes while a drag/drop, paste, or file-select upload is still in progress. - Align LoadAudio with the existing media upload lifecycle by setting `node.isUploading`, blocking concurrent uploads, and clearing the flag after upload completion. - Keep added-node model and missing-node scans on the original one-microtask path, while deferring added-node media scanning by one extra microtask so upload handlers can mark transient upload state before the scan reads widget values. ## Why Drag/drop and paste can create media loader nodes before the backing upload has settled. During that short window, the widget may contain a local filename that is not yet backend-resolvable, so missing media detection can surface a false missing asset. Refreshing works because the upload has completed by then. ## Follow-up - E2E coverage for this upload race will be handled in a follow-up PR together with E2E coverage for the annotated output-media path changes from #12069. ## Validation - `pnpm format` - `pnpm lint` - `pnpm typecheck` - `pnpm vitest run src/composables/graph/useErrorClearingHooks.test.ts src/platform/missingMedia/missingMediaScan.test.ts src/extensions/core/uploadAudio.test.ts src/composables/node/useNodeImageUpload.test.ts` - Re-ran `pnpm typecheck` after rebasing onto latest `main` - Pre-push `knip` hook passed Fixes FE-620 ## Screenshots Before https://github.com/user-attachments/assets/db7891de-a4b5-4cde-aa76-6340e6cdf7b2 After https://github.com/user-attachments/assets/9b99bb13-0d5b-4ff7-8f52-66eea6e417ec ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12111-fix-suppress-missing-media-scan-during-uploads-35b6d73d365081f3b54eed02874ccaa4) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
25c2d828c0 |
test: enable vitest/consistent-each-for and migrate .each → .for (#12161)
*PR Created by the Glary-Bot Agent*
---
Enables the oxlint rule `vitest/consistent-each-for` (configured to
prefer `.for` for `test`, `it`, `describe`, and `suite`) and migrates
every `.each` parameterized test in the repo to `.for`. Using `.for`
avoids accidentally splatting tuple elements into separate callback
arguments and exposes `TestContext` as the second callback argument.
The first commit covers the 38 lint-detected files (88 callsites):
renames `.each` → `.for` and updates callback signatures to destructure
when the data is an array of tuples (objects/primitives already work
unchanged with `.for`).
The follow-up commit addresses code review feedback: oxlint's rule does
not recognize `test.each` on extended test bases
(`baseTest.extend(...)`) and skips files in `ignorePatterns`
(`src/extensions/core/*`). These were converted manually so the policy
is uniform across the codebase.
## Verification
- `node_modules/.bin/oxlint src` — 0 errors, 0 `consistent-each-for`
violations
- `pnpm typecheck` — passes
- `pnpm test:unit` — all modified test files pass; pre-existing
environmental flakes (`GraphView.test.ts`, `ColorWidget.test.ts`, etc.,
unchanged here and flaky on `main` in this sandbox) are unrelated
- `pnpm lint` / `pnpm knip` — clean
- Manual verification: 362 tests across 6 representative converted
suites re-run in an interactive shell — all passing
Manual UI verification (Playwright/screenshots) is not applicable:
changes are test-file-only refactors with no production runtime or UI
behavior change.
## Notes on `.for` semantics
- Array-of-tuples (`[[a, b], ...]`) passes the tuple as a single arg, so
callbacks were changed from `(a, b) => …` to `([a, b]) => …`.
- Array-of-objects (`[{a}, …]`) already used destructuring — unchanged.
- Array-of-primitives (`['a', …]`) — callback signature unchanged.
- A handful of complex cases use a small `type Case = [...]` alias plus
`it.for<Case>([...])` to preserve tuple inference where TS narrowed
unions otherwise broke parameter types.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12161-test-enable-vitest-consistent-each-for-and-migrate-each-for-35e6d73d3650810c9417e07bdd9f27a2)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
|
||
|
Dante
|
ceb9936058 |
fix(i18n): clamp unsupported browser locales to a shipped tag (#11712)
## Summary
Sidebar buttons rendered literal i18n keys (e.g.
`sideToolbar.labels.assets`) on a fresh install when the user's
`navigator.language` base tag wasn't one of the 12 shipped locales —
German/Italian/Polish/Dutch/Brazilian-Portuguese users among others.
## Changes
- **What**: Add `resolveSupportedLocale()` that tries the full BCP-47
tag first (preserves `zh-TW`, `pt-BR`), then the base tag, then `'en'`.
Wire through both entry points (`createI18n`'s initial locale,
`Comfy.Locale`'s `defaultValue`) and clamp inside `loadLocale`,
propagating the resolved tag to `GraphView` so a stale stored
`Comfy.Locale='de'` from older builds also recovers.
- **Side benefit**: Brazilian Portuguese users were previously falling
through `pt-BR` → `pt` (unshipped) → broken. The full-tag-first lookup
now correctly lands them on the `pt-BR` bundle.
- **Breaking**: None.
- **Dependencies**: None.
## Root Cause
Three-link chain:
1. `Comfy.Locale`'s default was `() => navigator.language.split('-')[0]
|| 'en'`. German → `'de'` (unshipped).
2. `loadLocale('de')` silently `console.warn`'d and returned without
throwing.
3. `GraphView` then ran `i18n.global.locale.value = 'de'` anyway.
4. `st(key, fallback) = te(key) ? t(key) : fallback`. vue-i18n's `te()`
checks **only** the current locale and ignores `fallbackLocale` — every
key missed → `st()` returned the literal key string.
Two pathways reached the broken state (defaultValue path, and
unset-setting path through `createI18n`'s own `navigator.language`
snapshot); the new helper closes both.
## Review Focus
- `loadLocale` now returns `SupportedLocale` (was `void`). Old `void`
callers continue to compile; the only change is `GraphView` consuming
the return value.
- Unit-tested in `src/i18n.test.ts` (added `resolveSupportedLocale`
block + updated the `loadLocale` unsupported-locale case from "warn" to
"clamp to en").
- Self-reproduced via Playwright with `navigator.language='de-DE'` +
fresh-install state on both `main` (shows the bug) and this branch
(shows the fix). Spec saved at
`temp/scripts/issue-10563-locale-bug.spec.ts`.
Fixes #10563
FE-480 — https://linear.app/comfyorg/issue/FE-480
## Screenshots
**Before** (from #10563, on `main`):
<img width="258" height="399" alt="Sidebar with literal i18n keys"
src="https://github.com/user-attachments/assets/098d1d76-8e89-4237-813f-5f030b34e51e"
/>
**After** (this branch, same `navigator.language='de-DE'`):
<img width="367" height="793" alt="Screenshot 2026-04-28 at 2 07 38 PM"
src="https://github.com/user-attachments/assets/9d279de3-50a8-4774-999f-ab4c3018a9ef"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11712-fix-i18n-clamp-unsupported-browser-locales-to-a-shipped-tag-3506d73d3650812f89d2f0fe3199de3a)
by [Unito](https://www.unito.io)
|
||
|
Deep Mehta
|
bb420fe2c7 |
feat: add model-to-node mappings for new model directories (#12151)
## Summary Add entries to `MODEL_NODE_MAPPINGS` so the model browser's "Use" button correctly creates a loader node for five model directories that currently have no mapping. ## Changes - **What**: 5 new entries in `src/platform/assets/mappings/modelNodeMappings.ts`: - `background_removal` → `LoadBackgroundRemovalModel` / `bg_removal_name` (ComfyUI v0.21+ core) - `frame_interpolation` → `FrameInterpolationModelLoader` / `model_name` (ComfyUI v0.21+ core) - `film` → `FILM VFI` / `ckpt_name` (ComfyUI-Frame-Interpolation) - `ultralytics/bbox` → `UltralyticsDetectorProvider` / `model_name` (ComfyUI-Impact-Pack) - `ultralytics/segm` → `UltralyticsDetectorProvider` / `model_name` (ComfyUI-Impact-Pack) - **Breaking**: none ## Review Focus - Node class names and input keys were cross-checked against the ComfyUI v0.21.0 source and the published Impact-Pack / Frame-Interpolation node definitions - Both `ultralytics/bbox` and `ultralytics/segm` map to the same node (`UltralyticsDetectorProvider`); its `model_name` combo accepts values from both subdirectories (`bbox/...` and `segm/...`) - `film` and `frame_interpolation` are separate directories serving different node packs — keeping them as distinct entries rather than collapsing under a parent ## Test plan - [ ] In the model browser, clicking "Use" on `birefnet.safetensors` creates a `LoadBackgroundRemovalModel` node with the model preselected - [ ] Same for one model in each of: `frame_interpolation`, `film`, `ultralytics/bbox`, `ultralytics/segm` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12151-feat-add-model-to-node-mappings-for-new-model-directories-35d6d73d365081ff834bf6eb610da160) by [Unito](https://www.unito.io) |
||
|
Dante
|
02e1ba2968 |
fix: Load Image preview retains deleted asset (FE-230) (#11493)
## Summary After deleting an asset, the Load Image node kept displaying the deleted thumbnail — both in the node body and in the picker dropdown (All / Imported / Generated tabs), even after a workflow reload. - Fixes FE-230 - Source: Slack https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1776715727656809 ## Root Cause Three distinct paths kept the deleted asset visible: 1. **Node-body preview cache** — `useMediaAssetActions.deleteAssets` never cleared `node.imgs` / `node.videoContainer` / the `nodeOutputStore` Vue ref, so the canvas renderer kept its cached frame. 2. **Live-delete dropdown gap** — the picker reads from `outputMediaAssets.media` (the asset list) and from `missingMediaStore.missingMediaCandidates` (verified-missing names). On live delete, neither was updated for the deleted asset, so the dropdown filter had nothing to drop. 3. **Synthetic "selected" placeholder** — `useWidgetSelectItems.missingValueItem` rebuilt any orphaned `modelValue` as a fake item with a `/api/view?filename=...` preview URL. Browsers had cached that URL pre-delete, so the deleted thumbnail still rendered with a blue checkmark even after the filter dropped the real asset entry. A subtler issue compounded #2/#3: candidate names stored in `missingMediaStore` are raw widget values (e.g. `sub/foo.png [output]`), but the dropdown computed comparison keys differently per source (asset list uses bare `asset.name`, widget option list uses bare filename). Names with a subfolder prefix slipped through the filter. ## Fix - **`clearNodePreviewCacheForFilenames`** (existing helper, refactored): exports `findNodesReferencingFilenames` + `extractFilenameFromWidgetValue`. Uses `nodeOutputStore.removeNodeOutputs` so the **reactive** Pinia ref updates, not just the legacy `app.nodeOutputs` mirror. Also clears `node.videoContainer` for Load Video. - **`markDeletedAssetsAsMissingMedia`** (new): on successful deletion, surfaces the affected widgets through `missingMediaStore` immediately so the dropdown filter has something to drop without waiting for verification. - **`useMissingMediaPreviewSync`** (new): watches `missingMediaStore` and clears `node.imgs` / `node.videoContainer` / Vue preview source for nodes referencing confirmed-missing media on workflow load — covers the post-reload case. - **`useWidgetSelectItems`**: normalizes both sides of the missing-media filter via `extractFilenameFromWidgetValue` (strips `[input|output|temp]` annotation + subfolder prefix), and suppresses `missingValueItem` when the value is in the missing-media store so the cached-thumbnail "selected" placeholder doesn't appear. ## Red-Green Verification | Commit | CI Status | Run | |--------|-----------|-----| | `test: FE-230 add failing test for Load Image preview cache clearing` | 🔴 Failure — test caught the bug | https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/24700188700 | | `fix: FE-230 clear Load Image preview cache when asset is deleted` | 🟢 Success | https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/24700265884 | ## Test Plan - [x] Unit coverage: 78 tests across 5 files (preview-cache helper, mark-deleted-as-missing, missing-media-preview-sync, widget-select-items missing-media filter incl. subfolder-prefix case, useMediaAssetActions integration) - [x] Live delete: Load Image node preview clears, dropdown drops the asset across All / Imported / Generated, no synthetic "selected" placeholder - [x] Post-reload: missing-media verification → `useMissingMediaPreviewSync` clears the preview, dropdown drops the asset - [x] Linear FE-230 auto-links via the Source line ## Scope note In-session and session-restore are both covered. If the backend/CDN continues serving the deleted `filename`/`asset_hash` after deletion, a cross-session reopen may still render stale bytes from cache — that's a backend/CDN concern tracked separately. ## demo ### before https://github.com/user-attachments/assets/e4d3a40e-0d46-43ad-985c-22ce7e0d3faf ### after https://github.com/user-attachments/assets/fcac9387-4c07-4be2-bcdd-d1a6192fe962 |
||
|
jaeone94
|
8f68be5699 |
fix: handle annotated output media paths in missing media scan (#12069)
## Summary
This PR fixes missing-media false positives for annotated media widget
values such as:
```txt
photo.png [output]
clip.mp4 [input]
147257c95a3e957e0deee73a077cfec89da2d906dd086ca70a2b0c897a9591d6e.png [output]
clip.mp4[input] // Cloud compact form
```
The change is intentionally scoped to the missing-media detection
pipeline for:
- `LoadImage`
- `LoadImageMask`
- `LoadVideo`
- `LoadAudio`
It preserves the raw widget value on `MissingMediaCandidate.name` for UI
display, grouping, replacement, and user-facing missing-media rows.
Normalized values are used only as comparison keys during verification.
## Diff Size
`main...HEAD` line diff is currently:
- Production/runtime code: `+478 / -37` (`515` changed lines)
- Unit test code: `+960 / -47` (`1,007` changed lines)
- Total: `+1,438 / -84` (`1,522` changed lines)
The PR looks large mostly because it locks both Cloud and OSS/Core
runtime paths with unit coverage; the production/runtime change is about
one third of the total diff.
## What Changed
- Added missing-media-scoped annotation helpers for detection-only path
normalization.
- Core/OSS recognizes spaced suffixes like `file.png [output]`.
- Cloud also recognizes compact suffixes like `file.png[output]`.
- User-selectable trailing `input` and `output` annotations are
normalized for matching.
- Unknown annotations and middle-of-filename annotations are left
unchanged.
- Added shared file-path helpers in `formatUtil`:
- `joinFilePath(subfolder, filename)`
- `getFilePathSeparatorVariants(filepath)`
- Updated media verification to compare candidates against both raw and
normalized match keys.
- Kept input candidates and generated output candidates in separate
identifier sets so an input asset cannot accidentally satisfy an output
reference with the same name.
- Moved missing-media source loading into `missingMediaAssetResolver` so
`missingMediaScan` remains focused on scan/verification orchestration.
- Updated Cloud generated-media verification to use the Cloud assets API
instead of job history:
- Cloud input candidates use input/public assets.
- Cloud output candidates use `output` tagged assets.
- Kept OSS/Core generated-media verification history-based, matching the
current generated-picker/widget availability model.
## Runtime Verification Paths
### Cloud
Cloud stores generated outputs as asset records. For an annotated output
value, this PR verifies against the `output` asset tag rather than job
history.
```txt
Widget value
"147257...d6e.png [output]"
|
v
Detection keys
"147257...d6e.png [output]"
"147257...d6e.png"
|
v
Cloud asset sources
input candidates -> /api/assets?include_tags=input&include_public=true
output candidates -> /api/assets?include_tags=output&include_public=true
|
v
Match against
asset.name
asset.asset_hash
subfolder/asset.name
subfolder/asset.asset_hash
slash and backslash separator variants
```
Example:
```ts
candidate.name = 'abc123.png [output]'
asset.name = 'ComfyUI_00001_.png'
asset.asset_hash = 'abc123.png'
asset.tags = ['output']
// Result: not missing
```
### OSS / Core
Core widget options for the normal loader nodes are input-folder based.
Annotated output values are resolved by Core through
`folder_paths.get_annotated_filepath()`, but the current generated
picker path is history-backed. This PR keeps OSS generated verification
aligned with that widget availability model instead of treating the full
output folder as the source of truth.
```txt
Widget value
"subfolder/photo.png [output]"
|
v
Detection keys
"subfolder/photo.png [output]"
"subfolder/photo.png"
|
v
OSS generated source
fetchHistoryPage(...)
|
v
History preview_output
filename: "photo.png"
subfolder: "subfolder"
|
v
Generated match keys
"subfolder/photo.png"
"subfolder\\photo.png"
```
This means OSS/Core verification is about whether the generated media is
currently available through the same generated/history-backed path the
widget uses, not a full disk-level executability check across the entire
output directory.
## Why Not Consolidate All Annotated Path Parsers
There are existing annotated-path parsers in image widget, Load3D, and
path creation code. This PR does not replace them.
The helper added here is detection-only: it strips annotations to build
comparison keys for missing-media verification. Parser consolidation
across widget implementations is intentionally left out of scope to keep
this fix narrow.
## Known Follow-Ups / Out Of Scope
- FE-620 tracks the separate video drag-and-drop upload race between
upload completion and missing-media detection.
- Published/shared workflow assets are still not fully represented by
`/api/assets?include_public=true`; that remains a backend/API contract
issue.
- A future backend/API contract that answers “is this workflow media
executable?” would be preferable to stitching together runtime-specific
FE sources.
- OSS/Core full output-folder scanning via `/internal/files/output` was
considered, but that endpoint is internal, shallow (`os.scandir`), and
not the same source currently used by the generated picker flow.
## Validation
- `pnpm test:unit -- missingMediaAssetResolver missingMediaScan
mediaPathDetectionUtil formatUtil`
- touched files `oxfmt`
- touched files `oxlint --fix`
- touched files `eslint --cache --fix --no-warn-ignored`
- `pnpm typecheck`
- pre-commit `pnpm knip --cache`
- pre-push `pnpm knip --cache`
`knip` passes with the existing tag hint:
```txt
Unused tag in src/scripts/metadata/flac.ts: getFromFlacBuffer → @knipIgnoreUnusedButUsedByCustomNodes
```
## Screenshots
Before
https://github.com/user-attachments/assets/50eab565-3160-4a57-a758-87ec2c09071e
After
https://github.com/user-attachments/assets/08adcbbd-c3fc-43f9-b86c-327e4eb5abd8
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12069-fix-handle-annotated-output-media-paths-in-missing-media-scan-3596d73d365081f4afa3d4dd45cad3da)
by [Unito](https://www.unito.io)
|
||
|
Henry Lee
|
ca54877f9d |
fix(assets): strip directory annotation from input filenames (#12086)
## Summary
Imported assets render as a generic check-check icon instead of a
thumbnail because the OSS `/internal/files/{type}` endpoint returns
annotated filenames (`photo.png [input]`) that the assets-sidebar mapper
passes through verbatim, which breaks extension-based media-type
detection.
## Changes
- **What**: Strip ComfyUI's trailing directory-type annotation (`
[input]`, ` [output]`, `[temp]`) in `mapInputFileToAssetItem` so `name`,
`id`, and the generated `/view?filename=…` URL all use the canonical
on-disk filename. Adds a focused unit test.
- **Breaking**: None.
- **Dependencies**: None.
## Review Focus
### Root cause
ComfyUI core PR
[comfyanonymous/ComfyUI#13078](https://github.com/comfyanonymous/ComfyUI/pull/13078)
(April 2026) changed `/internal/files/{type}` to append the directory
type to each entry:
```python
# api_server/routes/internal/internal_routes.py
return web.json_response(
[f"{entry.name} [{directory_type}]" for entry in sorted_files], status=200
)
```
The annotation is the wire format `LoadImage`-style widgets expect, so
the backend change is correct. The assets-sidebar mapper treated the
response strings as raw filenames. After
[#8914](https://github.com/Comfy-Org/ComfyUI_frontend/pull/8914) changed
`getMediaTypeFromFilename` to default unknown extensions to `'other'`,
every input asset now routes to `MediaOtherTop` and renders as
`icon-[lucide--check-check]`:
```
getMediaTypeFromFilename("photo.png [input]").split('.').pop() === "png [input]" → 'other'
```
The strip happens at data ingestion so every consumer of
`AssetItem.name` (sidebar grid, list, filter, gallery, drag-drop,
delete) gets the canonical filename automatically. OSS-only — Cloud
paths get clean names from the cloud API and are unaffected.
Reproduces locally on stock OSS ComfyUI on `main` of both repos; no
public issue tracker entry.
## Screenshots (if applicable)
Before:
<img width="1091" height="718" alt="image"
src="https://github.com/user-attachments/assets/ff1f070d-da39-4e5a-bc6d-99b7214f7da8"
/>
After:
<img width="1089" height="716" alt="image"
src="https://github.com/user-attachments/assets/7123d9bf-f7dd-4430-b6f7-f6702b70baaa"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12086-fix-assets-strip-directory-annotation-from-input-filenames-35a6d73d365081e9b9eed7d8630d6f0b)
by [Unito](https://www.unito.io)
|
||
|
Simon Pinfold
|
a4faaa0159 |
fix: disable ultralytics asset-browser registration (#12075)
*PR Created by the Glary-Bot Agent* --- ## Summary Disable the `UltralyticsDetectorProvider` model-to-node mapping so the node falls back to the static combo populated from `/api/object_info`, restoring pre-#8468 behavior on cloud. ## Why PR #8468 opted `UltralyticsDetectorProvider` into the cloud asset-widget path, which exposed a latent mismatch in cloud asset metadata for nested-directory model folders. The bug has two independent halves, and a fix that addresses only one will still leave the workflow broken at execution time: - **Tag lookup mismatch.** Cloud stores tags as combined values like `ultralytics/bbox`, while the asset query asks for split tags (`models` + `ultralytics`) with exact-match filtering — so the dropdown returns no results. - **Submitted value mismatch.** Cloud stores filenames as basenames, but the node expects subdirectory-prefixed values (e.g. `bbox/face_yolov8m.pt`) that the static combo path normally produces. Both halves require cloud-side fixes (asset ingestion + metadata) before the asset-browser registration can be safely re-enabled. Until then, removing the registration restores the working static-combo behavior so users are unblocked. ## Changes - `src/platform/assets/mappings/modelNodeMappings.ts`: comment out the `['ultralytics', 'UltralyticsDetectorProvider', 'model_name']` entry with a note pointing at BE-689 and the re-enablement criteria. - `src/stores/modelToNodeStore.test.ts`: drop the now-stale ultralytics expectations from `EXPECTED_DEFAULT_TYPES`, `MOCK_NODE_NAMES`, and the hierarchical-fallback `it.each` cases. ## Verification Local quality gates: - `pnpm typecheck` — clean - `pnpm lint` — clean (3 pre-existing warnings, 0 errors) - `pnpm format:check` — clean - `pnpm knip` — clean (1 pre-existing warning unrelated to this change) - `pnpm test:unit -- src/stores/modelToNodeStore.test.ts` — 51/51 passing Manual runtime verification (dev server + Playwright against the live module): - `MODEL_NODE_MAPPINGS` no longer contains any entry where `[0] === 'ultralytics'` or `[1] === 'UltralyticsDetectorProvider'` (84 entries total, 0 ultralytics). - `useModelToNodeStore().getNodeProvider('ultralytics')` returns `null` after `registerDefaults()`, so the asset-widget path is no longer triggered for this node. - `getNodeProvider('ultralytics/bbox')` also returns `null`, confirming hierarchical fallback no longer resolves to the disabled mapping. - `getNodeProvider('checkpoints')` still resolves to `CheckpointLoaderSimple`, confirming unrelated mappings are intact. End-to-end cloud verification (actually exercising the asset-browser path against cloud-seeded ultralytics metadata) is not possible in the local sandbox, since the regression depends on the cloud's asset ingestion data shape. The change is a single mapping-table removal that reverts to the well-exercised static-combo path covered by the updated unit tests. Long-term cloud-side fix is tracked in BE-689. - Fixes BE-689 ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12075-fix-disable-ultralytics-asset-browser-registration-35a6d73d36508179b394f0915e69742e) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Dante
|
0ef98de8eb |
fix: make credits help icon a tooltip button in cloud user popover (FE-617) (#12072)
## Summary The help icon (lucide circle-help) next to the credits balance in the cloud user popover was a bare `<i>` with `v-tooltip` and `cursor-help`. PrimeVue tooltip on a bare `<i>` did not fire reliably and the icon had no focus/keyboard semantics, so users saw "no hover action and not clickable". Wrap the icon in `<Button variant="muted-textonly" size="icon-sm">`, matching the existing pattern in `InfoButton.vue` and `MissingPackGroupRow.vue`. Same change applied to `CurrentUserPopoverLegacy.vue` and `CurrentUserPopoverWorkspace.vue`, which shared the broken pattern. - Fixes FE-617 - https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778191473621829 ## Red-Green CI Verification The branch was force-pushed back to the test-only commit so CI could run against it, then restored to the fix commit. | Commit | CI: Tests Unit | Outcome | |--------|---------------|---------| | `test:` ( |
||
|
jaeone94
|
c8c0e53865 |
fix: remove asset hash verification (#12061)
## Summary This PR removes the `/api/assets/hash/:hash` verification path from missing media/model detection. I decided to remove this path for two reasons: 1. The Cloud runtime implementation and the OpenAPI/generated FE contract do not agree on the hash format that this endpoint represents. In current Cloud data, the dominant asset_hash shape is <64-hex>.<extension> (for example, abc123....png), while the OpenAPI/generated FE contract expects a blake3:<hash> style value. That makes this path either dead code that should never be reached, or, when it is reached, a request that always returns 400 and only adds unnecessary noise. 2. Even if the format is reconciled, the Cloud implementation is a global deduplication-oriented lookup, not an access-aware check for whether the current workflow can use a resource. In theory, it can return success for another user's personal asset, so it is the wrong primitive for missing asset detection. Because of that, this PR makes the existing asset list/store based checks the primary verification path and removes the hash-specific helpers, service method, and tests. ## Known follow-ups These are known issues that are intentionally not solved in this PR: 1. Published assets are not exposed through `/api/assets?...include_public=true`. This is a backend issue and can still cause mismatch between missing-asset detection and resources that preview/run successfully. 2. Shared workflow import has an ordering issue. The API contract issue is being hotfixed separately under FE-603. 3. Annotated media paths can still be detected incorrectly. I will prepare follow-up PRs for these, starting with the annotated media path issue because that is the most critical frontend-side gap. ## Validation - `pnpm exec vitest run src/platform/assets/services/assetService.test.ts src/platform/missingMedia/missingMediaScan.test.ts src/platform/missingModel/missingModelScan.test.ts` - `pnpm lint:unstaged` - `pnpm typecheck` - `pnpm knip` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12061-fix-remove-asset-hash-verification-3596d73d365081a088f8dfc874724c1d) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
a7aa124c10 |
FE-407: fix(assets): show 3D thumbnail without reopening the panel (#11972)
## Summary After persistThumbnail uploads the preview, patch the in-memory asset by name (the cross-API stable id) so an open Asset panel reflects the new preview_url. Media3DTop also picks up the patched preview_url directly, bypassing the IntersectionObserver gate. ## Screenshots (if applicable) before https://github.com/user-attachments/assets/ba0b753f-fede-43c0-b790-5c19a82455f9 after https://github.com/user-attachments/assets/6273ec0b-0d2e-4355-9889-68c3550f1a72 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11972-FE-407-fix-assets-show-3D-thumbnail-without-reopening-the-panel-3576d73d365081febcbfe6ae84a4a68b) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
0658c1ac9c |
refactor: align asset pagination schema (#11899)
## Summary Align the asset list pagination schema with generated ingest-types metadata and remove the now-unneeded missing `has_more` fallback branch. ## Changes - **What**: Reuse `zListAssetsResponse` for `total` and `has_more`, keep the local loose `AssetItem` shape, and simplify `getAllAssetsByTag()` to trust the required `has_more` contract. - **Breaking**: None. - **Dependencies**: None. ## Review Focus This is PR 1 of 4 in the missing asset follow-up stack: 1. This PR - Asset schema / pagination cleanup 2. #11900 - Missing asset hash verification utility cleanup 3. #11901 - Browser regression coverage for public input assets 4. #11902 - TanStack Query public-input cache replacement The key decision is intentionally narrow: pagination metadata now comes from generated ingest-types, but asset item validation remains locally loose to avoid changing UI/store synthetic asset shapes in this PR. `asset_hash` nullability remains unchanged because absent-vs-null hash semantics are still a backend/API contract follow-up. Addresses #11894 ## Screenshots (if applicable) N/A |
||
|
Dante
|
0bc951fd12 |
fix: clarify unsaved-changes modal buttons and fix sign-out 3-state (#11669)
## Summary The dirtyClose modal had three buttons (`Cancel | No | Save`) and the sign-out flow collapsed two distinct outcomes (deny vs. dismiss) into a single early return — so today clicking "No" *cancels* sign-out instead of signing out without saving, and clicking "Save" never actually saves before logging out. This PR drops `Cancel` for `dirtyClose`, gives each caller a context-specific deny label, and fixes the sign-out 3-state handling. - Fixes [FE-419](https://linear.app/comfyorg/issue/FE-419/unsaved-changes-modal-uses-confusing-button-labels) ## Changes - **What**: - `ConfirmationDialogContent.vue`: hide `Cancel` for `type='dirtyClose'`; add `denyLabel?: string` prop; autofocus `Save` (preserves work on Enter). - `dialogService.confirm()`: accept and forward `denyLabel`. - `useAuthActions.logout`: handle `null` (cancel) / `false` (sign out anyway, no save) / `true` (save each modified workflow, then logout) distinctly. Pass `denyLabel: 'Sign out anyway'`. - `workflowService.closeWorkflow`: pass `denyLabel: 'Close anyway'`. - i18n: add `auth.signOut.signOutAnyway` and `sideToolbar.workflowTab.closeAnyway`. - **Breaking**: none. The `denyLabel` prop is optional and falls back to `g.no`. ## Review Focus - The "Save" branch in `useAuthActions.logout` now iterates `workflowStore.modifiedWorkflows` and awaits `useWorkflowService().saveWorkflow(workflow)` for each before calling `authStore.logout()`. The close-tab path (`workflowService.closeWorkflow`) was already correct — only the sign-out path needed the same shape. - `ConfirmationDialogContent` autofocus moves from `Cancel` (gone for `dirtyClose`) to `Save`. The dialog is still dismissable via ESC / outside-click, which routes through `dialogComponentProps.onClose → resolve(null)` — sign-out and close-tab both treat `null` as cancel. - Out of scope: the native browser `beforeunload` warning (`UnloadWindowConfirmDialog.vue`) is a separate flow and never reaches the in-app modal. ## Tests - Unit (`useAuthActions.test.ts`, new): logout handles `null` / `false` / `true` / no-modified-workflows; saves *every* modified workflow before `authStore.logout`; passes `denyLabel='Sign out anyway'`. - Unit (`ConfirmationDialogContent.test.ts`): Cancel hidden for `dirtyClose`; custom `denyLabel` rendered; falls back to `g.no` when omitted. - E2E (`workflowTabs.spec.ts`): modified-tab close shows `Close anyway` (not `No`) and no `Cancel`; clicking `Close anyway` removes the tab; ESC keeps the tab. ## screenshot ### AS IS <img width="816" height="379" alt="Screenshot 2026-04-27 at 5 40 19 PM" src="https://github.com/user-attachments/assets/a8e39403-bf72-455a-8d86-6ceb1f94ac85" /> <img width="923" height="396" alt="Screenshot 2026-04-27 at 5 40 38 PM" src="https://github.com/user-attachments/assets/08031c7c-b3a6-45d7-a4dc-5dcb4e63cfa0" /> ### TO BE <img width="1661" height="872" alt="Screenshot 2026-04-27 at 5 43 40 PM" src="https://github.com/user-attachments/assets/b89d160b-be66-450e-981e-32b1591f6841" /> <img width="1488" height="584" alt="Screenshot 2026-04-27 at 5 44 21 PM" src="https://github.com/user-attachments/assets/b3a141a7-1f3b-4f25-85a9-49529229c28b" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11669-fix-clarify-unsaved-changes-modal-buttons-and-fix-sign-out-3-state-34f6d73d365081bf8afad8e146b3b990) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
0446ca7a18 |
fix: route default topbar feedback button to Typeform (#11863)
*PR Created by the Glary-Bot Agent* --- ## Summary PR #10890 routed the legacy action bar feedback button and the Help Center feedback item to the nightly Typeform survey, but the **default topbar feedback button** in `WorkflowTabs.vue` still called `buildFeedbackUrl()` and opened Zendesk. Since `Comfy.UI.TabBarLayout` defaults to `Default` (not `Legacy`), most Cloud/Nightly users were clicking the WorkflowTabs button and never reaching the Typeform survey — explaining the lack of survey responses. ## Changes - Added a shared `buildFeedbackTypeformUrl(source)` helper in `platform/support/config.ts` that tags the survey URL with: - `distribution`: `ccloud` / `oss-nightly` / `oss` (preserves the build-tagging the old `buildFeedbackUrl()` sent to Zendesk so responses stay segmented) - `source`: `topbar` / `action-bar` / `help-center` (identifies which UI entry point launched the survey) Tags are passed via the URL fragment (Typeform's hidden-field convention), so they reach the survey but are never sent to the server in the request line. - `WorkflowTabs.vue`: replaced `buildFeedbackUrl()` with `buildFeedbackTypeformUrl('topbar')`. - `cloudFeedbackTopbarButton.ts` and `HelpCenterMenuContent.vue`: use the shared builder with their respective source labels instead of inline URL literals. - Removed the now-unused `buildFeedbackUrl()` and `ZENDESK_FEEDBACK_FORM_ID` (knip-clean). `buildSupportUrl()` is preserved — `Comfy.ContactSupport` (the Help Center "Help" item) still routes to Zendesk as before. - Added unit tests for the builder, the WorkflowTabs feedback button, the legacy action bar button, and the Help Center feedback item (covering both the Cloud/Nightly Typeform path and the OSS `Comfy.ContactSupport` fallback). ## Verification - `pnpm format`, `pnpm lint`, `pnpm typecheck`, `pnpm knip`: clean (one pre-existing unrelated lint warning in `useWorkspaceBilling.test.ts`) - `pnpm test:unit` (impacted scope): 506/506 passing, including 13 new tests ## Review Focus - Cloud/Nightly gating in `WorkflowTabs.vue` (`v-if="isCloud || isNightly"`) is unchanged and matches PR #10890's gating philosophy. - The Help Center "Help" item and `Comfy.ContactSupport` command intentionally still route to Zendesk — feedback ≠ support. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11863-fix-route-default-topbar-feedback-button-to-Typeform-3556d73d3650815fb446dac33095d4be) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
jaeone94
|
1ab9752af8 |
fix: keep Reka overlays above PrimeVue dialogs (#12038)
## Summary Temporarily patch FE-569 by keeping the affected portaled Reka dropdowns and menus above their containing PrimeVue dialogs when PrimeVue auto z-index state has been elevated. ## Changes - **What**: Added a small compatibility helper, `usePrimeVueOverlayChildStyle`, that returns an anchor ref plus a computed inline style for child popover content. The helper finds the nearest PrimeVue dialog mask (`.p-dialog-mask` / `.p-overlay-mask`) from the parent surface and, only when found, applies `parent z-index + 1` to the affected Reka overlay content. - **What**: Applied that helper at the exact PrimeVue parent surfaces where the issue was found. This PR does not add a global overlay policy and does not change every Reka select/dropdown in the app. - **What**: Added optional `contentStyle`/`selectContentStyle` plumbing only where needed so the style reaches the actual portaled Reka overlay root. - **What**: Added focused unit coverage for the helper contract: no PrimeVue parent preserves existing stacking, PrimeVue dialog/overlay masks render child content above the parent, low parent z-index values respect the Reka floor, and invalid z-index values do not inject an inline override. - **Approach**: This is intentionally a minimal, parent-scoped band-aid. It avoids a global PrimeVue overlay scanner because global sampling can be polluted by unrelated persistent PrimeVue roots such as Toast and would turn this fix into a broader layering policy. - **Approach**: The patch targets the confirmed failure mode: a Reka child overlay rendering below its owning PrimeVue dialog after PrimeVue autoZIndex has been elevated. It does not attempt to solve PrimeVue z-index globally. - **Lifecycle**: This is temporary migration compatibility. PrimeVue dialogs and controls are being incrementally migrated to Reka UI, so `usePrimeVueOverlayChildStyle` and the optional style props added for FE-569 should be removed once the affected parent surfaces move to Reka. - **Breaking**: None. New props are optional and no public API contract is changed. - **Dependencies**: None. ## Patched Entry Points This PR pinpoints the six affected user-facing surfaces below. Each patch is applied from the PrimeVue dialog parent and passed only to the Reka child overlay content that can render underneath that parent. https://github.com/user-attachments/assets/d0d1522a-ffc7-4934-9e7a-06b83e20f809 1. **Workflow Template Library filters** - **How to enter**: click the Templates button in the left sidebar, or open the Comfy menu and choose **Browse Templates**. - **Affected elements**: the template filter popovers in `WorkflowTemplateSelectorDialog`: **Model**, **Use case**, **Runs on**, and **Sort by**. - **Patch point**: `WorkflowTemplateSelectorDialog.vue` anchors to the template dialog content filter area and passes `selectContentStyle` to the affected `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/3641fa24-da51-4392-a904-9085f8a5a2f4 2. **Manager dialog header controls** - **How to enter**: open Manager from the top/menu Manager entry when the new Manager UI is available. - **Affected elements**: the Manager header controls in `ManagerDialog`: search mode `SingleSelect`, search autocomplete suggestions, and **Sort** `SingleSelect`. - **Patch point**: `ManagerDialog.vue` anchors to the dialog header and passes `selectContentStyle` to those three Reka overlays. https://github.com/user-attachments/assets/cf25cc06-f851-48ef-9d9c-9ec2da8afc06 3. **Asset Browser filter bar** - **How to enter**: open the Asset Browser from an eligible model widget browse action, the Model Library flow, or another `useAssetBrowserDialog` caller. - **Affected elements**: `AssetFilterBar` controls: **File formats**, **Base models**, **Ownership**, and **Sort by**. - **Patch point**: `AssetBrowserModal.vue` anchors to the PrimeVue dialog header and passes the style through `AssetFilterBar` to its `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/e27bd805-10c0-4b3b-97f3-9e11faa47021 4. **Asset Browser model info panel** - **How to enter**: open Asset Browser, select an asset, then use the right-side model info panel. - **Affected element**: the **Model type** select in `ModelInfoPanel`. - **Patch point**: `AssetBrowserModal.vue` reuses the same parent-scoped style and passes it to `ModelInfoPanel` as `selectContentStyle`. https://github.com/user-attachments/assets/5e9f7ef0-ebd7-4987-ba1b-2137c034086f 5. **Upload Model confirmation step** - **How to enter**: open Asset Browser, click **Upload**, enter/fetch model metadata, then proceed to the confirmation step. - **Affected element**: the **Model type** `SingleSelect` in `UploadModelConfirmation`. - **Patch point**: `UploadModelConfirmation.vue` anchors within the upload dialog content and passes `selectContentStyle` to the model type selector. https://github.com/user-attachments/assets/ec145f26-8621-455b-915e-bedee47e1cbd 6. **Settings > Keybinding panel controls** - **How to enter**: open Settings from the sidebar/menu, then select the **Keybinding** panel. - **Affected elements**: the keybinding preset select, the preset overflow dropdown menu, and the row context menu inside `KeybindingPanel`. - **Patch point**: `KeybindingPanel.vue` anchors to the settings dialog panel and passes `keybindingOverlayContentStyle` only to those Reka overlay roots. ## Review Focus - Confirm the patch stays narrowly scoped to the six known PrimeVue parent + Reka child overlay surfaces above. - Confirm `contentStyle` reaches the actual portaled Reka overlay content in each patched path. - Confirm the fallback behavior preserves existing stacking when no PrimeVue parent overlay is found; in that case the helper returns an empty style object and leaves existing Tailwind z-index classes alone. - Please avoid expanding this into a larger overlay refactor. The goal is a clean, backport-friendly compatibility patch while the Reka migration continues. Validation performed: - `pnpm exec vitest run src/composables/usePopoverSizing.test.ts` - `pnpm typecheck` - `pnpm lint` (passes with existing unrelated warnings only) - `pnpm format:check` - commit hook lint-staged checks (`oxfmt`, `stylelint`, `oxlint`, `eslint --fix`, `pnpm typecheck`) - pre-push `pnpm knip` Linear: FE-569 ## Bug Screenshots https://github.com/user-attachments/assets/e73761af-9867-4c50-ab0d-4e32e59011e1 https://github.com/user-attachments/assets/145daf4d-3268-428b-9987-1e1afd0b866f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12038-fix-keep-Reka-overlays-above-PrimeVue-dialogs-3596d73d365081e7af49dbc4d3905962) by [Unito](https://www.unito.io) |
||
|
Dante
|
e469611f6d |
perf: memoize asset display transform across filter tab switches (#11491)
## Root cause `useAssetBrowser`'s `filteredAssets` computed re-ran `.map(transformAssetForDisplay)` over the full result set on every tab switch. `transformAssetForDisplay` allocates fresh `badges`/`stats` objects, walks `tags`, calls `getAssetBaseModels`, and runs i18n date formatting per asset — none of which were memoized. Switching All / Inputs / Outputs forced N transforms per click and produced brand-new `AssetDisplayItem` references, which also defeated `:key`-based diffing in `AssetGrid` / `VirtualGrid` and re-rendered every visible card. ## Fix Memoize `transformAssetForDisplay` at module scope with a `WeakMap<AssetItem, AssetDisplayItem>`. Unchanged assets reuse the same display item across tab switches; the GC reclaims entries when assets are released. ## Before / after (n=200 assets, 6 tab switches: inputs → outputs → all → inputs → outputs → all) | Metric | Before | After | | ------------------------------- | -----: | ----: | | `transformAssetForDisplay` runs | 800 | 0 | | Wall time (Vitest harness) | 13.2 ms | 8.1 ms | | Reused `AssetDisplayItem` refs | 0 | 200 | Measured via `src/platform/assets/composables/useAssetBrowser.perf.test.ts` plus a temporary `process.stderr.write` harness. ## Red / green | Commit | Purpose | | --------- | ------- | | |
||
|
Christian Byrne
|
ad6cbf7cbe |
feat: align cloud batch count limit with server-side queue cap (#11876)
*PR Created by the Glary-Bot Agent* --- Raises `Comfy.QueueButton.BatchCountLimit` on cloud from `32` to `100` to match the server-side `MaxQueuedJobsPerUser` cap (`cloud/infrastructure/dynamicconfig/prod/config.json:3`). The desktop default was already `100` and is unchanged — collapsing both branches to the same constant. Addresses Discord feature request: [Increase queue batch limit from 200](https://discord.com/channels/1218270712402415686/1243609826299220039/1499104231381012641). ## Change ```diff - defaultValue: isCloud ? 32 : 100, + defaultValue: 100, ``` The setting is read dynamically by all batch count UIs (`BatchCountEdit.vue`, `LinearControls.vue`). ## Why 100 (not 512) Original ask was 200→512. Investigation showed: - The actual previous default was `100` (desktop) / `32` (cloud), not 200. - Cloud enforces `MaxQueuedJobsPerUser = 100` per workspace server-side. A higher frontend cap can't unlock more queued work — extra prompts just get rejected with `QUEUE_LIMIT`. - Frontend submits prompts as N sequential `POST /prompt` calls (no batched-prompt endpoint), so the UI cap is purely about how many clicks it takes — not throughput. - Going from 32 → 100 lets cloud users match the server cap in one click instead of 4. No new behavior is unlocked. ## Known limitation (pre-existing, not introduced here) The new max equals the absolute server cap, not the user's remaining capacity. A user with already-queued work can hit `QUEUE_LIMIT` mid-batch. The pre-existing 32 limit had the same shape (just at a smaller scale); deriving the UI max from `cap - outstanding` would require polling and reactive state and is out of scope for a one-line setting bump. ## Verification - `pnpm typecheck` — passes - `pnpm lint` — 0 errors (1 pre-existing warning in unrelated test file) - `pnpm test:unit` — `BatchCountEdit.test.ts` (3 tests) + `src/platform/settings/**` (70 tests) all pass - **Manual (Playwright)**: - `settingStore.get('Comfy.QueueButton.BatchCountLimit')` returns `100` at runtime - Typing `999` into the batch count widget clamps to `100` - Increment button is disabled at `100` (max reached) ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11876-feat-align-cloud-batch-count-limit-with-server-side-queue-cap-3566d73d3650819b8d01dbf83d1a8e49) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Benjamin Lu
|
270c7e34f4 |
fix: hide Google free-tier copy in webviews (#11924)
Stacked on #10699. - Hide Google-specific free-tier promo copy when embedded webviews block Google SSO. - Use GitHub-only fallback copy when returning from email signup in that state. - Remove the unused export from the Google SSO blocked-reason type so knip stays clean. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11924-fix-hide-Google-free-tier-copy-in-webviews-3566d73d36508168be7ed28cbe455d9f) by [Unito](https://www.unito.io) --------- Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
Christian Byrne
|
ea2e8e59f2 |
test: add MembersPanelContent unit tests (#11402)
## Summary Add 27 unit tests for MembersPanelContent component covering workspace views, member management, and billing states. ## Changes - **What**: New test file for MembersPanelContent with 27 tests across 8 describe blocks (personal workspace, owner view, member view, sorting, search filtering, pending invites, single seat plan, member count display) ## Review Focus - Uses `@testing-library/vue` + `@testing-library/user-event` per project conventions - Component stubs (Button, SearchInput, Menu, UserAvatar) for isolation - Reactive mock refs in `vi.hoisted()` shared across `vi.mock()` calls ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11402-test-add-MembersPanelContent-unit-tests-3476d73d36508107abcbce95b72b3fb7) by [Unito](https://www.unito.io) |
||
|
Dante
|
94b570a177 |
test: rename misnamed Mixpanel test and cover the actual provider class (#11749)
## Summary The existing \`MixpanelTelemetryProvider.test.ts\` was misnamed: it only tested \`getExecutionContext\` from \`../../utils/getExecutionContext\`, never the provider class itself — provider coverage sat at **0%** despite a 239-line test file living next to it. This PR: 1. **Renames** the existing test file to \`src/platform/telemetry/utils/getExecutionContext.test.ts\` (co-located with the source it actually tests). Updates its relative import to \`./getExecutionContext\`. 2. **Adds** a fresh \`src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.test.ts\` covering the provider class. Lifts provider coverage from **0% → 81.1%** lines (functions 73.5%, branches 88.5%). ## Test Coverage (new tests) Constructor / initialization: - Without \`mixpanel_token\`, warns and disables itself; subsequent \`trackXxx\` calls are no-ops. - With \`mixpanel_token\`, dynamically imports mixpanel-browser, calls \`init\`, and after \`loaded()\` fires identifies users via \`onUserResolved\`. Queueing semantics: - Events fired before \`loaded()\` are queued and flushed in order once Mixpanel reports ready. Filtering: - Events listed in the default \`disabledEvents\` set (e.g. \`workflow_opened\`) are suppressed. Direct dispatchers (parameterized \`it.each\`): - 16 \`trackXxx\` methods covered: signup/auth/login, subscription lifecycle, credit topup events, template lifecycle, workflow imported/saved, default-view, enter-linear, share-flow, execution success/error. - \`trackApiCreditTopupButtonPurchaseClicked\` payload includes \`credit_amount\`. - \`trackEmailVerification\` dispatches the matching \`USER_EMAIL_VERIFY_*\` event for each stage. - \`trackSubscription\` maps \`'modal_opened'\` and \`'subscribe_clicked'\` to their distinct events. - \`trackRunButton\` populates \`RunButtonProperties\` from the execution context. - \`trackWorkflowExecution\` consumes the latest \`trigger_source\` from \`trackRunButton\`, then resets it to \`'unknown'\`. Survey: - On \`'submitted'\`, normalized properties are written to \`Mixpanel.people\`. - On \`'opened'\`, \`Mixpanel.people\` is not touched. Topup delegation: - \`startTopupTracking\`, \`clearTopupTracking\`, \`checkForCompletedTopup\` all forward to the \`topupTracker\` utility. ## Testing \`\`\`bash pnpm vitest run src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.test.ts pnpm vitest run src/platform/telemetry/utils/getExecutionContext.test.ts \`\`\` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11749-test-rename-misnamed-Mixpanel-test-and-cover-the-actual-provider-class-3516d73d365081609c54f34bd2d8b00d) by [Unito](https://www.unito.io) --------- Co-authored-by: Christian Byrne <cbyrne@comfy.org> |
||
|
Christian Byrne
|
b83602fd23 |
feat: hide Google SSO button in embedded webviews (#10699)
Hide the Google SSO login/signup button when the app runs inside an embedded webview (Android WebView, iOS WKWebView, social app in-app browsers), where Google blocks OAuth with a `403 disallowed_useragent` error. Fixes #7017 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10699-feat-hide-Google-SSO-button-in-embedded-webviews-3326d73d365081048e35d9d678fe1a2f) by [Unito](https://www.unito.io) --------- Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com> |
||
|
Christian Byrne
|
341fef46a9 |
refactor: replace unsafe as Error assertions with type guards (#11845)
## Summary Replaces all 7 production `as Error` type assertions with proper `instanceof Error` narrowing or a new `toError()` helper, and adds an ESLint rule to prevent new ones. First slice of #11429 (the `as Error` category — 9 total occurrences, 7 production + 2 in a test file left untouched). ## Changes - **What**: - New `src/utils/errorUtil.ts` exporting `toError(value: unknown): Error` and `getErrorMessage(value: unknown): string | undefined`. `toError` returns the value unchanged if already an `Error`, otherwise wraps it (handles strings, `undefined`, JSON-serializable objects, and circular refs via `String()` fallback). - Refactored 7 production call sites: - `src/services/gateway/registrySearchGateway.ts` — `toError(error)` for `lastError` assignment in fallback loop - `src/platform/cloud/onboarding/auth.ts` (×2) — `toError(error)` for `captureApiError` Sentry calls - `src/renderer/extensions/vueNodes/widgets/composables/audio/useAudioRecorder.ts` — `toError(err)` before forwarding to `options.onError` - `src/extensions/core/load3d/LoaderManager.ts` — replaced `error as Error & { response?: ... }` cast inside `isNotFoundError` with `'response' in error` + nested narrowing - `apps/desktop-ui/src/stores/maintenanceTaskStore.ts` — inline `error instanceof Error ? error.message : String(error)` - `apps/desktop-ui/src/components/maintenance/TaskListPanel.vue` — inline `error instanceof Error ? error.message : undefined` - New ESLint rule (`no-restricted-syntax` block named `comfy/no-unsafe-error-assertion`) banning `TSAsExpression TSTypeReference[typeName.name='Error']` in `src/**` and `apps/*/src/**`, with test files (`*.test.ts`, `*.spec.ts`) excluded. - 12 unit tests for the new helpers in `src/utils/errorUtil.test.ts`. - **Breaking**: none - **Dependencies**: none ## Review Focus - The lint rule is scoped to non-test source files. Test files retain freedom to use `as Error` for fixture construction; only 2 occurrences exist (in `teamWorkspaceStore.test.ts` and `errorDialog.spec.ts`) and they're intentional. - `toError` is duplicated as inline `instanceof` narrowing in `apps/desktop-ui/` rather than imported, since the desktop-ui workspace doesn't share `@/utils/` with the main app and adding a path mapping for one helper felt heavier than two inline guards. - Remaining `as`-on-DOM categories (HTMLElement ×133, HTMLInputElement ×55, HTMLCanvasElement ×36, KeyboardEvent ×7, Element ×3, MouseEvent ×2, Event ×2) are intentionally left for follow-up PRs to keep this one reviewable. Refs #11429 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11845-refactor-replace-unsafe-as-Error-assertions-with-type-guards-3546d73d36508137a015c4f9e8708f23) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
285421a87c |
feat: add queue progress overlay feature survey (#11560)
*PR Created by the Glary-Bot Agent* --- ## Summary Registers a new nightly feature survey for the Queue Progress Overlay using the existing feature-survey registry (same pattern as the merged node-search survey, PRs #8175/#8355/#9934). - New registry entry `queue-progress-overlay` → Typeform `HZ5saxry`, threshold **16**, 5s display delay. - `trackFeatureUsed()` wired at the major user-initiated handlers inside the overlay so the survey triggers regardless of panel location (floating-right v1 or docked-left v2). - Run button and other ActionBar items that the overlay pops over from are deliberately **not** tracked — tracking is scoped to interactions that originate inside the job panel / queue progress overlay itself. ## Tracked interactions Both variants share most sub-components, so tracking is instrumented once at each logical surface: - **`QueueProgressOverlay.vue`** (v1 container): `viewAllJobs`, `interruptAll`, `cancelQueuedWorkflows`, `onClearHistoryFromMenu`, `toggleAssetsSidebar`, `onCancelItem`, `onDeleteItem`, `inspectJobAsset` - **`QueueOverlayExpanded.vue`**: job tab switches - **`JobHistorySidebarTab.vue`** (v2 docked): job tab switches, `clearQueuedWorkflows`, `onClearHistory`, `onCancelItem`, `onDeleteItem`, `onViewItem` - **`JobFilterActions.vue`** (shared): workflow filter + sort mode selections - **`JobHistoryActionsMenu.vue`** (shared): docked-history toggle + run-progress-bar toggle Deliberately **not tracked** to keep the signal clean: - Hover handlers (ambient preview behaviour) - Search-box keystrokes (debounced typing) - Context menu open and menu-item dispatch — menu actions either bubble through already-tracked terminal handlers (e.g. inspect-asset → `onViewItem`) or are secondary operations (copy-id, open-workflow, download). Avoids double-counting per code review feedback. ## How it works (inherits from existing infrastructure) 1. `surveyRegistry.ts` drives `NightlySurveyController` → `NightlySurveyPopover`, which handles the Typeform embed. 2. Eligibility already gated on `isNightly && !isCloud && !isDesktop`, once-per-user, 4-day global cooldown across all surveys, and opt-out. 3. Typeform response routing to #C0ALLT6Q3SQ is handled on the Typeform side. ## Verification - `pnpm typecheck` ✅ - `pnpm lint` ✅ (no new warnings) - `pnpm knip` ✅ - `pnpm test:unit` on `src/components/queue`, `src/components/sidebar/tabs/JobHistorySidebarTab`, `src/platform/surveys` → **123/123 passing** - Pre-commit hooks (stylelint, oxfmt, oxlint, eslint, typecheck) all pass - Manual: dev server + backend boot cleanly, app loads without new runtime errors, `localStorage['Comfy.FeatureUsage']` layout verified to match what `useFeatureUsageTracker` writes ## Notes - Survey key `queue-progress-overlay` covers both v1 (floating-right) and v2 (docked-sidebar) per product guidance: _"This should trigger regardless of the location of the panel (docked from left or floating on right)."_ Both surfaces are the same product feature — the survey is intentionally scoped to the whole job-panel experience. ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11560-feat-add-queue-progress-overlay-feature-survey-34b6d73d3650819a9a50fd67fd9b5941) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
jaeone94
|
04918360eb |
Use hash lookup for missing asset detection (#11873)
## Summary
Use exact BLAKE3 hash lookups first for missing model/media detection,
and add a separate public-inclusive input asset cache so public input
assets are considered missing-detection candidates without changing the
user-only input assets shown in the UI.
## Changes
- **What**:
- Added `assetService.checkAssetHash()` for `HEAD
/api/assets/hash/{hash}` status-only existence checks.
- Added strict BLAKE3 hash helpers so only `blake3:<64 hex>` media
values and raw 64-hex BLAKE3 model metadata are sent to the hash
endpoint.
- Updated missing media detection to group BLAKE3 candidates by hash,
resolve them through the hash endpoint, and fall back to the legacy
asset list path for invalid/unverifiable/non-hash values.
- Updated missing model detection to use hash lookup for BLAKE3-backed
asset-supported candidates before falling back to the existing node-type
asset matching path.
- Added `assetService.getInputAssetsIncludingPublic()` backed by a
dedicated cache that fetches input assets with `include_public=true` for
missing media fallback checks.
- Kept `assetsStore.inputAssets` user-only for widget/UI display, while
invalidating the public-inclusive missing-detection cache when input
assets may change.
- Added abort handling for paginated asset fetches and shared
public-input cache callers so one aborted caller does not cancel the
shared fetch for other callers.
- Added regression coverage for hash lookup, fallback behavior, abort
paths, public input fallback detection, and cache invalidation.
- **Dependencies**: None.
- **Change size**:
- Production code: 4 files, 400 insertions, 24 deletions, net +376.
- Test code: 4 files, 806 insertions, 59 deletions, net +747.
- Total: 8 files, 1206 insertions, 83 deletions, net +1123.
## Review Focus
- The public-inclusive input asset cache is intentionally separate from
`assetsStore.inputAssets`. The existing store data is user-only and
drives the asset widgets/sidebar, so using it for missing input
detection misses public assets. Making that store public-inclusive would
change UI data semantics; this PR instead keeps the UI dataset unchanged
and adds a missing-detection-specific cache in `assetService`.
- Hash lookup is only used when the workflow exposes a valid BLAKE3
hash. Filename-like values and invalid hash values still use the legacy
fallback path.
- Missing model detection keeps the existing fallback behavior for
non-hash candidates and for hash checks that are invalid or fail
transiently.
- Async model download cache refresh behavior is left unchanged; this PR
avoids coupling model download completion to input asset cache
invalidation.
- No browser/e2e test was added because this changes the missing asset
detection data path, not UI interaction or rendering. The behavioral
coverage is in unit tests for the asset service and the missing
media/model scanners.
## Follow-up Items
- Fix `assetsStore.updateAssetTags()` partial-failure recovery. If
`removeAssetTags()` succeeds and `addAssetTags()` fails, the local model
asset cache can roll back to tags that the backend has already removed;
this should be handled in a focused model asset cache PR.
- Consider extracting shared hash-verification flow used by missing
media and missing model scans after this behavior stabilizes.
- Consider adding a concurrency cap or short-lived request cache for
large workflows with many unique hash lookups.
- Consider splitting `assetService.ts` further, e.g. hash helpers, abort
utilities, and the public-inclusive input asset cache.
- Consider tightening the asset hash service API shape so callers do not
directly depend on HTTP-oriented statuses such as `invalid`.
- Consider adding broader mutation-path coverage for public-inclusive
input cache invalidation once the cache has more consumers.
Linear: FE-534
## Screenshots (if applicable)
Before <false positive / missing image / public asset>
https://github.com/user-attachments/assets/db7ce2a9-b169-4fae-bf9f-98bb93d3ee6d
After
https://github.com/user-attachments/assets/29af9f9e-b536-4fcd-a426-3add40bcb165
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11873-Use-hash-lookup-for-missing-asset-detection-3556d73d36508165babafb16614be0d8)
by [Unito](https://www.unito.io)
|
||
|
Christian Byrne
|
5cad2c952b |
refactor+test: extract useSubscriptionCheckout composable, rewrite tests (#11396)
## Summary Adds 20 component tests for `SubscriptionRequiredDialogContentWorkspace.vue` covering: - **Initial rendering**: pricing table display, close/back button visibility, out_of_credits reason message - **Close button**: calls onClose callback - **Subscribe click flow**: pricing→preview transitions (new subscription & upgrade), error toasts for disallowed/missing/failed previews, monthly billing cycle - **Back button**: returns from preview to pricing step - **Add credit card**: handles subscribed status (success toast + close), needs_payment_method (opens Stripe URL), error state - **Confirm transition**: success path with close emit, error toast on failure - **Resubscribe**: success path with toast + close, error toast on failure ## Testing ```bash pnpm test:unit -- src/platform/workspace/components/SubscriptionRequiredDialogContentWorkspace.test.ts ``` All 20 tests pass. Quality gates (typecheck, lint, format, knip) pass. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11396-test-add-component-tests-for-SubscriptionRequiredDialogContentWorkspace-3476d73d36508156a218dcb67a2a334e) 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> |
||
|
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> |
||
|
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> |
||
|
Dante
|
9df4e02189 |
refactor: unify media asset downloads (#11717)
## Summary Unifies media asset download actions behind a single `downloadAssets(assets?)` API to avoid single and multi asset download path drift. ## Changes - **What**: Replaces `downloadAsset` and `downloadMultipleAssets` with `downloadAssets`, preserving no-arg media context fallback and explicit asset arrays. - **Dependencies**: None. ## Review Focus Download behavior for single-card, context-menu, and sidebar bulk actions should continue to use the same ZIP-export path for cloud multi-output jobs. Fixes #11715 ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11717-refactor-unify-media-asset-downloads-3506d73d3650810d8bcec9c0194e743d) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
AustinMroz
|
1c541d8577 |
Short circuit asset reuploads, simplify node dnd (#11691)
When an output is dragged from the assets panel onto a node, outputs were being reuploaded. This logic has been simplified to instead reference the existing asset by resolving the annotated path. As part of this change, async drop handlers on nodes are also fixed. Rather than placing obligation of event handling on client code, not respecting async handlers, or completely ignoring return types, the vue drop handler will now simply set `app.dragOverNode` and allow the `document` drop handler to resolve node drag/drop operations without any of the difficulty from propagation. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11691-Short-circuit-asset-reuploads-simplify-node-dnd-34f6d73d36508157af86e6cf09229781) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
517da289f6 |
feat: Search - add ghost node following setting and increase opacity (#11365)
## Summary Adds setting to disable the node auto-follow cursor behavior when adding nodes from the search, and increased the visibilty of Vue ghost nodes. ## Changes - **What**: - add setting - increase opacity - add test ## 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) Before <img width="452" height="517" alt="image" src="https://github.com/user-attachments/assets/369c0d90-5352-482b-a1b3-36180bffb3ee" /> After <img width="440" height="536" alt="image" src="https://github.com/user-attachments/assets/2066fdd4-6eb4-4bfb-ac7c-559fc99de57d" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11365-feat-Search-add-ghost-node-following-setting-and-increase-opacity-3466d73d3650811b9c27ed4cc930816d) by [Unito](https://www.unito.io) |
||
|
Dante
|
69e68847d9 |
test: add unit tests for workspaceAuthStore retry/race paths (#11674)
## Summary
Extends `useWorkspaceAuth.test.ts` with the retry, race,
storage-resilience, and Zod-validation paths the existing suite did not
exercise: exponential backoff on `TOKEN_EXCHANGE_FAILED`, immediate
context clearing on permanent errors, stale-refresh abort when the user
switches workspaces mid-flight, and resilience to `sessionStorage` quota
errors.
## Changes
- **What**: Adds 6 Vitest cases across three new `describe` blocks
(`refreshToken retry/race paths`, `persistToSession resilience`, `Zod
validation on token response`). Reuses the existing `mockGetIdToken`,
`mockTeamWorkspacesEnabled`, `vi.useFakeTimers()`, and
`mockTokenResponse` fixtures.
## Review Focus
- The retry test uses `vi.runAllTimersAsync()` so the three backoff
sleeps (1s + 2s + 4s) drain in a single tick instead of slowing the
suite. Both `console.warn` (per-attempt) and `console.error`
(final-failure) are silenced.
- The race test resolves the in-flight refresh fetch with a token tied
to the OLD workspace AFTER `switchWorkspace('workspace-other')` has run,
so the assertion fails loudly if the stale-request guard regresses.
- The sessionStorage spy targets the instance method
(`vi.spyOn(sessionStorage, 'setItem')`); spying
`Storage.prototype.setItem` does not intercept happy-dom's per-instance
method.
## Testing
\`\`\`bash
pnpm exec vitest run
src/platform/workspace/stores/useWorkspaceAuth.test.ts
pnpm format -- src/platform/workspace/stores/useWorkspaceAuth.test.ts
pnpm lint
pnpm typecheck
pnpm knip
\`\`\`
33 tests pass (27 prior + 6 new).
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11674-test-add-unit-tests-for-workspaceAuthStore-retry-race-paths-34f6d73d365081e6a8ecce59a156585e)
by [Unito](https://www.unito.io)
|
||
|
Simon Pinfold
|
c4043637d6 |
fix: route context menu Download through downloadMultipleAssets (#11700)
*PR Created by the Glary-Bot Agent* --- ## Summary The asset context menu's Download action called `downloadAsset` directly, so multi-output jobs only downloaded the preview file instead of all outputs. Route through `downloadMultipleAssets`, which detects multi-output jobs and creates a ZIP export in cloud mode and falls back to the single-download path otherwise. ## Changes - **What**: Swap `actions.downloadAsset(asset)` for `actions.downloadMultipleAssets([asset])` in the per-asset context menu Download command, and extend the existing unit test to assert the routing. - **Breaking**: none - **Dependencies**: none ## Review Focus For single-output assets the behavior is unchanged: `downloadMultipleAssets([asset])` falls through to the individual-download path when `hasMultiOutputJobs` is false and `assets.length === 1` (see `useMediaAssetActions.ts:106`). Verified manually — right-clicking a single-output asset and clicking Download still produces one file download to the correct `/api/view` URL. ## Notes This is a focused replacement for the stale #10948. Compared to that branch: - Drops the unrelated `bootstrapStore` API-key auth changes (scope creep). - Drops the new `assets.cloud.spec.ts` Playwright spec — cloud asset-export E2E coverage was added in #11610 (`browser_tests/tests/sidebar/assets.spec.ts`), so a separate cloud spec for this routing change would mostly duplicate it. - Keeps the unit-test change minimal: extends the existing `ContextMenu` stub with a `model` prop watcher and adds one new test, rather than rewriting the whole file from `@testing-library/vue` to `@vue/test-utils`. ## Verification - `pnpm test:unit` (MediaAssetContextMenu.test.ts and useMediaAssetActions.test.ts) - `pnpm typecheck` - `pnpm lint` - `pnpm format` / `oxfmt --check` - `pnpm knip` - Manual: started the OSS dev server, generated a single-output asset via the queue API, opened the assets sidebar, right-clicked the asset, and confirmed the Download menu item triggers a single-file download (screenshot attached). ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11700-fix-route-context-menu-Download-through-downloadMultipleAssets-34f6d73d365081eb8135e8b699640d97) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Alexander Brown
|
9d61b4df06 |
feat: ECS Phase 0b — ID type aliases (FE-166/475/476/477) (#11699)
## Summary
ECS Phase 0b — type-only ID aliases. Builds on FE-165 (centralized
version counter, base of this PR) and adds a tranche of named ID aliases
plus mechanical adoption at known call sites.
This PR now covers four child tickets:
- **FE-166** — adds `GroupId` (in `LGraphGroup.ts`) and `SlotIndex` (in
`interfaces.ts`); re-exported from the litegraph barrel.
- **FE-475** — mechanical adoption of existing aliases (`NodeId`,
`LinkId`, `RerouteId`, `GroupId`, `SlotIndex`, `ExecutionId`) across
litegraph at the audit-listed sites:
`LGraphState.lastGroupId/lastLinkId/lastRerouteId`,
`LGraphExtra.linkExtensions`, `ISerialisedGroup.id`,
`ISerialisedGraph.last_link_id`, `LinkNetwork.removeReroute`,
`INodeOutputSlot.slot_index`, `LGraphNode.{setOutputDataType,
getInputDataType, getOutputPos}` slot params,
`ExecutableNodeDTO.inputs[].linkId` + execution-id locals, and
`RenderLink/MovingLinkBase.fromSlotIndex` (plus subclasses that
redeclare).
- **FE-476** — adds `SubgraphId = UUID` in `LGraph.ts`; adopted at
`_subgraphs` Map, `findUsedSubgraphIds`, `getDirectSubgraphIds`, and
`ExportedSubgraphInstance.type`. Re-exported from the litegraph barrel.
- **FE-477** — adds app-domain entity aliases at their closest
schema/types files: `WorkflowId`, `AssetId`, `PromptId` (propagated as
existing `JobId`), `TaskId`, `UserId`, `WorkspaceId`,
`WorkspaceInviteId`, `NodePackId`. Adopted at primary use sites (entity
id fields, store state, service signatures).
## Entity reference
### ID aliases at a glance
| Alias | Underlying | Defined in | Identifies |
| --- | --- | --- | --- |
| `NodeId` | `number \| string` | `litegraph/LGraphNode.ts` | A node
within a graph |
| `LinkId` | `number` | `litegraph/LLink.ts` | A connection between two
slots |
| `RerouteId` | `number` | `litegraph/Reroute.ts` | A reroute waypoint
on a link |
| `GroupId` *(new)* | `number` | `litegraph/LGraphGroup.ts` | A visual
group of nodes |
| `SlotIndex` *(new)* | `number` | `litegraph/interfaces.ts` | A slot's
position on a node |
| `ExecutionId` | `string` | `litegraph/types/serialisation.ts` | A node
within a subgraph instance |
| `SubgraphId` *(new)* | `UUID` | `litegraph/LGraph.ts` | A subgraph
definition |
| `WorkflowId` *(new)* | `string` |
`platform/workflow/validation/schemas/workflowSchema.ts` | A saved
workflow document |
| `AssetId` *(new)* | `string` |
`platform/assets/schemas/assetSchema.ts` | A binary asset (model, image,
etc.) |
| `JobId` *(reused as `PromptId`)* | `string` | `schemas/apiSchema.ts` |
A queued prompt execution |
| `TaskId` *(new)* | `string` | `platform/tasks/services/taskService.ts`
| A backend background task |
| `UserId` *(new)* | `string` | `types/authTypes.ts` | An authenticated
user |
| `WorkspaceId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A workspace |
| `WorkspaceInviteId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A pending workspace invite |
| `NodePackId` *(new)* | `string` |
`workbench/extensions/manager/types/comfyManagerTypes.ts` | A Comfy
Registry / Manager node pack |
### How the entities relate
```mermaid
flowchart TB
subgraph LG["🎨 Litegraph entities"]
direction TB
Graph["LGraph<br/>id: SubgraphId"]
Subgraph["Subgraph<br/>id: SubgraphId"]
Node["LGraphNode<br/>id: NodeId"]
Link["LLink<br/>id: LinkId"]
Reroute["Reroute<br/>id: RerouteId"]
Group["LGraphGroup<br/>id: GroupId"]
Slot["INodeSlot<br/>slot_index: SlotIndex"]
Exec["ExecutableNodeDTO<br/>id: ExecutionId"]
end
subgraph AP["🌐 App-domain entities"]
direction TB
Workflow["ComfyWorkflow<br/>id: WorkflowId"]
Job["Prompt / Job<br/>id: JobId ≡ PromptId"]
Task["Task<br/>id: TaskId"]
Asset["Asset<br/>id: AssetId"]
Workspace["Workspace<br/>id: WorkspaceId"]
Invite["WorkspaceInvite<br/>id: WorkspaceInviteId"]
User["User<br/>id: UserId"]
Pack["NodePack<br/>id: NodePackId"]
end
Subgraph -. extends .-> Graph
Graph --> Node
Graph --> Link
Graph --> Group
Node --> Slot
Link --> Reroute
Link --> Slot
Node -. instantiates .-> Subgraph
Exec -. wraps .-> Node
Workflow -. serializes .-> Graph
Job --> Workflow
Job --> Task
Task --> Asset
Workspace --> Workflow
User --> Workspace
Workspace --> Invite
Pack -. provides .-> Node
```
Solid arrows are containment / direct references; dashed arrows are *“is
a kind of”* (`extends`) or cross-layer relationships (e.g. a
`ComfyWorkflow` *serializes* an `LGraph`; a `NodePack` *provides* node
definitions).
## Explicit non-goals
- `LGraphState.lastNodeId` is intentionally kept as bare `number`
(auto-increment counter; would widen if aliased to `NodeId = number |
string`).
- No new `SubgraphSlotId` alias — verified subsidiary (subgraph IO slots
are addressed via `SUBGRAPH_INPUT_ID/OUTPUT_ID` sentinel + numeric array
index, not by UUID alone).
- No `WidgetName`, `SlotName`, `WorkspaceMemberId` — verified subsidiary
(only meaningful inside a parent or as a relationship).
- No re-typing of `LGraph.id` / `Subgraph.id` — references adopt
`SubgraphId`, but the inherited UUID typing is left intact (minimal
diff).
## Type-only
All changes are structural-equivalent type aliases. No runtime behavior
changes. No new exports beyond the aliases themselves. No generated code
modified.
## Verification
- `pnpm typecheck` ✅
- `pnpm knip` ✅
- Scoped `npx eslint` on changed files ✅
- Lint-staged hooks (oxfmt, oxlint, eslint, typecheck) passed on every
commit
## Notes for reviewers
This branch was rebased onto `main` after FE-165 (`a441364a5`, PR
#11698) merged independently — the auto-skipped FE-165 commit is no
longer part of this PR. Six commits remain (oldest → newest):
| Commit | Maps to | Summary |
| --- | --- | --- |
| `e8e7ff795` | FE-166 | Add `GroupId` and `SlotIndex` aliases + barrel
re-exports |
| `e0bcb75a0` | FE-476 | Add `SubgraphId = UUID` alias |
| `2c136afb9` | FE-477 + FE-475 bulk | Add app-domain aliases;
mechanical adoption of
`NodeId`/`LinkId`/`RerouteId`/`GroupId`/`SlotIndex`/`ExecutionId`/`SubgraphId`
at audit-listed litegraph sites |
| `06d6e6a8b` | FE-477 | Adopt `TaskId` in asset stores |
| `f943e1c2b` | FE-476 | Adopt `SubgraphId` at remaining UUID reference
sites (`LGraphCanvas` clipboard map + paste, `SubgraphNode.type`) |
| `1739d5241` | review feedback | Tighten alias usage:
`linkExtensions.parentId: RerouteId`, drop redundant `String()` wraps in
`executionStore`, type `assetExportStore` map as `Map<TaskId,
AssetExport>` |
FE-475's mechanical adoption is bundled into `2c136afb9` rather than a
dedicated commit (parallel-agent execution on a shared working tree);
the substitutions themselves are complete — see the diff under
`src/lib/litegraph/src/`. PR will be squash-merged, so commit
granularity is informational.
Fixes FE-166
Fixes FE-475
Fixes FE-476
Fixes FE-477
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Dante
|
00974d6339 |
test: add E2E tests for publish flow wizard (#10770)
## Summary - Add Playwright E2E test coverage for the ComfyHub publish workflow dialog - Create `PublishDialog` page object fixture with programmatic dialog opening via Vite dynamic imports - Create `PublishApiHelper` for mocking all publish flow API endpoints (`/hub/profiles/me`, `/hub/labels`, `/hub/workflows`, `/userdata/*/publish`, `/assets/from-workflow`, `/hub/assets/upload-url`) - Add `data-testid` attributes to 6 publish flow components for stable E2E locators - 17 test scenarios across 7 describe blocks covering wizard navigation, form interactions, profile gate, save prompt, and publish submission ## Test plan - [ ] Run `pnpm test:browser:local -- --grep "Publish dialog"` against local dev server - [ ] Verify wizard navigation through Describe → Examples → Finish steps - [ ] Verify profile gate flow (with/without profile) - [ ] Verify save prompt for unsaved workflows - [ ] Verify publish success/failure scenarios Fixes #9079 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10770-test-add-E2E-tests-for-publish-flow-wizard-3346d73d3650818094d5fc3a84593402) by [Unito](https://www.unito.io) --------- Co-authored-by: dante <dante@danteui-MacStudio.local> Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
878ffb70cc |
test: add unit tests for assetService (#11670)
## Summary
Extends `assetService.test.ts` (which previously only covered
`shouldUseAssetBrowser`) with behavioral tests for the network-bound
methods: metadata fetch error mapping, base64 upload validation,
async-vs-sync upload routing, delete error propagation, model-folder
filtering, update validation, and tag-filtered list defaults.
## Changes
- **What**: Adds 10 Vitest cases across `getAssetMetadata`,
`uploadAssetFromBase64`, `uploadAssetAsync`, `deleteAsset`,
`getAssetModelFolders`, `updateAsset`, and `getAssetsByTag`. Reuses the
existing hoisted `mockDistributionState` / `mockSettingStoreGet` setup
and the existing `vi.mock('@/scripts/api')` boundary; adds local
`buildResponse` and `validAsset` helpers scoped to this file.
## Review Focus
- The localized error path is covered through public methods
(`getAssetMetadata`) rather than reaching for the internal
`getLocalizedErrorMessage`.
- `getAssetModelFolders` test asserts that the request URL omits
`include_public` (the internal call site passes no `includePublic`),
matching the conditional in `handleAssetRequest`.
- `uploadAssetAsync` tests pin the discriminated-union shape (`type:
'async' | 'sync'`) for both 202 and 200 responses.
## Testing
\`\`\`bash
pnpm exec vitest run src/platform/assets/services/assetService.test.ts
pnpm format -- src/platform/assets/services/assetService.test.ts
pnpm lint
pnpm typecheck
pnpm knip
\`\`\`
All 16 tests pass (6 prior + 10 new).
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11670-test-add-unit-tests-for-assetService-34f6d73d36508117b1aafaf463e9c820)
by [Unito](https://www.unito.io)
|
||
|
Simon Pinfold
|
3a05a37323 |
Fix naming strategy for multi-job asset exports (#11610)
## Summary Use `group_by_job_time` for exports spanning multiple jobs while keeping single-job exports on `preserve`, and add regression coverage for the new naming-strategy behavior. ## Changes - **What**: updated the asset export payload and request typing for the new naming-strategy values, added unit coverage for single-job vs multi-job export requests, added `@cloud` sidebar browser coverage for export payloads, and adjusted the cloud Playwright setup helpers so setup API calls can hit the backend directly and Firebase auth is seeded on the app origin - **Breaking**: none - **Dependencies**: none ## Review Focus Please sanity-check the cloud Playwright harness changes in `ComfyPage` and `CloudAuthHelper`, plus the single-job vs multi-job export naming-strategy assertions in the new browser tests. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11610-Fix-naming-strategy-for-multi-job-asset-exports-34c6d73d365081a68a88ea38d897578f) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Benjamin Lu
|
3ea75e1c48 |
fix: localize secret date labels (#11524)
## Summary
Localizes secret list date labels through Vue I18n date formatting
instead of the browser default numeric date format, with Storybook
coverage for design review.
## Changes
- **What**: Replaces `toLocaleDateString()` with `useI18n().d(..., {
dateStyle: 'medium' })` for `SecretListItem` dates.
- **What**: Updates `SecretListItem` expectations to match the Vue I18n
date formatter.
- **What**: Adds `SecretListItem` stories for default, never-used,
loading, and disabled states.
- **Dependencies**: None.
## Review Focus
Stacked on #11480 to keep the escaping fix scoped. Please confirm
whether the localized medium date style matches design/product
expectations.
## Screenshots (if applicable)
https://f3ba7229.comfy-storybook.pages.dev/?path=/story/platform-secrets-secretlistitem--default
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11524-fix-localize-secret-date-labels-3496d73d3650814bb70bfb3f870a31cd)
by [Unito](https://www.unito.io)
|
