Skip to content

Commit ccb7424

Browse files
committed
Merge remote-tracking branch 'origin/main' into synap5e/feat/model-type-flag-features-endpoint
# Conflicts: # src/composables/useFeatureFlags.test.ts
2 parents 26becd1 + 4e0e647 commit ccb7424

299 files changed

Lines changed: 24066 additions & 10753 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
---
2+
name: publishing-a-new-package
3+
description: 'Publishes a new package from this monorepo to npm under @comfyorg and proves it is consumable from another repo. Covers workflow scaffolding, the first-publish 404, catalog: rewriting, trusted publishing, and the consumer smoke test. Use when adding a package to packages/, publishing to npm for the first time, or when an npm publish workflow fails.'
4+
---
5+
6+
# Publishing a New Package
7+
8+
Getting a package onto npm is not done when the workflow goes green. It is done
9+
when someone in another repo can install it and it works. Most of the failures
10+
below happen after the "publish succeeded" line.
11+
12+
## The first publish is different
13+
14+
**A brand-new package name will fail the first CI publish unless the token was
15+
scoped for it**, and the error does not say so. npm returns:
16+
17+
```text
18+
[E404] 404 Not Found - PUT https://registry.npmjs.org/@comfyorg%2fyour-package
19+
```
20+
21+
A 404 on `PUT` means the token may publish _existing_ packages in the scope but
22+
may not _create_ a new name — a granular token whose write access is a
23+
hand-picked package list cannot include a package that does not exist yet. It
24+
reads like "the package doesn't exist" — which is true and irrelevant — and
25+
sends you looking for a workflow bug that isn't there. `--access public` is
26+
already set; the registry URL is already right.
27+
28+
Two ways out, both fine:
29+
30+
1. Give the CI token read+write on the whole `@comfyorg` scope — the narrowest
31+
grant that can create a new name, and preferable to all-packages access —
32+
then re-run the workflow.
33+
2. Publish once by hand, then make sure the CI token covers the new name.
34+
35+
Trusted publishing (OIDC) cannot bootstrap either — npm requires the package to
36+
exist before a trusted publisher can be configured ([npm/cli#8544](https://github.com/npm/cli/issues/8544)).
37+
So the ordering is always: first publish by token → configure trusted publisher
38+
→ switch CI to OIDC.
39+
40+
## Never `npm publish` from this repo
41+
42+
Workspace packages use pnpm catalog specifiers:
43+
44+
```json
45+
"dependencies": { "@iconify/utils": "catalog:" }
46+
```
47+
48+
`pnpm publish` rewrites those to real ranges when it packs. `npm publish` ships
49+
the literal string `"catalog:"`, and every consumer install breaks. The tarball
50+
looks fine locally either way — the damage only shows up in the consumer.
51+
52+
Check before you publish anything:
53+
54+
```sh
55+
cd packages/<name>
56+
pnpm pack --pack-destination /tmp
57+
tar -xzOf /tmp/comfyorg-<name>-<version>.tgz package/package.json | jq .dependencies
58+
```
59+
60+
Every value must be a real range. If you see `catalog:`, you used the wrong tool.
61+
62+
## Scaffolding the workflows
63+
64+
Copy the four-workflow set from an existing package — `design-system` and
65+
`desktop-ui` are the references:
66+
67+
| Workflow | Role |
68+
| ----------------------------- | ------------------------------------------------------- |
69+
| `publish-<pkg>.yaml` | `workflow_call` + `workflow_dispatch`; does the publish |
70+
| `publish-<pkg>-on-merge.yaml` | fires on merged PR with the `Release` label |
71+
| `version-bump-<pkg>.yaml` | dispatch → opens a version PR labelled `Release` |
72+
| `ci-<pkg>-pack.yaml` | on PR — typecheck + assert tarball contents |
73+
74+
The pack check must allowlist `package.json`, `LICENSE`, **and `README.md`**.
75+
npm force-includes all three regardless of the `files` field, so a guard that
76+
only permits the first two rejects any package that has a readme.
77+
78+
## Before the first publish
79+
80+
- **Write a README.** Without one the npm page renders empty, which defeats
81+
publishing for another team to discover.
82+
- **Declare peer dependencies.** Anything the consumer must supply — Tailwind,
83+
Vue — belongs in `peerDependencies`, not `devDependencies`. A devDependency
84+
tells the consumer nothing.
85+
- **Export `./package.json`.** Tooling reads it; an `exports` map that omits it
86+
throws `ERR_PACKAGE_PATH_NOT_EXPORTED`.
87+
- **Check `files` against the exports map.** Every path in `exports` must be
88+
covered by `files`, or the target is simply absent from the tarball. Nothing
89+
catches this at install time — neither `npm pack` nor `npm install` resolves
90+
export targets — so it surfaces as a consumer resolution error the first time
91+
something imports that entry.
92+
93+
## Releasing after the first time
94+
95+
Run the version-bump workflow → it opens a PR labelled `Release` → merge it →
96+
`publish-<pkg>-on-merge` publishes and posts to Slack. A manual dispatch at an
97+
already-published version is a **no-op**: the `Check if version already on npm`
98+
step finds it and skips. If you want to test the pipeline, you need a new
99+
version number.
100+
101+
## Prove it is consumable
102+
103+
This is the step people skip, and it is the only one that finds real problems.
104+
In a _different_ repo — ideally one on npm rather than pnpm, since that is the
105+
path where `catalog:` would explode:
106+
107+
```sh
108+
npm install @comfyorg/<name>
109+
```
110+
111+
Then import it somewhere real, build, and grep the build output to confirm the
112+
thing you imported actually reached the bundle. Import **every** entry in the
113+
`exports` map while you are there — a subpath whose target never made it into
114+
the tarball fails only here. A green build proves the import resolved; it does
115+
not prove the values landed. For CSS, point the check at the consumer's own
116+
build output — the path below is Nuxt's, so substitute whatever your consumer
117+
emits:
118+
119+
```sh
120+
grep -o -- "--your-token:[^;]*" .output/public/_nuxt/*.css
121+
```
122+
123+
Open that consumer change as a PR and keep the preview link — it is the
124+
evidence that the publish worked end to end.
125+
126+
## Trusted publishing
127+
128+
Once the package exists, configure it on npmjs.com under package settings:
129+
130+
- Organization / repository / **workflow filename** — use the reusable workflow
131+
that actually runs the publish (`publish-<pkg>.yaml`), not the on-merge wrapper.
132+
- **Environment name — leave blank** unless the publish job declares
133+
`environment:`. A mismatch fails every publish.
134+
- **Allow `npm publish` only.** `npm stage publish` publishes unlisted pending
135+
manual approval; we do not use it.
136+
137+
Then grant OIDC at **both** workflow layers — the caller job that does
138+
`uses: ./.github/workflows/publish-<pkg>.yaml`, and the publish job inside the
139+
reusable workflow. A called workflow can never hold more than the calling job
140+
does, so setting this on the inner job alone leaves it with no token and the
141+
publish quietly falls back to `NPM_TOKEN`:
142+
143+
```yaml
144+
permissions:
145+
contents: read
146+
id-token: write
147+
```
148+
149+
Keep `NODE_AUTH_TOKEN` in place until an OIDC publish has actually succeeded.
150+
`[WARN] Skipped OIDC` in the log means it silently fell back to the token —
151+
treat that as a failure to chase down, not a warning to scroll past. Suspect
152+
`pnpm/action-setup` first: every workflow here still pins `v4.4.0`
153+
(`fc06bc1257f339d1d5d8b3a19a8cae5388b55320`), the version that broke pnpm's OIDC
154+
publish in [pnpm#11513](https://github.com/pnpm/pnpm/issues/11513) — closed once
155+
the reporter bumped the action, not by a pnpm release. Only after a real OIDC
156+
publish should you tighten the org to require 2FA and disallow tokens; doing it
157+
earlier removes the only working path.
158+
159+
## Announce it
160+
161+
Post the npm link, the install line, and the consumer PR preview link. "It's
162+
published" is not actionable; "here is the import and here is it working" is.

.github/actions/comment-release-links/action.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ runs:
6363
MARKER='desktop-release-summary'
6464
LINKS_VALUE='npm desktop UI|https://www.npmjs.com/package/@comfyorg/desktop-ui/v/{{version}}'
6565
;;
66+
packages/design-system/package.json)
67+
MARKER='design-system-release-summary'
68+
LINKS_VALUE='npm design-system|https://www.npmjs.com/package/@comfyorg/design-system/v/{{version}}'
69+
;;
6670
esac
6771
6872
DIFF_PREFIX='v'
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
name: 'CI: Design System Pack Check'
2+
3+
on:
4+
pull_request:
5+
branches-ignore: [wip/*, draft/*, temp/*]
6+
paths:
7+
- 'packages/design-system/**'
8+
9+
concurrency:
10+
group: ${{ github.workflow }}-${{ github.ref }}
11+
cancel-in-progress: true
12+
13+
permissions:
14+
contents: read
15+
16+
jobs:
17+
verify-pack-contents:
18+
runs-on: ubuntu-latest
19+
steps:
20+
- uses: actions/checkout@v6
21+
22+
- name: Setup frontend
23+
uses: ./.github/actions/setup-frontend
24+
25+
- name: Typecheck
26+
run: pnpm -C packages/design-system typecheck
27+
28+
- name: Verify packed tarball only contains src/css and src/icons
29+
shell: bash
30+
run: |
31+
set -euo pipefail
32+
UNEXPECTED=$(
33+
pnpm -C packages/design-system pack --dry-run --json |
34+
jq -r '
35+
.files[].path
36+
| select(
37+
. != "package.json"
38+
and . != "LICENSE"
39+
and . != "README.md"
40+
and (startswith("src/css/") | not)
41+
and (startswith("src/icons/") | not)
42+
)
43+
'
44+
)
45+
if [ -n "$UNEXPECTED" ]; then
46+
echo "::error title=Unexpected files in tarball::Packed tarball contains files outside src/css and src/icons:" >&2
47+
echo "$UNEXPECTED" >&2
48+
exit 1
49+
fi
50+
echo 'Packed tarball contains only expected files.'
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Assigns the on-call release sheriff to backport and release version-bump PRs
2+
# so they are never left unowned. Details: docs/release-process.md.
3+
name: 'PR: Assign Release Sheriff'
4+
5+
on:
6+
pull_request_target:
7+
types: [opened, reopened, ready_for_review, labeled]
8+
schedule:
9+
- cron: '23 * * * *'
10+
workflow_dispatch:
11+
12+
permissions:
13+
contents: read
14+
15+
# Serialize runs so two sweeps cannot race on the same PR.
16+
concurrency:
17+
group: assign-release-sheriff
18+
cancel-in-progress: false
19+
20+
jobs:
21+
assign:
22+
name: Assign release sheriff
23+
# Cheap runner-spinup gate only; release-sheriff.ts is the authoritative filter.
24+
if: >-
25+
github.repository == 'Comfy-Org/ComfyUI_frontend' &&
26+
(github.event_name != 'pull_request_target' ||
27+
contains(github.event.pull_request.labels.*.name, 'backport') ||
28+
contains(github.event.pull_request.labels.*.name, 'Release') ||
29+
contains(github.event.pull_request.title, 'backport') ||
30+
startsWith(github.event.pull_request.head.ref, 'version-bump-'))
31+
runs-on: ubuntu-latest
32+
permissions:
33+
contents: read
34+
issues: write
35+
pull-requests: write
36+
37+
steps:
38+
# pull_request_target checks out the base branch, never PR head code.
39+
- name: Checkout repository
40+
uses: actions/checkout@v6
41+
42+
- name: Setup frontend
43+
uses: ./.github/actions/setup-frontend
44+
45+
- name: Assign release sheriff
46+
env:
47+
GH_TOKEN: ${{ github.token }}
48+
GH_REPO: ${{ github.repository }}
49+
DATADOG_API_KEY: ${{ secrets.DATADOG_API_KEY }}
50+
DATADOG_APP_KEY: ${{ secrets.DATADOG_APP_KEY }}
51+
run: pnpm exec tsx scripts/release-sheriff/release-sheriff.ts

.github/workflows/pr-backport.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,11 @@ jobs:
351351
PR_DATA=$(gh pr view ${{ inputs.pr_number }} --json title,author)
352352
PR_TITLE=$(echo "$PR_DATA" | jq -r '.title')
353353
PR_AUTHOR=$(echo "$PR_DATA" | jq -r '.author.login')
354+
PR_AUTHOR_IS_BOT=$(echo "$PR_DATA" | jq -r '.author.is_bot')
354355
else
355356
PR_TITLE=$(jq -r '.pull_request.title' "$GITHUB_EVENT_PATH")
356357
PR_AUTHOR=$(jq -r '.pull_request.user.login' "$GITHUB_EVENT_PATH")
358+
PR_AUTHOR_IS_BOT=$(jq -r '.pull_request.user.type == "Bot"' "$GITHUB_EVENT_PATH")
357359
fi
358360
359361
for backport in ${{ steps.backport.outputs.success }}; do
@@ -370,6 +372,10 @@ jobs:
370372
PR_NUM=$(echo "${PR_URL}" | grep -o '[0-9]*$')
371373
372374
if [ -n "${PR_NUM}" ]; then
375+
if [ "${PR_AUTHOR_IS_BOT}" != "true" ]; then
376+
gh pr edit "${PR_NUM}" --add-assignee "${PR_AUTHOR}" \
377+
|| echo "::warning::Failed to assign @${PR_AUTHOR} to PR #${PR_NUM}"
378+
fi
373379
gh pr merge "${PR_NUM}" --auto --squash --repo "${{ github.repository }}" \
374380
|| echo "::warning::Failed to enable auto-merge for PR #${PR_NUM}"
375381
gh pr comment "${PR_NUMBER}" --body "@${PR_AUTHOR} Successfully backported to #${PR_NUM}"

0 commit comments

Comments
 (0)