chore: add CI safety rules to backport-management skill (#10164)

Adds lessons learned from a bulk backport session where 69 PRs were admin-merged without CI checks, shipping 3 test failures to core/1.41. **Changes:** - **SKILL.md**: CI Safety Rules section, wave verification with `pnpm test:unit`, continuous backporting recommendation, Never Admin-Merge Without CI lesson - **execution.md**: Wait-for-CI step after automation, `gh pr checks --watch` for manual cherry-picks, CI Failure Triage section with common failure categories - **logging.md**: Wave verification log template, CI failure report table in session report ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10164-chore-add-CI-safety-rules-to-backport-management-skill-3266d73d365081aa856de1fb85a31887) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com>
2026-04-20 06:20:11 +00:00 · 2026-03-25 12:17:43 -07:00
parent 08ea013c51
commit 88079250eb
3 changed files with 96 additions and 23 deletions
--- a/.claude/skills/backport-management/SKILL.md
+++ b/.claude/skills/backport-management/SKILL.md
@@ -18,12 +18,20 @@ Cherry-pick backport management for Comfy-Org/ComfyUI_frontend stable release br

 ## 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/`                        |
+| Item           | Value                                                                       |
+| -------------- | --------------------------------------------------------------------------- |
+| Repo           | `~/ComfyUI_frontend` (Comfy-Org/ComfyUI_frontend)                           |
+| Merge strategy | Auto-merge via workflow (`--auto --squash`); `--admin` only after CI passes |
+| Automation     | `pr-backport.yaml` GitHub Action (label-driven, auto-merge enabled)         |
+| Tracking dir   | `~/temp/backport-session/`                                                  |
+
+## CI Safety Rules
+
+**NEVER merge a backport PR without all CI checks passing.** This applies to both automation-created and manual cherry-pick PRs.
+
+- **Automation PRs:** The `pr-backport.yaml` workflow now enables `gh pr merge --auto --squash`, so clean PRs auto-merge once CI passes. Monitor with polling (`gh pr list --base TARGET_BRANCH --state open`). Do not intervene unless CI fails.
+- **Manual cherry-pick PRs:** After `gh pr create`, wait for CI before merging. Poll with `gh pr checks $PR --watch` or use a sleep+check loop. Only merge after all checks pass.
+- **CI failures:** DO NOT use `--admin` to bypass failing CI. Analyze the failure, present it to the user with possible causes (test backported without implementation, missing dependency, flaky test), and let the user decide the next step.

 ## Branch Scope Rules

@@ -108,11 +116,15 @@ 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
+source ~/.nvm/nvm.sh && nvm use 24 && pnpm install && pnpm typecheck && pnpm test:unit
 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.
+If typecheck or tests fail, stop and investigate before continuing. A broken branch after wave N means all subsequent waves will compound the problem.
+
+### Never Admin-Merge Without CI
+
+In a previous bulk session, all 69 backport PRs were merged with `gh pr merge --squash --admin`, bypassing required CI checks. This shipped 3 test failures to a release branch. **Lesson: `--admin` skips all branch protection, including required status checks.** Only use `--admin` after confirming CI has passed (e.g., `gh pr checks $PR` shows all green), or rely on auto-merge (`--auto --squash`) which waits for CI by design.

 ## Continuous Backporting Recommendation

--- a/.claude/skills/backport-management/reference/execution.md
+++ b/.claude/skills/backport-management/reference/execution.md
@@ -19,23 +19,44 @@ done
 # Wait 3 minutes for automation
 sleep 180

-# Check which got auto-PRs
+# Check which got auto-PRs (auto-merge is enabled, so clean ones will self-merge after CI)
 gh pr list --base TARGET_BRANCH --state open --limit 50 --json number,title
 ```

-## Step 2: Review & Merge Clean Auto-PRs
+> **Note:** The `pr-backport.yaml` workflow now enables `gh pr merge --auto --squash` on automation-created PRs. Clean PRs will auto-merge once CI passes — no manual merge needed for those.
+
+## Step 2: Wait for CI & Merge Clean Auto-PRs
+
+Most automation PRs will auto-merge once CI passes (via `--auto --squash` in the workflow). Monitor and handle failures:

 ```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
