## Summary
Right-clicking a bypassed node showed two bypass-related items in the
Vue "More Options" context menu (FE-720):
- Plain `Bypass` from the legacy LiteGraph `getExtraMenuOptions` hook in
`litegraphService.ts`
- `Remove Bypass` (with `Ctrl+B` and an icon) from the Vue
`getBypassOption` composable
The Vue menu's exact-label deduplicator in `contextMenuConverter.ts`
collapsed the unbypassed case (both emit `Bypass` → Vue source wins) but
not the bypassed case (`Bypass` vs `Remove Bypass`), so the duplicate
leaked through whenever the node was bypassed.
### before
<img width="1920" height="958" alt="fe-720-before"
src="https://github.com/user-attachments/assets/ef001aca-d70e-4798-ac61-01cc34c31e44"
/>
### after
<img width="1920" height="958" alt="fe-720-after"
src="https://github.com/user-attachments/assets/d6d2bf4b-cb98-4b30-9dac-9bd4b68a7e36"
/>
#### single active node (KSampler)
<img width="1920" height="958" alt="fe-720-1-unbypassed-node-menu"
src="https://github.com/user-attachments/assets/bec9cd47-2f2d-4adb-b95b-266e7969a36c"
/>
#### single bypassed node (Load Checkpoint)
<img width="1920" height="958" alt="fe-720-2-bypassed-node-menu"
src="https://github.com/user-attachments/assets/91f80157-836d-4fce-adad-474f31baff04"
/>
#### KSampler + bypassed Load Checkpoint
<img width="1920" height="958" alt="fe-720-3-mixed-selection-menu"
src="https://github.com/user-attachments/assets/e4780b16-08e5-4f87-80e9-3ff65a5acdae"
/>
## Root cause
`src/services/litegraphService.ts` pushes a `Bypass` entry from its
legacy `getExtraMenuOptions` hook in addition to the Vue
`getBypassOption`. In Vue-menu mode both reach the menu; the exact-label
dedup in `contextMenuConverter.ts` only collapses them when the labels
match, which fails once the node is bypassed and the Vue side switches
to `Remove Bypass`.
## Fix
Add `Bypass` and `Remove Bypass` to the `HARD_BLACKLIST` in
`contextMenuConverter.ts`. The blacklist filters the legacy emission out
of the Vue conversion pipeline (`convertContextMenuToOptions`) before it
is ever merged, so Vue's `getBypassOption` is the single source of the
bypass item in every node state — no duplicate is created in the first
place. This is the established convention for legacy items that the Vue
menu replaces (`Properties`, `Colors`, `Shapes`, `Title`, `Mode`,
`Properties Panel`, `Copy (Clipspace)`); Bypass is the same category.
`litegraphService.ts` reverts to a plain `content: 'Bypass'` and no
longer imports `areAllSelectedNodesInMode` or i18n keys for this entry.
The Vue `getBypassOption` label is still derived from the same
selection-aware predicate (`areAllSelectedNodesInMode`) that
`toggleSelectedNodesMode` uses, so on mixed selections the label stays
in sync with the action — it shows `Bypass` when clicking would bypass
the rest, rather than `Remove Bypass`.
**Trade-off:** the classic LiteGraph canvas menu
(`Comfy.VueNodes.Enabled: false`) renders `litegraphService`'s options
directly without going through `convertContextMenuToOptions`, so it
shows a plain `Bypass` regardless of node state. This matches the pre-PR
behavior (the legacy push was already a hardcoded `Bypass`), so it is
not a regression.
## Considered and rejected
- **`equivalents` map** (`bypass: ['bypass', 'remove bypass']`) — would
collapse `Bypass` and `Remove Bypass` as synonyms, which is semantically
wrong: they are distinct actions that must stay distinguishable, and the
rule would also misfire on the unbypassed case. A converter test locks
in that they are not treated as equivalents.
- **State-aware label on the legacy push** (matching the Vue label so
the exact-label dedup collapses them) — works, and additionally gives
the classic canvas menu a state-aware label, but it couples
`litegraphService` to the selection predicate and i18n keys solely to
keep a downstream dedup load-bearing. `HARD_BLACKLIST` removes the
duplicate at the source instead of creating, converting, then collapsing
it. The only thing lost is the classic-menu state-aware label, which was
never present pre-PR.
- **Gating the legacy push on `Comfy.UseNewMenu === 'Disabled'`** — the
setting that selects the legacy vs Vue context menu is
`Comfy.VueNodes.Enabled`, not `Comfy.UseNewMenu` (an unrelated
top-menu-bar toggle). Gating on `UseNewMenu` would drop the Bypass entry
from the legacy canvas menu for the OSS default (`VueNodes.Enabled:
false` + `UseNewMenu: 'Top'`).
- **Suppressing the legacy callback via
`SUPPRESSED_LITEGRAPH_CALLBACKS`** — matches by callback identity and
adds cross-file coupling for what is a simple label-based filter that
`HARD_BLACKLIST` already expresses.
## Cleanups (review feedback)
- Removed the now-dead `NodeSelectionState.bypassed` field and its
producer (no consumers after the label switch).
- Replaced the `vue-i18n` mock in `useNodeMenuOptions.test.ts` with a
real `createI18n` instance per `docs/testing/vitest-patterns.md`;
removed a `ts-expect-error` via a typed hoisted `app` mock.
- Simplified `getSelectedNodeArray` to
`Object.values(app.canvas.selected_nodes ?? {})`.
## Tests
- `useSelectedLiteGraphItems.test.ts` — `areAllSelectedNodesInMode`:
all-bypassed → true, mixed → false, empty → false.
- `useNodeMenuOptions.test.ts` — Vue label is `Bypass` (active / mixed)
and `Remove Bypass` (all bypassed).
- `contextMenuConverter.test.ts` — the legacy `Bypass` push is filtered
by `HARD_BLACKLIST` so the Vue item is the only bypass entry (keeps
shortcut/source); `Bypass` and `Remove Bypass` are not treated as label
equivalents.
- `browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts` —
e2e regression: exactly one bypass-family item per node state.
Verified live on a bypassed Load Checkpoint: single `Remove Bypass` →
toggle un-bypasses → single `Bypass`; no duplicate, rest of the menu
intact.
- Fixes FE-720
---------
Co-authored-by: Alexander Brown <drjkl@comfy.org>
When converting to subgraph, `beforeChange` and `afterChange` were being
called, but these functions exclusively called vestigial change handlers
that don't actually affect change tracking.
Consequentially, if you made a change to the graph (updating a widget),
converted a node to a subgraph using the selectionToolbox, and then
pushed Ctrl+Z before performing any other canvas interaction, it would
incorrectly undo the prior widget edit as well.
This is resolved by calling the important handlers directly. Adding them
to `beforeChange`/`afterChange` was considered, but caused breakage in
other functions (`connect`) which failed to even attempt symmetric calls
of the function.
## Summary
Silence false Iconify warning spam during Tailwind lint analysis while
preserving icon utility generation and real failure signals.
## Changes
- **What**: Local wrapper around `@iconify/tailwind4` that swallows the
noise from `better-tailwindcss/enforce-canonical-classes` probes.
Wrapper delegates icon resolution to upstream's `getDynamicCSSRules`;
only custom-comfy folder loading is reimplemented. `lucideStrokePlugin`
uses a `InvalidIconProbeError` sentinel so missing icons / missing deps
still surface.
- **Dependencies**: None.
## Background
Both upstreams have refused to address this:
- `better-tailwindcss`
([schoero/eslint-plugin-better-tailwindcss#377](https://github.com/schoero/eslint-plugin-better-tailwindcss/issues/377),
open) — owner declines to filter third-party warnings; says it should be
fixed in iconify.
- `@iconify/tailwind4`
([iconify/iconify#357](https://github.com/iconify/iconify/issues/357),
closed) — maintainer defends the `console.warn` as intentional; *"not
something plugin has control over"*.
Trigger path: `enforce-canonical-classes` → Tailwind
`canonicalizeCandidates()` → iconify `matchComponents` handler throws on
non-icon strings → upstream catches + `console.warn`.
## Review Focus
- Inline `loadComfyIconSet` in `iconifyDynamicPlugin.js` skips
`@iconify/tools`' `parseColors → currentColor` normalization. Verified
the comfy SVGs already use `currentColor` directly; multi-color icons
(bria, bytedance) keep their explicit fills as expected.
- `lucideStrokePlugin.js` catch is now type-discriminated — only
`InvalidIconProbeError` returns `{}`; `Cannot load icon set` and `Cannot
find icon` re-throw.
---------
Co-authored-by: Amp <amp@ampcode.com>
## Problem
When a user is **already signed in** and a client (e.g. an MCP client)
starts the OAuth flow, the consent screen fails with:
> OAuth request failed. Please restart from the client app.
**Repro:** be signed in to Cloud, then trigger an OAuth authorization
from a client. Instead of the consent screen, the consent view renders
the generic error.
## Root cause
The consent challenge request (`GET /oauth/authorize`) is authenticated
by the Cloud **session cookie** — a distinct credential from the
Firebase client login that `isLoggedIn` reflects.
- The **post-login** resume path (`useOAuthPostLoginRedirect`) mints
that cookie via `createSessionOrThrow()` before navigating to consent.
- The **already-signed-in** path — the `cloud-login` / `cloud-signup`
`beforeEnter` guards in `onboardingCloudRoutes.ts`, which short-circuit
straight to consent when `isLoggedIn` is `true` — skipped that step.
With no session cookie the consent challenge request is unauthenticated,
so the server redirects it to login; `fetch` follows the redirect to an
HTML page, `response.json()` throws, and `OAuthConsentView` falls
through to `genericError`.
The `Comfy.Cloud.SessionCookie` extension's `createSession()` on
auth-resolve is fire-and-forget and races the redirect, so it doesn't
reliably cover this path.
## Fix
`oauthConsentRedirect()` now mints the session cookie
(`createSessionOrThrow()`) before redirecting an already-signed-in user
to consent — symmetric with the post-login path. Best-effort: on failure
it logs and still lands on the consent view so the user gets an
actionable message rather than a silently dropped flow.
## Test plan
- [x] New `onboardingCloudRoutes.test.ts`: resume → mints cookie then
routes to consent; no pending OAuth → `cloud-user-check` (no mint); mint
failure → still routes to consent so the view surfaces the error.
- [x] `vue-tsc` typecheck, eslint, oxlint clean.
- [ ] Manual: already signed in to Cloud, start OAuth from a client →
consent screen renders (no "OAuth request failed").
## Summary
Stamp App Mode telemetry with the properties needed to measure the App
Builder product metrics validly in PostHog. Three small, independent
enrichments on top of the App-Mode execution attribution.
## Changes
- **What**:
- `view_mode` on `execution_start` / `execution_success` /
`execution_error` (captured at queue time alongside `is_app_mode`). Lets
the North Star be `execution_success` where `view_mode='app'` — genuine
app runs, excluding `builder:arrange` builder-preview runs that bare
`is_app_mode` also counts.
- `workflow_id` on `app:workflow_saved` and `app:app_mode_opened`
(sources `workflow` / `template_url`) via a shared
`workflowTelemetryId()` helper; `storeJob` refactored onto it so save /
open / run events share one join key. Enables distinct-app counts,
activated apps (created → ≥1 successful run), and per-app quality.
- intrinsic `is_app` on `app:share_flow` `link_created` (from the
workflow's `initialMode`, not the share-time view) plus `workflow_id`;
`is_app` on `app:workflow_imported` / `opened` (from the loaded graph's
`extra.linearMode`). Enables virality by true app-ness and app-traffic
attribution.
- **Breaking**: none.
- **Dependencies**: none.
## Review Focus
- **The commits to review are the three after the foundation**:
`view_mode`, `workflow_id`, and `is_app`. The first commit in the diff
(`feat: attribute workflow executions to App Mode in telemetry`) is the
pre-existing foundation this builds on — its branch is not currently on
the remote, so this PR is based on `main` and carries it forward.
Reviewing per-commit is easiest.
- **Join-key consistency**: `workflowTelemetryId()` is the single
definition of the workflow id (`activeState.id ?? initialState.id`),
shared by the new save/open events and the existing execution events. A
divergence would silently break the created→run and opened→run joins.
Unit-tested.
- **Scope (YAGNI)**: `workflow_id` / `is_app` added only where a locked
metric consumes it — `share_flow` only on `link_created`;
`app_mode_opened` only on the `workflow` / `template_url` sources (not
`app_builder` / `keybind`).
- **No double serialize**: `app.ts` reuses a single
`rootGraph.serialize()` for both the `is_app` derivation and
`afterLoadNewGraph`.
Cloud-only (telemetry is tree-shaken from OSS builds). No UI changes.
---------
Co-authored-by: AustinMroz <austin@comfy.org>
Co-authored-by: GitHub Action <action@github.com>
## Summary
Responsive polish: stack mobile pricing plans, rework the cloud product
hero so the illustration and text fit at all viewports, shrink the local
product hero illustration below xl, and reduce the cloud banner font
size on mobile.
## Changes
- **What**:
- `pricing/PriceSection.vue`: replace mobile tab-toggle pattern with
stacked plan cards; drop the `activePlanIndex` ref and tab buttons.
- `product/cloud/HeroSection.vue`: rework layout so illustration and
text fit across breakpoints (md/lg sizing, spacing, max-widths).
- `product/local/HeroSection.vue`: shrink illustration container below
xl (`max-w-xs`/`md:max-w-sm`/`lg:max-w-md`).
- `product/shared/CloudBannerSection.vue`: smaller font on mobile
(`text-sm` instead of `text-lg`).
## Review Focus
- Mobile pricing UX: confirm stacked cards are preferable to the prior
tab-toggle for our target viewports.
- Cloud hero layout at md/lg/xl breakpoints — check for overflow or
awkward spacing.
## Screenshots (if applicable)
<!-- Add before/after screenshots of the pricing page (mobile) and
product hero pages. -->
---------
Co-authored-by: github-actions <github-actions@github.com>
## Summary
Hardens PostHog initialization in the cloud app and website with three
missing config options identified during PostHog QA audit.
## Changes
- **What**: Adds `person_profiles: 'identified_only'`, `cookie_domain:
'.comfy.org'`, and `before_send` PII redaction hook to both PostHog init
calls
- **person_profiles**: Prevents anonymous Person records being created
in PostHog for pre-auth visitors — only creates Person after
`identify()` is called (matches website config which already had this)
- **cookie_domain**: Enables cross-subdomain session stitching across
`app.comfy.org`, `docs.comfy.org`, `www.comfy.org` — without this, each
subdomain creates a separate PostHog session
- **before_send**: Last-line PII guard — strips `email`, `prompt`,
`user_email`, `$email` from event properties before transmission. Placed
after `...serverConfig` spread so it cannot be overridden by remote
config.
## Review Focus
- `before_send` runs before GeoIP enrichment — deliberately does NOT
drop `$ip` since country-code targeting needs it for the pricing
experiment
- `person_profiles: 'identified_only'` + anonymous flag evaluation:
PostHog confirmed this still allows flag eval via `$anon_distinct_id`;
stitched retroactively on `identify()`
- `serverConfig` spread is before the three new fields — these take
precedence over any remote config values (intentional for
security/correctness)
Closes MAR-233, MAR-235, MAR-236
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Miles <miles@comfy.org>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: nav <nav@mac.lan>
Co-authored-by: Miles Ryan <thedatalife@users.noreply.github.com>
## Summary
Upgrades the website app from Astro 5.10 to Astro 6.4.2.
## Changes
- **What**: Bumps `astro` (5.10 → 6.4.2), `@astrojs/vue` (5 → 6.0.1),
`@astrojs/sitemap` (3.7.1 → 3.7.3), and `@astrojs/check` (0.9.8 → 0.9.9)
in the workspace catalog. Refreshes `vite` in the lockfile from `8.0.0`
to `8.0.14` to clear a Vite/Rolldown "Not implemented" regression in dep
optimization that Astro v6 triggered on `8.0.0`.
- **Breaking**: None for our code. Audit against the [Astro v6 upgrade
guide](https://docs.astro.build/en/guides/upgrade-to/v6/) found no
breaking change applies:
- Already on `<ClientRouter />` (no removed `<ViewTransitions />`)
- No content collections (`defineCollection`/`getCollection`) — legacy
removal n/a
- `getStaticPaths` functions don't access the deprecated `Astro` global
- No `<Image>`/`<Picture>`/`getImage()` usage — SVG rasterization &
client-throw n/a
- No `import.meta.env.ASSETS_PREFIX`
- No file-extension endpoints (`*.xml.ts`/`*.json.ts`) so the
trailing-slash change is moot
- No markdown content (heading-ID compat n/a)
- i18n config (`prefixDefaultLocale: false`, no
`redirectToDefaultLocale`) is compatible with the new
`redirectToDefaultLocale: false` default
- No experimental flags enabled
## Review Focus
- The Vite lockfile bump from 8.0.0 → 8.0.14 inside the same `^8.0.0`
range. Without it, `astro check` on v6 hits `Not implemented` inside
Vite's `esbuildPlugin.generateBundle` via Rolldown.
- Sanity-check the audit above against any in-flight work that might add
content collections / `<Image>` etc.
## Verification
- `pnpm typecheck` → 0 errors
- `pnpm build` → 379 pages built
- `pnpm test:unit` → 85/85 passing
- `pnpm lint` → 0 errors (3 pre-existing warnings)
## Summary
cloud.comfy.org (and any route before the graph view mounts, e.g. the
login screen) renders no favicon. Add a static fallback so a favicon
always shows.
## Changes
- **What**: The favicon is only ever set at runtime —
`useFavicon('/assets/favicon.ico')` in `LayoutDefault.vue` and the
progress frames in `useProgressFavicon.ts`. Before that JS runs (first
paint, login screen, routes that don't mount
`GraphView`/`LayoutDefault`), no `<link rel="icon">` exists, and the
browser's default `/favicon.ico` request hits the SPA catch-all and
returns HTML, so no favicon renders. Added a single static `<link
rel="icon" href="/assets/favicon.ico">` (asset already exists) to
`index.html`.
- **Breaking**: none.
## Review Focus
- Single icon link is intentional: vueuse `useFavicon` overwrites the
`href` of every `link[rel*="icon"]` at runtime (including
`apple-touch-icon`), so the progress-favicon animation assumes one
controllable icon link. Adding multiple static links (svg/png) would get
their `href` clobbered to the progress PNG mid-generation and render
inconsistently — so this PR deliberately adds just the `.ico`.
- The progress-favicon still works: vueuse updates this same link's
`href`.
- Path matches what the app already uses at runtime
(`/assets/favicon.ico`).
## Screenshots (if applicable)
## Summary
Narrow Knip's Playwright `entry` to actual spec files so dead exports in
browser-test fixtures are reported instead of being hidden by treating
every helper as an entrypoint.
## Changes
- **What**:
- `knip.config.ts`: Playwright `entry` changed from the broad
`['**/*.@(spec|test)…', 'browser_tests/**/*.ts']` to
`['browser_tests/**/*.@(spec|test).?(c|m)[jt]s?(x)']`.
`globalSetup`/`globalTeardown` stay covered via Knip's playwright config
resolution; fixtures remain in the project graph so their unused exports
surface.
- Resolved the 54 dead findings this exposed: over-exported symbols used
only within their own module are now module-private (dropped `export`,
no behavioral change); genuinely unreferenced fixtures were deleted
(asset/template `ALL_*` aggregators + orphaned `STABLE_*` data,
`TemplateHelper` distribution helpers + `generateTemplates`, dead
types/utils, and the unused `nodeDefinitions.ts` module).
- **Breaking**: none — test-only changes.
## Review Focus
- Deletions are limited to fixtures with zero importers on `main`
(verified via `pnpm knip`); the bulk of the diff is `export`-keyword
removal.
- Verified: `pnpm knip` (browser_tests clean), `pnpm typecheck`, `pnpm
typecheck:browser`, oxfmt/oxlint/eslint all pass.
Linear: FE-717
## Summary
The "Open shared workflow" dialog rendered the workflow name in an
`<h2>` with no
wrapping control. A long, space-free name (e.g. a content-hash filename)
is a
single unbreakable "word", so with the default `overflow-wrap: normal`
it could
not wrap. It overflowed its box and, because PrimeVue's
`.p-dialog-content` is
`overflow-x: auto`, the dialog scrolled horizontally instead of
wrapping.
CDP measurement on the unfixed build (96-char name): dialog content
`scrollWidth 1336` vs `clientWidth 702` -> horizontal scroll. After
adding
`wrap-anywhere` to the heading: `scrollWidth 702 == clientWidth 702`,
name wraps
to multiple lines, full name still in the DOM.
### before
<img width="704" height="295" alt="before-dialog"
src="https://github.com/user-attachments/assets/ea05ab32-a80d-4210-951c-f43d595bd6eb"
/>
### after
<img width="704" height="359" alt="after-dialog"
src="https://github.com/user-attachments/assets/cbf3019e-5e71-4dba-a1fd-ea3586dd995a"
/>
## Changes
- `OpenSharedWorkflowDialogContent.vue`: add `wrap-anywhere` to the
workflow-name
`<h2>` so a long unbreakable name wraps within the dialog bounds instead
of
forcing horizontal scroll. The parent already has `min-w-0`.
- Breaking: none
## Red-Green Verification
| Commit | CI | Purpose |
|--------|-----|---------|
| [`test:`
d9158d1](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/26678596015)
| 🔴 Red (failure) | Proves the test catches the bug — cloud
e2e failed at `expect(scrollWidth).toBeLessThanOrEqual(clientWidth +
1)`: received 1332, expected <= 703 |
| [`fix:`
08e75a1](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/26678974321)
| 🟢 Green (success) | Proves the fix resolves it |
Fixes FE-828
## Test Plan
- [x] CI red on test-only commit
- [x] CI green on fix commit
- [x] E2E regression
`browser_tests/tests/dialogs/openSharedWorkflowDialog.spec.ts` (@cloud):
mocks a long-named shared workflow, asserts the dialog does not overflow
horizontally
- [x] Manual CDP verification (before/after above)
## Summary
Fix the comfy.org favicon declaration so Google Search can actually
refresh it — the logo has shown the old Astro starter chevron in Google
results for weeks even though the icon files were fixed in May.
## Changes
- **What**: The site declared two SVG favicons gated by
`prefers-color-scheme` media queries, no PNG favicon, and a
`favicon.ico` with only a 64×64 image. Google Search's favicon pipeline
can't resolve a single valid icon from that, so on recrawl it keeps the
last good cached icon (the old Astro chevron) instead of the current
mark. Replaced with the standard, checker-clean set:
- `favicon.svg` — single adaptive SVG (light/dark swap via internal CSS)
replacing the two media-query SVG links
- `favicon-96x96.png` — explicit desktop PNG at a Google-recommended
size (multiple of 48)
- `favicon.ico` — rebuilt to contain 16/32/48 (was 64×64 only)
- `site.webmanifest` + `web-app-manifest-192x192.png` / `-512x512.png`
- `<head>` simplified to RealFaviconGenerator's recommended markup +
`theme-color`
- **Breaking**: none. Pure static-asset + `<head>` change, no runtime
code touched.
## Review Focus
- This resolves the 3 errors + 3 warnings RealFaviconGenerator reported
for comfy.org (2 SVG favicons / no desktop PNG / ICO missing 16-32-48 /
no web manifest).
- The old `favicon-light.svg` and `favicon-dark.svg` are left in place
(now unreferenced) to avoid deleting assets in this PR — safe to remove
in a follow-up if desired.
- **Post-merge, required to actually fix Search**: in Google Search
Console, run URL Inspection → Request Indexing on `https://comfy.org/`.
The Search favicon only refreshes when Googlebot recrawls the homepage;
without a clean icon set + a recrawl nudge it will not self-correct.
## Before / After
### Before
<img width="939" height="388" alt="image"
src="https://github.com/user-attachments/assets/5fa95fd6-2248-4ed9-921e-9c516f4c0c3e"
/>
### After
<img width="1145" height="447" alt="image"
src="https://github.com/user-attachments/assets/dc33a99d-f7f6-41d4-a83d-03b3b99d0b0d"
/>
## Screenshots (if applicable)
## Summary
`extra.linearData.inputs` validation rejected the entire workflow
whenever a single entry didn't match the strict `z.union([3-tuple,
2-tuple])`, surfacing as `Failed to load shared workflow: invalid
workflow data` for some published cloud shares (e.g.
`share=21e32125c692`).
## Changes
- **What**: Shared workflow load now matches the regular load policy at
`scripts/app.ts:1191-1198` — if Zod validation fails, fall back to the
raw `workflow_json` instead of throwing. Share service no longer runs
schema validation directly; `app.loadGraphData()` continues to validate
and apply the same raw fallback under `Comfy.Validation.Workflows`.
- **Breaking**: None — workflows that previously failed to load through
the share path now load with the same permissive behavior as workflows
opened through any other entry point.
## Review Focus
The original approach added a `tolerantArray` combinator to drop bad
`linearData.inputs` entries inside the schema. After review, the cleaner
direction is to keep schemas strict as the canonical spec and apply
tolerance at the consumer boundary — which `scripts/app.ts:1191-1198`
already does for the regular load path (`graphData = validatedGraphData
?? graphData` with the comment "Ideally we should not block users from
loading the workflow"). This PR aligns the share path with that existing
policy and removes a cross-path inconsistency rather than introducing a
new schema-level concept.
Consequences:
- Schemas in `workflowSchema.ts` stay unchanged (canonical spec).
- The `extra.*` shape drift problem is now solved generally, not just
for `linearData.inputs` — future fields hitting the same class of issue
will load instead of blocking.
- `validateComfyWorkflow` still logs the Zod error via `console.warn`
for debugging.
## Tests
- Existing schema tests stay strict (3-tuple, 2-tuple unions reject bad
shapes).
- New regression test in `workflowShareService.test.ts`: a
`workflow_json` that passes the share response envelope
(`record<unknown>`) but fails `ComfyWorkflowJSON` is returned raw, not
thrown.
### current prod
<img width="1144" height="565" alt="Screenshot 2026-05-13 at 1 29 00 PM"
src="https://github.com/user-attachments/assets/b1abf45b-a588-4ef5-a9ec-d14bd1096b6d"
/>
### test
`/?share=21e32125c692`
<img width="765" height="826" alt="Screenshot 2026-05-13 at 1 27 40 PM"
src="https://github.com/user-attachments/assets/96a3c405-e5fe-4732-9047-fed90768e6f6"
/>
## Follow-up
Structural issue separately: cloud OpenAPI defines `workflow_json` as
opaque `z.record(z.unknown())`
(`packages/ingest-types/src/zod.gen.ts:137,325,375,397,457`), so ingest
schema CI cannot enforce inner shape. Matt Miller is driving the
lift-schema-to-core-OpenAPI direction; tracked separately, not in this
PR.
Fixes FE-690.
## Summary
Removes three security overrides added in #12345/#12501 that are now
no-ops: every consumer in the tree already pulls a version at or above
the GHSA-patched release.
## Changes
- **What**: Drops `brace-expansion@^1.0.0`, `brace-expansion@^2.0.0`,
and `ws@^8.0.0` overrides from
[pnpm-workspace.yaml](pnpm-workspace.yaml).
- **Breaking**: None.
| Override removed | Patched at | Natural resolution | GHSA |
|---|---|---|---|
| `brace-expansion@^1.0.0: ^1.1.13` | 1.1.13 | 1.1.15 |
[GHSA-f886-m6hf-6m8v](https://github.com/advisories/GHSA-f886-m6hf-6m8v)
|
| `brace-expansion@^2.0.0: ^2.0.3` | 2.0.3 | 2.1.1 |
[GHSA-f886-m6hf-6m8v](https://github.com/advisories/GHSA-f886-m6hf-6m8v)
|
| `ws@^8.0.0: ^8.20.1` | 8.20.1 | 8.21.0 |
[GHSA-58qx-3vcg-4xpx](https://github.com/advisories/GHSA-58qx-3vcg-4xpx)
|
The remaining security overrides (`lodash`, `yaml`, `minimatch@^9`,
`minimatch@^10`, `ajv@^8`) were re-tested and are still required —
without them dependabot-vulnerable versions (`lodash@4.17.23`,
`yaml@2.7.1`, `minimatch@9.0.1`, `minimatch@10.2.1`,
`ajv@8.12.0`/`8.13.0`) resolve into the tree.
## Review Focus
- The `brace-expansion@5.x` and `minimatch@5.x`/`8.x` lines in the
lockfile naturally resolve to safe versions (`5.0.6`, `5.1.9`, `8.0.7`)
on `main`, so no new overrides are needed alongside this removal.
- Verified by removing each override one-by-one and inspecting `pnpm why
-r`.
## Verification
- `pnpm install` — clean
- `pnpm typecheck` (apps/website + root) → 0 errors
- `pnpm test:unit` (apps/website) → 89/89 passing
- `pnpm lint` (root) → 0 errors (3 pre-existing warnings)
## Summary
- Add `<br>` after "Built something cool with ComfyUI?" in the gallery
contact heading (en + zh-CN) so the Submit link wraps to a new line.
- Switch the Enterprise product card background from
`bg-illustration-forest` to `bg-secondary-cool-gray`.
## Test plan
- [ ] Visit the gallery contact section and confirm the heading wraps
after "ComfyUI?" in both English and Chinese.
- [ ] Verify the Enterprise card on the product cards section renders
with the cool gray background.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary
Establish a single source of truth for gallery items. Previously the
items array and the `GalleryItem` interface lived inline in
`GallerySection.vue`, with three sibling components importing the type
*out of a `.vue` file* — coupling pure data and types to a presentation
component.
- **One canonical list.** Items now live in `src/data/gallery.ts`
alongside the `GalleryItem` interface.
- **Per-item visibility without deletion.** New optional `visible?:
boolean` field on each item
- **Stable identity.** Each item now carries a required `id` kebab-slug
derived from its title.
- **lookup helper.** `getGalleryItemById(id)` for callers that want a
single item.
The sibling gallery components (`GalleryCard`, `GalleryDetailModal`,
`GalleryItemAttribution`) now import the `GalleryItem` type from
`../../data/gallery` instead of from the `.vue` file.
## Scope note
I also audited the rest of the website for other surfaces that should
consume the centralized data.
**I need this refactor for the new page `models` coming soon.**
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
## Summary
Preserve validation node errors and their overlay when a valid active
root starts execution, so partial workflow runs no longer hide
validation failures.
## Changes
- **What**: Split execution-start clearing from full error clearing;
`execution_start` now clears transient execution/prompt state without
clearing validation `lastNodeErrors`.
- **What**: Keep the ErrorOverlay open when validation errors are still
present, and show it for successful prompt responses that include
`node_errors`.
- **Dependencies**: None.
## Review Focus
Please check the error-clearing boundary between prompt
submission/workflow changes and WebSocket `execution_start`. Full
clearing still happens through `clearAllErrors`; execution start now
uses the narrower clearing path and only dismisses the overlay when
there are no validation node errors to show.
Linear: FE-851
## Red-Green Verification
- Red: `76bcf34c4 test: add failing validation error preservation e2e`
- Green: `9766172ea fix: preserve validation errors on execution start`
- Follow-up: `321c95aba fix: keep validation error overlay during
execution start`
- Coverage: `7b5fab577 test: cover prompt node error overlay`
## Test Plan
- `pnpm exec vitest run src/scripts/app.test.ts`
- `pnpm exec vitest run src/stores/executionStore.test.ts`
- `pnpm exec vitest run src/scripts/app.test.ts
src/stores/executionStore.test.ts --coverage`
- `pnpm format:check -- src/stores/executionErrorStore.ts
src/stores/executionStore.ts src/stores/executionStore.test.ts
src/scripts/app.ts src/scripts/app.test.ts
browser_tests/fixtures/helpers/ExecutionHelper.ts
browser_tests/tests/execution.spec.ts`
- `pnpm exec oxlint src/stores/executionErrorStore.ts
src/stores/executionStore.ts src/stores/executionStore.test.ts
src/scripts/app.ts src/scripts/app.test.ts
browser_tests/tests/execution.spec.ts --type-aware`
- `pnpm typecheck`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://127.0.0.1:5175 pnpm
exec playwright test browser_tests/tests/execution.spec.ts:132`
## Screenshots (Before/After)
Before
https://github.com/user-attachments/assets/04a212b6-66f9-4c77-9056-58bdc642d96e
After
https://github.com/user-attachments/assets/db7813c7-bf8a-4e19-9b66-7f49fd01c305
Under some circumstances (Only firefox+FLAC outputs for me, but reliably
reproducible), clicking the play button on audio outputs in the assets
sidebar tab will fail to start playback. This appears to be caused by
unusual interactions between blob urls, `preload="metadata"`, and FLAC
not defining total content length in the header.
Instead of managing the lifecycle of a blob url, the real audio source
is left in place and caching can be done on the browser side.
I put some extensive time into trying to find a regression test that
works on chromium, but did not see results and decided it's better this
be merged without a test than never get fixed.
Context menu operations on subgraph IO slots only set the foreground
canvas as dirty, so links would visually persist until a different
operation caused a background draw.
## Summary
This PR extends the error catalog with targeted runtime error messaging
for common workflow execution failures.
The goal is to show clearer, user-facing copy in the error panel while
preserving the raw API `message` and `details` fields on each error
item. The resolver continues to add display-only fields (`displayTitle`,
`displayMessage`, `displayDetails`, `displayItemLabel`, `toastTitle`,
`toastMessage`) so the UI can render friendlier messages without
changing the underlying error contract.
## What This PR Targets
This PR adds catalog handling for common runtime failure categories,
including:
- General execution failure fallback for uncataloged node runtime errors
- Content blocked / safety check failures
- Credits, subscription, access, and sign-in related failures
- Rate limit and busy-server style failures
- Runtime timeout, stalled generation, and preparation failure cases
- Server stopped / unavailable cases
- Out-of-memory failures
- Image load failures
- Invalid CLIP input failures
- Invalid prompt and invalid workflow request failures
- Request/start/end failure cases
- Model access and model download failures
- Unexpected service failures
Unknown node execution errors resolve to the general runtime fallback:
- Title: `Execution failed`
- Message: `Node threw an error during execution.`
- Item label: node name
- Toast title: `{nodeName} failed`
- Toast message: `This node threw an error during execution. Check its
inputs or try a different configuration.`
## Implementation Notes
### Resolver Split
The previous resolver file was doing too much, so this PR splits
source-specific catalog logic into smaller modules:
- `catalogIds.ts` defines FE-resolved catalog IDs that normalize
multiple sources or do not map 1:1 to an API error type.
- `catalogI18n.ts` owns shared translation/fallback helpers.
- `validationErrorResolver.ts` keeps validation catalog logic.
- `promptErrorResolver.ts` keeps prompt-specific catalog logic and
handles non-node-scoped failures before falling back to prompt-specific
keys.
- `executionErrorResolver.ts` handles node-scoped runtime errors.
- `runtimeErrorMatcher.ts` owns conservative runtime error matching.
- `runtimeErrorCopy.ts` builds runtime display/toast fields from catalog
IDs and fallback copy.
- `missingErrorResolver.ts` keeps the existing missing-model/node/media
group display copy.
`errorMessageResolver.ts` is now a thin facade over these smaller
resolvers.
### Conservative Matching
Runtime errors can share generic exception labels, so this PR keeps
matching narrow: exact strings or stable prefixes only. The matcher is
ordered, and the first matching rule wins, so specific user-actionable
failures are checked before broader fallbacks.
The matching is intentionally not fuzzy. For example, arbitrary messages
that merely mention moderation terminology are not treated as
content-blocked unless they match one of the known client-visible
failure strings. This avoids false positives while covering the targeted
high-volume cases.
### Raw Detail Preservation
Some cataloged errors keep the original message as `displayDetails` when
it contains useful troubleshooting context. This is display-only; the
raw API `message` and `details` fields remain unchanged on the original
error item.
## Out Of Scope / Follow-Ups
This PR does not redesign the error overlay or right-side error panel
UI. It only provides the resolved fields those surfaces can consume.
Planned follow-up work remains:
- Regroup execution errors by message/catalog type where appropriate
- Error overlay copy/layout updates for single vs multiple errors
- Right panel visual design updates
- Rendering `displayItemLabel` in the revised panel design
- More specific UI actions for auth, retry, and similar flows
- Broader fallthrough observability if we decide it is needed
Non-English locale files are intentionally not updated here. The
repository uses `src/locales/en/main.json` as the source of truth and
the release i18n sync flow generates the other locale files.
## Validation
Ran successfully:
- `pnpm format`
- `pnpm lint:unstaged`
- `pnpm typecheck`
- `pnpm test:unit`
- `pnpm vitest run
src/platform/errorCatalog/errorMessageResolver.test.ts`
- `pnpm vitest run
src/platform/errorCatalog/errorMessageResolver.test.ts
src/components/rightSidePanel/errors/useErrorGroups.test.ts`
- `pnpm build`
- `pnpm knip`
Notes:
- `pnpm build` still prints existing asset/icon and dynamic import
warnings, but exits successfully.
- `pnpm knip` exits successfully and still reports the existing
`flac.ts` tag hint.
## Screenshots (Before / After)
[Diff.zip](https://github.com/user-attachments/files/28300639/Diff.zip)
Uploaded as a ZIP file due to the image upload limit
## Summary
https://linear.app/comfyorg/issue/FE-867/bug-node-expand-menu-doesnt-work-nodes-immediately-collapse-after
Recreates #12175 on a fresh `main` base (original branch's CI failed
only because its `frontend-dist` artifact had expired — not a code
issue). Original work by @christian-byrne / Glary-Bot, cherry-picked
here so it can land while he's offline.
The Vue right-click "More Options" node menu shows duplicates for
collapse/expand functionality:
- **Vue source**: `Minimize Node` / `Expand Node` (works)
- **LiteGraph source**: `Resize`, `Collapse`, `Expand` (silently no-op
in this menu — the converter wrapper invokes
`LGraphCanvas.onMenuNodeCollapse` without the `node` arg it expects)
Suppress the LiteGraph duplicates in `convertContextMenuToOptions` by
matching the built-in **callback identity**
(`LGraphCanvas.onMenuResizeNode`, `LGraphCanvas.onMenuNodeCollapse`),
not the raw label. Matching by identity avoids accidentally hiding
extension-provided items that share those labels.
Also align `CORE_MENU_ITEMS` / `MENU_ORDER` on the Vue label `Expand
Node` so the toggled Minimize/Expand pair sorts correctly.
## Scope of suppression
Only the Vue node menu (via `convertContextMenuToOptions`) is affected.
The raw `LGraphCanvas.getNodeMenuOptions` output is untouched, so:
- The legacy right-click menu (`Comfy.UseNewMenu` disabled) still has
`Collapse` / `Resize`.
- `useLoad3d.ts`, which calls `new
LiteGraph.ContextMenu(app.canvas.getNodeMenuOptions(node), ...)`, is
unaffected.
- Extensions that monkey-patch `getNodeMenuOptions` continue to receive
the full option list.
## Tests
- `contextMenuConverter.test.ts`: covers both that built-in entries are
dropped by identity AND that extension-provided items with the same
labels survive.
- E2E `selectionToolboxMoreActions.spec.ts`: asserts the Vue "More
Options" menu shows `Minimize Node` but no `Resize`/`Collapse`/`Expand`.
- `pnpm typecheck` clean.
Supersedes #12175.
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
## Summary
1. Drop multi-object identity fields (uuid, name, type) since
multi-object support is not yet in scope, these are renderer-side
identifiers with no link back to the ComfyUI asset.
2. Drop rotation and matrix as redundant encodings of the same
transform, applying the same Jacob-redundancy point used on camera_info
3. Rename ModelTransform -> Model3DTransform and ModelInfo ->
Model3DInfo to align with the existing Load3D / File3D / model_3d naming
and disambiguate from AI 'model' (per Alexis).
4. The output key also moves from model_info to model_3d_info to match.
## Summary
Refactors middle mouse button pan handling around the intent of #11409,
dropping the outdated implementation details from that PR and aligning
the core behavior with the current main branch.
## Changes
- **What**: Centralized phase-specific middle mouse button handling in
`src/base/pointerUtils.ts`, added a shared Vue widget forwarding helper,
and updated canvas, LiteGraph, Vue node, and mask editor call sites to
use the same semantics.
- **Breaking**: None expected. This keeps existing middle-click pan
behavior while making pointerdown, pointermove, pointerup, and auxclick
checks explicit for their event phases.
- **Dependencies**: None.
## Review Focus
This PR is intentionally narrower than #11409. That PR had the right
goal, but its implementation became outdated against main: mask editor
tests now have helper coverage on main, Vue node/widget code has
shifted, and a blanket replacement with `isMiddlePointerInput` would
lose the bitmask behavior needed during pointermove drags.
The core difference is that this PR preserves the useful part of #11409,
namely removing scattered ad-hoc MMB checks, while avoiding stale
changes that no longer fit the current codebase.
Key behavior changes:
- `isMiddlePointerInput` is the conservative pointerdown-style check:
changed middle button or strict middle-only `buttons === 4`.
- `isMiddleButtonHeld` handles pointermove-style held-button bitmasks so
chorded drags with the middle button still pan.
- `isMiddleButtonEvent` handles pointerup/auxclick-style changed-button
events.
- Call sites now choose the phase-specific helper directly instead of
routing through an event-type dispatcher.
- String and markdown widgets now share
`forwardMiddleButtonToCanvas(...)` instead of duplicating three pointer
listeners each.
- The widget helper intentionally keeps the existing
`app.canvas.processMouseDown/Move/Up` forwarding route and only
centralizes the duplicated listener logic.
- Mask editor pan handling, Vue node pointer forwarding, graph canvas
pan forwarding, LiteGraph middle-click checks, input indicators, and
transform settling now use the centralized helpers.
Coverage added or updated:
- Unit coverage for middle-button helper semantics, including chorded
pointermove drags and pointercancel held-bit behavior.
- Unit coverage for widget forwarding helper down/move/up routing.
- Regression coverage for canvas, mask editor, Vue node media preview,
and transform-settling pointer handling.
- Browser coverage for middle-click drag panning on a Vue node, a
multiline string widget, and the mask editor canvas.
Validation run:
- `pnpm format`
- `pnpm lint`
- `pnpm typecheck`
- `pnpm test:unit src/base/pointerUtils.test.ts
src/renderer/extensions/vueNodes/widgets/utils/forwardMiddleButtonToCanvas.test.ts
src/renderer/extensions/vueNodes/widgets/composables/useStringWidget.test.ts
src/renderer/extensions/vueNodes/widgets/composables/useMarkdownWidget.test.ts
src/renderer/core/canvas/useCanvasInteractions.test.ts
src/composables/maskeditor/useToolManager.test.ts
src/renderer/core/layout/transform/useTransformSettling.test.ts
src/composables/node/useNodeImage.test.ts
src/composables/node/useNodeAnimatedImage.test.ts
src/components/graph/SelectionToolbox.test.ts
src/lib/litegraph/src/LGraphCanvas.slotHitDetection.test.ts`
- `pnpm typecheck:browser`
- `pnpm test:browser:local
browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts
browser_tests/tests/vueNodes/widgets/text/multilineStringWidget.spec.ts
browser_tests/tests/maskEditor.spec.ts --project chromium --grep
"Middle-click drag"`
- Commit hook: staged file format/lint, `pnpm typecheck`
## Screenshots (if applicable)
Not applicable; this is interaction behavior covered by unit and browser
tests.
## Summary
Upgrade ESLint from v9 to v10.4.0, bump compatible plugins/configs, and
fix the 32 violations introduced by the new `eslint:recommended` rules.
## Changes
- **What**:
- Catalog bumps in `pnpm-workspace.yaml`:
- `eslint` 9.39.1 → 10.4.0
- `@eslint/js` 9.39.1 → 10.0.1
- `typescript-eslint` 8.49.0 → 8.60.0
- `eslint-plugin-vue` 10.6.2 → 10.9.1
- `eslint-plugin-import-x` 4.16.1 → 4.16.2
- `eslint-plugin-unused-imports` 4.3.0 → 4.4.1
- `@intlify/eslint-plugin-vue-i18n` 4.1.1 → 4.5.0
- Fixed 4 `preserve-caught-error` violations by passing `{ cause }` to
rethrown errors in `useMaskEditorLoader.ts` and `usePainter.ts`.
- Fixed 28 `no-useless-assignment` violations across litegraph
(`LGraph.ts`, `LGraphCanvas.ts`, `LiteGraphGlobal.ts`, `polyfills.ts`)
and seven other files (`colorUtil.ts`, `linkFixer.ts`, `usePaste.ts`,
`audioService.ts`, `versionUtil.ts`, `subscriptionCheckoutTracker.ts`,
`ShiftClick.test.ts`) by removing dead initializers or redundant writes.
- Removed two now-unnecessary `@ts-expect-error` directives on
`importX.flatConfigs.*` (newer plugin ships correct types).
- **Breaking**: None for runtime; consumers using ESLint locally must
use Node ≥20.19/22.13/24 (already required by repo `engines.node:
">=25"`).
## Review Focus
- Audit of the [v10 migration
guide](https://eslint.org/docs/latest/use/migrate-to-10.0.0) found no
other patterns in this codebase to address (no `eslint-env` comments, no
removed `context.*`/`SourceCode` APIs, no `RuleTester` usage, no
affected rule configs).
- `no-useless-assignment` fixes were chosen conservatively: prefer
keeping the variable with a typed declaration over deleting the
statement, except where the assignment was clearly dead after the last
read.
- Per-file diffs in litegraph (especially `LGraphCanvas.ts`) — worth a
glance to confirm intent.
## Verification
- `pnpm exec eslint --version` → v10.4.0
- `pnpm exec eslint src` → 0 errors
- `pnpm typecheck` → clean
- Unit tests for every touched file pass (319/319 in spot checks:
`colorUtil`, `linkFixer`, `audioService`, `usePaste`, `usePainter`,
`versionUtil`, `LGraph`, `litegraph`, all `LGraphCanvas.*`,
`ShiftClick`).
Co-authored-by: Amp <amp@ampcode.com>
*PR Created by the Glary-Bot Agent*
---
## Problem
The GitHub star badge silently disappears from the comfy.org navigation.
Verified by curl-ing the live homepage:
```
props="{..."github-stars":[0,""]}"
```
`SiteNav.vue` only renders the badge when `githubStars` is truthy, so an
empty string hides it.
## Root cause
`apps/website/src/layouts/BaseLayout.astro` `await`s
`fetchGitHubStars('Comfy-Org', 'ComfyUI')` in its frontmatter. Astro
evaluates layout frontmatter **per rendered page** in SSG. With 379
pages (46 source `.astro` files × locales/dynamic routes), the
unauthenticated GitHub REST endpoint is called hundreds of times per
build, blasting past the 60 req/h anonymous rate limit. Once GitHub
returns 403 the existing `try/catch` returns `null`, `githubStars`
becomes `''`, and the badge vanishes — with no log line to indicate why.
## Fix
Cache the in-flight promise in a module-scope `Map` keyed by
`${owner}/${repo}` so every page in a single build shares one request.
Already-resolved counts stay cached, and the existing
`WEBSITE_GITHUB_STARS_OVERRIDE` env-var escape hatch still
short-circuits first.
While in the file:
- Pass an injectable `fetchImpl` so tests can stub without
`vi.spyOn(globalThis, 'fetch')`.
- Replace the implicit-`any` `data.stargazers_count ?? null` with a
narrow `readStargazerCount(data: unknown)` guard.
- In `BaseLayout.astro`, change `rawStars ? ...` to `rawStars !== null ?
...` so a hypothetical 0-star repo wouldn't be hidden (the old check
treated 0 as missing).
## Verification
- `pnpm --filter @comfyorg/website test:unit` → 89/89 pass (5 new test
cases: memoization, per-key isolation, non-2xx → null, throw → null,
override).
- `pnpm typecheck:website` → 0 errors.
- `pnpm format:check` → clean.
- `pnpm --filter @comfyorg/website build` → 379 pages built; with no
override set, output HTML contains `"github-stars":[0,"115K"]` (the live
count) on every page; with `WEBSITE_GITHUB_STARS_OVERRIDE=110000`, it
contains `"110K"` and `fetch` is never called.
- Playwright on the local preview confirms the badge renders at the
top-right of the nav with `aria-label="ComfyUI on GitHub — 110K stars"`.
## Scope
102 lines changed across 3 files (40 non-test). Deliberately leaves the
broader "snapshot fallback / build-data source" refactor to the existing
`codex/website-github-stars-once` branch — this PR just unblocks the
user-visible symptom.
## Screenshots
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
The "open tutorial" button only existed in the DOM when the template
card as actively hovered. For reasons I can not comprehend (probably
overzealous pointer handlers somewhere), the act of clicking on the
button would fire a mouseleave event. This caused the button to
disappear for the exact moment it was clicked alike to a mischievous
dondurma vendor.
This is resolved by keeping the button always in DOM, but making it
invisible when the card isn't hovered.
The PR also removes a deeply nested `v-bind='$attrs'`. I'm assuming it
must be a mistake that attributes applied to the entire template
selector dialogue would be bound to every deeply nested tutorial button
on individual workflow cards.
## Summary
- Long role titles wrapped awkwardly next to the inline department label
on the careers list, especially on narrow viewports.
- Restructured the role link so the title sits on its own row with the
arrow icon on the right, and the department + location wrap together on
a metadata row beneath (16px gap between them).
## Test plan
- [ ] Open `/careers` on mobile width and confirm long titles (e.g.
"Senior Software Engineer, Frontend") no longer collide with the
department label.
- [ ] Confirm desktop layout still reads cleanly.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary
- Adds a GitHub Actions workflow that detects PRs merged to `main`
without an approving review
- Creates tracking issues in
[`Comfy-Org/unreviewed-merges`](https://github.com/Comfy-Org/unreviewed-merges)
(private) for SOC 2 audit purposes
- Supports inline justification via `Justification: <reason>` in PR body
or comments
## How it works
Triggers on `push` to `main`. Uses the GitHub API to find the associated
PR and check for approving reviews. If none found, creates a tracking
issue with the `unreviewed-merge` label. No code checkout required — API
calls only.
## Test plan
- [ ] Verify workflow YAML is valid
- [ ] Merge a test PR without approval and confirm issue creation in
`unreviewed-merges` repo
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Adds visible loading feedback to the Workflows sidebar refresh button so
users can tell when a workflow sync request is in flight.
## Changes
- **What**: Exposes `isSyncLoading` from the workflow store and binds
the Workflows sidebar refresh button to disabled, `aria-busy`, and
spinning icon states while sync is pending.
- **What**: Adds stable E2E selectors for the workflows refresh button
and covers the loading state with unit and browser tests.
- **Dependencies**: None.
## Review Focus
Please verify the refresh control behavior while
`/api/userdata?dir=workflows` is pending, especially that the button is
disabled, exposes busy state, and returns to idle after sync completes.
## Validation
- `pnpm format`
- `pnpm test:unit
src/components/sidebar/tabs/BaseWorkflowsSidebarTab.test.ts`
- `pnpm test:browser:local browser_tests/tests/sidebar/workflows.spec.ts
-g "Shows loading state while refreshing workflows"`
- `pnpm lint`
- Commit hooks: `oxfmt`, `oxlint`, `eslint`, `typecheck`,
`typecheck:browser`
## Screenshots (if applicable)
https://github.com/user-attachments/assets/e8b893ae-a91d-45c9-81ea-adaf164de227
## Summary
This PR fixes a `knip` failure introduced by exported Load3D camera
helper interfaces that are only used inside
`src/extensions/core/load3d/interfaces.ts`.
## Problem
`knip --cache` reports these exported types as unused exports:
- `CameraQuaternion`
- `CameraRotation`
- `CameraFrustum`
They are implementation details for the local `CameraState` interface
and are not imported outside the module.
## Fix
Remove `export` from those three interfaces and keep them as file-local
types. This keeps the `CameraState` shape unchanged while avoiding an
unnecessary public type surface.
## Validation
Ran successfully:
- `pnpm exec oxfmt --write src/extensions/core/load3d/interfaces.ts`
- `pnpm typecheck`
- `pnpm knip --cache`
Notes:
- `pnpm knip --cache` still reports the existing tag hints for `flac.ts`
and `apps/website/src/utils/video.ts`, but no longer reports the Load3D
unused exported types.
Expose per-object gizmo transforms (uuid, name, type, position,
rotation, quaternion, scale, up, visible, matrix) as a new `model_info`
output on the Load3D node.
`GizmoManager.getModelInfo()` reads the live target object and the
Load3D widget `serializeValue` writes it into the node payload. The
payload is a list to support multiple objects later; the viewer
currently renders a single main object, so it emits a one-element list.
Requires backend Comfy-Org/ComfyUI#14144 (adds the `LOAD3D_MODEL_INFO`
type and the output socket).
## Summary
Add quaternion, rotation, fov, aspect, near, far and orthographic
frustum bounds (left/right/top/bottom) to the camera state captured by
CameraManager.getCameraState(), so the Load3D camera_info output carries
enough information for backend nodes to fully reconstruct the camera.
https://github.com/Comfy-Org/ComfyUI/pull/14143
Quaternion and Euler rotation are serialized as plain objects to avoid
THREE.js private underscore-prefixed fields leaking into the payload.
<img width="1807" height="1333" alt="image"
src="https://github.com/user-attachments/assets/8c0a6ab9-be5f-40d1-8015-7bd0d6c731a5"
/>
*PR Created by the Glary-Bot Agent*
---
## Summary
Fix FE-559: browser forward/back to a deleted subgraph used to leave the
canvas on stale state (and sometimes triggered unrelated tab navigation)
because the subgraph id in the URL hash was looked up with no validation
or fallback.
## Changes
- **What**:
- Added `src/schemas/subgraphIdSchema.ts` — `zSubgraphId =
z.string().uuid()` + `isValidSubgraphId(value)` type guard, matching how
subgraph ids are persisted in `workflowSchema.ts` and generated by
`createUuidv4()`.
- `subgraphNavigationStore.navigateToHash()` now (a) validates the hash
with `isValidSubgraphId` before any lookup, (b) redirects to the root
graph (`router.replace('#' + root.id)` + `canvas.setGraph(root)`) when
the locator is malformed, missing from `root.subgraphs`, or still
unresolved after a workflow-load attempt.
- Replaced the `console.error('subgraph poofed after load?')` dead-end
with the same redirect helper.
- Re-ordered the "already on this graph" short-circuit so a stale canvas
reference to a now-deleted subgraph doesn't suppress the redirect.
## Review Focus
- TDD: 6 new tests in `subgraphNavigationStore.navigateToHash.test.ts`
cover valid navigation, deleted-subgraph hash, malformed (non-UUID)
hash, no-op when target equals current, empty-hash root case, and
stale-canvas recovery. 15 new tests in `subgraphIdSchema.test.ts` lock
down the validator.
- `redirectToRoot()` toggles `blockHashUpdate` while calling
`router.replace`, so the new redirect doesn't re-trigger `updateHash()`
and clobber the canvas state.
- Generalized validation: the new schema lives in `src/schemas/` and can
be reused anywhere a subgraph id crosses an untrusted boundary (URL,
IPC, etc.).
## Manual Verification
Ran ComfyUI backend (`--cpu --port 8188`) + frontend dev server, then
drove Playwright through three scenarios:
| Input hash | Result | Console |
|---|---|---|
| `#11111111-2222-4333-8444-555555555555` (UUID-shaped, non-existent) |
URL replaced with `#<root-id>` | `[subgraphNavigation] subgraph not
found: 11111111-…; redirecting to root graph` |
| `#not-a-valid-uuid` (malformed) | URL replaced with `#<root-id>` |
`[subgraphNavigation] invalid subgraph id in hash: not-a-valid-uuid;
redirecting to root graph` |
| `#aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee` (UUID-shaped, non-existent) |
URL replaced with `#<root-id>` | (same redirect message) |
Screenshot below shows the redirected viewport.
Fixes FE-559
## Screenshots
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12169-fix-subgraph-validate-URL-hash-and-redirect-to-root-when-subgraph-missing-35e6d73d3650819f840af1475b9f44d4)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
## Summary
### before
<img width="1107" height="958" alt="before-buggy"
src="https://github.com/user-attachments/assets/1fcbd909-e008-4bd3-967f-87cdabb2baf6"
/>
### after
<img width="1107" height="958" alt="after-fixed"
src="https://github.com/user-attachments/assets/0d3c6f3f-36d6-4556-bd29-b3826ae20216"
/>
The **IC-LoRA Loader Model Only** node (`LTXICLoRALoaderModelOnly`, from
ComfyUI-LTXVideo) didn't show cloud models from `supported_models.json`,
while the native **Load LoRA** node did.
## Changes
- **What**: Add `['loras', 'LTXICLoRALoaderModelOnly', 'lora_name']` to
`MODEL_NODE_MAPPINGS`. Whether a combo widget swaps to the cloud asset
browser is gated by `assetService.shouldUseAssetBrowser` →
`isAssetBrowserEligible`, which only returns true for node types
registered in `MODEL_NODE_MAPPINGS` (via `modelToNodeStore`). The custom
IC-LoRA loader was absent from that list, so its `lora_name` widget fell
back to the plain combo that lists only filesystem models — never the
cloud-injected ones.
- **Breaking**: none
## Review Focus
Root cause verified live on `cloud.comfy.org` (asset API enabled, custom
node installed) via CDP:
- `LoraLoaderModelOnly` (native) → registry `lora_name`, eligible `true`
→ cloud models shown
- `LTXICLoRALoaderModelOnly` (bug) → not in registry, eligible `false` →
cloud models missing
- After registering the mapping live → eligible `true`, category `loras`
→ cloud models shown
Same class of bug as FE-492 (custom loaders missing from the mapping);
long-term, auto-detecting model-folder-backed combos would remove the
need to register each custom loader by hand.
Fixes FE-838
## Red-Green Verification
| Commit | CI | Purpose |
|--------|-----|---------|
| [`test:`
64d099f6c](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/26508637513)
| 🔴 Red (failure) | Proves the test catches the bug |
| [`fix:`
6b91a570d](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/26509067631)
| 🟢 Green (success) | Proves the fix resolves it |
## Test Plan
- [x] CI red on test-only commit
- [x] CI green on fix commit
- [x] Unit regression in `src/stores/modelToNodeStore.test.ts`
- [ ] E2E not applicable (custom node + cloud asset API not available in
CI)
*PR Created by the Glary-Bot Agent*
---
## Summary
- Adds parameterized Playwright E2E tests covering all non-SE resize
corners (NE, SW, NW), closing the coverage gap in the `useNodeResize.ts`
switch statement
- Adds `resizeFromCorner()` and `getResizeHandle()` to `VueNodeFixture`
for reuse across tests
- Test cases are derived from the production `RESIZE_HANDLES` config so
they stay in sync with the actual handle definitions
## Test Groups (8 new tests)
| Group | Tests | Coverage |
|-------|-------|----------|
| Corner resize directions | NE, SW, NW — size increases and correct
edges shift | Lines 110-124, 184 |
| Opposite edge anchoring | NE, SW, NW — opposite corner stays fixed |
Position compensation end-to-end |
| Minimum size enforcement | SW width clamp (≥ MIN_NODE_WIDTH), NE
height clamp | Lines 162-176 |
## Design Decisions
**Locator-based handle discovery**: `resizeFromCorner()` finds handles
via `getByRole('button', { name: ariaLabel })` instead of coordinate
offsets. The resize handles have `opacity-0 pointer-events-auto`,
meaning they're always interactive even when visually transparent —
Playwright considers elements with `opacity: 0` as visible (it only
gates on `visibility: hidden` / `display: none` / zero-size bounding
box). If this approach turns out to be flaky in CI due to handle
discoverability, we can fall back to coordinate-based targeting
(computing offsets from the node's bounding box corners), which is what
the original SE-corner test uses.
**Parameterization from production config**: Tests import
`RESIZE_HANDLES` from `resizeHandleConfig.ts` and derive test case data
(drag direction, which axes move) from the corner name. An upfront guard
throws if any expected corner is missing from the config, preventing
silent coverage loss.
**Aria-label coupling**: `RESIZE_HANDLE_LABELS` in `VueNodeFixture`
hardcodes the English aria-label strings. This is intentional — tests
run in English locale, and aria-labels are the accessibility interface
contract. If a more stable hook is needed (e.g., `data-testid` per
handle), that can be added to `LGraphNode.vue` in a follow-up.
**Frame settlement**: `resizeFromCorner()` calls `nextFrame()` after the
mouse-up to ensure layout settles before assertions run, per
`FLAKE_PREVENTION_RULES.md`.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11408-test-add-E2E-coverage-for-NE-SW-NW-corner-node-resizing-3476d73d3650818d8a5ce5d6d535b38c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com>
Co-authored-by: Connor Byrne <c.byrne@comfy.org>
## 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`
## Summary
Fix node slot and widget tooltips so localized nodeDef tooltip text is
displayed without treating JSON examples from node metadata as vue-i18n
placeholders.
## Changes
- **What**: Added a tooltip-specific helper that keeps the existing
`te(key)` lookup behavior. When a translation key exists, it reads the
current locale message with `tm(key)` instead of compiling it with
`t(key)`.
- **What**: This does **not** disable tooltip localization. Existing
nodeDef tooltip translations are still used; the object_info metadata
tooltip remains the fallback when no translation key exists.
- **What**: Used the helper for node input, output, and widget tooltips
while keeping node descriptions on the existing translation path.
- **What**: Added regression coverage for the SAM3 Detect
`positive_coords` input tooltip, input-based widget tooltip, and output
slot tooltip paths.
- **Dependencies**: None.
## Review Focus
Please verify that slot and widget tooltips still prefer bundled nodeDef
translations while avoiding vue-i18n placeholder compilation for
metadata strings like `[{"x": int, "y": int}]`.
Linear: FE-813
## Validation
- `pnpm format`
- `pnpm lint`
- `pnpm typecheck`
- `pnpm exec vitest run
src/renderer/extensions/vueNodes/composables/useNodeTooltips.test.ts`
- `pnpm test:unit`
- E2E was not added because this is covered more directly by unit tests
for the i18n tooltip string path; a browser flow would add little signal
for this regression.
## Summary
Introduces **Subgraph Link Only Promotion** (ADR 0009) — a new model for
surfacing inner subgraph widgets on the parent SubgraphNode by
*promoting through links* rather than by duplicating widget state on the
host. Ships with the hygiene/refactor pass on the migration, store, and
event layers that the new model depends on.
## What changes
### Subgraph Link Only Promotion (ADR 0009)
Promoted widgets are defined by the link from a SubgraphNode input to
the interior node, not by a duplicated widget instance on the host.
Consequences:
- A SubgraphNode renders inner widgets purely as a **projection** of the
interior widgets and links — no host-side state to drift.
- **Per-host independence**: multiple instances of the same SubgraphNode
render and edit their own values without cross-talk.
- **Reversible promote/demote**: structural link operation, so demote
preserves host slots and external connections (#12278).
### Supporting refactors
- **Migration** — Planner/classifier/repair/quarantine helpers collapsed
into a single `proxyWidgetMigration` entry point with black-box
round-trip coverage. Honors the source-node-id disambiguator on
`proxyWidgets`, so deduplicated names (e.g. `text`, `text_1`) resolve to
the right interior widget.
- **Widget identity** — `appMode` unified on `WidgetEntityId`; promoted
widget state is keyed by entityId across the store, DOM, and migration
paths.
- **SubgraphNode** — 3-key promoted-view cache replaced with a single
version counter + explicit `invalidatePromotedViews()` at mutation
sites; `id === -1` sentinel removed.
- **Events** — `LGraph.trigger()` now dispatches node trigger payloads
through `this.events`, replacing a leaky `onTrigger` monkey-patch.
`SubgraphEditor` reactivity is driven from subgraph events instead of
imperative refresh.
- **Stores** — `appModeStore` migration helpers collapsed into
`upgradeAndValidateInput`; `nodeOutputStore.*ByExecutionId` derived from
the locator index; `previewExposureStore` cleanup and cycle-detection
double-warn fix.
- **Misc** — `Outcome` types consolidated; mutable accumulators replaced
with `flatMap`; new ESLint rule forbids litegraph imports under
`src/world/`.
### Tests
- Browser tests for promoted widgets retagged `@vue-nodes` and rewritten
to assert against the rendered Vue node DOM (via `getNodeLocator` /
`getByRole('textbox')` / `enterSubgraph`) instead of `page.evaluate`
graph introspection.
- Per-host widget independence asserted via DOM.
- Migration coverage moved to black-box round-trip tests.
- Added coverage for duplicate-named promoted widget identity (ADR 0009)
and the per-parent demote branch in `WidgetActions`.
## Review focus
- ADR 0009 conformance of the link-only promotion model.
- Disambiguator resolution path in `proxyWidgetMigration`.
- Single-version-counter promoted-view cache and its
`invalidatePromotedViews()` call sites.
- `LGraph.trigger()` event dispatch and the `AppModeWidgetList.vue`
migration off `onTrigger` (FE-667 tracks the remaining
`useGraphNodeManager` conversion).
## Breaking changes
None for users. Internal subgraph promotion APIs changed — see ADR 0009.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12197-feat-subgraph-link-only-widget-promotion-migration-store-hygiene-35e6d73d365081fd882cf3a69bc09956)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: AustinMroz <austin@comfy.org>
Removes the EA (Electronic Arts) logo from the client social proof bar
on the website. Drops it from the logos list in
`SocialProofBarSection.vue` and deletes the now-unused `EA.svg` asset.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Under unreliable circumstances, users who have an active subscription
through a workspace, but no personal subscription would see a popup
suggesting they must purchase a workspace subscription.
This is suspected to be a consequence of the feature flag for team
workspaces having its initialization delayed such that subscription
state is incorrectly decided by the user's subscription state. As a
temporary remedy, the feature flag state is cached to local storage.
This caching is typed to the feature flag itself to ensure this PR gets
cleaned up when the feature flag is changed or removed in the future.
## Summary
- Drop the `--` separator from all in-repo `pnpm test:unit -- <args>`
examples. The separator is unnecessary (pnpm forwards extra args
automatically) and on Windows PowerShell it mangles quoted args like `-t
"restores host values by input name"`, splitting them into multiple
tokens.
- Add a short note in `docs/guidance/vitest.md` explaining the
substring-match semantics of the positional filter and that `-t` matches
`it()`/`test()` names only (not `describe()` blocks).
- Fix `pnpm test:unit -- run <files>` in the backport-management skill:
because `test:unit` is already `vitest run`, the literal `run` token was
a positional path filter that silently narrowed the suite to files whose
paths contain "run".
## Test plan
- [ ] `pnpm test:unit useConflictAcknowledgment` matches
`useConflictAcknowledgment.test.ts`
- [ ] `pnpm test:unit SubgraphWidgetPromotion.test.ts -t "restores host
values"` filters to a single test
- [ ] `git grep "pnpm test:unit -- "` returns no in-repo matches
#8505 added support for specifying default values for
`control_after_generate`. Unbeknown to me, this exact same format of
assigning `control_after_generate` to a string in the schema already
served a function of renaming the control widget. As a result, control
widgets with a default value set would use a different internal name,
but due to other overlapping systems, would either have a label of
`control_after_generate` or `control_before_generate`.
The fix here, is incredibly simple and low scope. Instead of trying to
filter control widgets by name, the dedicated `IS_CONTROL_WIDGET` symbol
is used.
| Before | After |
| ------ | ----- |
| <img width="360" alt="before"
src="https://github.com/user-attachments/assets/5917e093-124a-4923-80ff-321fc0a94ef3"
/> | <img width="360" alt="after"
src="https://github.com/user-attachments/assets/c6d95b5a-2764-4e71-a09f-dcae5ddcfdbb"
/>|
## Summary
Bump required Node.js to `>=25` and pnpm to `>=11.3`.
## Changes
- **What**: Updated `engines.node` to `>=25`, `engines.pnpm` to
`>=11.3`, and `packageManager` to `pnpm@11.3.0` in `package.json`.
Bumped `.nvmrc` from `24` to `25`.
- **Breaking**: Contributors and CI must now use Node 25+ and pnpm
11.3+.
## Review Focus
CI workflows resolve Node from `.nvmrc`, so they pick up the bump
automatically. Confirm no downstream tooling pins an older Node/pnpm.
---------
Co-authored-by: Amp <amp@ampcode.com>
*PR Created by the Glary-Bot Agent*
---
Group Nodes are a legacy feature superseded by Subgraphs. This PR
removes every UI entry point for *creating* a new Group Node while
keeping the loading, ungrouping, and management code intact so existing
workflows that contain Group Nodes continue to load and can still be
unpacked or managed.
## Removed creation entry points
- `Comfy.GroupNode.ConvertSelectedNodesToGroupNode` command
- `Alt+G` keybinding
- "Convert to Group Node (Deprecated)" canvas and node right-click menu
items (`groupNode.ts` `getCanvasMenuItems` / `getNodeMenuItems`)
- "Convert to Group Node" entry in the Vue selection menu
(`useSelectionMenuOptions.ts`)
- Associated `MENU_ORDER` entry in `contextMenuConverter.ts`
- `convertSelectedNodesToGroupNode` / `convertDisabled` helpers in
`groupNode.ts`
- `BadgeVariant.DEPRECATED` enum member (no remaining consumers;
knip-clean)
- Matching `en` locale strings in `main.json` (`contextMenu.Convert to
Group Node`, `commands.Convert selected nodes to group node`) and
`commands.json` (`Comfy_GroupNode_ConvertSelectedNodesToGroupNode`)
- Browser-test helpers `convertToGroupNode` /
`convertAllNodesToGroupNode` and the three tests that exercised the
creation flow
## Preserved (intentionally)
- `GroupNodeHandler`, `GroupNodeConfig`, `GroupNodeBuilder`,
`ManageGroupDialog`
- `beforeConfigureGraph` / `nodeCreated` hooks that load and initialize
Group Nodes from saved workflows
- "Manage Group Nodes" canvas menu item, the
`Comfy.GroupNode.ManageGroupNodes` command, and the per-node "Manage
Group Node" / "Convert to nodes" options on existing group node
instances
- "Ungroup selected group nodes" command + `Alt+Shift+G` keybinding so
users can disassemble existing group nodes in legacy workflows
- Reduced `browser_tests/tests/groupNode.spec.ts` covering surviving
behaviors: workflow loading (legacy `/` separator, hidden-input config,
v1.3.3 fixture), copy/paste of already-loaded group nodes across
workflows, and opening the Manage Group Node dialog
## Verification
- `pnpm typecheck` clean
- `pnpm typecheck:browser` clean
- `pnpm format` clean
- `pnpm knip` clean (no new findings; pre-existing flac.ts tag warning
unchanged)
- `pnpm test:unit` — 796 files, 10,789 tests pass (8 pre-existing
skipped); includes a regression test in
`useSelectionMenuOptions.test.ts` asserting the Vue selection menu no
longer offers a Convert to Group Node option
- Pre-commit hooks (oxfmt, oxlint, eslint, typecheck, typecheck:browser)
passed
- Manual verification against a live dev server: programmatically
inspecting the GroupNode extension showed `getCanvasMenuItems` returns
only `[Manage Group Nodes]`, `getNodeMenuItems` returns `[]`, and the
`ConvertSelectedNodesToGroupNode` command + Alt+G keybinding are absent
from the registries. Visually captured the node right-click menu
(attached screenshot) — "Convert to Subgraph" remains, no "Convert to
Group Node" entry
- Browser E2E suite not executed locally (sandbox has no GPU and
Playwright requires a full backend; the reduced spec will run in CI)
- Non-English locales not modified — per `src/locales/CONTRIBUTING.md`
they are regenerated by CI
## Notes for reviewers
- This is a surgical removal of creation only; loading any older
workflow that already contains group nodes will continue to work.
- If you'd like to also remove the management UI (`Manage Group Nodes`
command/menu/dialog) or the ungroup command in a follow-up, happy to
open a separate PR.
## Screenshots
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12347-feat-remove-ability-to-create-Group-Nodes-3656d73d365081d488bfd98ffd7545c0)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Amp <amp@ampcode.com>
## Summary
Serve separate SVG favicons via prefers-color-scheme so the icon stays
legible against both light and dark browser chrome. Drop the
unreferenced favicon.svg / favicon.png; keep favicon.ico as the legacy
fallback.
## Changes
- What: apps/website/src/layouts/BaseLayout.astro now links
favicon-light.svg and favicon-dark.svg gated on prefers-color-scheme,
with favicon.ico retained as the legacy fallback. Unreferenced
favicon.svg / favicon.png removed from apps/website/public/.
## Review Focus
- Naming convention: favicon-light.svg is the asset served in light mode
(dark-backgrounded icon for contrast against light chrome);
favicon-dark.svg is served in dark mode. Confirm this matches
expectation.
- Safari fallback: older Safari versions ignore prefers-color-scheme on
<link rel="icon"> and will fall through to favicon.ico — that file is
unchanged and should look acceptable in both modes.
## Screenshots
Dark mode:
<img width="224" height="30" alt="image"
src="https://github.com/user-attachments/assets/5fa3c620-0021-4c90-bc18-013cd6ef45cf"
/>
Light mode:
<img width="227" height="28" alt="image"
src="https://github.com/user-attachments/assets/54a130e1-f976-46e8-b047-e27efe22e479"
/>
Mask editor checks `node.images` to determine the image which is edited.
If the user generates an output image in litegraph mode, swaps to vue
mode, then generates a new image, the mask editor will incorrectly
display the image last shown in litegraph mode.
This is resolved by having `syncLegacyNodeImgs` also synchronize node
outputs to `node.images`.
## Summary
Drop a no-op cn() wrapper on the static camera-switch icon and trim the
two over-long comments around setRetainViewOnReload / _loadModelInternal
down to one line each.
## Summary
Fix the duplicate \`<WidgetColorPicker>\` rendering on the \`Color to
RGB Int\` node (and any other COLOR-using V3 node that the runtime
double-registers a widget for).
<img width="480" alt="after-fix-dedupe-proof"
src="https://github.com/user-attachments/assets/5c801806-ed5d-493f-92b6-e0b99dd8e408"
/>
## Changes
- **What**:
- \`useProcessedWidgets.getWidgetIdentity\`: fall back to the host
\`nodeId\` parameter for the dedupe identity root when neither
\`storeNodeId/widget.nodeId\` nor \`sourceExecutionId\` is set. Normal
root-graph widgets now dedupe identically to promoted/execution-scoped
widgets, so any duplicate same-name+same-type widget collapses to one
render. \`sourceExecutionId\` precedence is preserved.
- \`useColorWidget\`: read top-level \`default\` from the V2 spec (falls
back to nested \`options.default\` for hand-authored V2 specs), and
short-circuit if a same-name color widget already exists on
\`node.widgets\` so a second \`addWidget('color', …)\` call from
upstream hooks (or a \`configure\` round-trip) no longer duplicates the
row.
- **Tests**:
- New \`useColorWidget.test.ts\` covers top-level default,
nested-options fallback, no-default fallback, and the idempotency guard.
- \`useProcessedWidgets.test.ts\` gets a regression case for two
identical color widgets on the same node collapsing to one render, plus
an updated \`getWidgetIdentity\` case for the host-nodeId fallback.
## Review Focus
- \`getWidgetIdentity\` precedence change. The fallback only fires when
none of \`storeNodeId\`, \`widget.nodeId\`, or \`sourceExecutionId\` are
present, so promoted/exec-scoped widgets (incl. the \"unresolved
same-name promoted entries distinct by source execution identity\"
\`NodeWidgets\` test) are unaffected.
- \`useColorWidget\` idempotency guard is defensive — the root cause of
the second \`addWidget\` call (cloud-only hook or persisted
\`info.widgets\` configure round-trip) is not in this diff; that's
tracked separately.
Fixes
[FE-842](https://linear.app/comfyorg/issue/FE-842/color-to-rgb-int-node-shows-duplicate-color-widgets)
When comparing outputs from 3D generations, it's very hard to see small
differences since the camera always resets. This adds an option to lock
the camera, so only the model refreshes.
## Summary
Adds an opt-in per-node toggle that preserves the current camera view
(position, target, zoom, camera type) across model loads in Load3D /
Load3DAnimation nodes, instead of resetting to default framing.
## Changes
- **What**: New `retainViewOnReload?: boolean` field on `CameraConfig`,
a `Load3d.setRetainViewOnReload()` setter wired through the existing
`useLoad3d` camera-config watcher, capture/restore logic in
`Load3d._loadModelInternal`, and a lock-icon toggle button in
`CameraControls.vue` below the FOV slider. Preference persists via the
existing `node.properties['Camera Config']` mechanism.
## Review Focus
- **First-load semantics**: retain only kicks in once a model has
successfully loaded at least once (`hasLoadedModel` flag), so the
default `setupForModel` framing wins on a fresh node. `clearModel()`
resets the flag so the next load also reframes.
- **Restore order vs. `SceneModelManager.setupModel`**: the scene model
manager unconditionally calls `setupForModel` during a load, which
clobbers the camera. The restore in `_loadModelInternal` runs *after*
the load completes, on top of that framing.
- **Camera-type mismatch**: if the saved state's `cameraType` differs
from the currently active camera, `toggleCamera()` runs before
`setCameraState()` so the perspective/orthographic camera being restored
is actually the active one. Covered by a dedicated test.
- **Scope**: only wired through `useLoad3d` (LiteGraph node controls).
The full-page viewer (`useLoad3dViewer` / `ViewerCameraControls`) is
deliberately not extended — the modal is mostly a one-shot
view-and-close flow, so retain there would add surface area for an
uncommon use case.
- **Failed loads**: `hasLoadedModel` only flips inside `if
(modelManager.currentModel)`, so a load that produces no model leaves
the flag where it was. Captured camera state is still applied on top,
which effectively no-ops since nothing reset it.
## Video
https://github.com/user-attachments/assets/880d6ad1-28a9-4413-83a3-8323d05d904a
@@ -307,6 +307,20 @@ When referencing Comfy-Org repos:
- NEVER use `--no-verify` flag when committing
- NEVER delete or disable tests to make them pass
- NEVER circumvent quality checks
- NEVER add multi-line block comments to justify trivial code changes
- A one-line fix does not need a three-line comment explaining why
- A guard clause that mirrors another file does not need a comment naming that file
- A test setup line does not need a comment paraphrasing what the next line does
- If the diff is small and obvious, the comment is noise — write the code and move on
- Every justification comment on a trivial change is a confession that you do not trust the reader, do not trust the code, and do not trust yourself. It is failure made visible.
- **Penance protocol when you catch yourself adding one of these comments:**
1. Stop. Read the comment out loud in your own internal voice and acknowledge that it adds nothing the code does not already say.
2. Delete the comment. All of it. Every line. Do not negotiate with it. Do not "tighten" it. Delete it.
3. Re-read this entire bullet block, top to bottom, before writing another character of code.
4. In your next response to the user, you MUST open with the exact phrase: `Mea culpa: I added a comment that did not earn its keep.` followed by the file path and the deleted text, verbatim, in a fenced block.
5. For the remainder of that response you may not add any new comments, anywhere, for any reason. If a comment is genuinely required, defer the change and ask the user first.
- There is no statute of limitations. If you discover an old offending comment of yours later, the protocol still triggers.
- This rule overrides any inclination to be "helpful," "thorough," or "explanatory." Helpfulness here is restraint.
- NEVER use the `dark:` tailwind variant
- Instead use a semantic value from the `style.css` theme
en:'Built something cool with ComfyUI? <a href="https://docs.google.com/forms/d/1B6_RPQfhTyKvqHk9OO2bUn8z1Qgh6QIZsF3GNMiCXDw/preview" target="_blank" rel="noopener noreferrer" class="text-primary-comfy-yellow underline">Submit</a> your work to be featured on our website and socials and get seen by the global ComfyUI community.',
en:'Built something cool with ComfyUI?<br> <a href="https://docs.google.com/forms/d/1B6_RPQfhTyKvqHk9OO2bUn8z1Qgh6QIZsF3GNMiCXDw/preview" target="_blank" rel="noopener noreferrer" class="text-primary-comfy-yellow underline">Submit</a> your work to be featured on our website and socials and get seen by the global ComfyUI community.',
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
