Skip to content

Commit 221cbeb

Browse files
Merge branch 'Comfy-Org:main' into fix-ogg-syntax-9256
2 parents 9364af6 + 02e1ba2 commit 221cbeb

413 files changed

Lines changed: 33769 additions & 6994 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.

.claude/skills/backport-management/SKILL.md

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,18 @@ Cherry-pick backport management for Comfy-Org/ComfyUI_frontend stable release br
99

1010
## Quick Start
1111

12-
1. **Discover** — Collect candidates from Slack bot + git log gap (`reference/discovery.md`)
13-
2. **Analyze** — Categorize MUST/SHOULD/SKIP, check deps (`reference/analysis.md`)
14-
3. **Human Review** — Present candidates in batches for interactive approval (see Interactive Approval Flow)
15-
4. **Plan** — Order by dependency (leaf fixes first), group into waves per branch
16-
5. **Execute** — Label-driven automation → worktree fallback for conflicts (`reference/execution.md`)
17-
6. **Verify** — After each wave, verify branch integrity before proceeding
18-
7. **Log & Report** — Generate session report (`reference/logging.md`)
12+
1. **Discover** — Collect candidates from Slack bot + git log gap, then **reconcile both lists** (`reference/discovery.md`)
13+
2. **Pre-filter by path** — Auto-skip PRs whose changed files are entirely under `apps/website/`, `browser_tests/`, `.github/`, `packages/design-system/`, `packages/{cloud,registry}-types/`, `.claude/`, `docs/`. Don't read PR bodies for these — they don't ship to core ComfyUI users (`reference/analysis.md`)
14+
3. **Verify target file existence** — For each surviving candidate, run `git cat-file -e origin/$TARGET:$path` for primary changed files. If they don't exist on the target, auto-mark SKIP with reason `feature-not-on-branch`
15+
4. **Tiered triage** — Bucket into **Tier 1 (core editor must-haves)**, **Tier 2 (cloud-distribution only)**, **Tier 3 (skip)** before reviewing individually (`reference/analysis.md`)
16+
5. **Analyze** — Categorize remaining MUST/SHOULD, check deps (`reference/analysis.md`)
17+
6. **Human Review** — Present candidates in batches for interactive approval, with tier context attached (see Interactive Approval Flow)
18+
7. **Plan** — Order by dependency (leaf fixes first), group into waves per branch
19+
8. **Test-then-resolve dry-run** — Classify clean vs conflict before committing time (`reference/execution.md`)
20+
9. **Execute** — Label-driven automation for clean PRs → worktree fallback for conflicts (`reference/execution.md`)
21+
10. **Public-API conflict review** — If conflict resolution touches a public LiteGraph callback, extension API, or `node.*` method, consult oracle for compat-regression review BEFORE pushing (`reference/execution.md`)
22+
11. **Verify** — Per-PR validation (typecheck + targeted tests + lint on changed files) AND per-wave verification (full typecheck + test:unit on branch HEAD)
23+
12. **Log & Report** — Generate session report + author accountability report + Slack status update (`reference/logging.md`)
1924

2025
## System Context
2126