+# Wait for CI to complete (~45 minutes for full suite)
+sleep 2700
+
+# Check which PRs are still open (CI may have failed, or auto-merge succeeded)
+STILL_OPEN_PRS=$(gh pr list --base TARGET_BRANCH --state open --limit 50 --json number --jq '.[].number')
+RECENTLY_MERGED=$(gh pr list --base TARGET_BRANCH --state merged --limit 50 --json number,title,mergedAt)
+
+# For PRs still open, check CI status
+for pr in $STILL_OPEN_PRS; do
+  CI_FAILED=$(gh pr checks $pr --json name,state --jq '[.[] | select(.state == "FAILURE")] | length')
+  CI_PENDING=$(gh pr checks $pr --json name,state --jq '[.[] | select(.state == "PENDING" or .state == "QUEUED")] | length')
+  if [ "$CI_FAILED" != "0" ]; then
+    # CI failed — collect details for triage
+    echo "PR #$pr — CI FAILED:"
+    gh pr checks $pr --json name,state,link --jq '.[] | select(.state == "FAILURE") | "\(.name): \(.state)"'
+  elif [ "$CI_PENDING" != "0" ]; then
+    echo "PR #$pr — CI still running ($CI_PENDING checks pending)"
+  else
+    # All checks passed but didn't auto-merge (race condition or label issue)
+    gh pr merge $pr --squash --admin
+    sleep 3
+  fi
 done
 ```

+**⚠️ If CI fails: DO NOT admin-merge to bypass.** See "CI Failure Triage" below.
+
 ## Step 3: Manual Worktree for Conflicts

 ```bash
@@ -63,6 +84,13 @@ for PR in ${CONFLICT_PRS[@]}; do
  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+$')
+
+  # Wait for CI before merging — NEVER admin-merge without CI passing
+  echo "Waiting for CI on PR #$NEW_PR..."
+  gh pr checks $NEW_PR --watch --fail-fast || {
+    echo "⚠️ CI failed on PR #$NEW_PR — skipping merge, needs triage"
+    continue
+  }
  gh pr merge $NEW_PR --squash --admin
  sleep 3
 done
@@ -82,7 +110,7 @@ After completing all PRs in a wave for a target branch:
 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
+source ~/.nvm/nvm.sh && nvm use 24 && pnpm install && pnpm typecheck && pnpm test:unit
 git worktree remove /tmp/verify-TARGET --force
 ```

@@ -132,7 +160,8 @@ 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
+# Wait for CI after rebase before merging
+gh pr checks $PR --watch --fail-fast && gh pr merge $PR --squash --admin
 ```

 ## Lessons Learned
@@ -146,5 +175,31 @@ gh pr merge $PR --squash --admin
 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.
+10. **Verify after each wave** — run `pnpm typecheck && pnpm test:unit` 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.
+12. **Never admin-merge without CI** — `--admin` bypasses all branch protections including required status checks. A bulk session of 69 admin-merges shipped 3 test failures. Always wait for CI to pass first, or use `--auto --squash` which waits by design.
+
+## CI Failure Triage
+
+When CI fails on a backport PR, present failures to the user using this template:
+
+```markdown
+### PR #XXXX — CI Failed
+
+- **Failing check:** test / lint / typecheck
+- **Error:** (summary of the failure message)
+- **Likely cause:** test backported without implementation / missing dependency / flaky test / snapshot mismatch
+- **Recommendation:** backport PR #YYYY first / skip this PR / rerun CI after fixing prerequisites
+```
+
+Common failure categories:
+
+| Category                    | Example                                  | Resolution                                |
+| --------------------------- | ---------------------------------------- | ----------------------------------------- |
+| Test without implementation | Test references function not on branch   | Backport the implementation PR first      |
+| Missing dependency          | Import from module not on branch         | Backport the dependency PR first, or skip |
+| Snapshot mismatch           | Screenshot test differs                  | Usually safe — update snapshots on branch |
+| Flaky test                  | Passes on retry                          | Re-run CI, merge if green on retry        |
+| Type error                  | Interface changed on main but not branch | May need manual adaptation                |
+
+**Never assume a failure is safe to skip.** Present all failures to the user with analysis.
--- a/.claude/skills/backport-management/reference/logging.md
+++ b/.claude/skills/backport-management/reference/logging.md
@@ -5,9 +5,9 @@
 Maintain `execution-log.md` with per-branch tables:

 ```markdown
-| PR#   | Title | Status                            | Backport PR | Notes   |
-| ----- | ----- | --------------------------------- | ----------- | ------- |
-| #XXXX | Title | ✅ Merged / ⏭️ Skip / ⏸️ Deferred | #YYYY       | Details |
+| PR#   | Title | CI Status                      | Status                            | Backport PR | Notes   |
+| ----- | ----- | ------------------------------ | --------------------------------- | ----------- | ------- |
+| #XXXX | Title | ✅ Pass / ❌ Fail / ⏳ Pending | ✅ Merged / ⏭️ Skip / ⏸️ Deferred | #YYYY       | Details |
 ```

 ## Wave Verification Log
@@ -19,6 +19,7 @@ Track verification results per wave:

 - PRs merged: #A, #B, #C
 - Typecheck: ✅ Pass / ❌ Fail
+- Unit tests: ✅ Pass / ❌ Fail
 - Issues found: (if any)
 - Human review needed: (list any non-trivial conflict resolutions)
 ```
@@ -41,6 +42,11 @@ Track verification results per wave:

 | PR# | Branch | Conflict Type | Resolution Summary |

+## CI Failure Report
+
+| PR# | Branch | Failing Check | Error Summary | Cause | Resolution |
+| --- | ------ | ------------- | ------------- | ----- | ---------- |
+
 ## Automation Performance

 | Metric                      | Value |