Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-07-15 11:44:10 +00:00
Code Issues Packages Projects Releases Wiki Activity
8,298 Commits 1,690 Branches 699 Tags
5cf647d71ecda4b7db94181f48b942db640a90a3
Commit Graph

5 Commits

Author SHA1 Message Date
jaeone94
72d1261983 [bugfix] Use Desktop2 bridge for missing model downloads (#12710) 
## Summary

Fixes the Desktop2 missing-model download path so the frontend calls the
Desktop2 download bridge directly when it is available, instead of
relying on the browser `<a download>` fallback that Desktop2 currently
has to intercept indirectly.

This addresses Linear FE-956, where missing-model downloads on Windows
could open the OS Save As dialog. The issue was reproducible when the
frontend language was not English: switching the UI language back to
English made the download succeed again.

## Root Cause

Desktop2 currently has compatibility logic that watches/intercepts the
frontend missing-model download flow from outside the FE code. That
interception depends on FE-rendered DOM details, including localized
accessible labels such as the missing-model download button
`aria-label`.

In English, Desktop2 could find the expected download controls and cache
the missing-model metadata before the FE-created `<a>` download was
clicked. In non-English locales, the localized label no longer matched
Desktop2's selector, so the Desktop2 interception path missed the
download. The FE then continued down the browser download path, which
Electron surfaced as a native Save As dialog on Windows.

## Changes

- Adds a narrow Desktop2 runtime bridge check in
`missingModelDownload.ts`:
  - if `window.__comfyDesktop2.downloadModel` exists
  - and `window.__comfyDesktop2Remote` is not set
- then FE calls `window.__comfyDesktop2.downloadModel(model.url,
model.name, model.directory)` directly and returns early.
- Keeps remote Desktop2 sessions on the existing browser fallback path
by preserving the `__comfyDesktop2Remote` guard.
- Leaves the existing OSS browser fallback and legacy desktop
`isDesktop` download-store path intact.
- Logs Desktop2 bridge failures so rejected promises or synchronous
bridge throws do not become unhandled errors.
- Adds regression coverage for:
- Desktop2 bridge path taking priority over browser and legacy desktop
fallbacks.
- rejected Desktop2 bridge calls being logged without falling back to
browser download.
- synchronously thrown Desktop2 bridge failures being logged without
crashing or falling back to browser download.
  - remote Desktop2 sessions continuing to use browser fallback.

## User Impact

Desktop2 users should no longer depend on localized FE DOM text for
missing-model downloads. In particular, non-English UI locales should
route missing-model downloads through Desktop2's managed downloader
instead of opening the OS Save As dialog.

## Validation

- Manually verified the issue is fixed in Desktop2 using a locally built
FE dist served through ComfyUI with `--front-end-root`.
- Verified Korean locale no longer triggers the Save As dialog and the
missing-model download succeeds through Desktop2.
- Verified the new regression test fails when the production bridge fix
is reverted.
- Covered the FE-side contract with unit tests because a true end-to-end
assertion of the Windows native Save As dialog is not currently
practical in the FE browser-test infrastructure. The FE tests can verify
that clicking missing-model download routes into
`window.__comfyDesktop2.downloadModel`; they cannot directly prove
Electron/Windows native dialog behavior. That full native-dialog
regression belongs in Desktop2/Electron integration coverage.
- Ran:
- `pnpm exec oxfmt --check
src/platform/missingModel/missingModelDownload.ts
src/platform/missingModel/missingModelDownload.test.ts`
  - `pnpm lint:unstaged`
- `pnpm exec vitest run
src/platform/missingModel/missingModelDownload.test.ts`
  - `pnpm typecheck`
  - `pnpm build`
- Pre-commit hook passed: `oxfmt`, `oxlint`, `eslint`, `typecheck`.
- Pre-push hook passed: `knip --cache` completed with existing tag hints
only.
- Ran a 3-round local Claude review loop; final verdict was approve with
no Blocker/Major findings.

## Follow-up Work

- Define and document the FE/Desktop2 bridge contract explicitly,
including the expected semantics of `downloadModel` resolving `false`
versus rejecting.
- Add a shared or canonical TypeScript declaration for
`window.__comfyDesktop2` and `window.__comfyDesktop2Remote` if more FE
code starts depending on these globals.
- Remove Desktop2's DOM/aria/class-based missing-model download
interception after a sufficient FE compatibility window, so Desktop2 no
longer depends on FE DOM structure or localized labels.
- Add Desktop2 integration/e2e coverage for missing-model downloads in
non-English locales, ideally including Windows where the Save As dialog
was observed. This is the right layer for a true native Save As
regression test.
- Optionally add a lighter FE browser E2E that injects a fake
`window.__comfyDesktop2.downloadModel` and verifies the missing-model UI
calls that bridge. This would validate the FE contract, but it would
still not replace Desktop2/Electron coverage for native dialog behavior.
- Decide on user-facing failure UX for Desktop2 bridge download failures
once Desktop2 defines whether failures, cancellations, and
already-queued downloads are represented by rejection or by `false`.

## Notes

This intentionally does not fall back to browser download when the
Desktop2 bridge resolves `false`. Falling back there could reintroduce
the exact Save As dialog behavior this PR fixes, and the meaning of
`false` should be clarified in the Desktop2 bridge contract before FE
invents user-facing behavior for it.

A true E2E test for this bug would need to exercise Desktop2/Electron on
Windows and assert that the native Save As dialog is not opened. The
current FE browser-test infrastructure cannot observe that native
Desktop2 behavior directly, so this PR uses focused unit regression
coverage for the FE routing contract plus manual Desktop2 verification.
 2026-06-09 16:42:19 +00:00
jaeone94
7caba4408d fix: open model library for desktop model downloads (#12478) 
## Summary

Open the Model Library sidebar when a missing-model download starts on
desktop so users can immediately see Electron download progress. This is
a temporary UX patch until the dedicated desktop missing-model download
progress bar lands.

## Changes

- **What**: Activates the `model-library` sidebar tab before starting
desktop missing-model downloads.
- **Dependencies**: None.

## Review Focus

Confirm the minimal behavior is scoped to the desktop download path and
covers both individual downloads and Download all through the shared
`downloadModel()` helper. This should remain small and easy to remove
once the progress bar flow is available.

## Screenshots (if applicable)



https://github.com/user-attachments/assets/d5b01db7-46b5-4a52-bb11-45e75a422474



## Test Plan

- `pnpm format`
- `pnpm lint`
- `pnpm typecheck`
- `pnpm test:unit`
- `pnpm knip`
 2026-05-27 15:25:26 +00:00
Hunter
4c35add5bc feat: add civitai.red hostname support (#11349) 
*PR Created by the Glary-Bot Agent*

---

## Summary

Civitai split its domain — NSFW content moved to `civitai.red` while
`civitai.com` stays SFW. The frontend only recognized `civitai.com`
URLs, causing the import button to silently reject `.red` links. This
was the root cause of 8+ support tickets in 3 days.

Companion to backend PR: https://github.com/Comfy-Org/cloud/pull/3259

## Changes

### Import source recognition
- **`civitaiImportSource.ts`**: Added `'civitai.red'` to `hostnames`
array — this is the primary fix for "button doesn't recognize the links"

### Missing model auto-download
- **`missingModelDownload.ts`**: Added `'https://civitai.red/'` to
`ALLOWED_SOURCES`

### URL detection utilities
- **`formatUtil.ts`**: `isCivitaiModelUrl()` now accepts `civitai.red`
URLs with proper hostname validation
- **`assetMetadataUtils.ts`**: `getSourceName()` returns "Civitai" for
`.red` URLs

### Tests (4 files)
- `useUploadModelWizard.test.ts`: Added civitai.red hostnames and URL
test case
- `missingModelDownload.test.ts`: Added civitai.red cases for
`toBrowsableUrl` and `isModelDownloadable`
- `assetMetadataUtils.test.ts`: Added civitai.red case for
`getSourceName`
- `useMissingModelInteractions.test.ts`: Updated mock hostnames
- `formatUtil.test.ts`: Added civitai.red cases for `isCivitaiModelUrl`

## Not changed (intentionally)
- `getAssetSourceUrl()` ARN fallback (line 88) — ARNs don't carry domain
info, `civitai.com` is correct default
- `fetchCivitaiMetadata()` API URL (line 109) — REST API works on both
domains, keeping `civitai.com`

Resolves BE-353

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11349-feat-add-civitai-red-hostname-support-3456d73d3650810d9c62ef4ad95ae031)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Terry Jia <terryjia88@gmail.com>
 2026-04-19 04:51:01 +00:00
jaeone94
a3cf6fcde0 fix: convert download URLs to browsable page URLs in Copy Url button (#10228) 
## Summary

- Copy Url button in MissingModelRow now copies a browsable page URL
instead of a direct download URL
- HuggingFace: `/resolve/` → `/blob/` (file page with model info and
download button)
- Civitai: strips `/api/download` or `/api/v1` prefix (model page)

## Changes

- Add `toBrowsableUrl()` to `missingModelDownload.ts` — converts
download URLs to browsable page URLs for HuggingFace and Civitai
- Update `MissingModelRow.vue` to use `toBrowsableUrl()` when copying
- Add 5 unit tests covering HuggingFace, Civitai, and non-matching URL
cases

## Test plan

- [x] Unit tests pass (14/14) — `toBrowsableUrl` covered by 5 dedicated
tests
- [x] Lint, format, typecheck pass
- [x] Manual: load workflow with missing HuggingFace models, click Copy
Url, verify copied URL opens the file page
- [x] Manual: load workflow with missing Civitai models, click Copy Url,
verify copied URL opens the model page

### Why no E2E test

The Copy Url button is only visible when `!isCloud &&
model.representative.url && !isAssetSupported`. Writing an E2E test for
the clipboard content would require:

1. A test fixture with a real HuggingFace/Civitai URL (fragile — depends
on external availability)
2. Granting `clipboard-read` permission in Playwright context
3. Ensuring `isAssetSupported` evaluates to `false` against the local
test server's node definitions

The URL transformation logic is a pure function, fully covered by unit
tests. E2E clipboard content verification has no existing patterns in
the codebase and would be environment-dependent and flaky.

---------

Co-authored-by: Jin Yi <jin12cc@gmail.com>
 2026-03-18 13:19:52 +09:00
jaeone94
e2ef041170 feat: surface missing models in Error Tab for OSS and remove legacy dialog (#9921) 
## Summary
- Surface missing models in the Error Tab for OSS environments,
replacing the legacy modal dialog
- Add Download button per model and Download All button in group header
with file size display
- Move download business logic from `components/dialog/content` to
`platform/missingModel`
- Remove legacy missing models dialog components and composable

## Changes
- **Pipeline**: Remove `isCloud` guard from `scanAllModelCandidates` and
`surfaceMissingModels` so OSS detects missing models
- **Grouping**: Group non-asset-supported models by directory in OSS
instead of lumping into UNSUPPORTED
- **UI**: Add Download button (matching Install Node Pack design) and
Download All header button
- **Store**: Add `folderPaths`/`fileSizes` state with setter methods,
race condition guard
- **Cleanup**: Delete `MissingModelsContent`, `MissingModelsHeader`,
`MissingModelsFooter`, `useMissingModelsDialog`, `missingModelsUtils`
- **Tests**: Add OSS/Cloud grouping tests, migrate Playwright E2E to
Error Tab, improve test isolation
- **Snapshots**: Reset Playwright screenshot expectations since OSS
missing model error detection now causes red highlights on affected
nodes
- **Accessibility**: Add `aria-label` with model name, `aria-expanded`
on toggle, warning icon for unknown category

## Test plan
- [x] Unit tests pass (86 tests)
- [x] TypeScript typecheck passes
- [x] knip passes
- [x] Load workflow with missing models in OSS → Error Tab shows missing
models grouped by directory
- [x] Download button triggers browser download with correct URL
- [x] Download All button downloads all downloadable models
- [x] Cloud environment behavior unchanged
- [x] Playwright E2E: `pnpm test:browser:local -- --grep "Missing models
in Error Tab"`

## Screenshots


https://github.com/user-attachments/assets/12f15e09-215a-4c58-87ed-39bbffd1359c

 


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9921-feat-surface-missing-models-in-Error-Tab-for-OSS-and-remove-legacy-dialog-3236d73d365081f0a9dfc291978f5ecf)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
 2026-03-15 22:46:47 +09:00