diff --git a/.claude/skills/reviewing-unit-tests/SKILL.md b/.claude/skills/reviewing-unit-tests/SKILL.md new file mode 100644 index 0000000000..dcf8a1499d --- /dev/null +++ b/.claude/skills/reviewing-unit-tests/SKILL.md @@ -0,0 +1,156 @@ +--- +name: reviewing-unit-tests +description: Use when reviewing Vitest unit-test diffs in ComfyUI_frontend, especially new mocks, store tests, component tests, or bugfix regression tests. +--- + +# Reviewing Unit Tests for ComfyUI_frontend + +## Overview + +Review for behavior and current repo rules, not motion. Compare to authoritative rules, not prior diffs or legacy snippets. + +## Review Workflow + +1. Identify the test type: component, store, composable, util, or bugfix regression. +2. Name the behavior the test proves. If you cannot say it in one sentence, request changes. +3. Open the authoritative doc section before judging structure. +4. Scan the red flags below. +5. State the verdict first. Name the failure mode. Cite the doc or rule. + +## Source of Truth / Precedence + +When docs and examples conflict, use this order: + +1. Explicit repo rules, lint rules, and note blocks. +2. [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) +3. Rule sections in [`docs/testing/unit-testing.md`](../../../docs/testing/unit-testing.md), [`docs/testing/store-testing.md`](../../../docs/testing/store-testing.md), and [`docs/testing/component-testing.md`](../../../docs/testing/component-testing.md) +4. Example snippets +5. Prior diffs + +Apply these repo-specific clarifications: + +- [`docs/testing/component-testing.md`](../../../docs/testing/component-testing.md) starts with the authoritative rule: new component tests use `@testing-library/vue` with `@testing-library/user-event`. The `@vue/test-utils` snippets below it are legacy examples. +- [`docs/testing/store-testing.md`](../../../docs/testing/store-testing.md) still contains `as any` examples. Treat them as legacy snippets, not approval for new or edited test code. +- If docs conflict, prefer the stricter newer rule and call out the doc ambiguity. Do not approve through it. +- Motion != fix. + +## 30-Second Red Flags + +| If you see... | Failure mode | Default action | +| ----------------------------------------------------------------------------------------- | ------------------------------- | ------------------------------------------------------------- | +| New `@vue/test-utils` import in a new component test | legacy test API | Request changes | +| `vi.mock('vue-i18n', ...)` | mocked i18n | Request changes | +| `as any`, `@ts-expect-error`, `as Mock`, `as ReturnType`, `as unknown as X` | unnecessary cast or type escape | Request changes unless the author proves no safer type exists | +| `getXMock()`, renamed wrapper, or helper that only returns a mocked value | alias-by-renaming | Request changes | +| `beforeEach` recreates the return object for a module-mocked composable or service | shared mock setup drift | Request changes | +| Assertions only check defaults, mock plumbing, or CSS hooks | non-behavioral test | Request changes | +| Bugfix test has no proof it fails on pre-fix code | unproven regression | Request changes | + +## Rationalization Table + +| Excuse | Reality | +| ----------------------------------- | ------------------------------------------------------------------------------------------------------ | +| "I restructured the mocks" | If the indirection stayed, nothing improved. Flag `alias-by-renaming`. | +| "The docs do it" | Rule, note, and lint beat legacy snippet. Compare to the current rule, not the nearest example. | +| "TypeScript required the cast" | `vi.mocked()` usually narrows mock methods. Assertion-only references need no cast. | +| "Putting it in `beforeEach` is DRY" | Recreating module mock state in hooks hides singleton behavior and drifts from the documented pattern. | +| "It is only a nit" | Explicit repo-rule violations are never nits. | +| "No behavior changed, just cleanup" | Motion != fix. Ask what behavior got stronger. | +| "Mental revert is enough" | For bugfix tests, establish red on pre-fix code or ask the author to show it. | + +## Mocking Rules + +- Fail helpers that do not remove repeated setup, encode domain meaning, or simplify assertions. Barely earning the abstraction is not enough. +- For composables with reactive or singleton state, define stable mock state inside the `vi.mock()` factory. Access it per test via the composable itself. See [`docs/testing/unit-testing.md`](../../../docs/testing/unit-testing.md) "Mocking Composables with Reactive State". +- This does not ban local test data builders or per-test `vi.spyOn(...)`. +- Mock seams, not the project-owned module you are trying to exercise. For store tests, prefer real Pinia plus `createTestingPinia({ stubActions: false })` per [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) and [`docs/testing/store-testing.md`](../../../docs/testing/store-testing.md). + +### Alias-by-Renaming + +```ts +// Before +const mockAdd = vi.hoisted(() => vi.fn()) + +// After: same indirection, new name +function getToastAddMock() { + return useToast().add +} +``` + +If the wrapper only renames or relays a mocked value, fail it. Inline the lookup at the call site or fetch the singleton mock via the documented pattern. + +### `vi.mocked()` Scope + +| Use case | `vi.mocked()` required? | +| --------------------------------------------------------------- | ----------------------- | +| `.mockReturnValue`, `.mockResolvedValue`, `.mockImplementation` | Yes | +| `.mock.calls`, `.mock.results` | Yes | +| `expect(fn).toHaveBeenCalled()` | No | +| `expect(fn).toHaveBeenCalledWith(...)` | No | + +- Flag casts whenever `vi.mocked()` would narrow correctly. +- Do not add `vi.mocked()` around assertion-only references just for style. + +### Reset Hygiene + +- Flag per-mock `mockClear()` or `mockReset()` when `vi.clearAllMocks()` or `vi.resetAllMocks()` already runs in the relevant hook chain. +- Review for redundancy or broken state management. Do not bikeshed `clearAllMocks` vs `resetAllMocks` unless behavior depends on it. + +### Third-Party Seams + +- Distinguish trivial hooks from behavior-rich APIs. +- Mocking single-method third-party hooks like `primevue/usetoast` is usually acceptable. +- That exception does not justify mocking behavior-rich third-party modules. + +### `vue-i18n` + +- Never mock `vue-i18n` in component tests. +- Use real `createI18n` per [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) and the shared [`testI18n`](../../../src/components/searchbox/v2/__test__/testUtils.ts) setup. + +## Test-Body Rules + +| Smell | Review bar | +| ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| Change-detector test | Reject. Default values alone prove nothing. | +| Mock-only assertion | Accept collaborator-call assertions only when the call is the meaningful external effect and the test also exercises the triggering behavior. | +| Non-behavioral assertion | Reject tests that only check classes, utility hooks, or styling internals. | +| New component test using `@vue/test-utils` | Request changes. Use `@testing-library/vue` plus `@testing-library/user-event`. | +| `any`, `as any`, or `@ts-expect-error` in new or edited test code | Request changes unless the author proves no safer type exists. Legacy doc snippets do not authorize it. | + +## Bugfix Regression Proof + +For `fix:` PRs or bugfix diffs: + +1. Identify the production change that fixes the bug. +2. Verify the new test fails on pre-fix code, or ask the author to show it. +3. If the test passes on broken code, request changes. + +A regression test that never proves red does not pin the bug. + +## Review Output Rules + +- State verdict before procedural questions. +- Do not lead with approval language like `LGTM, just one nit` or `approve and move on?`. +- Name the failure mode directly: `alias-by-renaming`, `unnecessary cast`, `mocked i18n`, `mock-only assertion`, `unproven regression`. +- Link the authoritative doc section in the review comment. +- If an explicit repo rule, lint rule, or authoritative doc note is violated, do not downgrade it to "minor deviation" or "nit". + +## Quick Reference + +| When you see... | Read this | +| ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| New `vi.mock(...)` for a composable | [`docs/testing/unit-testing.md`](../../../docs/testing/unit-testing.md) -> "Mocking Composables with Reactive State" | +| New store test or store mock | [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) setup + [`docs/testing/store-testing.md`](../../../docs/testing/store-testing.md) | +| New component test | Top note in [`docs/testing/component-testing.md`](../../../docs/testing/component-testing.md) | +| `vue-i18n` in a component test | [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) + [`src/components/searchbox/v2/__test__/testUtils.ts`](../../../src/components/searchbox/v2/__test__/testUtils.ts) | +| Cast around a mock | [`docs/guidance/typescript.md`](../../../docs/guidance/typescript.md) -> "Type Assertion Hierarchy" | + +## Key Files to Read + +| Purpose | Path | +| ------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | +| Composable mocking patterns | [`docs/testing/unit-testing.md`](../../../docs/testing/unit-testing.md) | +| Store testing patterns | [`docs/testing/store-testing.md`](../../../docs/testing/store-testing.md) | +| Repo-wide Vitest setup defaults | [`docs/testing/vitest-patterns.md`](../../../docs/testing/vitest-patterns.md) | +| Component testing rule for new tests | [`docs/testing/component-testing.md`](../../../docs/testing/component-testing.md) | +| Real i18n setup | [`src/components/searchbox/v2/__test__/testUtils.ts`](../../../src/components/searchbox/v2/__test__/testUtils.ts) | diff --git a/.github/actions/ashby-pull/action.yaml b/.github/actions/ashby-pull/action.yaml new file mode 100644 index 0000000000..d1b27f6c67 --- /dev/null +++ b/.github/actions/ashby-pull/action.yaml @@ -0,0 +1,23 @@ +name: Ashby Pull +description: 'Refresh the apps/website Ashby roles snapshot from the Ashby job board API' +inputs: + api_key: + description: 'Ashby API key (WEBSITE_ASHBY_API_KEY).' + required: true + job_board_name: + description: 'Ashby job board name (WEBSITE_ASHBY_JOB_BOARD_NAME).' + required: true +runs: + using: 'composite' + steps: + # Note: this action assumes the frontend repo is checked out at the workspace root. + + - name: Setup frontend + uses: ./.github/actions/setup-frontend + + - name: Refresh Ashby snapshot + shell: bash + env: + WEBSITE_ASHBY_API_KEY: ${{ inputs.api_key }} + WEBSITE_ASHBY_JOB_BOARD_NAME: ${{ inputs.job_board_name }} + run: pnpm --filter @comfyorg/website ashby:refresh-snapshot diff --git a/.github/actions/changes-filter/action.yaml b/.github/actions/changes-filter/action.yaml new file mode 100644 index 0000000000..3a37ca4a32 --- /dev/null +++ b/.github/actions/changes-filter/action.yaml @@ -0,0 +1,87 @@ +# Outputs default to 'true' for non-pull_request events (push, merge_group): +# granular path filtering is a PR-only optimization. This avoids the silent +# skip footgun where a job gated on e.g. `app-website-changes == 'true'` +# would never run on push. +# +# Shared dependency files (root package.json, pnpm-lock.yaml, +# pnpm-workspace.yaml) are folded into every app-* and packages-changes +# output so a lockfile bump correctly invalidates each granular gate. They +# are NOT folded into docs-changes. +# +# Two paths-filter steps are needed because predicate-quantifier=every is +# required for the negated globs in `should-run` but breaks multi-pattern +# OR filters like `docs:` and `deps:`. +# +# Requires the caller to have checked out the repository. + +name: 'Detect Path Changes' +description: > + Computes typed *-changes outputs and a back-compat should-run for + path-gated CI jobs. + +outputs: + should-run: + description: 'Any file outside `apps/`, `docs/`, `.storybook/`, or `**/*.md` changed.' + value: ${{ github.event_name != 'pull_request' || steps.relevant.outputs.relevant == 'true' }} + app-website-changes: + description: 'Shared deps or `apps/website/**` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' || steps.filter.outputs.app_website == 'true' }} + app-desktop-changes: + description: 'Shared deps or `apps/desktop-ui/**` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' || steps.filter.outputs.app_desktop == 'true' }} + app-frontend-changes: + description: 'Shared deps or `src/**` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' || steps.filter.outputs.app_frontend == 'true' }} + packages-changes: + description: 'Shared deps or `packages/**` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' || steps.filter.outputs.packages == 'true' }} + storybook-changes: + description: 'Shared deps or `.storybook/**` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' || steps.filter.outputs.storybook == 'true' }} + docs-changes: + description: '`docs/**` or any `**/*.md` changed (deps NOT folded in).' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.docs == 'true' }} + dependency-changes: + description: 'Root `package.json`, `pnpm-lock.yaml`, or `pnpm-workspace.yaml` changed.' + value: ${{ github.event_name != 'pull_request' || steps.filter.outputs.deps == 'true' }} + +runs: + using: composite + steps: + - name: Filter typed changes + if: ${{ github.event_name == 'pull_request' }} + id: filter + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + with: + filters: | + app_website: + - 'apps/website/**' + app_desktop: + - 'apps/desktop-ui/**' + app_frontend: + - 'src/**' + packages: + - 'packages/**' + storybook: + - '.storybook/**' + docs: + - 'docs/**' + - '**/*.md' + deps: + - 'package.json' + - 'pnpm-lock.yaml' + - 'pnpm-workspace.yaml' + + - name: Filter relevant changes + if: ${{ github.event_name == 'pull_request' }} + id: relevant + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + with: + predicate-quantifier: 'every' + filters: | + relevant: + - '**' + - '!apps/**' + - '!docs/**' + - '!.storybook/**' + - '!**/*.md' diff --git a/.github/workflows/ci-dist-telemetry-scan.yaml b/.github/workflows/ci-dist-telemetry-scan.yaml index 1821efd95d..c5e53d13fd 100644 --- a/.github/workflows/ci-dist-telemetry-scan.yaml +++ b/.github/workflows/ci-dist-telemetry-scan.yaml @@ -12,17 +12,30 @@ permissions: contents: read jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + should-run: ${{ steps.changes.outputs.should-run }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + scan: + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: actions/checkout@v6 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 - name: Use Node.js - uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 + uses: actions/setup-node@v6 with: node-version-file: '.nvmrc' cache: 'pnpm' diff --git a/.github/workflows/ci-oss-assets-validation.yaml b/.github/workflows/ci-oss-assets-validation.yaml index a145ca04f1..c05399d4dc 100644 --- a/.github/workflows/ci-oss-assets-validation.yaml +++ b/.github/workflows/ci-oss-assets-validation.yaml @@ -14,16 +14,29 @@ permissions: contents: read jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + should-run: ${{ steps.changes.outputs.should-run }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + validate-fonts: + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: actions/checkout@v6 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 - name: Use Node.js - uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 + uses: actions/setup-node@v6 with: node-version-file: '.nvmrc' cache: 'pnpm' @@ -68,15 +81,17 @@ jobs: echo '✅ No proprietary fonts found in dist' validate-licenses: + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: actions/checkout@v6 - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 - name: Use Node.js - uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 + uses: actions/setup-node@v6 with: node-version-file: '.nvmrc' cache: 'pnpm' diff --git a/.github/workflows/ci-perf-report.yaml b/.github/workflows/ci-perf-report.yaml index b265e0ebb7..bd6c286f37 100644 --- a/.github/workflows/ci-perf-report.yaml +++ b/.github/workflows/ci-perf-report.yaml @@ -3,10 +3,8 @@ name: 'CI: Performance Report' on: push: branches: [main, core/*] - paths-ignore: ['**/*.md'] pull_request: branches-ignore: [wip/*, draft/*, temp/*] - paths-ignore: ['**/*.md'] concurrency: group: perf-${{ github.ref }} @@ -16,12 +14,24 @@ permissions: contents: read jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + should-run: ${{ steps.changes.outputs.should-run }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + perf-tests: - if: github.repository == 'Comfy-Org/ComfyUI_frontend' + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' && github.repository == 'Comfy-Org/ComfyUI_frontend' }} runs-on: ubuntu-latest timeout-minutes: 30 container: - image: ghcr.io/comfy-org/comfyui-ci-container:0.0.12 + image: ghcr.io/comfy-org/comfyui-ci-container:0.0.17 credentials: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ci-size-data.yaml b/.github/workflows/ci-size-data.yaml index b3e4598fce..ad8e7266c7 100644 --- a/.github/workflows/ci-size-data.yaml +++ b/.github/workflows/ci-size-data.yaml @@ -16,9 +16,22 @@ permissions: contents: read jobs: - collect: + changes: if: github.repository == 'Comfy-Org/ComfyUI_frontend' runs-on: ubuntu-latest + permissions: + contents: read + outputs: + should-run: ${{ steps.changes.outputs.should-run }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + + collect: + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' }} + runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 diff --git a/.github/workflows/ci-tests-e2e.yaml b/.github/workflows/ci-tests-e2e.yaml index 97785d254e..76ff79d873 100644 --- a/.github/workflows/ci-tests-e2e.yaml +++ b/.github/workflows/ci-tests-e2e.yaml @@ -4,7 +4,6 @@ name: 'CI: Tests E2E' on: push: branches: [main, master, core/*, desktop/*] - paths-ignore: ['**/*.md'] pull_request: branches-ignore: [wip/*, draft/*, temp/*] merge_group: @@ -15,36 +14,20 @@ concurrency: cancel-in-progress: true jobs: - # Detect whether e2e-relevant files changed. Required checks see "skipped" - # (which counts as passing) when only docs/apps/storybook files are touched, - # avoiding the stall that paths-ignore would cause. changes: runs-on: ubuntu-latest permissions: contents: read outputs: - should_run: ${{ github.event_name != 'pull_request' || steps.filter.outputs.e2e }} + should-run: ${{ steps.changes.outputs.should-run }} steps: - - name: Checkout repository - if: ${{ github.event_name == 'pull_request' }} - uses: actions/checkout@v6 - - name: Check for e2e-relevant changes - if: ${{ github.event_name == 'pull_request' }} - id: filter - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 - with: - predicate-quantifier: 'every' - filters: | - e2e: - - '**' - - '!apps/**' - - '!docs/**' - - '!.storybook/**' - - '!**/*.md' + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter setup: needs: changes - if: ${{ needs.changes.outputs.should_run == 'true' }} + if: ${{ needs.changes.outputs.should-run == 'true' }} runs-on: ubuntu-latest steps: - name: Checkout repository @@ -82,7 +65,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 container: - image: ghcr.io/comfy-org/comfyui-ci-container:0.0.16 + image: ghcr.io/comfy-org/comfyui-ci-container:0.0.17 credentials: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} @@ -140,7 +123,7 @@ jobs: needs: setup runs-on: ubuntu-latest container: - image: ghcr.io/comfy-org/comfyui-ci-container:0.0.16 + image: ghcr.io/comfy-org/comfyui-ci-container:0.0.17 credentials: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} @@ -194,7 +177,7 @@ jobs: merge-reports: needs: [changes, playwright-tests-chromium-sharded] runs-on: ubuntu-latest - if: ${{ !cancelled() && needs.changes.outputs.should_run == 'true' }} + if: ${{ !cancelled() && needs.changes.outputs.should-run == 'true' }} steps: - name: Install pnpm uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 @@ -233,7 +216,7 @@ jobs: steps: - name: Check E2E results env: - SHOULD_RUN: ${{ needs.changes.outputs.should_run }} + SHOULD_RUN: ${{ needs.changes.outputs.should-run }} SHARDED: ${{ needs.playwright-tests-chromium-sharded.result }} BROWSERS: ${{ needs.playwright-tests.result }} run: | @@ -251,7 +234,7 @@ jobs: runs-on: ubuntu-latest if: >- ${{ - needs.changes.outputs.should_run == 'true' && + needs.changes.outputs.should-run == 'true' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false }} @@ -278,7 +261,7 @@ jobs: if: >- ${{ always() && - needs.changes.outputs.should_run == 'true' && + needs.changes.outputs.should-run == 'true' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false }} diff --git a/.github/workflows/ci-tests-storybook.yaml b/.github/workflows/ci-tests-storybook.yaml index d2fa26826c..f53a254cf9 100644 --- a/.github/workflows/ci-tests-storybook.yaml +++ b/.github/workflows/ci-tests-storybook.yaml @@ -8,10 +8,29 @@ on: branches: [main] jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + storybook-changes: ${{ steps.changes.outputs.storybook-changes }} + app-frontend-changes: ${{ steps.changes.outputs.app-frontend-changes }} + packages-changes: ${{ steps.changes.outputs.packages-changes }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + # Post starting comment for non-forked PRs comment-on-pr-start: + needs: changes runs-on: ubuntu-latest - if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false + if: | + github.event_name == 'pull_request' + && github.event.pull_request.head.repo.fork == false + && (needs.changes.outputs.storybook-changes == 'true' + || needs.changes.outputs.app-frontend-changes == 'true' + || needs.changes.outputs.packages-changes == 'true') permissions: pull-requests: write steps: @@ -30,8 +49,13 @@ jobs: # Build Storybook for all PRs (free Cloudflare deployment) storybook-build: + needs: changes runs-on: ubuntu-latest - if: github.event_name == 'pull_request' + if: | + github.event_name == 'pull_request' + && (needs.changes.outputs.storybook-changes == 'true' + || needs.changes.outputs.app-frontend-changes == 'true' + || needs.changes.outputs.packages-changes == 'true') outputs: conclusion: ${{ steps.job-status.outputs.conclusion }} workflow-url: ${{ steps.workflow-url.outputs.url }} @@ -67,8 +91,15 @@ jobs: # Chromatic deployment only for version-bump-* branches or manual triggers chromatic-deployment: + needs: changes runs-on: ubuntu-latest - if: github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && startsWith(github.head_ref, 'version-bump-')) + if: | + github.event_name == 'workflow_dispatch' + || (github.event_name == 'pull_request' + && startsWith(github.head_ref, 'version-bump-') + && (needs.changes.outputs.storybook-changes == 'true' + || needs.changes.outputs.app-frontend-changes == 'true' + || needs.changes.outputs.packages-changes == 'true')) outputs: conclusion: ${{ steps.job-status.outputs.conclusion }} workflow-url: ${{ steps.workflow-url.outputs.url }} @@ -107,9 +138,15 @@ jobs: # Deploy and comment for non-forked PRs only deploy-and-comment: - needs: [storybook-build] + needs: [changes, storybook-build] runs-on: ubuntu-latest - if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false && always() + if: | + always() + && github.event_name == 'pull_request' + && github.event.pull_request.head.repo.fork == false + && (needs.changes.outputs.storybook-changes == 'true' + || needs.changes.outputs.app-frontend-changes == 'true' + || needs.changes.outputs.packages-changes == 'true') permissions: pull-requests: write contents: read diff --git a/.github/workflows/ci-tests-unit.yaml b/.github/workflows/ci-tests-unit.yaml index 3fc3095bd6..352eb8a49d 100644 --- a/.github/workflows/ci-tests-unit.yaml +++ b/.github/workflows/ci-tests-unit.yaml @@ -4,10 +4,8 @@ name: 'CI: Tests Unit' on: push: branches: [main, master, dev*, core/*, desktop/*] - paths-ignore: ['**/*.md'] pull_request: branches-ignore: [wip/*, draft/*, temp/*] - paths-ignore: ['**/*.md'] merge_group: concurrency: @@ -15,7 +13,20 @@ concurrency: cancel-in-progress: true jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + should-run: ${{ steps.changes.outputs.should-run }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + test: + needs: changes + if: ${{ needs.changes.outputs.should-run == 'true' }} runs-on: ubuntu-latest steps: diff --git a/.github/workflows/ci-vercel-website-preview.yaml b/.github/workflows/ci-vercel-website-preview.yaml index 7a26fd178d..3588cfc2bf 100644 --- a/.github/workflows/ci-vercel-website-preview.yaml +++ b/.github/workflows/ci-vercel-website-preview.yaml @@ -52,6 +52,9 @@ jobs: run: vercel pull --yes --environment=preview - name: Build project artifacts + env: + WEBSITE_ASHBY_API_KEY: ${{ secrets.WEBSITE_ASHBY_API_KEY }} + WEBSITE_ASHBY_JOB_BOARD_NAME: ${{ secrets.WEBSITE_ASHBY_JOB_BOARD_NAME }} run: vercel build - name: Fetch head commit metadata @@ -146,6 +149,9 @@ jobs: run: vercel pull --yes --environment=production - name: Build project artifacts + env: + WEBSITE_ASHBY_API_KEY: ${{ secrets.WEBSITE_ASHBY_API_KEY }} + WEBSITE_ASHBY_JOB_BOARD_NAME: ${{ secrets.WEBSITE_ASHBY_JOB_BOARD_NAME }} run: vercel build --prod - name: Deploy project artifacts to Vercel diff --git a/.github/workflows/ci-website-build.yaml b/.github/workflows/ci-website-build.yaml index 832854c2eb..211ee86960 100644 --- a/.github/workflows/ci-website-build.yaml +++ b/.github/workflows/ci-website-build.yaml @@ -4,23 +4,29 @@ name: 'CI: Website Build' on: push: branches: [main, master, website/*] - paths: - - 'apps/website/**' - - 'packages/design-system/**' - - 'pnpm-lock.yaml' pull_request: branches-ignore: [wip/*, draft/*, temp/*] - paths: - - 'apps/website/**' - - 'packages/design-system/**' - - 'pnpm-lock.yaml' concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + app-website-changes: ${{ steps.changes.outputs.app-website-changes }} + packages-changes: ${{ steps.changes.outputs.packages-changes }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + build: + needs: changes + if: ${{ needs.changes.outputs.app-website-changes == 'true' || needs.changes.outputs.packages-changes == 'true' }} runs-on: ubuntu-latest steps: @@ -30,4 +36,7 @@ jobs: uses: ./.github/actions/setup-frontend - name: Build website + env: + WEBSITE_ASHBY_API_KEY: ${{ secrets.WEBSITE_ASHBY_API_KEY }} + WEBSITE_ASHBY_JOB_BOARD_NAME: ${{ secrets.WEBSITE_ASHBY_JOB_BOARD_NAME }} run: pnpm --filter @comfyorg/website build diff --git a/.github/workflows/ci-website-e2e.yaml b/.github/workflows/ci-website-e2e.yaml index 79032241e9..ea8e7f0592 100644 --- a/.github/workflows/ci-website-e2e.yaml +++ b/.github/workflows/ci-website-e2e.yaml @@ -3,25 +3,29 @@ name: 'CI: Website E2E' on: push: branches: [main] - paths: - - 'apps/website/**' - - 'packages/design-system/**' - - 'packages/tailwind-utils/**' - - 'pnpm-lock.yaml' pull_request: branches-ignore: [wip/*, draft/*, temp/*] - paths: - - 'apps/website/**' - - 'packages/design-system/**' - - 'packages/tailwind-utils/**' - - 'pnpm-lock.yaml' concurrency: group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.ref }} cancel-in-progress: true jobs: + changes: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + app-website-changes: ${{ steps.changes.outputs.app-website-changes }} + packages-changes: ${{ steps.changes.outputs.packages-changes }} + steps: + - uses: actions/checkout@v6 + - id: changes + uses: ./.github/actions/changes-filter + website-e2e: + needs: changes + if: ${{ needs.changes.outputs.app-website-changes == 'true' || needs.changes.outputs.packages-changes == 'true' }} runs-on: ubuntu-latest container: image: mcr.microsoft.com/playwright:v1.58.1-noble @@ -45,6 +49,8 @@ jobs: run: pnpm install --frozen-lockfile - name: Build website + env: + WEBSITE_GITHUB_STARS_OVERRIDE: 110000 run: pnpm --filter @comfyorg/website build - name: Run Playwright tests @@ -161,7 +167,11 @@ jobs: post-starting-comment: # Safe to comment from pull_request trigger: fork PRs are excluded by the guard below. # This avoids a ci-*/pr-* workflow_run split for a comment that must appear immediately. - if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false + needs: changes + if: | + github.event_name == 'pull_request' + && github.event.pull_request.head.repo.fork == false + && (needs.changes.outputs.app-website-changes == 'true' || needs.changes.outputs.packages-changes == 'true') runs-on: ubuntu-latest permissions: pull-requests: write diff --git a/.github/workflows/pr-update-playwright-expectations.yaml b/.github/workflows/pr-update-playwright-expectations.yaml index 308c54b74f..3291d45767 100644 --- a/.github/workflows/pr-update-playwright-expectations.yaml +++ b/.github/workflows/pr-update-playwright-expectations.yaml @@ -77,7 +77,7 @@ jobs: needs: setup runs-on: ubuntu-latest container: - image: ghcr.io/comfy-org/comfyui-ci-container:0.0.16 + image: ghcr.io/comfy-org/comfyui-ci-container:0.0.17 credentials: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/pr-update-website-screenshots.yaml b/.github/workflows/pr-update-website-screenshots.yaml index 87f8163996..9e8d58e564 100644 --- a/.github/workflows/pr-update-website-screenshots.yaml +++ b/.github/workflows/pr-update-website-screenshots.yaml @@ -18,6 +18,7 @@ jobs: timeout-minutes: 15 permissions: contents: write + issues: write pull-requests: read # Trigger: (1) label, (2) /slash-command, or (3) checkbox in E2E status comment # ⚠️ This condition is duplicated on `post-starting-comment` — keep them in sync. @@ -86,6 +87,8 @@ jobs: run: pnpm install --frozen-lockfile - name: Build website + env: + WEBSITE_GITHUB_STARS_OVERRIDE: 110000 run: pnpm --filter @comfyorg/website build - name: Update screenshots @@ -137,7 +140,10 @@ jobs: name: 'Update Website Screenshots' }) } catch (e) { - // Label may already be removed + if (e.status !== 404) { + throw e + } + core.info('Label "Update Website Screenshots" was already removed') } post-starting-comment: diff --git a/.github/workflows/release-website.yaml b/.github/workflows/release-website.yaml new file mode 100644 index 0000000000..8ec080bddd --- /dev/null +++ b/.github/workflows/release-website.yaml @@ -0,0 +1,59 @@ +# Description: Manual workflow to refresh the apps/website Ashby roles snapshot +# and open a PR. Merging the PR triggers the existing Vercel website production +# deploy via ci-vercel-website-preview.yaml. +name: 'Release: Website' + +on: + workflow_dispatch: + +concurrency: + group: release-website + cancel-in-progress: true + +jobs: + refresh-snapshot: + if: github.repository == 'Comfy-Org/ComfyUI_frontend' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + ref: main + persist-credentials: false + + - name: Refresh Ashby snapshot + uses: ./.github/actions/ashby-pull + with: + api_key: ${{ secrets.WEBSITE_ASHBY_API_KEY }} + job_board_name: ${{ secrets.WEBSITE_ASHBY_JOB_BOARD_NAME }} + + - name: Create Pull Request + uses: peter-evans/create-pull-request@c0f553fe549906ede9cf27b5156039d195d2ece0 # v8.1.0 + with: + token: ${{ secrets.PR_GH_TOKEN }} + commit-message: 'chore(website): refresh Ashby roles snapshot' + title: 'chore(website): refresh Ashby roles snapshot' + body: | + Automated refresh of `apps/website/src/data/ashby-roles.snapshot.json` + from the Ashby job board API. + + **Flow:** + 1. `Release: Website` workflow ran (manual trigger). + 2. This PR opens with the regenerated snapshot. + 3. `CI: Vercel Website Preview` deploys a preview for review. + 4. Merging to `main` triggers the production Vercel deploy. + + The snapshot fallback in `apps/website/src/utils/ashby.ts` remains + intact: builds without `WEBSITE_ASHBY_API_KEY` continue to use the + committed snapshot. + + Triggered by workflow run `${{ github.run_id }}`. + branch: chore/refresh-ashby-snapshot-${{ github.run_id }} + base: main + labels: | + Release:Website + delete-branch: true diff --git a/.husky/pre-push b/.husky/pre-push index dcb5564518..642dd8ac88 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,5 +1,13 @@ #!/usr/bin/env bash +# Skip in CI: the canonical knip check runs in ci-lint-format on every +# PR, and bot workflows (e.g. i18n-update-core) populate ComfyUI/ via +# setup-comfyui-server, which contaminates knip's project glob with the +# devtools copy under custom_nodes and produces false-positive failures. +if [ -n "${CI-}" ]; then + exit 0 +fi + # Run Knip with cache via package script pnpm knip 1>&2 diff --git a/apps/website/README.md b/apps/website/README.md index a20338aecb..047940ed02 100644 --- a/apps/website/README.md +++ b/apps/website/README.md @@ -113,6 +113,31 @@ git commit apps/website/src/data/ashby-roles.snapshot.json The script exits non-zero on any non-fresh outcome so stale/empty snapshots can't be accidentally committed. +## HubSpot contact form + +The contact page uses HubSpot's hosted form embed for the interest form: + +```html + +
+``` + +The localized `/zh-CN/contact` page uses the same portal and script with form +ID `6885750c-02ef-4aa2-ba0d-213be9cccf93`. + +This keeps submission handling, validation, anti-spam updates, and field +configuration in HubSpot. The local implementation in +`src/components/contact/HubspotFormEmbed.vue` only loads the hosted script and +renders the documented embed container. + ## Scripts - `pnpm dev` — Astro dev server diff --git a/apps/website/e2e/visual-responsive.spec.ts-snapshots/contact-form-1-sm-visual-linux.png b/apps/website/e2e/visual-responsive.spec.ts-snapshots/contact-form-1-sm-visual-linux.png index 65a242f834..a0f2f36815 100644 Binary files a/apps/website/e2e/visual-responsive.spec.ts-snapshots/contact-form-1-sm-visual-linux.png and b/apps/website/e2e/visual-responsive.spec.ts-snapshots/contact-form-1-sm-visual-linux.png differ diff --git a/apps/website/package.json b/apps/website/package.json index 1f72fc97e3..5a4aa57c3b 100644 --- a/apps/website/package.json +++ b/apps/website/package.json @@ -26,6 +26,7 @@ "cva": "catalog:", "gsap": "catalog:", "lenis": "catalog:", + "posthog-js": "catalog:", "vue": "catalog:", "zod": "catalog:" }, diff --git a/apps/website/public/robots.txt b/apps/website/public/robots.txt index b1931edbd8..5e6114b55e 100644 --- a/apps/website/public/robots.txt +++ b/apps/website/public/robots.txt @@ -1,4 +1,33 @@ +# robots.txt for comfy.org +# Open to all crawlers — including AI/LLM bots — for maximum visibility +# in AI-powered search, chat-based answer engines, and traditional search. +# Granular UAs are listed explicitly to signal intent; rules are shared +# via stacked user-agent records (RFC 9309 §2.2). + User-agent: * +User-agent: Googlebot +User-agent: Bingbot +User-agent: DuckDuckBot +User-agent: GPTBot +User-agent: ChatGPT-User +User-agent: OAI-SearchBot +User-agent: Google-Extended +User-agent: ClaudeBot +User-agent: Claude-Web +User-agent: anthropic-ai +User-agent: PerplexityBot +User-agent: Perplexity-User +User-agent: Applebot +User-agent: Applebot-Extended +User-agent: Bytespider +User-agent: Amazonbot +User-agent: CCBot +User-agent: Meta-ExternalAgent +User-agent: Meta-ExternalFetcher +User-agent: Diffbot Allow: / +Disallow: /_astro/ +Disallow: /_website/ +Disallow: /_vercel/ Sitemap: https://comfy.org/sitemap-index.xml diff --git a/apps/website/src/components/contact/FormSection.vue b/apps/website/src/components/contact/FormSection.vue index dbefd4b80c..114a16ae34 100644 --- a/apps/website/src/components/contact/FormSection.vue +++ b/apps/website/src/components/contact/FormSection.vue @@ -1,13 +1,12 @@