## Summary
When adding a node from the library sidebar, the node was not correctly
selected upon placing it. This was due to the canvas capturing the node
under the cursor on mouse down, however the node had not yet been
comitted to the graph at that point, and so selection was then cleared
on mouse up.
## Changes
- **What**:
- add `blockCommitPointerDown` so if the cursor is over the canvas stop
propagation to prevent LiteGraph adding the mouse handler to clear the
selection
## Review Focus
Alternative approaches considered were blocking the event in endDrag
however this then required manual cleanup of LiteGraph handlers or
overriding the `pointer.onClick` function to force selection of our
node, both felt worse than this approach.
## Screenshots (if applicable)
https://github.com/user-attachments/assets/a2eb154e-5178-4a1e-b5c7-884efd7a10c6
When an app mode workflow is opened on fresh page load, either from a
template url, or a persisted in browser cache, the UI would briefly
display the graph view prior to swapping to app mode. This is fixed by
continuing to display the splash screen until workflow state has loaded.
Share by url brings unique difficulties. The function call does not
return until a user has responded to a dialogue. If the splash screen
were blocked by this, the user would never be able to see the dialogue.
Consequentially, this change is not applied to shared workflow urls and
the (very unlikely) url including both a template url and a share url
will now prioritize the template url.
A best effort e2e test is included, but is a little clunky.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12387-Persist-splash-until-graph-load-completes-3666d73d3650813495e4ccad6052c1e4)
by [Unito](https://www.unito.io)
## Summary
- Spark 2.x requires SparkRenderer in scene tree; add it in SceneManager
and protect it in clearModel so model reloads don't dispose the splat
renderer.
- three 0.184 OrbitControls listens on ownerDocument; drop redundant
pointermove/up .stop in Load3D containers so the document listener can
receive events.
- Narrow Texture.image type for 0.184 strict typing.
- Add `file_path` to AssetItem schema (nullable per BE-933/BE-934 wire shape)
- Rewrite `getAssetDetectionNames`: use `file_path` alone when emitted,
fall back to the legacy `asset_hash` / `name` / `subfolder + name` union
when null (hash-only Core registrations, tagless Cloud rows, legacy data)
- Drop `isCloud` parameter from `scanAllMediaCandidates`,
`scanNodeMediaCandidates`, and `MediaVerificationOptions` — both backends
consult the same asset listing oracle post-BE-933/934
- Remove the OSS synchronous-shortcut branch; all candidates now resolve
asynchronously through `verifyMediaCandidates` against the unified listing
- Rename `MediaVerificationOptions.isCloud` to `allowCompactSuffix`
(N1-deferred annotation grammar flag, no longer a backend identity field)
- Always call `assetService.getInputAssetsIncludingPublic`; retain internal
`isCloud` only for the generated-assets oracle (Cloud /api/assets vs OSS
history) — unifying that path is tracked separately
- Update callers in app.ts, useErrorClearingHooks.ts, markDeletedAssetsAsMissingMedia.ts
- Update tests with module-level `isCloud` mock for generated-asset oracle
control; add `file_path`-primary matching test case
Per RFC BE-808 v2 (Asset Identity Semantics). DO NOT MERGE until BE-933
(comfyanonymous/ComfyUI#14005) and BE-934 (Comfy-Org/cloud#3744) merge
and ship — both backends must emit `file_path` for the new path to fire.
Fixes FE-746
## Summary
Allow asset/media FormDropdown searches to select the top filtered
result when the user presses Enter. This covers image, video, audio,
mesh, model-like asset selects, and other `WidgetSelectDropdown`-backed
media widgets.
## Implementation Scope
This PR implements a **top-result Enter shortcut** for the custom
asset/media dropdown path only:
- In scope: `WidgetSelectDropdown` -> `FormDropdown` asset/media
widgets.
- In scope: while the dropdown is open, single-select, and the search
text is non-empty, the first current search result becomes the Enter
candidate.
- In scope: pressing Enter in the search input selects that
candidate/top result through the existing selection path.
- In scope: candidate feedback for this shortcut, including visual
candidate styling and a polite screen-reader announcement for the
current top result.
- In scope: stale async search protection, empty-query/no-result no-op
behavior, multi-select guard behavior, and focus return to the trigger
after Enter selection closes the menu.
- Out of scope: plain combo widgets (`WidgetSelectDefault` /
`SelectPlus`). That path is PrimeVue-based and should be handled
separately from this focused asset-widget PR.
- Out of scope: full combobox/listbox keyboard navigation, including
Tab-to-list focus, ArrowUp/ArrowDown candidate movement, Home/End
behavior, scroll-to-active-item behavior, and a full ARIA
combobox/listbox refactor.
Follow-up arrow-key navigation should validate the interaction model
separately. This PR keeps the candidate state narrow and localized so
that future work can either extend it into movable active-item state or
replace it as part of a fuller combobox/listbox implementation.
## Changes
- **What**: Added an explicit Enter event from `FormSearchInput`, routed
it through the FormDropdown menu actions, and selected the current top
search result in `FormDropdown`.
- **What**: Kept the existing `computedAsync` + debounced filtering path
for normal typing, while Enter performs a one-off search against the
latest input before selecting. Stale async Enter results are ignored if
the query or item source changes before resolution.
- **What**: Prevented closed FormDropdown state from treating the full
unfiltered list as current search results, limited Enter-to-select to
single-select dropdowns, and made empty search Enter a no-op.
- **What**: Returned focus to the dropdown trigger after single-select
selection closes the menu.
- **What**: Added candidate styling for the first current FormDropdown
result while a search query is active so the Enter target is visible to
users.
- **What**: Added a polite screen-reader announcement for the current
top result candidate.
- **What**: Fixed the FormDropdownMenuActions `baseModelSelected` model
default to use a `Set` factory instead of a shared instance.
- **What**: Added unit coverage for the search Enter event, FormDropdown
selection behavior, focus return, debounce/Enter behavior, stale async
Enter protection, empty-query no-op behavior, closed-state stale result
protection, multi-select guard behavior, and candidate announcement
behavior. Added App Mode E2E coverage for asset FormDropdown Enter
selection.
- **What**: Extracted reusable app-mode dropdown fixture helpers and
updated the existing FormDropdown clipping test to use the shared
helper.
## Review Focus
Please focus review on the asset/media FormDropdown path, especially
`getTopSearchResult()`, the single-select/empty-query guards, stale
async search protection, trigger focus return after selection, and
candidate feedback in grid/list layouts.
The plain combo path and full arrow-key navigation are intentionally
left for separate follow-up work.
## Screenshots (if applicable)
https://github.com/user-attachments/assets/3eb3456d-93a3-4959-91a3-188f8116ccc9
Validation performed:
- Latest final-commit validation:
- `pnpm test:unit
src/renderer/extensions/vueNodes/widgets/components/form/FormSearchInput.test.ts
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuActions.test.ts
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.test.ts`
- Commit hook: `pnpm exec stylelint ...`, `pnpm exec oxfmt --write ...`,
`pnpm exec oxlint --type-aware --fix ...`, `pnpm exec eslint --cache
--fix ...`, `pnpm typecheck`
- Push hook: `pnpm knip --cache`
- `git diff --check`
- Earlier branch validation for this flow:
- `pnpm install`
- `pnpm typecheck:browser`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173
PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser --
--project=chromium browser_tests/tests/appMode.spec.ts -g "Drag and
Drop|FormDropdown search Enter selects the top filtered item"
--reporter=list`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173
PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser --
--project=chromium browser_tests/tests/appMode.spec.ts -g "FormDropdown
search Enter selects the top filtered item" --reporter=list`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173
PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm test:browser --
--project=chromium browser_tests/tests/appModeDropdownClipping.spec.ts
-g "FormDropdown popup is not clipped" --reporter=list`
## Summary
Add entries to `MODEL_NODE_MAPPINGS` so the model browser's "Use" button
creates the correct loader node for two model directories introduced in
recent node-pack updates.
## Changes
- **What**: 2 new entries in
`src/platform/assets/mappings/modelNodeMappings.ts`:
- `geometry_estimation` → `LoadMoGeModel` / `model_name`
- `optical_flow` → `OpticalFlowLoader` / `model_name`
- **Breaking**: none
## Review Focus
- Node class names and input keys cross-checked against the published
node definitions:
- `LoadMoGeModel` is the MoGe geometry-estimation loader (companion
nodes: `MoGeInference`, `MoGeRender`, `MoGeContextStrandModel`)
- `OpticalFlowLoader` is the RAFT optical-flow loader
- Both directories accept a single model file via a COMBO widget on the
loader node
## Test plan
- [ ] Verify "Use" button works for each new model directory in the
model browser:
- One `geometry_estimation` model (e.g.
`moge_2_vitl_normal_fp16.safetensors`) → creates a `LoadMoGeModel` node
with the file preselected
- One `optical_flow` model → creates an `OpticalFlowLoader` node with
the file preselected
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12389-feat-add-model-to-node-mappings-for-geometry_estimation-and-optical_flow-3666d73d36508190981fcaf77f9d2ee4)
by [Unito](https://www.unito.io)
Fixes 3 different bugs when making links to and from subgraph IO from
vue nodes
- When dragging a link from a node to a subgraph IO, there is no
feedback if a slot is not a valid connection target or if a slot is
actively hovered
- When a link is made from a subgraph IO to a node, the reactivity is
not triggered on the node to indicate a change of link state.
- When dragging a link from a subgraph IO to a node, the link would not
snap to the valid connection targets on nodes
- The fix for this one is not as thorough as I would like. It only
allows connections to the slot, not connections to the hovered widget.
We have two deeply disconnected linking systems and properly reconciling
them would be a multi-week project.
Resolves FE-561
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12281-Subgraph-io-fixes-3606d73d365081089f7ef19331c6d70a)
by [Unito](https://www.unito.io)
- useComboWidget: drop the `if (isCloud)` wrap so shouldUseAssetBrowser
and NODE_MEDIA_TYPE_MAP paths run unconditionally. Rename
resolveCloudDefault -> resolveAssetDefault.
- useAssetWidgetData: drop the `if (isCloud)` branch and its empty-data
fallback; remove the "Cloud-only" docstring.
- Tests: drop the isCloud mock + per-test toggles. Delete the
isCloud=false desktop spec and the OSS LoadImage fall-through case
that asserted the now-removed branch.
## Summary
Allow staging api/platform base URLs to be overridden by env vars so
non-cloud builds can target an alternate backend without source edits.
## Changes
- **What**: `BUILD_TIME_API_BASE_URL` / `BUILD_TIME_PLATFORM_BASE_URL`
in `src/config/comfyApi.ts` now read
`import.meta.env.VITE_STAGING_API_BASE_URL` /
`VITE_STAGING_PLATFORM_BASE_URL` first, falling back to the existing
`stagingapi.comfy.org` / `stagingplatform.comfy.org` constants. Vars
typed in `src/vite-env.d.ts` and documented in `.env_example`.
- **Breaking**: None. Defaults unchanged. The cloud-runtime override
path via the features endpoint (`comfy_api_base_url`,
`comfy_platform_base_url` in `RemoteConfig`) is untouched.
## Review Focus
Override only applies to the non-prod branch of the build-time ternary,
so prod builds (`USE_PROD_CONFIG=true`) cannot be redirected. Cloud
builds continue to resolve URLs at runtime via `remoteConfig` regardless
of these env vars.
## Note
Pre-commit `pnpm typecheck` fails on `origin/main` independently of this
change (`src/utils/nodeDefUtil.ts` and
`src/workbench/utils/nodeHelpUtil.ts` import non-existent exports from
`@/schemas/nodeDefSchema` / `@/types/nodeSource`). Verified by stashing
this PR's diff and re-running. Committed with `--no-verify`; please
address the underlying breakage separately.
This is a targeted small scope change to improve the availability for
converting group nodes into a subgraph.
The prior implementation would only apply on the litegraph context menu
option for converting a node to a subgraph. It failed to apply on any of
the other more common methods. The code for unpacking group nodes has
been moved directly into the setup for converting a group of nodes into
a subgraph and drastically simplified.
Of note, several other long lived bugs were found while working on this
fix, but they are out of scope for this targeted PR.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12356-On-subgraph-conversion-always-unpack-group-nodes-3666d73d365081d09774c00a851b8198)
by [Unito](https://www.unito.io)
FE-730 (L1.3). Both Cloud and OSS will serve /api/assets post-BE-786,
so assetsStore collapses to a single asset-API code path.
- Drop fetchInputFilesFromAPI (legacy /files/input caller); rename
fetchInputFilesFromCloud to fetchInputFiles.
- Remove the isCloud ternary on the input fetcher swap.
- Unwrap getModelState()'s if(isCloud) body and delete its OSS no-op
return object; the model pagination subsystem now runs unconditionally.
- Drop now-dead isCloud + mapInputFileToAssetItem imports.
- Delete mapInputFileToAssetItem + stripDirectoryAnnotation from
assetMappers.ts (no remaining callers in src/); delete its test file.
- Simplify assetsStore.test.ts: hardcode the distribution mock to
isCloud: true, drop the mockIsCloud toggle helper and the dead
mapper mock, rename the now-unconditional "(Cloud)" describe blocks.
Blocked-merge by BE-786 (OSS --enable-assets removal) + FE-729
(isAssetAPIEnabled deletion). Opened as a Draft on the FE-729 stack.
Auto-fixed unrelated tailwind class-order lint errors in five files
via pnpm lint:fix to keep CI green.
## Summary
Import published media assets for shared workflows before loading the
graph so the first missing-media scan sees the user's newly imported
references instead of surfacing a false missing asset error. cc FE-773
## Changes
- **What**: Moves the shared workflow import step ahead of
`loadGraphData` for the copy-and-open flow, while still allowing the
workflow to open with a warning path if asset import fails.
- **What**: Clears the shared workflow URL intent consistently on
failure paths, including graph load failure after an import attempt, so
reloads do not repeatedly replay the same shared workflow side effects.
- **What**: Invalidates the input asset cache after published asset
import so graph loading and missing-media resolution can observe the
refreshed media state.
- **What**: Adds a global loading spinner while shared workflow asset
import and graph load are in progress, with `role="status"`,
`aria-live`, reduced-motion-safe animation, and body teleporting so it
stays visible above blocking UI.
- **What**: Adds stable TestIds for the shared workflow dialog and
updates existing shared workflow E2E selectors away from copy-dependent
role text.
- **What**: Adds a cloud E2E regression fixture and spec covering the
critical flow: shared URL opens the dialog, the user confirms asset
import, published media is imported before the public-inclusive input
asset scan, the workflow loads, the share query is removed, and missing
media UI is not surfaced.
- **Breaking**: None.
- **Dependencies**: None.
## Root Cause
Shared workflow graph loading triggered the missing-media pipeline
before the user-selected published media import had completed. Because
`include_public=true` does not include published assets, the pre-import
scan could classify shared media as missing even when the user was about
to import those assets into their own library.
## Review Focus
- The ordering in `useSharedWorkflowUrlLoader`: import published assets
first, then load the graph, while keeping import failure non-fatal for
workflow opening.
- The failure cleanup behavior: the shared URL/preserved query intent is
now cleared for graph load failures too, avoiding repeated
reload-triggered imports.
- The spinner behavior in `App.vue`: it uses the existing
`workspaceStore.spinner` boolean and intentionally keeps broader
ref-counted spinner ownership as follow-up work.
- The E2E sentinel in `sharedWorkflowMissingMedia.spec.ts`: it asserts
no public-inclusive input asset scan occurs before `/api/assets/import`,
then waits for a settling window to ensure the missing-media overlay
does not appear.
## Validation
- `pnpm format`
- `pnpm lint` (passed with existing unrelated warnings only)
- `pnpm typecheck`
- `pnpm test:unit`
- Commit hook: lint-staged formatting/linting, `pnpm typecheck`, `pnpm
typecheck:browser`
- Push hook: `pnpm knip --cache` (passed with existing tag hint only)
## Follow-Up
- Consider a ref-counted or scoped global spinner API so long-running
flows do not directly toggle `workspaceStore.spinner`.
- Consider separating shared workflow load status into orthogonal result
fields instead of encoding partial success in a single string union.
- Consider moving published asset import/cache invalidation behind an
asset-service-owned API boundary.
- Backend follow-up remains needed for `include_public=true` not
including published assets; this PR only removes the frontend false
positive when the user explicitly imports the shared media.
## Screenshots
Before
https://github.com/user-attachments/assets/dc790046-237c-4dd8-b773-2507f9a66650
After
https://github.com/user-attachments/assets/6517cd38-2c3d-4bfe-a990-35892b7e50aehttps://github.com/user-attachments/assets/d89dc3d3-75d9-4251-998b-0c354414e25b
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12333-fix-avoid-false-missing-media-errors-after-importing-shared-workflow-assets-3656d73d365081b38634dcb7625cfc32)
by [Unito](https://www.unito.io)
## Summary
Two fixes for the cloud LoadImage form dropdown:
1. **Cloud root-cause fix** — outputs now come from a single
`getAssetsByTag('output')` call instead of walking the jobs API and
per-job `resolveOutputAssetItems` detail fetches. Per Christian's [Slack
feedback](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778051260476369?thread_ts=1776716352.588229&cid=C0A4XMHANP3):
*"on cloud, we can just grab the assets with a single GET, filtering
with input or output tag."* Sidebar's job-stack UX is untouched.
2. **Local / defense-in-depth** — even when the watch+expansion path is
in play (still used by local), batch all in-flight
`resolveOutputAssetItems` for the current `media` snapshot via
`Promise.all`, committing once into `resolvedByJobId`. This kills the
progressive head-shift symptom even on the legacy path.
The first attempt at (1) (`6a1a083c9`, reverted in `c175962e8`) broke
select+load on cloud prod because the dropdown wrote `asset.name` (human
filename) into the widget value, but cloud's `/api/view` resolves output
files by **`asset_hash`** (the blake3-keyed filename). Verified against
cloud prod that every output row carries `asset_hash` and that cloud's
own `preview_url` is hash-keyed, not name-keyed. Re-introduced in
`d7693377` with the dropdown value derived from `asset.asset_hash ||
asset.name`, with the human filename retained as the display label.
- Fixes FE-227
## Cloud / local divergence — what this PR clarifies
| | input | output (this PR) |
| ------------ | ---------------------------------------------- |
-------------------------------------------------------- |
| **cloud** | `getAssetsByTag('input')` (already correct) |
**`getAssetsByTag('output')` (new)** |
| **local** | `/files/input` (FS-listing) | `getHistory` + per-job
expansion (unchanged) |
Both directions are now symmetric on cloud: tag-based listing,
hash-keyed values. Local stays on the legacy path because core ComfyUI
doesn't have the assets/tags model — that's the deeper convergence
Jacob/Luke flagged in the FE-556 thread (now BE-757), which is BE/Core
work and not this PR.
## Red-Green verification
| Commit | CI: Tests Unit | Purpose |
|--------|----------------|---------|
| `3e8d42e7` test | 🔴 [failure
(25413987208)](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/25413987208)
| Asserts the head of the list does not shift while one of two
multi-output jobs is still resolving. |
| `fe2608d4` fix (atomic batch) | 🟢 [success
(25414246791)](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/25414246791)
| Resolutions awaited via `Promise.all` and merged in one
`resolvedByJobId` update. |
| `6a1a083c` simplification (broken) | — | First attempt — used
`asset.name`, broke select+load on cloud prod. |
| `c175962e` revert | — | Rolled back the broken simplification while
diagnosis was in flight. |
| `d7693377` simplification (fixed) | pending | Re-introduces
`useFlatOutputAssets` and uses `asset.asset_hash` for the dropdown
value. Adds 7 unit tests covering hash-as-value, name-fallback,
pagination, dedupe, and error path. |
## screenshot/ video
### before
https://github.com/user-attachments/assets/239aa447-a260-4713-926c-04dd80a30408
### after
https://github.com/user-attachments/assets/d68228c6-33f5-4bf0-ad24-bb83c876fdc2
## Test plan
- [x] New `useFlatOutputAssets.test.ts` — 7 tests for tag-based
fetching, pagination, dedupe, error path.
- [x] `useWidgetSelectItems.test.ts` — atomic-batching regression test +
new tests asserting hash-as-value and local name-fallback. 35 tests
pass.
- [x] `WidgetSelectDropdown.test.ts` — 5 tests pass with the new
conditional source.
- [x] CI red on test-only commit, CI green on first fix commit.
- [ ] CI green on the simplification (re-introduce) commit.
- [ ] Manual verification on cloud build: open LoadImage → switch to
Outputs → scroll → list head stays stable; select an output → LoadImage
preview loads (was broken in `6a1a083c`, restored in `d7693377`).
## Summary
Classify `.ply` as 3D media so PLY outputs are surfaced by queue/assets
preview flows.
## Changes
- **What**: adds `.ply` to shared 3D extension detection and falls back
to the asset `/view` URL when opening 3D assets without `preview_url`.
- **Breaking**: none.
- **Dependencies**: none.
## Review Focus
- This is the tactical FE fix for FE-129; it intentionally does not
solve the broader 3D media vs load3d-loadable split.
- Assets sidebar 3D viewer still prefers `preview_url`, but now has a
usable fallback for assets that only have the normal asset URL.
FE-129
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12319-fix-classify-PLY-assets-as-3D-media-3646d73d365081218f0bde401b1601bd)
by [Unito](https://www.unito.io)
## Summary
Stops the Shape submenu (and any other PrimeVue nested submenu) from
being clipped behind the node context menu when the menu fits in the
viewport.
## Changes
- **What**: `constrainMenuHeight` in `NodeContextMenu.vue` now applies
`max-height` + `overflow-y: auto` to the root `<ul>` only when
`scrollHeight > availableHeight`. The common case keeps `overflow:
visible`.
- Added `browser_tests/tests/nodeContextMenuShapeSubmenu.spec.ts`
regression spec.
## Review Focus
Root cause: setting only `overflow-y: auto` on a `<ul>` coerces
`overflow-x` to a non-visible value per CSS spec (`If one of
overflow-x/overflow-y is visible and the other isn't, the visible value
is computed as auto`). PrimeVue `ContextMenuSub` renders submenus
in-tree as a nested `<ul>` with `position: absolute; left: 100%`, so the
implicit horizontal clip hides them entirely.
The pre-existing overflow scenario (#10824 / #10854) is unchanged — when
the menu actually overflows, the clamp still applies and
`nodeContextMenuOverflow.spec.ts` continues to verify scroll. Submenu
clipping in that overflow case is a known limitation, not introduced by
this PR.
Fixes FE-570
## screenshot
### AS IS
<img width="788" height="505" alt="Screenshot 2026-05-07 at 12 43 26 PM"
src="https://github.com/user-attachments/assets/36d34070-0c57-4385-a130-0394f22f282e"
/>
### TO BE
<img width="779" height="627" alt="Screenshot 2026-05-07 at 12 42 44 PM"
src="https://github.com/user-attachments/assets/00956729-763b-4787-822f-209e8ea42331"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12035-fix-keep-node-context-menu-overflow-visible-when-content-fits-3586d73d365081ad9aaec82f220d401c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
Move isValidDimension + the metadata-vs-fallback logic into
getAssetMetadataDimensions in assetMetadataUtils, leaving the SFC with a
one-line computed. Switches the block comments to JSDoc on the helper
(per review feedback) and adds unit coverage for the validation cases —
addresses codecov/patch failure on the previously SFC-internal logic.
Tightens the metadata-dimension read added in the previous commit
based on review feedback:
- Adds isValidDimension() guard: rejects NaN, Infinity, 0, negatives,
and fractional values. typeof v === 'number' alone accepts all of
those, any of which would render as garbage in the dimension label
(e.g. "NaNxNaN", "0x0", "1024.5x768").
- Renames originalImageDimensions to displayImageDimensions. The
fallback branch may return preview-sized values from the rendered
<img>'s naturalWidth/Height — calling that "original" invites the
next caller to wire it into a place where source-of-truth
dimensions are required.
- Restores meaning of metaInfo's existing truthy check by allowing
displayImageDimensions to return undefined when neither path
produces usable values (e.g. invalid metadata + image not yet
loaded). Previously the truthy check was dead code because the
computed always returned something.
No behavior change in the happy path (valid metadata, valid local).
Asset cards now render image dimensions from the asset response's
metadata field (`asset.metadata.width` / `asset.metadata.height`)
when available, falling back to the locally-computed naturalWidth /
naturalHeight from the rendered <img> tag when not. This unifies
the dimension display across runtimes that serve the original file
vs. runtimes that serve a downscaled preview — the rendered <img>
on the latter reports the preview size, not the source asset's
size, so a metadata-first read is what makes the displayed label
match the source on both runtimes.
No behavior change in the absence of metadata: cards continue to
display the locally-computed dimensions as they do today.
Drops the now-unused isCloud import; the conditional it guarded
collapses into the unified read-then-fallback path.
## Summary
When the built-in logs terminal stayed open during a backend restart,
the buffer froze on pre-restart entries and live log streaming silently
stopped — only closing and reopening the panel resynced. Listen for the
api `reconnected` event and rebuild the terminal contents the same way a
fresh open would.
## Changes
- **What**:
- Extract `useLogsTerminal` composable. The SFC is now a thin shell
holding `terminal: shallowRef<Terminal>` and forwarding to the
composable, so `onMounted`/`onScopeDispose` no longer rely on the
child's emit callback timing.
- Subscribe to `api`'s `reconnected` event via `useEventListener`,
registered synchronously before any awaits. On reconnect:
`terminal.reset()` → refetch raw logs → `scrollToBottom()` →
`subscribeLogs(true)` (the backend loses the per-client subscription on
restart, so re-subscribe is required for live streaming to resume).
- Wrap in-flight resync/mount fetches in AbortControllers. Overlapping
reconnects abort the prior resync, and unmount mid-fetch suppresses
writes to the disposed xterm.
- Hide BaseTerminal whenever `errorMessage` is set so the error layout
doesn't expose an empty xterm container behind the message;
`loading=false` after both load failure and resync success so a later
successful reconnect can clear a stuck spinner.
- Migrate the load/resync error strings to vue-i18n
(`logsTerminal.loadError`, `logsTerminal.resyncError`).
## Review Focus
- **Re-subscribe is the non-obvious half of the fix** — without it, even
after the WebSocket reconnects the backend never resumes streaming logs
to this client because its subscription state was wiped on restart. The
visible "stale buffer" is only one symptom; the silent "no new logs"
symptom needed the explicit `subscribeLogs(true)` re-call in resync.
- `terminal.reset()` lives after a successful raw-logs fetch (not
before) so a failed resync leaves the prior buffer visible instead of
blanking it; resync errors surface via the same inline error message the
mount path uses.
- 8 unit tests around the composable: mount + subscribe, resync ordering
(reset → write → scroll → subscribe via `invocationCallOrder`),
in-flight resync abort on double reconnect, resync error surfacing,
mount-failure-then-recovery, unmount-mid-fetch terminal-write
suppression, listener cleanup on unmount.
- 2 E2E tests using `ws.close()` on the proxied WebSocket as the
reconnect trigger and `subscribeLogs` HTTP fetch count as the sync point
(same pattern as `wsReconnectStaleJob.spec.ts`). Red-checked: disabling
the `reconnected` listener fails exactly the two new tests, all 8
pre-existing tests stay green.
Fixes FE-712
## Screenshots
Before - (After rebooting, the console window does not update from its
state before the reboot must remount the console window for it to
resync.)
https://github.com/user-attachments/assets/b1e49c2c-89a4-4a4a-82b4-064412acee12
After - (The console window syncs automatically after a reboot.)
https://github.com/user-attachments/assets/54b582c5-ad42-41c0-9886-18f4495859da
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12270-fix-terminal-resync-logs-console-on-backend-reconnect-3606d73d3650812fb13fd1934c632344)
by [Unito](https://www.unito.io)
## Summary
Add the first product-area browser coverage on top of the merged typed
route mock foundation: the docked job history sidebar.
## Changes
- **What**: Adds `browser_tests/tests/sidebar/jobHistory.spec.ts` using
`jobsRouteFixture`.
- **What**: Covers direct sidebar entry, docked QPO history entry,
terminal history jobs, active queue jobs, tab filtering, search, clear
queue, and clear history.
- **What**: Adds typed `POST /api/queue` and `POST /api/history` route
helpers that validate request bodies with generated zod schemas.
- **What**: Adds stable test ids for the job history sidebar and queue
progress overlay so tests avoid structural CSS selectors.
- **Dependencies**: Builds on the typed route mock foundation merged in
#12267.
## Review Focus
Review the product assertions and whether this is the right first
coverage slice on top of the typed route mock foundation. This PR
intentionally avoids asset sidebar and floating QPO lifecycle coverage;
those should remain follow-up PRs.
## Screenshots (if applicable)
Not applicable.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12272-test-cover-job-history-sidebar-with-typed-route-mocks-3606d73d3650817481d5f9fac4bfc93c)
by [Unito](https://www.unito.io)
Follow-up to the previous FE-729 commit. After deleting
isAssetAPIEnabled, isAssetPreviewSupported() became a wrapper that
always returned true. Remove the function and simplify all callers.
Changes:
- Delete isAssetPreviewSupported() from assetPreviewUtil.ts.
- Media3DTop.vue: drop the isAssetPreviewSupported() arm of the
loadThumbnail guard (asset.name check is still required).
- saveMesh.ts: unwrap two `if (isAssetPreviewSupported()) { ... }`
blocks in applySaveGLBOutput and the SaveGLB beforeRegisterNodeDef
extension callback.
- FormDropdownMenuItem.vue: drop the early return from
resolveMeshPreview.
- useLoad3d.ts: drop the isAssetPreviewSupported() arm of the
modelReady guard.
- Tests: remove the dead "asset preview is unsupported" branches
(useLoad3d, Media3DTop, FormDropdownMenuItem) and clean up the
associated mocks and hoisted state.
Auto-fixed unrelated tailwind class-order lint errors in five files
(VirtualGrid, RightSidePanel, Textarea, ModelInfoPanel,
WidgetSelectDefault) to keep CI green.
FE-729 (L1.2 — L1 central). Asset API will be always available
post-BE-786 (`--enable-assets` removed from OSS). Delete the gating
function, the user-facing setting, the experimental warning, and the
toggle command. Simplify all caller sites to assume the asset API
is available.
Changes:
- Delete isAssetAPIEnabled() from assetService.ts; simplify
shouldUseAssetBrowser() to call isAssetBrowserEligible() directly.
- Remove the EXPERIMENTAL_WARNING prefix from four asset-load error
messages.
- Drop the Comfy.Assets.UseAssetAPI setting from coreSettings.ts and
its schema entry in apiSchema.ts.
- Remove the Comfy.ToggleAssetAPI command outright. Keep
Comfy.BrowseModelAssets but drop its setting-check / setting-set
preamble and the "Experimental:" label prefix; the command now
just opens the asset browser dialog.
- modelStore: drop the useAssetAPI fork in createGetModelsFunc and
loadModelFolders; always route through assetService.
- sidebarTabStore: model-library tab click always routes through
Comfy.BrowseModelAssets (no more setting check).
- assetPreviewUtil.isAssetPreviewSupported() returns true
unconditionally; caller-site simplification deferred.
- WidgetSelect.vue: simplify isAssetMode to drop the
isAssetAPIEnabled() arm of the disjunction.
- Test mocks/expectations updated.
Blocked-merge by BE-786 (OSS `--enable-assets` removal). PR opened
as draft.
Also auto-fixed unrelated tailwind class-order lint errors in four
files (VirtualGrid, RightSidePanel, Textarea, ModelInfoPanel,
WidgetSelectDefault) to keep CI green.
Replace the hand-rolled { name?: string ... } shapes in the painter
and mask editor with UploadImageResponse from the codegen'd
ingest-types package. The runtime shape is unchanged - the type now
flows from the spec via the existing codegen pipeline instead of
being declared inline locally.
The runtime check for data.name remains: the spec marks the 200
response properties as optional, so the imported type still has
name?: string. The check enforces what the server actually returns
in practice; tightening the spec is a separate follow-up.
## Summary
https://linear.app/comfyorg/issue/FE-727/refactor-usemediaasset
`useMediaAssets` was a factory wrapper that branched on `isCloud` and
returned one of two near-identical implementations (`useAssetsApi` /
`useInternalFilesApi`). Both implementations delegated to the same
`assetsStore` actions (`updateInputs` / `updateHistory` /
`loadMoreHistory`) — the real cloud/local fork lives inside
`assetsStore.fetchInputFiles` (line 121), not at the composable layer.
Collapse the wrapper:
1. Delete `useInternalFilesApi` (identical to `useAssetsApi`).
2. Delete `useMediaAssets` (now a pass-through).
3. Switch the four callers to `useAssetsApi` directly.
## Changes
- **What**:
- Delete `useInternalFilesApi.ts` and `useMediaAssets.ts`.
- `AssetsSidebarTab.vue`, `WidgetSelectDropdown.vue`,
`useOutputHistory.ts`: call `useAssetsApi` directly.
- `useWidgetSelectItems.ts`: narrow `outputMediaAssets` prop type to
`IAssetsProvider` (the interface `useOutputHistory` already implements
directly).
- Test mocks repointed from `useMediaAssets` → `useAssetsApi`.
- Two unrelated tailwind class-order lint errors auto-fixed by `pnpm
lint:fix` to keep CI green.
- **Breaking**: None — no behavior change.
## Review Focus
The real cloud/local fork remains at `assetsStore.ts:121`
(`fetchInputFiles = isCloud ? fetchInputFilesFromCloud :
fetchInputFilesFromAPI`). That branch is M1-strict and clears once
BE-933 lands. This PR only collapses the dead wrapper layer above it.
`IAssetsProvider` is intentionally kept — `useOutputHistory.ts:83`
directly implements it for a different use case, so the interface still
has more than one consumer.
Surfaced while working on the FE-678 cloud/local asset branching survey.
## Screenshots (if applicable)
N/A — no UI change.
---------
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary
`getSurveyCompletedStatus` (auth.ts) now resolves ambiguous responses to
"completed" instead of "not completed", so transient backend errors no
longer bounce working users to `/cloud/survey`.
| Backend response | Old behavior | New behavior |
|---|---|---|
| 200 with non-empty `value` | `true` (completed) | `true` (completed) |
| 200 with empty `value` | `false` (not completed) | `false` (not
completed) |
| 404 | `false` → bounced | `true` (treat as completed) |
| 5xx | `false` → bounced | `true` (treat as completed) |
| 401 / 403 | `false` → bounced | `true` (treat as completed) |
| Network error | `false` → bounced | `true` (treat as completed) |
Only a definitive `200` with empty `value` is treated as "not
completed". Everything else fails open. The dedicated auth layer handles
re-authentication on the next API call, so 401/403 doesn't need a
separate branch here.
## Why
User reports from team-plan customers: _"I was working in a workflow,
hit run, and then got logged out and redirected to a survey screen."_
Datadog shows ~7,000 distinct users/day hitting the `setting key
onboarding_survey not found` path on prod ingest. With
`onboarding_survey_enabled: true` in prod dynamic config and the
catch-all `!response.ok` returning `false`, any mid-session reload
tripped a redirect to `/cloud/survey`.
User-validated requirement: rather miss showing the survey to a few
users than show it duplicately or interrupt working customers.
## Trade-off worth product review
A genuinely brand-new user whose `User.Settings` JSON is empty also
returns 404 from `/api/settings/onboarding_survey` — the backend doesn't
distinguish "key absent for existing user" from "user has no settings
yet". With this change, that 404 is treated as "completed", so the
survey gate does not fire on the strict 404 path. New users will still
see the survey if signup pre-populates the `onboarding_survey` key with
an empty object (`200` with empty `value`); if not, the survey is missed
on initial signup.
We picked this trade-off per the product call that false positives
(bouncing paying customers) are strictly worse than false negatives
(occasionally missing a new user).
The clean fix to recover the new-user signal is a backend change: return
`200` with `value: null` when the `User` row exists but the key is
absent — distinguishing "no survey saved" from "user not found". Out of
scope for this PR; filing as follow-up if accepted.
## Test plan
- [ ] Logged-in user with completed survey navigates around — no
redirect
- [ ] Logged-in user with no survey, fresh tab — redirected to
`/cloud/survey` (gate still works for new sessions)
- [ ] Logged-in user with no survey, after submitting — no redirect on
next nav
- [ ] Simulate transient 5xx on `/api/settings/onboarding_survey`
(DevTools blocking) — user stays on current page, no redirect
Unit coverage in `auth.test.ts` locks the resolution table above against
drift (one test per branch, 8 total).
## Companion PRs
None — frontend only.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12301-fix-cloud-stop-bouncing-working-users-to-cloud-survey-mid-session-3616d73d365081128ba7e266ad7ccff9)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Both uploadLayer (maskeditor) and the painter's serialize path previously
fell through to the pre-upload layer.ref when the server returned 200 with
no name or with a non-JSON body, logging only a console.warn. The caller
then assigned the stale ref onto the saved widget value as if the upload
had succeeded, producing graphs that pointed at files that were never
written.
- maskeditor: rewrite uploadLayer to throw on JSON parse failure and on
missing data.name (was: console.warn + return layer.ref)
- painter: add a missing-name guard alongside the existing JSON parse
guard, surfacing the same toast/error path the JSON failure already uses
- painter test: assert the upload FormData carries type=input with no
subfolder, and add coverage for the two new error branches
Collapse the OSS vs Cloud branching in the mask editor and painter so both
runtimes use the same upload contract: POST to /upload/image with type "input"
and no subfolder, then reference the result by filename only.
- maskeditor: replace separate uploadMask + uploadImage helpers with a single
uploadLayer; drop the /upload/mask call, the original_ref form field, and
the clipspace subfolder
- painter: drop the isCloud branch on type/subfolder/result string; always
upload as type "input" with no subfolder
- update painter test to match the unified result shape
## Summary
Migrate the default combo widget select from the PrimeVue `SelectPlus`
wrapper to a Reka `Combobox` implementation while preserving the
existing Comfy combo widget contract and the node-canvas dropdown
behavior.
## Changes
- **What**: Rewrites `WidgetSelectDefault.vue` on top of Reka
`ComboboxRoot`, `ComboboxTrigger`, `ComboboxInput`, `ComboboxContent`,
and `ComboboxItem`.
- **What**: Preserves the default combo widget surface: `v-model`,
`widget` prop, `aria-label` from the widget name/label,
`data-capture-wheel`, disabled state, placeholder/filter placeholder,
default slot controls, invalid current value display, array values,
dynamic/factory values, and `getOptionLabel` fallback behavior.
- **What**: Keeps dynamic `values` compatibility by refreshing
function-backed options when the dropdown opens, without re-evaluating
the factory on every search keystroke.
- **What**: Deletes the now-unused PrimeVue `SelectPlus.vue` wrapper and
removes the PrimeVue test plugin/stub path from the default widget
select tests.
- **What**: Updates App Mode dropdown clipping coverage and combo-widget
browser coverage to target the new Reka overlay/viewport structure.
- **Breaking**: No breaking change is intended for the documented Comfy
combo widget contract. This migration does not preserve incidental
PrimeVue `Select` prop pass-through from `widget.options`; that was a
side effect of wrapping PrimeVue rather than a stable widget API.
- **Dependencies**: No new dependencies.
## Review Focus
### Compatibility choices
The goal of this PR is a migration PR, not a broad behavior redesign.
The new implementation keeps the Comfy-specific combo contract rather
than attempting to emulate PrimeVue internals. In particular:
- `values` still accepts arrays and functions, and function values are
re-read on open to support dynamic/custom node option sources.
- `getOptionLabel(value) || value` is intentionally preserved to match
the sibling dropdown path and avoid turning an empty-string label into a
blank rendered option.
- Invalid/current values that are not present in the option list are
still rendered in the trigger instead of disappearing.
- `WidgetWithControl` continues to render its default slot in the
control area, with trigger text truncation preserved.
- App Mode `OverlayAppendToKey='body'` continues to map to a body portal
to avoid panel clipping.
### Visual alignment and screenshot updates
The previous PrimeVue implementation passed `size="small"`, which
injected internal `.p-select-sm .p-select-label` styling. That internal
PrimeVue style used its own small-select font size and padding,
overriding the surrounding widget sizing intent and making the select
trigger subtly taller with slightly larger text than nearby inline node
widget controls.
The Reka implementation intentionally keeps the normal widget styling
path instead of recreating that PrimeVue-specific internal override.
This means the trigger follows the same inline widget sizing direction
as neighboring controls rather than preserving the incidental PrimeVue
height/text-size delta. Because this is an expected visual difference
from the migration, the affected E2E screenshots should be recaptured
instead of treating the old PrimeVue select height as the target.
### Scrollbar and focus behavior
Reka provides the combobox/listbox semantics we want, including search,
arrow navigation, highlighted items, and Enter selection. The tricky
part is the canvas dropdown scrollbar behavior. The native Reka viewport
path hides/owns scrollbar behavior in a way that made it hard to
preserve the previous widget dropdown affordances, especially visible
scrollbars and mouse wheel capture over the node canvas.
To keep the previous behavior, this PR renders a dedicated scrollable
viewport inside `ComboboxContent` with the project scrollbar utilities
(`scrollbar-thin`, stable gutter, transparent track). That preserves
visible scroll affordance and allows wheel events over the dropdown to
scroll the list instead of zooming the canvas.
There was one Reka interaction to account for: pressing the native
scrollbar can be treated as a focus-outside event from the search input,
which previously closed the dropdown on mouse down or caused subsequent
wheel events to leak back to the canvas. The new
`useRestoreFocusOnViewportPointer` composable handles only that short
pointer gesture:
- viewport pointerdown marks a short-lived scrollbar/viewport
interaction,
- the next focus-outside event is prevented only if the search input can
be restored,
- the guard is cleared by `pointerup`, `pointercancel`, and a timeout so
normal outside clicks still close the dropdown.
### Tests and regression coverage
Unit coverage was updated around the new Reka implementation:
- option sources from arrays and functions,
- dynamic values refreshed on open but not on each search keystroke,
- selection updates and blank/undefined Reka emissions being ignored,
- search filtering and Reka keyboard selection behavior,
- disabled state, invalid current values, `getOptionLabel`, empty
results status, and WidgetWithControl slot preservation,
- composable coverage for pointerup, pointercancel, repeated pointerdown
listener cleanup, and no-input/no-op behavior.
Browser regression coverage now checks the canvas-specific interaction
surface:
- opening and selecting default combo widget options,
- wheel over the dropdown scrolls the list instead of zooming the
canvas,
- pressing the scrollbar does not close the dropdown,
- wheel capture still works after pressing the scrollbar,
- opening another node widget closes the previous dropdown,
- switching between node widgets preserves dropdown scroll capture,
- serialize/reload retains selected combo values.
## Screenshots (if applicable)
New
<img width="527" height="753" alt="스크린샷 2026-05-18 오전 1 36 27"
src="https://github.com/user-attachments/assets/2293d510-6965-4b84-9b12-b8528f8a734f"
/>
Old
<img width="496" height="473" alt="스크린샷 2026-05-18 오전 1 35 57"
src="https://github.com/user-attachments/assets/47c0e28a-27df-44a6-81a8-14fcc1f3bd8f"
/>
Reka Supports Auto highlight top item on search (Search -> Enter ->
Select 👍)
https://github.com/user-attachments/assets/9d633dfc-c23a-4e7a-8d39-b044c219f1f3
The default combo widget trigger has a small intentional visual delta
from the old PrimeVue path because the Reka implementation does not
recreate PrimeVue's internal `size="small"` label override.
https://github.com/user-attachments/assets/a9053a14-e39e-4d5e-a846-dcf9aeb0caed
## Validation
- `pnpm format`
- `pnpm lint` (passes; existing warning-only lint output remains in
unrelated tests)
- `pnpm typecheck`
- `pnpm typecheck:browser`
- `pnpm test:unit`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://127.0.0.1:5174 pnpm
exec playwright test
browser_tests/tests/vueNodes/widgets/combo/comboWidget.spec.ts
--project=chromium`
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12288-refactor-migrate-default-combo-widget-select-to-Reka-3616d73d365081fd8742c038a7dc7851)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
## Summary
When a workflow queues a 3D job (Preview3D / Load3D / SaveGLB) and the
user switches to another tab before the job completes, the WS `executed`
payload was silently dropped: `getNodeByExecutionId(app.rootGraph, …)`
resolves against the currently-visible workflow's graph, so the node
isn't found, `onExecuted` never fires, and the workflow JSON never
learns the path of the asset the backend just saved.
Worse, `setNodeOutputsByExecutionId` for top-level execution ids returns
the id verbatim without consulting rootGraph, so the old handler also
wrote the payload into the *active* (wrong) workflow's
`app.nodeOutputs`, polluting it and getting mis-attributed by
`ChangeTracker` snapshot/restore on subsequent tab switches.
This change introduces a single routing point — no new public API, no
new in-memory cache layer — that re-uses three pieces of infrastructure
that already exist independently:
1. `executionStore.jobIdToSessionWorkflowPath` already records which
workflow queued each prompt_id.
2. Each `ComfyWorkflow.ChangeTracker` already snapshots `nodeOutputs` on
`deactivate()` and replays it through the `app.nodeOutputs = …` setter
on `restore()`, which fires `onNodeOutputsUpdated` for every extension.
3. Audio nodes already implement `onNodeOutputsUpdated` to rehydrate
their widget from `nodeOutputs`. They have always been silently broken
in this same scenario (no one noticed because missing audio is less
visible than a missing 3D model); the fix here repairs audio for free as
a side effect.
The new behaviour:
- `app.ts` looks up the node against the active rootGraph *first*. Only
when found do we touch `nodeOutputStore` and call `node.onExecuted`
(existing path, unchanged). Moving the `nodeOutputStore` write inside
`if (node)` is the part that eliminates the cross-workflow pollution
above.
- When the node is not in the active rootGraph, the new
`routeExecutedToInactiveOwner` helper finds the owner workflow via
`jobIdToSessionWorkflowPath`, then writes the payload directly into
`owner.changeTracker.nodeOutputs` — the same field that `restore()`
already drains on tab switch back.
- Preview3D / SaveGLB add `onNodeOutputsUpdated` (mirroring the pattern
audio uses), normalise the path, write `node.properties['Last Time Model
File']` (already workflow-JSON-serialised), and apply it to the viewer.
The legacy `node.onExecuted` chain stays intact for live active-workflow
executions — both paths are idempotent.
## Screenshots (if applicable)
Before
https://github.com/user-attachments/assets/3803af1f-2eb6-41af-87ed-ac885a2eaad6
After
https://github.com/user-attachments/assets/72e1bed9-5f94-414d-ac31-fc925651d11b
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12295-FE-416-route-3D-node-outputs-to-owner-workflow-on-tab-switch-3616d73d3650817b908de48a32b1d6bd)
by [Unito](https://www.unito.io)
