Commit Graph

7 Commits

Author SHA1 Message Date
Alexander Brown
f566abdd6e ci: extract changes-filter composite action; fix docs-only PR stall (#11785)
## 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>
2026-05-01 18:06:29 +00:00
Alexander Brown
81e6282599 Chore: pnpm build ignores and version centralization (#10687)
## Summary

Just pnpm pieces. Centralize the pnpm version for corepack/actions.
Ignore builds from some recent deps.
2026-03-28 16:38:02 -07:00
sno
4a8f68a6bd ci: upgrade pnpm/action-setup to v4.4.0 (Node.js 24) (#10137)
Upgrades `pnpm/action-setup` from v4.2.0 to v4.4.0 across all 16
workflow files and the shared `setup-frontend` action.

## Why

GitHub Actions will force Node.js 24 as the default starting June 2,
2026. The v4.2.0 pin ran on Node.js 20 and emitted deprecation warnings
on every CI run. v4.4.0 was released specifically to address this,
updating the action runtime to Node.js 24.

- Fixes the warning: *"pnpm/action-setup@41ff72... Actions will be
forced to run with Node.js 24 by default starting June 2nd, 2026"*

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10137-ci-upgrade-pnpm-action-setup-to-v4-4-0-Node-js-24-3266d73d36508176b157fcd1d33f2274)
by [Unito](https://www.unito.io)
2026-03-17 05:53:24 -07:00
Benjamin Lu
750a2d23e0 chore: standardize on Node 24 (#9521)
## Summary

Standardize the repo's Node contract on 24 while centralizing workflow
resolution through `.nvmrc` so local setup, CI, and package metadata
stay aligned from one version file.

## Changes

- **What**: Add `package.json` `engines.node = 24.x`, switch every
`actions/setup-node` workflow in the repo to `node-version-file:
'.nvmrc'`, and update contributor and Playwright docs to point to
`.nvmrc` as the Node source of truth.

## Review Focus

The workflow behavior should be unchanged apart from sourcing the Node
version from `.nvmrc` instead of repeating literals like `20`, `22`,
`24.x`, or `lts/*`. GitHub's formatter also moved the new `engines`
block to the package metadata section near the end of `package.json`.

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-03-07 09:06:10 +00:00
Robin Huang
6c2680f0ba feat: Add PostHog telemetry provider (#9409)
Add PostHog as a telemetry provider for cloud builds so custom events
can be correlated with session recordings. Follows the same pattern as
MixpanelTelemetryProvider with dynamic import, event queuing, and
disabled events from remote config. Tree-shaken away in OSS builds.

The posthog-js package uses Apache-2.0 (verified from its LICENSE file)
but declares it as "SEE LICENSE IN LICENSE" in package.json, which
  the license checker can't parse.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9409-feat-Add-PostHog-telemetry-provider-31a6d73d3650818b8e86c772c6551099)
by [Unito](https://www.unito.io)
2026-03-05 16:19:35 -08:00
sno
06732b84bb Add Mixpanel detection to telemetry tree-shaking validation (#8826)
## Summary
Enhances the existing CI telemetry scan workflow to also detect Mixpanel
code in dist files, ensuring it's properly tree-shaken from OSS builds.

## Context
- Extends existing `ci-dist-telemetry-scan.yaml` (added in PR #8354)
- Based on analysis in closed PR #6777 (split into focused PRs)
- Complements GTM detection already in place
- Part of comprehensive OSS compliance effort

## Implementation
- Adds separate Mixpanel check step with specific patterns:
  - `mixpanel.init`
  - `mixpanel.identify` 
  - `MixpanelTelemetryProvider`
  - `mp.comfy.org`
  - `mixpanel-browser`
  - `mixpanel.track(`
- Separates GTM and Mixpanel checks for clarity
- Adds `DISTRIBUTION=localhost` env var to build step
- Excludes source maps from scanning

## Related
- Supersedes part of closed PR #6777
- Complements existing GTM check from PR #8354
- Related to PR #8623 (GTM-focused, may be redundant)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8826-Add-Mixpanel-detection-to-telemetry-tree-shaking-validation-3056d73d36508153bab5f55d4bb17658)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-20 02:31:37 -08:00
Benjamin Lu
dd4d36d459 fix: route gtm through telemetry entrypoint (#8354)
Wire checkout attribution into GTM events and checkout POST payloads.

This updates the cloud telemetry flow so the backend team can correlate checkout events without relying on frontend cookie parsing. We now surface GA4 identity via a GTM-provided global and include attribution on both `begin_checkout` telemetry and the checkout POST body. The backend should continue to derive the Firebase UID from the auth header; the checkout POST body does not include a user ID.

GTM events pushed (unchanged list, updated payloads):
- `page_view` (page title/location/referrer as before)
- `sign_up` / `login`
- `begin_checkout` now includes:
  - `user_id`, `tier`, `cycle`, `checkout_type`, `previous_tier` (if change flow)
  - `ga_client_id`, `ga_session_id`, `ga_session_number`
  - `gclid`, `gbraid`, `wbraid`

Backend-facing change:
- `POST /customers/cloud-subscription-checkout/:tier` now includes a JSON body with attribution fields only:
  - `ga_client_id`, `ga_session_id`, `ga_session_number`
  - `gclid`, `gbraid`, `wbraid`
- Backend should continue to derive the Firebase UID from the auth header.

Required GTM setup:
- Provide `window.__ga_identity__` via a GTM Custom HTML tag (after GA4/Google tag) with `{ client_id, session_id, session_number }`. The frontend reads this to populate the GA fields.

<img width="1416" height="1230" alt="image" src="https://github.com/user-attachments/assets/b77cf0ed-be69-4497-a540-86e5beb7bfac" />

## Screenshots (if applicable)

<img width="991" height="385" alt="image" src="https://github.com/user-attachments/assets/8309cd9e-5ab5-4fba-addb-2d101aaae7e9"/>

Manual Testing:
<img width="3839" height="2020" alt="image" src="https://github.com/user-attachments/assets/36901dfd-08db-4c07-97b8-a71e6783c72f"/>
<img width="2141" height="851" alt="image" src="https://github.com/user-attachments/assets/2e9f7aa4-4716-40f7-b147-1c74b0ce8067"/>
<img width="2298" height="982" alt="image" src="https://github.com/user-attachments/assets/72cbaa53-9b92-458a-8539-c987cf753b02"/>
<img width="2125" height="999" alt="image" src="https://github.com/user-attachments/assets/4b22387e-8027-4f50-be49-a410282a1adc"/>

To manually test, you will need to override api/features in devtools to also return this:

```
"gtm_container_id": "GTM-NP9JM6K7"
```

┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8354-fix-route-gtm-through-telemetry-entrypoint-2f66d73d36508138afacdeffe835f28a) by [Unito](https://www.unito.io)


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
  * Analytics expanded: page view tracking, richer auth telemetry (includes user IDs), and checkout begin events with attribution.
  * Google Tag Manager support and persistent checkout attribution (GA/client/session IDs, gclid/gbraid/wbraid).

* **Chores**
  * Telemetry reworked to support multiple providers via a registry with cloud-only initialization.
  * Workflow module refactored for clearer exports.

* **Tests**
  * Added/updated tests for attribution, telemetry, and subscription flows.

* **CI**
  * New check prevents telemetry from leaking into distribution artifacts.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-02-07 01:08:48 -08:00