Skip to content

Commit a7ba811

Browse files
committed
Merge remote-tracking branch 'origin/main' into matt/be-4815-lint-scope
# Conflicts: # eslint.config.ts # lint-staged.config.ts
2 parents 0abed6f + 860dfdd commit a7ba811

1,283 files changed

Lines changed: 175181 additions & 47216 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.

.agents/checks/pattern-compliance.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Steps:
2222
- Explicit return types on exported functions
2323
- Use `es-toolkit` for utility functions, NOT lodash. Flag any new `import ... from 'lodash'` or `import ... from 'lodash/*'`
2424
- Never use `z.any()` in Zod schemas — use `z.unknown()` and narrow
25+
- Never hand-declare or duplicate server/API response types locally — flag any local interface/type that reimplements a type already generated from an OpenAPI spec (e.g. `@comfyorg/ingest-types`, `@comfyorg/registry-types` in `packages/`) instead of importing it. This caused real bugs in PR #14771 (see `docs/guidance/typescript.md`)
2526

2627
### Vue (if applicable)
2728

.agents/setup

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ set -euo pipefail
33

44
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
55
cd "$repo_root"
6+
7+
if [[ "${AMP_ORB:-}" == 1 ]]; then
8+
gh auth setup-git
9+
fi
10+
611
node_major="$(tr -d '[:space:]' < "$repo_root/.nvmrc")"
712
node_home="$HOME/.local/share/comfyui-frontend-node-v$node_major"
813

@@ -56,16 +61,10 @@ if [[ ! -x "$node_home/bin/pnpm" ]] ||
5661
npm install --global --prefix "$node_home" "pnpm@$pnpm_version"
5762
fi
5863

59-
profile_marker="# ComfyUI Frontend orb toolchain"
60-
if ! grep -Fqx "$profile_marker" "$HOME/.bash_profile" 2>/dev/null; then
61-
cat >> "$HOME/.bash_profile" <<EOF
62-
63-
$profile_marker
64-
if [[ "\${PWD:-}" == "$repo_root" || "\${PWD:-}" == "$repo_root/"* ]]; then
65-
export PATH="$node_home/bin:\$PATH"
66-
fi
67-
EOF
68-
fi
64+
mkdir -p "$HOME/.local/bin"
65+
for executable in node npm npx pnpm pnpx; do
66+
ln -sfn "$node_home/bin/$executable" "$HOME/.local/bin/$executable"
67+
done
6968

7069
echo "Installing workspace dependencies"
7170
pnpm install --frozen-lockfile

.amp/services.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
services:
2+
frontend:
3+
command: pnpm dev:cloud --host 0.0.0.0 --port "$PORT"
4+
portal:
5+
title: ComfyUI Frontend
6+
description: Frontend dev server connected to the Comfy test cloud backend.

.claude/commands/comprehensive-pr-review.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ Look for:
146146
- Proper abstraction levels
147147
- Interface design and API clarity
148148
- Leftover debug code (console.log, commented code, TODO comments)
149+
- Hand-declared/inlined server or API response types that duplicate a type already generated from an OpenAPI spec (`@comfyorg/ingest-types`, `@comfyorg/registry-types` under `packages/`) instead of importing it — these silently drift from the real contract and caused real bugs in PR #14771
149150
150151
### 3.3 Library Usage Enforcement
151152

.claude/commands/create-hotfix-release.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,8 @@ For each commit:
280280
5. **Click "Publish release"**
281281
6. **CONFIRMATION REQUIRED**: Draft release published with "latest" unchecked?
282282
283+
Note: the `release-enforce-latest.yaml` workflow now auto-corrects the "latest" flag back to the highest stable semver release within a minute if this step is missed, so this checkbox is no longer a silent single point of failure. Still uncheck it manually — the workflow is a safety net, not a replacement for doing this right the first time.
284+
283285
### Step 14: Create ComfyUI Requirements.txt Update PR
284286
285287
**IMPORTANT**: Create PR to update ComfyUI's requirements.txt via fork:
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.

.coderabbit.yaml

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,39 @@ reviews:
6565
When warning, reference the specific ADR by number and link to `docs/adr/` for context. Frame findings as directional guidance since ADR 0003 and 0008 are in Proposed status.
6666
6767
path_instructions:
68+
- path: '**/*.ts'
69+
instructions: |
70+
Treat `docs/guidance/typescript.md` as required review context.
71+
- path: '**/*.vue'
72+
instructions: |
73+
Treat `docs/guidance/typescript.md` and
74+
`docs/guidance/vue-components.md` as required review context. For
75+
changed components or views under `src/components/` or `src/views/`,
76+
also apply `docs/guidance/design-standards.md` and assess accessibility.
77+
- path: '**/*.stories.ts'
78+
instructions: |
79+
Treat `docs/guidance/storybook.md` as required review context.
80+
- path: 'src/lib/litegraph/**'
81+
instructions: |
82+
Treat `docs/adr/README.md` as required review context. For widget
83+
serialization changes, also read
84+
`docs/WIDGET_SERIALIZATION.md`.
6885
- path: '**/*.test.ts'
6986
instructions: |
70-
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`, and `docs/guidance/vitest.md` as required review context for every changed Vitest test file.
87+
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`,
88+
`docs/guidance/vitest.md`, and `docs/testing/vitest-patterns.md` as
89+
required review context for every changed Vitest test file.
7190
- path: 'src/lib/litegraph/**/*.test.ts'
7291
instructions: |
73-
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`, `docs/guidance/vitest.md`, and `docs/testing/litegraph-testing.md` as required review context for every changed litegraph Vitest test file.
92+
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`,
93+
`docs/guidance/vitest.md`, `docs/testing/vitest-patterns.md`, and
94+
`docs/testing/litegraph-testing.md` as required review context for
95+
every changed LiteGraph Vitest test file.
7496
- path: '{browser_tests,apps/website/e2e}/**/*.spec.ts'
7597
instructions: |
76-
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`, and `docs/guidance/playwright.md` as required review context for every changed Playwright test file.
98+
Treat `.agents/checks/test-quality.md`, `docs/testing/README.md`,
99+
and `docs/guidance/playwright.md` as required review context for every
100+
changed Playwright test file. For
101+
`browser_tests/`, also read `browser_tests/README.md` and
102+
`browser_tests/AGENTS.md`, and apply
103+
`.agents/checks/playwright-e2e.md`.

0 commit comments

Comments
 (0)