@@ -107,6 +112,35 @@ Husky hooks fail in worktrees (can't find lint-staged config). Always use `git p
107112

108113
In the 2026-04-06 session: core/1.42 got 18/26 auto-PRs, cloud/1.42 got only 1/25. The cloud branch has more divergence. **Always plan for manual fallback** — don't assume automation will handle most PRs.
109114

115+
### Cherry-Picked Tests Can Reference Files Added By Earlier Unbackported PRs
116+
117+
A common conflict: PR A on main modifies a test file that was _added_ on main by an earlier PR B (not backported to the target). The cherry-pick of A reports "modify/delete" on B's test file because the file doesn't exist on the target. Adding the new file would smuggle in B's test scaffolding without B's runtime changes.
118+
119+
**Detection:** Conflict says `deleted in HEAD and modified in <PR>`. Verify with:
120+
121+
```bash
122+
git log --diff-filter=A --oneline origin/main -- path/to/test.ts
123+
```
124+
125+
If the introducing commit is **not** on the target branch, the test file isn't a real prerequisite for the runtime fix.
126+
127+
**Fix:** `git rm` the test file (drop it from the backport). Document in the commit body which PR introduced it on main and why dropping it is safe. The runtime fix itself usually doesn't depend on these tests — coverage exists at the integration layer.
128+
129+
### Backport-Only Compatibility Shims
130+
131+
When a PR's _mechanism_ relies on changes upstream that aren't on the older branch, a literal cherry-pick can recreate the original bug for any consumer still using the old contract. This is most dangerous for **public LiteGraph callbacks, extension APIs, and `node.*` methods** that custom-node packages depend on.
132+
133+
**Real example (#11541, core/1.43 backport):** The PR removed `LGraphNode.vue`'s legacy `handled === true` sync-return check from `handleDrop`, replacing it with `await node.onDragDrop(event, true)`. Safe on `main` because all in-repo `onDragDrop` handlers had migrated to participate in the new `claimEvent` flag. On `core/1.43`, `onDragDrop` is a public callback — custom-node packages with synchronous `onDragDrop` returning `true` would no longer have their event claimed, recreating the duplicate-node-creation bug the PR was fixing.
134+
135+
**Detection:** The PR's diff modifies a file that is part of a public extension API surface. Look for:
136+
137+
- `node.onXxx` callback assignments
138+
- Methods on `LGraphNode`, `LGraphCanvas`, `LGraph`, `Subgraph`
139+
- Public exports from `src/lib/litegraph/`
140+
- Type changes affecting `litegraph-augmentation.d.ts`
141+
142+
**Fix:** Add a backport-only compatibility shim that preserves the old contract while keeping the new fix. Document it explicitly in the commit body under a `## Backport-only compatibility fix` heading. Consult oracle for review before pushing — a bad shim is worse than no fix.
143+
110144
## Conflict Triage
111145

112146
**Always categorize before deciding to skip. High conflict count ≠ hard conflicts.**
@@ -147,6 +181,26 @@ Skip these without discussion:
147181
- **Features not on target branch** — e.g., Painter, GLSLShader, appModeStore on core/1.40
148182
- **Cloud-only PRs on core/\* branches** — Team workspaces, cloud queue, cloud-only login. (Note: app mode and Firebase auth are NOT cloud-only — see Branch Scope Rules)
149183

184+
### Path Pre-Filter (run BEFORE reading PR bodies)
185+
186+
For 50+ candidate PRs, classify by changed paths first to skip the unproductive ones without spending time on triage. Run `git show --stat $SHA` (or `gh pr view --json files`) and bucket:
187+
188+
| Path prefix | Bucket | Reason |
189+
| ---------------------------------------------- | ---------------------- | ------------------------------------------------ |
190+
| `apps/website/` | SKIP | Marketing/platform site, not core ComfyUI bundle |
191+
| `apps/desktop-ui/` | SKIP for `core/*` | Desktop app, separate release cadence |
192+
| `browser_tests/` only (no `src/`) | SKIP | Test-only |
193+
| `.github/workflows/` only | SKIP | CI/release infra |
194+
| `packages/design-system/` only | SKIP | Design tokens, not core |
195+
| `packages/{cloud,registry,ingest}-types/` only | SKIP | Generated types |
196+
| `.claude/`, `.agents/`, `docs/` | SKIP | Agent / documentation |
197+
| `*.stories.ts` only | SKIP | Storybook only |
198+
| `src/` (core editor) | KEEP — analyze further | Runtime/editor code that requires full triage |
199+
200+
A PR touches multiple paths? Keep it if **any** changed file is under `src/` (or other core paths) and run normal analysis. Auto-skip is conservative — only skip when _all_ paths match the SKIP buckets.
201+
202+
This filter alone removes ~30-50% of candidates in a typical session, leaving only the PRs that need real triage.
203+
150204
## Wave Verification
151205

152206
After merging each wave of PRs to a target branch, verify branch integrity before proceeding:

.claude/skills/backport-management/reference/analysis.md

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,89 @@ Check before backporting — these don't exist on older branches:
3939
- **App builder** — check per branch
4040
- **appModeStore.ts** — not on core/1.40
4141

42+
### Verify Target File Existence (Run Before Cherry-Pick)
43+
44+
Before cherry-picking any PR, confirm the files it modifies actually exist on the target branch. If they don't, the PR's runtime fix is for a feature that hasn't been added yet — skip cleanly without attempting cherry-pick:
45+
46+
```bash
47+
# For each file the PR changes
48+
for f in $(gh pr view $PR --json files --jq '.files[].path' | grep -v "^browser_tests/\|\.test\." ); do
49+
if ! git cat-file -e origin/$TARGET:$f 2>/dev/null; then
50+
echo "MISSING on $TARGET: $f"
51+
fi
52+
done
53+
```
54+
55+
If the _primary_ changed files (the runtime ones, not tests) are missing, mark the PR `SKIP / feature-not-on-branch`. This is faster than letting cherry-pick fail with modify/delete conflicts and gives a clean signal.
56+
57+
This check is the first thing that runs after the path pre-filter and BEFORE you spend time reading PR descriptions.
58+
59+
## Tiered Triage (Recommended for 30+ Candidates)
60+
61+
Before the interactive Y/N approval flow, bucket all surviving candidates into three tiers. This surfaces release-engineering decisions that a flat MUST/SHOULD list obscures:
62+
63+
### Tier 1 — Core Editor Must-Haves
64+
65+
User-facing bugs, crashes, data corruption, or security issues in code paths that exist on the target branch. These are the strongest backport candidates.
66+
67+
Indicators:
68+
69+
- `fix:` prefix and the bug is reproducible on the target branch
70+
- Crash guards, runtime null checks, race-condition fixes
71+
- Data-loss bugs (state not persisted, duplicates, drops)
72+
- Security hardening (CSRF, XSS, auth)
73+
- Vue Nodes 2.0 regression cluster (if the target ships Vue Nodes 2.0)
74+
- Subgraph correctness fixes
75+
- Public-API extension callback fixes
76+
77+
Recommend `Y` to user.
78+
79+
### Tier 2 — Cloud-Distribution Only
80+
81+
Bugs that only manifest on cloud-hosted distributions (Secrets panel, subscription flows, cloud signup, workspace tracking, etc.). Whether to backport depends on whether cloud ships from the target `core/*` branch in your release matrix.
82+
83+
Indicators:
84+
85+
- Files under `src/platform/secrets/`, `src/platform/subscription/`, signup flows
86+
- PR description mentions cloud staging issues
87+
- Fix gated behind cloud feature flags
88+
89+
Default: ask the cloud release rotation owner. If unsure, defer.
90+
91+
### Tier 3 — Skip
92+
93+
Path pre-filter caught most of these. The rest are PRs where the diff _touches_ `src/` but the practical impact is non-user-facing or scoped to features the target doesn't ship.
94+
95+
Indicators:
96+
97+
- All changes in test files even if the PR touched `src/` test files
98+
- Storybook stories only
99+
- Lint config / lint rule additions
100+
- Documentation comments
101+
- Internal refactors with no behavior change
102+
103+
### Presentation Format
104+
105+
When showing tier results to the user, format as:
106+
107+
```text
108+
Tier 1 (N PRs) — strong backport candidates
109+
- #11541 fix: stop duplicate node creation when dropping image on Vue nodes
110+
Why: Vue Nodes 2.0 regression — async onDragDrop bypassed handled-check, drops bubble to document, spawns extra LoadImage nodes
111+
- #10849 fix: store promoted widget values per SubgraphNode instance
112+
Why: Multiple instances overwriting each other's promoted widget values — data loss
113+
114+
Tier 2 (N PRs) — cloud-distribution release rotation should decide
115+
- #11636 fix: enable Chrome password autofill on signup form
116+
- ...
117+
118+
Tier 3 (N PRs) — skip recommended
119+
- #11586 fix: website polish (apps/website/ only)
120+
- ...
121+
```
122+
123+
Then run interactive Y/N over Tier 1 and Tier 2; Tier 3 gets confirmed-skip without per-PR review.
124+
42125
## Dep Refresh PRs
43126

44127
Always SKIP on stable branches. Risk of transitive dependency regressions outweighs audit cleanup benefit. If a specific CVE fix is needed, cherry-pick that individual fix instead.

.claude/skills/backport-management/reference/discovery.md

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Discovery — Candidate Collection
22

3+
**Run all sources, then reconcile.** No single source is authoritative:
4+
5+
- Slack bot may flag PRs that have already been backported (false positive)
6+
- Git gap may include PRs that don't need backport (test-only, design-system, website)
7+
- Bot can also miss PRs that landed without the right labels
8+
39
## Source 1: Slack Backport-Checker Bot
410

511
Use `slackdump` skill to export `#frontend-releases` channel (C09K9TPU2G7):
@@ -36,7 +42,43 @@ gh pr view $PR --json mergeCommit,title --jq '"Title: \(.title)\nMerge: \(.merge
3642
gh pr view $PR --json files --jq '.files[].path'
3743
```
3844

45+
## Source 4: Already-Backported PRs (cross-reference)
46+
47+
When the target branch already has some cherry-picks on it (e.g., partway through a release window), extract the originals to avoid re-backporting:
48+
49+
```bash
50+
# Get all original PR numbers already backported to TARGET since the last release tag
51+
git log --format="%H%n%B" $LAST_TAG..origin/$TARGET \
52+
| grep -oiE "(backport of|cherry.picked) #?[0-9]+" \
53+
| grep -oE "[0-9]+" \
54+
| sort -un > /tmp/already-backported.txt
55+
```
56+
57+
Subtract this list from your candidates.
58+
59+
## Reconciliation Workflow
60+
61+
```bash
62+
# 1. Slack bot list (parse from export)
63+
# /tmp/bot-flagged.txt — one PR# per line, sorted
64+
65+
# 2. Git gap fix/perf only
66+
MB=$(git merge-base origin/main origin/$TARGET)
67+
git log --format="%h|%s" $MB..origin/main \
68+
| grep -iE "^[a-f0-9]+\|(fix|perf)" \
69+
| grep -oE "#[0-9]+\)" | grep -oE "[0-9]+" \
70+
| sort -un > /tmp/gap-fixes.txt
71+
72+
# 3. Already backported (Source 4 above)
73+
74+
# 4. Candidates = (gap-fixes ∪ bot-flagged) − already-backported
75+
sort -u /tmp/gap-fixes.txt /tmp/bot-flagged.txt > /tmp/union.txt
76+
comm -23 /tmp/union.txt /tmp/already-backported.txt > /tmp/candidates.txt
77+
```
78+
79+
The result is the input to the path pre-filter (`SKILL.md` Quick Start step 2).
80+
3981
## Output: candidate_list.md
4082

4183
Table per target branch:
42-
| PR# | Title | Category | Flagged by Bot? | Decision |
84+
| PR# | Title | Source (bot/gap/both) | Path bucket | Tier | Decision |

0 commit comments

Comments
 (0)