From 37c6ddfcd9a55ea4665d607fa99b27ceeb01c265 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Thu, 12 Mar 2026 04:55:01 -0700 Subject: [PATCH] chore: add backport-management agent skill (#9619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a reusable agent skill for managing cherry-pick backports across stable release branches. ## What Agent skill at `.claude/skills/backport-management/` with routing-table SKILL.md + 4 reference files (discovery, analysis, execution, logging). ## Why Codifies lessons from backporting 57 PRs across cloud/1.41, core/1.41, and core/1.40. Makes future backport sessions faster and less error-prone. ## Key learnings baked in - Cloud-only PRs must not be backported to `core/*` branches (wasted effort) - Wave verification (`pnpm typecheck`) between batches to catch breakage early - Human review required for non-trivial conflict resolutions before admin-merge - MUST vs SHOULD decision guide with clear criteria - Continuous backporting preference over bulk sessions - Mermaid diagram as final session deliverable - Conflict triage table (never skip based on file count alone) - `gh api` for labels instead of `gh pr edit` (Projects Classic deprecation) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9619-chore-add-backport-management-agent-skill-31d6d73d3650815b9808c3916b8e3343) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action --- .claude/skills/backport-management/SKILL.md | 150 ++++++++++++++++++ .../backport-management/reference/analysis.md | 68 ++++++++ .../reference/discovery.md | 42 +++++ .../reference/execution.md | 150 ++++++++++++++++++ .../backport-management/reference/logging.md | 96 +++++++++++ 5 files changed, 506 insertions(+) create mode 100644 .claude/skills/backport-management/SKILL.md create mode 100644 .claude/skills/backport-management/reference/analysis.md create mode 100644 .claude/skills/backport-management/reference/discovery.md create mode 100644 .claude/skills/backport-management/reference/execution.md create mode 100644 .claude/skills/backport-management/reference/logging.md diff --git a/.claude/skills/backport-management/SKILL.md b/.claude/skills/backport-management/SKILL.md new file mode 100644 index 0000000000..449339c2fe --- /dev/null +++ b/.claude/skills/backport-management/SKILL.md @@ -0,0 +1,150 @@ +--- +name: backport-management +description: Manages cherry-pick backports across stable release branches. Discovers candidates from Slack/git, analyzes dependencies, resolves conflicts via worktree, and logs results. Use when asked to backport, cherry-pick to stable, manage release branches, do stable branch maintenance, or run a backport session. +--- + +# Backport Management + +Cherry-pick backport management for Comfy-Org/ComfyUI_frontend stable release branches. + +## Quick Start + +1. **Discover** — Collect candidates from Slack bot + git log gap (`reference/discovery.md`) +2. **Analyze** — Categorize MUST/SHOULD/SKIP, check deps (`reference/analysis.md`) +3. **Plan** — Order by dependency (leaf fixes first), group into waves per branch +4. **Execute** — Label-driven automation → worktree fallback for conflicts (`reference/execution.md`) +5. **Verify** — After each wave, verify branch integrity before proceeding +6. **Log & Report** — Generate session report with mermaid diagram (`reference/logging.md`) + +## System Context + +| Item | Value | +| -------------- | ------------------------------------------------- | +| Repo | `~/ComfyUI_frontend` (Comfy-Org/ComfyUI_frontend) | +| Merge strategy | Squash merge (`gh pr merge --squash --admin`) | +| Automation | `pr-backport.yaml` GitHub Action (label-driven) | +| Tracking dir | `~/temp/backport-session/` | + +## Branch Scope Rules + +**Critical: Match PRs to the correct target branches.** + +| Branch prefix | Scope | Example | +| ------------- | ------------------------------ | ----------------------------------------- | +| `cloud/*` | Cloud-hosted ComfyUI only | App mode, cloud auth, cloud-specific UI | +| `core/*` | Local/self-hosted ComfyUI only | Core editor, local workflows, node system | + +**⚠️ NEVER backport cloud-only PRs to `core/*` branches.** Cloud-only changes (app mode, cloud auth, cloud billing UI, cloud-specific API calls) are irrelevant to local users and waste effort. Before backporting any PR to a `core/*` branch, check: + +- Does the PR title/description mention "app mode", "cloud", or cloud-specific features? +- Does the PR only touch files like `appModeStore.ts`, cloud auth, or cloud-specific components? +- If yes → skip for `core/*` branches (may still apply to `cloud/*` branches) + +## ⚠️ Gotchas (Learn from Past Sessions) + +### Use `gh api` for Labels — NOT `gh pr edit` + +`gh pr edit --add-label` triggers Projects Classic deprecation errors. Always use: + +```bash +gh api repos/Comfy-Org/ComfyUI_frontend/issues/$PR/labels \ + -f "labels[]=needs-backport" -f "labels[]=TARGET_BRANCH" +``` + +### Automation Over-Reports Conflicts + +The `pr-backport.yaml` action reports more conflicts than reality. `git cherry-pick -m 1` with git auto-merge handles many cases the automation can't. Always attempt manual cherry-pick before skipping. + +### Never Skip Based on Conflict File Count + +12 or 27 conflicting files can be trivial (snapshots, new files). **Categorize conflicts first**, then decide. See Conflict Triage below. + +## Conflict Triage + +**Always categorize before deciding to skip. High conflict count ≠ hard conflicts.** + +| Type | Symptom | Resolution | +| ---------------------------- | ------------------------------------ | --------------------------------------------------------------- | +| **Binary snapshots (PNGs)** | `.png` files in conflict list | `git checkout --theirs $FILE && git add $FILE` — always trivial | +| **Modify/delete (new file)** | PR introduces files not on target | `git add $FILE` — keep the new file | +| **Modify/delete (removed)** | Target removed files the PR modifies | `git rm $FILE` — file no longer relevant | +| **Content conflicts** | Marker-based (`<<<<<<<`) | Accept theirs via python regex (see below) | +| **Add/add** | Both sides added same file | Accept theirs, verify no logic conflict | +| **Locale/JSON files** | i18n key additions | Accept theirs, validate JSON after | + +```python +# Accept theirs for content conflicts +import re +pattern = r'<<<<<<< HEAD\n(.*?)=======\n(.*?)>>>>>>> [^\n]+\n?' +content = re.sub(pattern, r'\2', content, flags=re.DOTALL) +``` + +### Escalation Triggers (Flag for Human) + +- **Package.json/lockfile changes** → skip on stable (transitive dep regression risk) +- **Core type definition changes** → requires human judgment +- **Business logic conflicts** (not just imports/exports) → requires domain knowledge +- **Admin-merged conflict resolutions** → get human review of the resolution before continuing the wave + +## Auto-Skip Categories + +Skip these without discussion: + +- **Dep refresh PRs** — Risk of transitive dep regressions on stable. Cherry-pick individual CVE fixes instead. +- **CI/tooling changes** — Not user-facing +- **Test-only / lint rule changes** — Not user-facing +- **Revert pairs** — If PR A reverted by PR B, skip both. If fixed version (PR C) exists, backport only C. +- **Features not on target branch** — e.g., Painter, GLSLShader, appModeStore on core/1.40 +- **Cloud-only PRs on core/\* branches** — App mode, cloud auth, cloud billing. These only affect cloud-hosted ComfyUI. + +## Wave Verification + +After merging each wave of PRs to a target branch, verify branch integrity before proceeding: + +```bash +# Fetch latest state of target branch +git fetch origin TARGET_BRANCH + +# Quick smoke check: does the branch build? +git worktree add /tmp/verify-TARGET origin/TARGET_BRANCH +cd /tmp/verify-TARGET +source ~/.nvm/nvm.sh && nvm use 24 && pnpm install && pnpm typecheck +git worktree remove /tmp/verify-TARGET --force +``` + +If typecheck fails, stop and investigate before continuing. A broken branch after wave N means all subsequent waves will compound the problem. + +## Continuous Backporting Recommendation + +Large backport sessions (50+ PRs) are expensive and error-prone. Prefer continuous backporting: + +- Backport bug fixes as they merge to main (same day or next day) +- Use the automation labels immediately after merge +- Reserve session-style bulk backporting for catching up after gaps +- When a release branch is created, immediately start the continuous process + +## Quick Reference + +### Label-Driven Automation (default path) + +```bash +gh api repos/Comfy-Org/ComfyUI_frontend/issues/$PR/labels \ + -f "labels[]=needs-backport" -f "labels[]=TARGET_BRANCH" +# Wait 3 min, check: gh pr list --base TARGET_BRANCH --state open +``` + +### Manual Worktree Cherry-Pick (conflict fallback) + +```bash +git worktree add /tmp/backport-$BRANCH origin/$BRANCH +cd /tmp/backport-$BRANCH +git checkout -b backport-$PR-to-$BRANCH origin/$BRANCH +git cherry-pick -m 1 $MERGE_SHA +# Resolve conflicts, push, create PR, merge +``` + +### PR Title Convention + +``` +[backport TARGET_BRANCH] Original Title (#ORIGINAL_PR) +``` diff --git a/.claude/skills/backport-management/reference/analysis.md b/.claude/skills/backport-management/reference/analysis.md new file mode 100644 index 0000000000..d62cd1c935 --- /dev/null +++ b/.claude/skills/backport-management/reference/analysis.md @@ -0,0 +1,68 @@ +# Analysis & Decision Framework + +## Categorization + +| Category | Criteria | Action | +| -------------------- | --------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| **MUST** | User-facing bug, crash, data corruption, security. Clear breakage that users will hit. | Backport (with deps if needed) | +| **SHOULD** | UX improvement, minor bug, small dep chain. No user-visible breakage if skipped, but improves experience. | Backport if clean cherry-pick; defer if conflict resolution is non-trivial | +| **SKIP** | CI/tooling, test-only, lint rules, cosmetic, dep refresh | Skip with documented reason | +| **NEEDS DISCUSSION** | Large dep chain, unclear risk/benefit, touches core types | Flag for human | + +### MUST vs SHOULD Decision Guide + +When unsure, ask: "If a user on this stable branch reports this issue, would we consider it a bug?" + +- **Yes** → MUST. The fix addresses broken behavior. +- **No, but it's noticeably better** → SHOULD. The fix is a quality-of-life improvement. +- **No, and it's cosmetic or internal** → SKIP. + +For SHOULD items with conflicts: if conflict resolution requires more than trivial accept-theirs patterns (content conflicts in business logic, not just imports), downgrade to SKIP or escalate to NEEDS DISCUSSION. + +## Branch Scope Filtering + +**Before categorizing, filter by branch scope:** + +| Target branch | Skip if PR is... | +| ------------- | ------------------------------------------------------------------- | +| `core/*` | Cloud-only (app mode, cloud auth, cloud billing, cloud-specific UI) | +| `cloud/*` | Local-only features not present on cloud branch | + +Cloud-only PRs backported to `core/*` are wasted effort — `core/*` branches serve local/self-hosted users who never see cloud features. Check PR titles, descriptions, and files changed for cloud-specific indicators. + +## Features Not on Stable Branches + +Check before backporting — these don't exist on older branches: + +- **Painter** (`src/extensions/core/painter.ts`) — not on core/1.40 +- **GLSLShader** — not on core/1.40 +- **App builder** — check per branch +- **appModeStore.ts** — not on core/1.40 + +## Dep Refresh PRs + +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. + +## Revert Pairs + +If PR A is reverted by PR B: + +- Skip BOTH A and B +- If a fixed version exists (PR C), backport only C + +## Dependency Analysis + +```bash +# Find other PRs that touched the same files +gh pr view $PR --json files --jq '.files[].path' | while read f; do + git log --oneline origin/TARGET..$MERGE_SHA -- "$f" +done +``` + +## Human Review Checkpoint + +Present decisions.md before execution. Include: + +1. All MUST/SHOULD/SKIP categorizations with rationale +2. Questions for human (feature existence, scope, deps) +3. Estimated effort per branch diff --git a/.claude/skills/backport-management/reference/discovery.md b/.claude/skills/backport-management/reference/discovery.md new file mode 100644 index 0000000000..ac826ce764 --- /dev/null +++ b/.claude/skills/backport-management/reference/discovery.md @@ -0,0 +1,42 @@ +# Discovery — Candidate Collection + +## Source 1: Slack Backport-Checker Bot + +Use `slackdump` skill to export `#frontend-releases` channel (C09K9TPU2G7): + +```bash +slackdump export -o ~/slack-exports/frontend-releases.zip C09K9TPU2G7 +``` + +Parse bot messages for PRs flagged "Might need backport" per release version. + +## Source 2: Git Log Gap Analysis + +```bash +# Count gap +git log --oneline origin/TARGET..origin/main | wc -l + +# List gap commits +git log --oneline origin/TARGET..origin/main + +# Check if a PR is already on target +git log --oneline origin/TARGET --grep="#PR_NUMBER" + +# Check for existing backport PRs +gh pr list --base TARGET --state all --search "backport PR_NUMBER" +``` + +## Source 3: GitHub PR Details + +```bash +# Get merge commit SHA +gh pr view $PR --json mergeCommit,title --jq '"Title: \(.title)\nMerge: \(.mergeCommit.oid)"' + +# Get files changed +gh pr view $PR --json files --jq '.files[].path' +``` + +## Output: candidate_list.md + +Table per target branch: +| PR# | Title | Category | Flagged by Bot? | Decision | diff --git a/.claude/skills/backport-management/reference/execution.md b/.claude/skills/backport-management/reference/execution.md new file mode 100644 index 0000000000..0fa8d032c6 --- /dev/null +++ b/.claude/skills/backport-management/reference/execution.md @@ -0,0 +1,150 @@ +# Execution Workflow + +## Per-Branch Execution Order + +1. Smallest gap first (validation run) +2. Medium gap next (quick win) +3. Largest gap last (main effort) + +## Step 1: Label-Driven Automation (Batch) + +```bash +# Add labels to all candidates for a target branch +for pr in $PR_LIST; do + gh api repos/Comfy-Org/ComfyUI_frontend/issues/$pr/labels \ + -f "labels[]=needs-backport" -f "labels[]=TARGET_BRANCH" --silent + sleep 2 +done + +# Wait 3 minutes for automation +sleep 180 + +# Check which got auto-PRs +gh pr list --base TARGET_BRANCH --state open --limit 50 --json number,title +``` + +## Step 2: Review & Merge Clean Auto-PRs + +```bash +for pr in $AUTO_PRS; do + # Check size + gh pr view $pr --json title,additions,deletions,changedFiles \ + --jq '"Files: \(.changedFiles), +\(.additions)/-\(.deletions)"' + # Admin merge + gh pr merge $pr --squash --admin + sleep 3 +done +``` + +## Step 3: Manual Worktree for Conflicts + +```bash +git fetch origin TARGET_BRANCH +git worktree add /tmp/backport-TARGET origin/TARGET_BRANCH +cd /tmp/backport-TARGET + +for PR in ${CONFLICT_PRS[@]}; do + # Refresh target ref so each branch is based on current HEAD + git fetch origin TARGET_BRANCH + git checkout origin/TARGET_BRANCH + + git checkout -b backport-$PR-to-TARGET origin/TARGET_BRANCH + git cherry-pick -m 1 $MERGE_SHA + + # If conflict — NEVER skip based on file count alone! + # Categorize conflicts first: binary PNGs, modify/delete, content, add/add + # See SKILL.md Conflict Triage table for resolution per type. + + # Resolve all conflicts, then: + git add . + GIT_EDITOR=true git cherry-pick --continue + + git push origin backport-$PR-to-TARGET + NEW_PR=$(gh pr create --base TARGET_BRANCH --head backport-$PR-to-TARGET \ + --title "[backport TARGET] TITLE (#$PR)" \ + --body "Backport of #$PR..." | grep -oP '\d+$') + gh pr merge $NEW_PR --squash --admin + sleep 3 +done + +# Cleanup +cd - +git worktree remove /tmp/backport-TARGET --force +``` + +**⚠️ Human review for conflict resolutions:** When admin-merging a PR where you manually resolved conflicts (especially content conflicts beyond trivial accept-theirs), pause and present the resolution diff to the human for review before merging. Trivial resolutions (binary snapshots, modify/delete, locale key additions) can proceed without review. + +## Step 4: Wave Verification + +After completing all PRs in a wave for a target branch: + +```bash +git fetch origin TARGET_BRANCH +git worktree add /tmp/verify-TARGET origin/TARGET_BRANCH +cd /tmp/verify-TARGET +source ~/.nvm/nvm.sh && nvm use 24 && pnpm install && pnpm typecheck +git worktree remove /tmp/verify-TARGET --force +``` + +If verification fails, stop and fix before proceeding to the next wave. Do not compound problems across waves. + +## Conflict Resolution Patterns + +### 1. Content Conflicts (accept theirs) + +```python +import re +pattern = r'<<<<<<< HEAD\n(.*?)=======\n(.*?)>>>>>>> [^\n]+\n?' +content = re.sub(pattern, r'\2', content, flags=re.DOTALL) +``` + +### 2. Modify/Delete (two cases!) + +```bash +# Case A: PR introduces NEW files not on target → keep them +git add $FILE + +# Case B: Target REMOVED files the PR modifies → drop them +git rm $FILE +``` + +### 3. Binary Files (snapshots) + +```bash +git checkout --theirs $FILE && git add $FILE +``` + +### 4. Locale Files + +Usually adding new i18n keys — accept theirs, validate JSON: + +```bash +python3 -c "import json; json.load(open('src/locales/en/main.json'))" && echo "Valid" +``` + +## Merge Conflicts After Other Merges + +When merging multiple PRs to the same branch, later PRs may conflict with earlier merges: + +```bash +git fetch origin TARGET_BRANCH +git rebase origin/TARGET_BRANCH +# Resolve new conflicts +git push --force origin backport-$PR-to-TARGET +sleep 20 # Wait for GitHub to recompute merge state +gh pr merge $PR --squash --admin +``` + +## Lessons Learned + +1. **Automation reports more conflicts than reality** — `cherry-pick -m 1` with git auto-merge handles many "conflicts" the automation can't +2. **Never skip based on conflict file count** — 12 or 27 conflicts can be trivial (snapshots, new files). Categorize first: binary PNGs, modify/delete, content, add/add. +3. **Modify/delete goes BOTH ways** — if the PR introduces new files (not on target), `git add` them. If target deleted files the PR modifies, `git rm`. +4. **Binary snapshot PNGs** — always `git checkout --theirs && git add`. Never skip a PR just because it has many snapshot conflicts. +5. **Batch label additions need 2s delay** between API calls to avoid rate limits +6. **Merging 6+ PRs rapidly** can cause later PRs to become unmergeable — wait 20-30s for GitHub to recompute merge state +7. **appModeStore.ts, painter files, GLSLShader files** don't exist on core/1.40 — `git rm` these +8. **Always validate JSON** after resolving locale file conflicts +9. **Dep refresh PRs** — skip on stable branches. Risk of transitive dep regressions outweighs audit cleanup. Cherry-pick individual CVE fixes instead. +10. **Verify after each wave** — run `pnpm typecheck` on the target branch after merging a batch. Catching breakage early prevents compounding errors. +11. **Cloud-only PRs don't belong on core/\* branches** — app mode, cloud auth, and cloud-specific UI changes are irrelevant to local users. Always check PR scope against branch scope before backporting. diff --git a/.claude/skills/backport-management/reference/logging.md b/.claude/skills/backport-management/reference/logging.md new file mode 100644 index 0000000000..ce0cc9f331 --- /dev/null +++ b/.claude/skills/backport-management/reference/logging.md @@ -0,0 +1,96 @@ +# Logging & Session Reports + +## During Execution + +Maintain `execution-log.md` with per-branch tables: + +```markdown +| PR# | Title | Status | Backport PR | Notes | +| ----- | ----- | --------------------------------- | ----------- | ------- | +| #XXXX | Title | ✅ Merged / ⏭️ Skip / ⏸️ Deferred | #YYYY | Details | +``` + +## Wave Verification Log + +Track verification results per wave: + +```markdown +## Wave N Verification — TARGET_BRANCH + +- PRs merged: #A, #B, #C +- Typecheck: ✅ Pass / ❌ Fail +- Issues found: (if any) +- Human review needed: (list any non-trivial conflict resolutions) +``` + +## Session Report Template + +```markdown +# Backport Session Report + +## Summary + +| Branch | Candidates | Merged | Skipped | Deferred | Rate | +| ------ | ---------- | ------ | ------- | -------- | ---- | + +## Deferred Items (Needs Human) + +| PR# | Title | Branch | Issue | + +## Conflict Resolutions Requiring Review + +| PR# | Branch | Conflict Type | Resolution Summary | + +## Automation Performance + +| Metric | Value | +| --------------------------- | ----- | +| Auto success rate | X% | +| Manual resolution rate | X% | +| Overall clean rate | X% | +| Wave verification pass rate | X% | + +## Process Recommendations + +- Were there clusters of related PRs that should have been backported together? +- Any PRs that should have been backported sooner (continuous backporting candidates)? +- Feature branches that need tracking for future sessions? +``` + +## Final Deliverable: Visual Summary + +At session end, generate a **mermaid diagram** showing all backported PRs organized by target branch and category (MUST/SHOULD), plus a summary table. Present this to the user as the final output. + +```mermaid +graph TD + subgraph branch1["☁️ cloud/X.XX — N PRs"] + C1["#XXXX title"] + C2["#XXXX title"] + end + + subgraph branch2must["🔴 core/X.XX MUST — N PRs"] + M1["#XXXX title"] + end + + subgraph branch2should["🟡 core/X.XX SHOULD — N PRs"] + S1["#XXXX-#XXXX N auto-merged"] + S2["#XXXX-#XXXX N manual picks"] + end + + classDef cloudStyle fill:#1a3a5c,stroke:#4da6ff,color:#e0f0ff + classDef coreStyle fill:#1a4a2e,stroke:#4dff88,color:#e0ffe8 + classDef mustStyle fill:#5c1a1a,stroke:#ff4d4d,color:#ffe0e0 + classDef shouldStyle fill:#4a3a1a,stroke:#ffcc4d,color:#fff5e0 +``` + +Use the `mermaid` tool to render this diagram and present it alongside the summary table as the session's final deliverable. + +## Files to Track + +- `candidate_list.md` — all candidates per branch +- `decisions.md` — MUST/SHOULD/SKIP with rationale +- `wave-plan.md` — execution order +- `execution-log.md` — real-time status +- `backport-session-report.md` — final summary + +All in `~/temp/backport-session/`.