[PR #212] [MERGED] Backport checker: don't flag dual-homed commits as needing backport #213

New issue

Closed

opened 2026-10-27 21:10:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-27 21:10:40 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/Comfy-PR/pull/212
Author: @claude[bot]
Created: 8/17/2026
Status: ✅ Merged
Merged: 8/23/2026
Merged by: @christian-byrne

Base: main ← Head: backport-checker-dual-homed-commits

📝 Commits (3)

e664cec fix: don't flag dual-homed commits as needing backport
3469b1f Merge remote-tracking branch 'origin/main' into backport-checker-dual-homed-commits
3d677dd fix(test): add MSW handlers for Slack users.info/conversations.info

📊 Changes

4 files changed (+167 additions, -8 deletions)

View changed files

📝 app/tasks/gh-frontend-backport-checker/index.spec.ts (+17 -0)
📝 app/tasks/gh-frontend-backport-checker/index.ts (+112 -6)
📝 src/test/msw-setup.ts (+3 -2)
➕ src/test/slack-handlers.ts (+35 -0)

📄 Description

Requested via Slack thread

What changed

app/tasks/gh-frontend-backport-checker/index.ts flagged PR #14387 and #15273 as "❗ Might need backport" on the v1.52.0 release report. Christian pointed out these are "dual-homed commits" per ComfyUI_frontend's docs/release-process.md#dual-homed-commits: a minor-version bump (x.y.0) freezes the previous minor by branching core/<prevMinor> + cloud/<prevMinor> from the commit right before the bump, so every commit merged to main since the last release ships in the new version and already sits, byte-for-byte, on those freshly-cut branches. Nothing to backport — the branch cut already did it.

I verified this directly against the repo. Both PRs' merge commits are literal ancestors of core/1.51/cloud/1.51 (which share a tip, 9a54e1f0):

$ git log --oneline 9a54e1f0 -200 | grep -iE "14387|15273"
947e35b fix(cloud): surface a failed support request instead of logging it (#15273)
38b07da fix: keep locked widget hover color consistent with node color (#14387)

$ git merge-base --is-ancestor 947e35b 9a54e1f0 && echo yes   # -> yes
$ git merge-base --is-ancestor 38b07da 9a54e1f0 && echo yes   # -> yes

Neither PR carries a core/1.*/cloud/1.* label (only size:*/risk:*), and neither mentions "backport" — that's why they were flagged. The checker's only branch-comparison logic (compareCommits(target, commit)) runs exclusively inside the per-labeled-target-branch loop, gated on backportStatusRaw === "needed". A PR with no explicit signal never reaches that check at all and defaults straight to "might need backport" — regardless of whether the commit is actually already on a stable branch. Notably, availableBackportTargetBranches (the full list of core/1.**/cloud/1.** branches) was already being fetched at the top of the run for exactly this kind of check, but was never passed anywhere — it was only used for a log line.

Why not "skip all x.y.0 releases"?

That was the other option on the table, but it's the wrong fix: a minor release can ship a genuinely unbackported fix alongside dual-homed ones (there's no rule that a x.y.0 release only contains freeze-range commits — anything merged in the same window qualifies). Suppressing by release type would silence real cases along with the false positives, with no way to tell them apart from the Slack report.

The fix

Before flagging a bugfix PR with no explicit backport signal, check whether its commit is already an ancestor of any currently-existing core/1.**/cloud/1.** branch (compareCommits(branch, commit) → "identical"/"behind"). If so, mark it completed on that branch instead of leaving it ambiguous.

This is a same-SHA ancestry check, not a release-type guess — it works for any release type, and it can't produce a false negative: a cherry-picked backport always gets a brand-new commit SHA, so a genuinely unbackported commit can never satisfy it by coincidence. The only way the same SHA appears on two branches is if one was literally cut from a point in history that already contains it — exactly what a minor-version branch cut does on purpose.

export function isAlreadyOnBranchStatus(status: string): boolean {
  return status === "identical" || status === "behind";
}

async function findDualHomedBranches(
  owner: string,
  repo: string,
  commitSha: string,
  candidateBranches: string[],
): Promise<string[]> {
  // compareCommits(branch, commitSha) against every currently existing
  // backport-target branch; returns the ones where it's already present.
}

availableBackportTargetBranches is now threaded through to processTask/processBugfixPR so this check has something to run against.

Before / After

Before: might need backport shown for both PRs on the v1.52.0 report, with no way to tell (from Slack) whether that's a real gap or a dual-homed non-issue.

After: both PRs resolve to completed against core/1.51/cloud/1.51 and drop out of the "Might need backport" section entirely — same outcome as if someone had manually verified and labeled them. A PR in the same situation but genuinely missing from every existing branch is unaffected and still gets flagged as before.

Testing

bun run typecheck (tsgo) — clean
bunx oxlint on both changed files — 0 warnings/errors
bunx oxfmt on both changed files — clean
bun test app/tasks/gh-frontend-backport-checker/ — 81 pass, 0 fail (3 new tests for isAlreadyOnBranchStatus; the network-calling findDualHomedBranches/processTask orchestration isn't unit-tested, matching this file's existing convention of only unit-testing pure helpers)

No live Slack/GitHub calls were made by this change.

Generated by Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/Comfy-PR/pull/212 **Author:** [@claude[bot]](https://github.com/apps/claude) **Created:** 8/17/2026 **Status:** ✅ Merged **Merged:** 8/23/2026 **Merged by:** [@christian-byrne](https://github.com/christian-byrne) **Base:** `main` ← **Head:** `backport-checker-dual-homed-commits` --- ### 📝 Commits (3) - [`e664cec`](https://github.com/Comfy-Org/Comfy-PR/commit/e664cecd84a2baa17ae1e94ccf7930064354257a) fix: don't flag dual-homed commits as needing backport - [`3469b1f`](https://github.com/Comfy-Org/Comfy-PR/commit/3469b1f3e494a27f8eea0fb8c5cd9b4f01d4e2bc) Merge remote-tracking branch 'origin/main' into backport-checker-dual-homed-commits - [`3d677dd`](https://github.com/Comfy-Org/Comfy-PR/commit/3d677dd6935828d9122cbce5db5ad787f6eb0405) fix(test): add MSW handlers for Slack users.info/conversations.info ### 📊 Changes **4 files changed** (+167 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `app/tasks/gh-frontend-backport-checker/index.spec.ts` (+17 -0) 📝 `app/tasks/gh-frontend-backport-checker/index.ts` (+112 -6) 📝 `src/test/msw-setup.ts` (+3 -2) ➕ `src/test/slack-handlers.ts` (+35 -0) </details> ### 📄 Description  _Requested via [Slack thread](https://comfy-organization.slack.com/archives/C09K9TPU2G7/p1787006943236709?thread_ts=1787006943.236709&cid=C09K9TPU2G7)_ ## What changed `app/tasks/gh-frontend-backport-checker/index.ts` flagged PR [#14387](https://github.com/Comfy-Org/ComfyUI_frontend/pull/14387) and [#15273](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15273) as "❗ Might need backport" on the v1.52.0 release report. Christian pointed out these are "dual-homed commits" per ComfyUI_frontend's `docs/release-process.md#dual-homed-commits`: a minor-version bump (x.y.0) freezes the previous minor by branching `core/<prevMinor>` + `cloud/<prevMinor>` from the commit right before the bump, so every commit merged to `main` since the last release ships in the new version **and** already sits, byte-for-byte, on those freshly-cut branches. Nothing to backport — the branch cut already did it. I verified this directly against the repo. Both PRs' merge commits are literal ancestors of `core/1.51`/`cloud/1.51` (which share a tip, `9a54e1f0`): ``` $ git log --oneline 9a54e1f0 -200 | grep -iE "14387|15273" 947e35b fix(cloud): surface a failed support request instead of logging it (#15273) 38b07da fix: keep locked widget hover color consistent with node color (#14387) $ git merge-base --is-ancestor 947e35b 9a54e1f0 && echo yes # -> yes $ git merge-base --is-ancestor 38b07da 9a54e1f0 && echo yes # -> yes ``` Neither PR carries a `core/1.*`/`cloud/1.*` label (only `size:*`/`risk:*`), and neither mentions "backport" — that's why they were flagged. The checker's only branch-comparison logic (`compareCommits(target, commit)`) runs exclusively inside the per-labeled-target-branch loop, gated on `backportStatusRaw === "needed"`. A PR with no explicit signal never reaches that check at all and defaults straight to "might need backport" — regardless of whether the commit is actually already on a stable branch. Notably, `availableBackportTargetBranches` (the full list of `core/1.**`/`cloud/1.**` branches) was already being fetched at the top of the run for exactly this kind of check, but was never passed anywhere — it was only used for a log line. ### Why not "skip all x.y.0 releases"? That was the other option on the table, but it's the wrong fix: a minor release can ship a genuinely unbackported fix *alongside* dual-homed ones (there's no rule that a x.y.0 release only contains freeze-range commits — anything merged in the same window qualifies). Suppressing by release type would silence real cases along with the false positives, with no way to tell them apart from the Slack report. ### The fix Before flagging a bugfix PR with no explicit backport signal, check whether its commit is already an ancestor of any currently-existing `core/1.**`/`cloud/1.**` branch (`compareCommits(branch, commit) → "identical"/"behind"`). If so, mark it `completed` on that branch instead of leaving it ambiguous. This is a same-SHA ancestry check, not a release-type guess — it works for any release type, and it can't produce a false negative: a cherry-picked backport always gets a brand-new commit SHA, so a genuinely unbackported commit can never satisfy it by coincidence. The only way the same SHA appears on two branches is if one was literally cut from a point in history that already contains it — exactly what a minor-version branch cut does on purpose. ```ts export function isAlreadyOnBranchStatus(status: string): boolean { return status === "identical" || status === "behind"; } async function findDualHomedBranches( owner: string, repo: string, commitSha: string, candidateBranches: string[], ): Promise<string[]> { // compareCommits(branch, commitSha) against every currently existing // backport-target branch; returns the ones where it's already present. } ``` `availableBackportTargetBranches` is now threaded through to `processTask`/`processBugfixPR` so this check has something to run against. ### Before / After **Before**: `might need backport` shown for both PRs on the v1.52.0 report, with no way to tell (from Slack) whether that's a real gap or a dual-homed non-issue. **After**: both PRs resolve to `completed` against `core/1.51`/`cloud/1.51` and drop out of the "Might need backport" section entirely — same outcome as if someone had manually verified and labeled them. A PR in the same situation but genuinely missing from every existing branch is unaffected and still gets flagged as before. ## Testing - `bun run typecheck` (tsgo) — clean - `bunx oxlint` on both changed files — 0 warnings/errors - `bunx oxfmt` on both changed files — clean - `bun test app/tasks/gh-frontend-backport-checker/` — 81 pass, 0 fail (3 new tests for `isAlreadyOnBranchStatus`; the network-calling `findDualHomedBranches`/`processTask` orchestration isn't unit-tested, matching this file's existing convention of only unit-testing pure helpers) No live Slack/GitHub calls were made by this change. --- _Generated by [Claude Code](https://claude.ai/code/session_01UTG7KUnGjiMcdEYaA8MrRJ)_ --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-27 21:10:40 +00:00