[pull] main from Comfy-Org:main

AGENTS.md

@@ -4,6 +4,7 @@ See @docs/guidance/\*.md for file-type-specific conventions (auto-loaded by glob
 
 - `docs/guidance/engineering.md` — general engineering guidelines, project philosophy, code-review checklist, external resource links
 - `docs/guidance/vue-components.md` — Vue 3 Composition API best practices
+- `docs/guidance/state-and-effects.md` — modelling a feature's state: one discriminated union, named events, a pure transition, effects reserved for synchronising outward
 - `docs/guidance/typescript.md` — TypeScript type-safety rules
 - `docs/guidance/vitest.md` — Vitest unit/component test conventions
 - `docs/guidance/playwright.md` — Playwright E2E conventions and API-mock typing table

apps/website/src/data/fdct.ts

@@ -24,20 +24,20 @@ export interface FdctTechnologist {
 const technologistIdentities = {
   'doug-hogan': {
     name: 'Doug Hogan',
-    avatarSrc: 'https://media.comfy.org/website/technologists/doug-hogan.png'
+    avatarSrc: 'https://media.comfy.org/website/technologists/doug-hogan_v2.png'
   },
   'chris-v': {
     name: 'Chris V.',
-    avatarSrc: 'https://media.comfy.org/website/technologists/chris-v.png'
+    avatarSrc: 'https://media.comfy.org/website/technologists/chris-v_v2.png'
   },
   'rob-losch': {
     name: 'Rob Losch',
-    avatarSrc: 'https://media.comfy.org/website/technologists/rob-losch_v2.png'
+    avatarSrc: 'https://media.comfy.org/website/technologists/rob-losch_v3.png'
   },
   'robert-paige': {
     name: 'Robert Paige',
     avatarSrc:
-      'https://media.comfy.org/website/technologists/robert-paige_v3.png'
+      'https://media.comfy.org/website/technologists/robert-paige_v4.png'
   }
 } as const
 

docs/guidance/state-and-effects.md

@@ -0,0 +1,157 @@
+---
+globs:
+  - 'src/**/*.ts'
+  - 'src/**/*.vue'
+---
+
+# State, Effects and Workflows
+
+How to represent a multi-step process — onboarding, a wizard, an upload, a
+checkout, anything with a beginning and an end — in a reactive framework.
+
+> **Events initiate work. One explicit state represents the workflow. Pure
+> derivations shape the UI. Effects only synchronise with external systems.**
+
+This applies to any framework; the Vue mapping is at the bottom.
+
+## 1. One state, not several booleans
+
+Hold the **minimum authoritative facts** needed to render and continue. Anything
+computable from them is not state.
+
+Avoid independent booleans for what is really one value. Prefer a discriminated
+union, so invalid combinations are structurally impossible rather than merely
+unlikely.
+
+```ts
+// ✗ four facts that must agree, and nothing makes them
+const steps = ref<Step[]>([])
+const stepIdx = ref(-1)
+const waiting = ref(false)
+const active = ref<Flow | null>(null)
+
+// ✓ one fact
+type FlowState =
+  | { phase: 'idle' }
+  | { phase: 'awaiting'; flow: Flow; steps: Step[]; idx: number }
+  | { phase: 'showing'; flow: Flow; steps: Step[]; idx: number }
+  | { phase: 'finished'; flow: Flow; outcome: Outcome }
+```
+
+**The tell:** if ending the workflow means resetting four variables in the right
+order, it was one state and you gave it four variables.
+
+## 2. Change state through named events and a pure transition
+
+```ts
+type FlowEvent =
+  | { type: 'started'; flow: Flow; steps: Step[] }
+  | { type: 'advanced' }
+  | { type: 'skipped' }
+
+function reduceFlow(state: FlowState, event: FlowEvent): FlowState // pure switch
+```
+
+Invalid transitions become harmless — an event that means nothing in the current
+phase returns the state untouched — and valid ones become reviewable in one
+place. This is the same reason most frameworks recommend a reducer once
+state-update logic gets complex.
+
+It is also exhaustively testable: every state crossed with every event, including
+the pairs that should do nothing.
+
+## 3. Async orchestration belongs in commands
+
+Do **not** build chains where each step is an effect reacting to the last:
+
+```
+saving changed → effect saves → saved changed → effect invalidates
+              → invalidated changed → effect navigates
+```
+
+Nobody can read that as one sequence, and the middle of it is reachable from
+places you did not intend. Give the whole causal sequence to one function:
+
+```ts
+async function startFlow(id: string) {
+  const data = await load(id)
+  if (!data) return false
+  dispatch({ type: 'started', flow: id, steps: build(data) })
+  await settle()
+  dispatch({ type: 'stepEntered', idx: 0 })
+  return true
+}
+```
+
+## 4. Derive everything derivable
+
+Do not store `isOpening`, `canTransition`, `isLast`, or a second copy of data
+that already exists. If a `computed` can name it, do not put it in a `ref` and
+keep it in sync by hand — the sync is where the bugs live.
+
+## 5. Reserve effects for synchronising with the outside
+
+**Good effects** — one-way, outward, and they write nothing anything else reads:
+
+- state changed → emit telemetry
+- state changed → write a setting
+- component disposed → abort a request or unsubscribe
+- external socket event → _dispatch an event_ (not: assign state directly)
+
+**Bad effects:**
+
+- pointer target changed → recalculate steps
+- success changed → advance the workflow
+- an effect that writes state a second effect reads
+
+If two effects communicate through shared state, that is a transition wearing a
+disguise. Put it in the reducer.
+
+## 6. Do not inspect the DOM for something a store already knows
+
+Avoid `getBoundingClientRect`, `getComputedStyle`, and `document.querySelector`
+for positions and sizes that a store holds — especially inside a `computed`,
+where every recompute becomes a layout read.
+
+For canvas-anchored UI: `layoutStore` holds node bounds and `useTransformState`
+mirrors the camera, both reactive, so a screen rect is a **derivation** rather
+than a measurement. Floating UI accepts a
+[virtual element](https://floating-ui.com/docs/virtual-elements) for exactly
+this.
+
+Two caveats worth knowing rather than discovering:
+
+- `layoutStore` bounds are the node's **body box**. The element renders one
+  `NODE_TITLE_HEIGHT` above `position` and the resize tracker subtracts it, so
+  anything deriving a rect must add it back — and a collapsed node is exactly one
+  title tall, which is zero without it.
+- Node ids are **graph-local**. An id resolved against one graph names a
+  different node in another, so anything holding one across a workflow or
+  subgraph change must pin the graph it resolved against.
+
+## 7. Reach for a formal state machine when it earns it
+
+Parallel regions, nested states, timed transitions, cancellation, replay — at
+that point a library (XState or equivalent) buys real guarantees.
+
+Below it, a discriminated union plus a pure reducer is the same model without the
+dependency, and the migration between them is mechanical. **There is currently no
+XState in this repo**; introducing it is a deliberate decision, not a default.
+
+Note that two genuinely parallel regions — say a wizard's progress and a
+long-running job's outcome — should be two small states, not one product type.
+
+## Vue mapping
+
+| Concern               | Where it goes                                 |
+| --------------------- | --------------------------------------------- |
+| Workflow state        | `ref` / `shallowRef` in a Pinia store         |
+| Transition            | pure function, its own module, no Vue imports |
+| Derived UI            | `computed`                                    |
+| External sync         | `watch` / `watchEffect`                       |
+| The workflow itself   | store method (a command)                      |
+| Complex state machine | XState or equivalent                          |
+
+**Mental model:** user or external event → command performs async work → typed
+event → pure state transition → reactive derived UI. Effects sit _beside_ this
+loop, only to synchronise with external systems.

eslint.config.ts

@@ -302,6 +302,34 @@ export default defineConfig([
       ]
     }
   },
+  // A layout read inside a derivation runs on every recompute, and a derivation
+  // that measures the DOM cannot be tested without one. See
+  // docs/guidance/state-and-effects.md.
+  //
+  // 'warn' rather than 'error' because four pre-existing instances remain, in
+  // BrushCursor.vue, WorkflowTabs.vue and SubgraphBreadcrumb.vue. Promote to
+  // 'error' once those are derived from stores instead.
+  {
+    files: ['src/**/*.ts', 'src/**/*.vue'],
+    ignores: ['**/*.test.ts', '**/*.spec.ts'],
+    rules: {
+      'no-restricted-syntax': [
+        'warn',
+        {
+          selector:
+            "CallExpression[callee.name='computed'] CallExpression[callee.property.name='getBoundingClientRect']",
+          message:
+            'Do not measure the DOM inside a computed - every recompute becomes a layout read. Derive from a store instead. See docs/guidance/state-and-effects.md.'
+        },
+        {
+          selector:
+            "CallExpression[callee.name='computed'] CallExpression[callee.property.name=/^(getComputedStyle|querySelector|querySelectorAll)$/]",
+          message:
+            'Do not inspect the DOM inside a computed. Derive from a store instead. See docs/guidance/state-and-effects.md.'
+        }
+      ]
+    }
+  },
   {
     files: ['**/*.spec.ts'],
     ignores: ['browser_tests/tests/**/*.spec.ts', 'apps/*/e2e/**/*.spec.ts'],

src/components/dialog/content/signin/SignUpForm.test.ts

@@ -43,9 +43,6 @@ const mockReset = vi.fn()
 let emitTurnstileToken: ((token: string) => void) | undefined
 let emitTurnstileUnavailable: ((unavailable: boolean) => void) | undefined
 
-// The reset-on-toggle behavior lives in useTurnstileGate itself (see
-// useTurnstile.test.ts); this fake just wires token/unavailable through to
-// `waiting` the same way so SignUpForm's submit gating can be exercised.
 vi.mock('@/composables/auth/useTurnstile', () => ({
   useTurnstile: () => ({
     enabled: mockTurnstileEnabled
@@ -62,9 +59,8 @@ vi.mock('@/composables/auth/useTurnstile', () => ({
   })
 }))
 
-// Stub the real widget (which loads the external Turnstile script) with one that
-// exposes a spyable reset() and lets a test drive the v-model token/unavailable
-// the way a solved challenge (or a broken/slow widget) would.
+// The real widget loads an external Turnstile script; this stub exposes a
+// spyable reset() and lets a test drive the token/unavailable v-models.
 vi.mock('./TurnstileWidget.vue', async () => {
   const { defineComponent: defineMock } = await import('vue')
   return {
@@ -118,8 +114,6 @@ describe('SignUpForm', () => {
     return { ...utils, user }
   }
 
-  /** Render through a host that keeps a ref, so the parent-facing exposed
-   * `resetTurnstile()` can be invoked the way SignInContent would. */
   function renderWithRef() {
     const formRef = ref<{ resetTurnstile: () => void } | null>(null)
     const Host = defineComponent({
@@ -246,11 +240,6 @@ describe('SignUpForm', () => {
     })
   })
 
-  // Regression coverage for the shadow-mode race: previously submit was only
-  // gated in 'enforce' mode, so most real signups in 'shadow' mode raced
-  // ahead of the async Cloudflare challenge and reached the backend with an
-  // empty token. Gating now depends only on whether the widget is enabled
-  // (shadow or enforce both render it), so both modes behave identically here.
   describe('Turnstile submit gating', () => {
     it('disables the submit button until a token is present', async () => {
       mockTurnstileEnabled.value = true
@@ -268,7 +257,10 @@ describe('SignUpForm', () => {
 
       await user.click(screen.getByRole('button', { name: signUpButton }))
 
-      expect(onSubmit).not.toHaveBeenCalled()
+      expect(
+        onSubmit,
+        'gating on enabled (not enforce) is what stops a shadow-mode signup racing ahead with an empty token'
+      ).not.toHaveBeenCalled()
     })
 
     it('emits submit with the token once the challenge is solved', async () => {
@@ -297,4 +289,66 @@ describe('SignUpForm', () => {
       expect(onSubmit).toHaveBeenCalledWith(expectedValues, undefined)
     })
   })
+
+  describe('Turnstile wait hint accessibility', () => {
+    it('announces the wait politely while the challenge is pending', async () => {
+      mockTurnstileEnabled.value = true
+      renderComponent()
+      await nextTick()
+
+      const hint = screen.getByRole('status')
+      expect(
+        hint,
+        'the hint is the only thing telling a screen-reader user why submit is unavailable'
+      ).toHaveTextContent(enMessages.auth.turnstile.submitBlockedHint)
+      expect(hint).toHaveAttribute('aria-live', 'polite')
+    })
+
+    it('points the disabled submit button at the hint', async () => {
+      mockTurnstileEnabled.value = true
+      const { user } = renderComponent()
+      await fillValidSignup(user)
+      await nextTick()
+
+      const submit = screen.getByRole('button', { name: signUpButton })
+      expect(
+        submit,
+        'an otherwise-valid form must stay disabled while the challenge is pending'
+      ).toBeDisabled()
+      expect(submit).toHaveAttribute(
+        'aria-describedby',
+        screen.getByRole('status').id
+      )
+    })
+
+    it('drops the description once the challenge resolves', async () => {
+      mockTurnstileEnabled.value = true
+      renderComponent()
+      await nextTick()
+
+      emitTurnstileToken!('token-xyz')
+      await nextTick()
+
+      expect(
+        screen.getByRole('button', { name: signUpButton })
+      ).not.toHaveAttribute('aria-describedby')
+    })
+  })
+
+  describe('double-submit throttling', () => {
+    it('emits once when the button is clicked twice in quick succession', async () => {
+      const onSubmit = vi.fn()
+      const { user } = renderComponent({ onSubmit })
+      await fillValidSignup(user)
+      const submit = screen.getByRole('button', { name: signUpButton })
+
+      await user.click(submit)
+      await user.click(submit)
+
+      expect(
+        onSubmit,
+        'an impatient double-click would otherwise create the account twice'
+      ).toHaveBeenCalledOnce()
+    })
+  })
 })

src/components/sidebar/tabs/AssetsSidebarTab.vue

@@ -229,7 +229,10 @@ import type { OutputAssetMetadata } from '@/platform/assets/schemas/assetMetadat
 import { getOutputAssetMetadata } from '@/platform/assets/schemas/assetMetadataSchema'
 import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
 import { getAssetDisplayName } from '@/platform/assets/utils/assetMetadataUtils'
-import { getAssetUrl } from '@/platform/assets/utils/assetUrlUtil'
+import {
+  getAssetSubfolder,
+  getAssetUrl
+} from '@/platform/assets/utils/assetUrlUtil'
 import type { MediaKind } from '@/platform/assets/schemas/mediaAssetSchema'
 import { resolveOutputAssetItems } from '@/platform/assets/utils/outputAssetUtil'
 import { isCloud } from '@/platform/distribution/types'
@@ -457,7 +460,7 @@ const galleryItems = computed(() => {
     const mediaType = getMediaTypeFromFilename(asset.name)
     const resultItem = new ResultItemImpl({
       filename: asset.name,
-      subfolder: '',
+      subfolder: getAssetSubfolder(asset),
       type: 'output',
       nodeId: '0',
       mediaType: mediaType === 'image' ? 'images' : mediaType