mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-05 21:54:50 +00:00
## Summary Extract a `.github/actions/changes-filter` composite action and adopt it across path-gated CI workflows, fixing the docs-only PR stall and removing duplicated `paths:` / `paths-ignore:` filtering across 8 workflows. ## Background Docs-only PRs stalled on required status checks because workflows using `paths-ignore: ['**/*.md']` never created a check run, while branch protection still required it. Observed on #11776 (the `test` check from `ci-tests-unit.yaml` never appeared). The fix pattern: keep the workflow triggered, gate downstream jobs on a `changes` job whose outputs are computed from a path filter. Skipped jobs count as passing under branch protection. ## What the action emits | Output | Meaning | |---|---| | `should-run` | Any file outside `apps/`, `docs/`, `.storybook/`, `**/*.md` changed. | | `app-website-changes` | Shared deps or `apps/website/**` changed. | | `app-desktop-changes` | Shared deps or `apps/desktop-ui/**` changed. | | `app-frontend-changes` | Shared deps or `src/**` changed. | | `packages-changes` | Shared deps or `packages/**` changed. | | `storybook-changes` | Shared deps or `.storybook/**` changed. | | `docs-changes` | `docs/**` or any `**/*.md` changed (deps NOT folded in). | | `dependency-changes` | Root `package.json`, `pnpm-lock.yaml`, or `pnpm-workspace.yaml` changed. | Shared deps are folded into every `app-*`, `packages-changes`, and `storybook-changes` output so a lockfile bump correctly invalidates each granular gate. Outputs default to `'true'` for non-`pull_request` events to avoid the silent-skip footgun on push / merge_group. ## Workflows migrated | Workflow | Gate | Notes | |---|---|---| | `ci-tests-unit.yaml` | `should-run` | Required check (`test`). Fixes the original stall. | | `ci-tests-e2e.yaml` | `should-run` | Required check (`e2e-status`). Replaces inline filter. | | `ci-perf-report.yaml` | `should-run` | Removes `paths-ignore`. | | `ci-website-build.yaml` | `app-website-changes \|\| packages-changes` | Refactor — not a required check, but moves to job-level gating. Filter scope broadens from `packages/{design-system}` to all `packages/**` (strictly safer). | | `ci-website-e2e.yaml` | `app-website-changes \|\| packages-changes` | Same restructure; `post-starting-comment` also gated to avoid spurious "tests are running" when E2E is skipped. | | `ci-dist-telemetry-scan.yaml` | `should-run` | New gate; was previously running on every PR including docs-only. | | `ci-oss-assets-validation.yaml` | `should-run` | Same. | | `ci-size-data.yaml` | `should-run` | Preserves existing repository guard on the new `changes` job. | | `ci-tests-storybook.yaml` | `storybook-changes \|\| app-frontend-changes \|\| packages-changes` | Gates 4 of 6 jobs. `deploy-production` (push to main) left ungated; `update-comment-with-chromatic` cascades naturally. | ## Branch protection (verified) Required status checks on `main` and `core/**`/`cloud/**`: `test`, `lint-and-format`, `e2e-status`. Only `test` and `e2e-status` use the composite — `lint-and-format` correctly stays unfiltered (must run on docs/apps too). The other 6 migrations are refactor wins (less wasted CI on docs/apps-only PRs), not stall fixes. ## Changes - **What**: New `.github/actions/changes-filter` composite + 8 workflow migrations to consume it. - **Breaking**: None. - **Dependencies**: New pin on `dorny/paths-filter@de90cc6` — already covered by `ci-validate-action-pins`. ## Review Focus - The `should-run` filter excludes `.storybook/**` (granular `storybook-changes` covers it instead). Storybook's gate combines all three: `storybook-changes || app-frontend-changes || packages-changes`. - Two `dorny/paths-filter` steps inside the composite — `predicate-quantifier=every` is required for the negated globs in `should-run` but breaks the multi-pattern OR filters. - The website filter scope intentionally broadens from `packages/{design-system,tailwind-utils}/**` to all `packages/**` for consistency and safety. Fixes #11776 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11785-ci-extract-changes-filter-composite-action-fix-docs-only-PR-stall-3526d73d36508172a1d7fe8c30fa6453) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>
112 lines
3.4 KiB
YAML
112 lines
3.4 KiB
YAML
name: 'CI: Dist Telemetry Scan'
|
|
|
|
on:
|
|
pull_request:
|
|
branches-ignore: [wip/*, draft/*, temp/*]
|
|
|
|
concurrency:
|
|
group: ${{ github.workflow }}-${{ github.ref }}
|
|
cancel-in-progress: true
|
|
|
|
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@v6
|
|
|
|
- name: Install pnpm
|
|
uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0
|
|
|
|
- name: Use Node.js
|
|
uses: actions/setup-node@v6
|
|
with:
|
|
node-version-file: '.nvmrc'
|
|
cache: 'pnpm'
|
|
|
|
- name: Install dependencies
|
|
run: pnpm install --frozen-lockfile
|
|
|
|
- name: Build project
|
|
run: pnpm build
|
|
env:
|
|
DISTRIBUTION: localhost
|
|
|
|
- name: Scan dist for GTM telemetry references
|
|
run: |
|
|
set -euo pipefail
|
|
echo '🔍 Scanning for Google Tag Manager references...'
|
|
if rg --no-ignore -n \
|
|
-g '*.html' \
|
|
-g '*.js' \
|
|
-e 'Google Tag Manager' \
|
|
-e '(?i)\bgtm\.js\b' \
|
|
-e '(?i)googletagmanager\.com/gtm\.js\\?id=' \
|
|
-e '(?i)googletagmanager\.com/ns\.html\\?id=' \
|
|
dist; then
|
|
echo '❌ ERROR: Google Tag Manager references found in dist assets!'
|
|
echo 'GTM must be properly tree-shaken from OSS builds.'
|
|
exit 1
|
|
fi
|
|
echo '✅ No GTM references found'
|
|
|
|
- name: Scan dist for Mixpanel telemetry references
|
|
run: |
|
|
set -euo pipefail
|
|
echo '🔍 Scanning for Mixpanel references...'
|
|
if rg --no-ignore -n \
|
|
-g '*.html' \
|
|
-g '*.js' \
|
|
-e '(?i)mixpanel\.init' \
|
|
-e '(?i)mixpanel\.identify' \
|
|
-e 'MixpanelTelemetryProvider' \
|
|
-e 'mp\.comfy\.org' \
|
|
-e 'mixpanel-browser' \
|
|
-e '(?i)mixpanel\.track\(' \
|
|
dist; then
|
|
echo '❌ ERROR: Mixpanel references found in dist assets!'
|
|
echo 'Mixpanel must be properly tree-shaken from OSS builds.'
|
|
echo ''
|
|
echo 'To fix this:'
|
|
echo '1. Use the TelemetryProvider pattern (see src/platform/telemetry/)'
|
|
echo '2. Call telemetry via useTelemetry() hook'
|
|
echo '3. Use conditional dynamic imports behind isCloud checks'
|
|
exit 1
|
|
fi
|
|
echo '✅ No Mixpanel references found'
|
|
|
|
- name: Scan dist for PostHog telemetry references
|
|
run: |
|
|
set -euo pipefail
|
|
echo '🔍 Scanning for PostHog references...'
|
|
if rg --no-ignore -n \
|
|
-g '*.html' \
|
|
-g '*.js' \
|
|
-e '(?i)posthog\.init' \
|
|
-e '(?i)posthog\.capture' \
|
|
-e 'PostHogTelemetryProvider' \
|
|
-e 'ph\.comfy\.org' \
|
|
-e 'posthog-js' \
|
|
dist; then
|
|
echo '❌ ERROR: PostHog references found in dist assets!'
|
|
echo 'PostHog must be properly tree-shaken from OSS builds.'
|
|
exit 1
|
|
fi
|
|
echo '✅ No PostHog references found'
|