[PR #211] [MERGED] Backport checker: skip irrelevant paths, resolve bot-authored PR attribution #209

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: backport-checker-bot-attribution-and-filtering

📝 Commits (2)

8ac4c76 feat(backport-checker): skip irrelevant paths, resolve bot-authored PR attribution
9ccbba7 fix: restore process.env after people.spec.ts to stop leaking a fake SLACK_BOT_TOKEN

📊 Changes

4 files changed (+584 additions, -6 deletions)

View changed files

📝 app/tasks/gh-frontend-backport-checker/index.spec.ts (+154 -1)
📝 app/tasks/gh-frontend-backport-checker/index.ts (+182 -4)
➕ lib/notion/people.spec.ts (+202 -0)
📝 lib/notion/people.ts (+46 -1)

📄 Description

Requested via Slack thread

What changed

Two improvements to app/tasks/gh-frontend-backport-checker/index.ts, per Christian's Slack request:

1. Skip PRs that only touch irrelevant paths

A bugfix PR is now skipped (not flagged as "might need backport") when every one of its changed files matches an ignore-path glob. If it touches anything else too, it's still flagged as before — this only suppresses PRs that are 100% out of scope.

The ignore list is an exported, easily-editable array (IGNORED_BACKPORT_PATH_GLOBS) near the top of the file:

export const IGNORED_BACKPORT_PATH_GLOBS: string[] = [
  "apps/website/**",     // comfy.org marketing site (Astro) — not the shipped frontend app
  ".github/**",          // CI/CD workflow + automation definitions
  ".husky/**",
  ".coderabbit.yaml",    // repo-level CI/lint/formatter tooling config
  ".oxlintrc.json",
  ".oxfmtrc.json",
  ".stylelintrc.json",
  ".pinact.yaml",
  ".yamllint",
  ".fallowrc.jsonc",
  "codecov.yml",
];

I inspected the ComfyUI_frontend repo layout to build this list — apps/website (Astro marketing site) and apps/desktop-ui (a real product, kept in scope) are the two apps/* packages; .github/workflows/ holds ~30 CI/CD pipelines. Changed files are fetched via pulls.listFiles (paginated with the existing ghPageFlow helper).

Flag for Christian: I deliberately did not add apps/desktop-ui — it's a real shipped app, not "irrelevant" — but wanted to call that out explicitly in case you meant to include it too. Also open to trimming/expanding the tooling-config file list; it's just an array, easy to adjust.

2. Resolve bot-authored PR attribution

When a PR's author is a bot/automation account (claude[bot], comfy-pr-bot, cloud-code-bot[bot], dependabot[bot], ... — detected via isBotLogin(), reusing the existing bot-comment-filter regex), the checker now tries to resolve the real human to tag:

Co-authored-by: trailer on the PR's commits (pulls.listCommits) — if the trailer's email is a GitHub noreply address (123+login@users.noreply.github.com), the embedded login is used directly; otherwise the trailer's name is matched against the Notion People database.
Fallback: PR description attribution line — this workspace's convention of putting _Requested by **Name**_ near the top of the PR body (added by whatever opens these PRs on someone's behalf), matched by name against the Notion People database.
Fallback to existing behavior — if neither resolves, the raw bot login is used as before, which the existing resolveSlackTagForAuthor pipeline already can't map to a Slack user, so it already falls through to tagging the release sheriff (unchanged).

What justified this, from inspecting the two example PRs:

#14206 — authored by claude[bot], single commit authored by Claude <noreply@anthropic.com> with no Co-authored-by: trailer. Body starts with _Requested by **nav** · [Slack thread](...)_. So resolution here comes entirely from the body-attribution fallback, matching "nav" → Notion People row with UniqueID title "Nav" → GitHub username nav-tej → Slack U0AMJURRLKV.
#15342 — same shape: claude[bot]-authored, single commit, no co-author trailer, body starts with _Requested by **Christian Byrne** · [Slack thread](...)_ → matches Notion row christian-byrne → Slack U087MJCDHHC.

Neither example actually has a Co-authored-by: trailer, so the co-author path is implemented per the request ("try to find co-author... fall back to attribution") but untested against a real example — I added unit test coverage for its parsing logic (noreply-email login extraction, trailer regex) using synthetic fixtures instead.

To make the name → GitHub-username matching possible, I extended lib/notion/people.ts:

PersonMapping gained a name field sourced from the row's UniqueID (title) column — needed because the nav-tej row has no linked Notion Person (that field was empty), so the title column is the only place "Nav" appears.
New findGithubUsernameByPersonName(name) matches case-insensitively on full name or first-name-only (e.g. "Christian" → "Christian Byrne"), skipping inactive rows — mirrors the existing findSlackIdByGithubUsername conventions.

Flag for Christian: the fallback order (co-author trailer, then body attribution, then existing bot-login-falls-through-to-sheriff behavior) matches your message, but worth double-checking that's the priority you want — and whether comfy-pr-bot / cloud-code-bot[bot] should even be treated as "bot" for this purpose (their PRs are automated release/CI bumps with no human requester, so the fallback will just find nothing and fall through harmlessly, but flagging in case you'd rather scope this to claude[bot] only).

Testing

bun run typecheck (tsgo) — clean
bun run lint (oxlint) — 0 warnings/errors on changed files (pre-existing warnings elsewhere in the repo are unrelated)
bun run fmt (oxfmt) — clean
bun test app/tasks/gh-frontend-backport-checker/ lib/notion/ — 107 pass, 0 fail
- Added path-filtering, bot-detection, and attribution-regex tests to index.spec.ts, including fixtures built from the real bodies of PR #14206 and #15342.
- Added lib/notion/people.spec.ts (new — no prior test file existed) covering findGithubUsernameByPersonName, mocking the Notion client per the pattern already used in app/tasks/gh-priority-sync/index.spec.ts.

No live Slack/Notion/GitHub calls were made by this change; nothing was posted to Slack or Notion as part of this work.

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/211 **Author:** [@claude[bot]](https://github.com/apps/claude) **Created:** 8/17/2026 **Status:** ✅ Merged **Merged:** 8/21/2026 **Merged by:** [@christian-byrne](https://github.com/christian-byrne) **Base:** `main` ← **Head:** `backport-checker-bot-attribution-and-filtering` --- ### 📝 Commits (2) - [`8ac4c76`](https://github.com/Comfy-Org/Comfy-PR/commit/8ac4c76c150f43f748e89c9f5a272184c54da4b2) feat(backport-checker): skip irrelevant paths, resolve bot-authored PR attribution - [`9ccbba7`](https://github.com/Comfy-Org/Comfy-PR/commit/9ccbba765c0990fe563d480acb289d0ea2df62d1) fix: restore process.env after people.spec.ts to stop leaking a fake SLACK_BOT_TOKEN ### 📊 Changes **4 files changed** (+584 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `app/tasks/gh-frontend-backport-checker/index.spec.ts` (+154 -1) 📝 `app/tasks/gh-frontend-backport-checker/index.ts` (+182 -4) ➕ `lib/notion/people.spec.ts` (+202 -0) 📝 `lib/notion/people.ts` (+46 -1) </details> ### 📄 Description  _Requested via [Slack thread](https://comfy-organization.slack.com/archives/C09K9TPU2G7/p1786609614107509?thread_ts=1786609614.107509&cid=C09K9TPU2G7)_ ## What changed Two improvements to `app/tasks/gh-frontend-backport-checker/index.ts`, per Christian's Slack request: ### 1. Skip PRs that only touch irrelevant paths A bugfix PR is now skipped (not flagged as "might need backport") when **every** one of its changed files matches an ignore-path glob. If it touches anything else too, it's still flagged as before — this only suppresses PRs that are 100% out of scope. The ignore list is an exported, easily-editable array (`IGNORED_BACKPORT_PATH_GLOBS`) near the top of the file: ```ts export const IGNORED_BACKPORT_PATH_GLOBS: string[] = [ "apps/website/**", // comfy.org marketing site (Astro) — not the shipped frontend app ".github/**", // CI/CD workflow + automation definitions ".husky/**", ".coderabbit.yaml", // repo-level CI/lint/formatter tooling config ".oxlintrc.json", ".oxfmtrc.json", ".stylelintrc.json", ".pinact.yaml", ".yamllint", ".fallowrc.jsonc", "codecov.yml", ]; ``` I inspected the `ComfyUI_frontend` repo layout to build this list — `apps/website` (Astro marketing site) and `apps/desktop-ui` (a real product, kept **in scope**) are the two `apps/*` packages; `.github/workflows/` holds ~30 CI/CD pipelines. Changed files are fetched via `pulls.listFiles` (paginated with the existing `ghPageFlow` helper). **Flag for Christian**: I deliberately did *not* add `apps/desktop-ui` — it's a real shipped app, not "irrelevant" — but wanted to call that out explicitly in case you meant to include it too. Also open to trimming/expanding the tooling-config file list; it's just an array, easy to adjust. ### 2. Resolve bot-authored PR attribution When a PR's author is a bot/automation account (`claude[bot]`, `comfy-pr-bot`, `cloud-code-bot[bot]`, `dependabot[bot]`, ... — detected via `isBotLogin()`, reusing the existing bot-comment-filter regex), the checker now tries to resolve the real human to tag: 1. **`Co-authored-by:` trailer** on the PR's commits (`pulls.listCommits`) — if the trailer's email is a GitHub noreply address (`123+login@users.noreply.github.com`), the embedded login is used directly; otherwise the trailer's name is matched against the Notion People database. 2. **Fallback: PR description attribution line** — this workspace's convention of putting `_Requested by **Name**_` near the top of the PR body (added by whatever opens these PRs on someone's behalf), matched by name against the Notion People database. 3. **Fallback to existing behavior** — if neither resolves, the raw bot login is used as before, which the existing `resolveSlackTagForAuthor` pipeline already can't map to a Slack user, so it already falls through to tagging the release sheriff (unchanged). **What justified this**, from inspecting the two example PRs: - [#14206](https://github.com/Comfy-Org/ComfyUI_frontend/pull/14206) — authored by `claude[bot]`, single commit authored by `Claude <noreply@anthropic.com>` with **no** `Co-authored-by:` trailer. Body starts with `_Requested by **nav** · [Slack thread](...)_`. So resolution here comes entirely from the body-attribution fallback, matching `"nav"` → Notion People row with `UniqueID` title `"Nav"` → GitHub username `nav-tej` → Slack `U0AMJURRLKV`. - [#15342](https://github.com/Comfy-Org/ComfyUI_frontend/pull/15342) — same shape: `claude[bot]`-authored, single commit, no co-author trailer, body starts with `_Requested by **Christian Byrne** · [Slack thread](...)_` → matches Notion row `christian-byrne` → Slack `U087MJCDHHC`. Neither example actually has a `Co-authored-by:` trailer, so the co-author path is implemented per the request ("try to find co-author... fall back to attribution") but untested against a real example — I added unit test coverage for its parsing logic (noreply-email login extraction, trailer regex) using synthetic fixtures instead. To make the name → GitHub-username matching possible, I extended `lib/notion/people.ts`: - `PersonMapping` gained a `name` field sourced from the row's `UniqueID` (title) column — needed because the `nav-tej` row has no linked Notion `Person` (that field was empty), so the title column is the only place "Nav" appears. - New `findGithubUsernameByPersonName(name)` matches case-insensitively on full name or first-name-only (e.g. "Christian" → "Christian Byrne"), skipping inactive rows — mirrors the existing `findSlackIdByGithubUsername` conventions. **Flag for Christian**: the fallback order (co-author trailer, then body attribution, then existing bot-login-falls-through-to-sheriff behavior) matches your message, but worth double-checking that's the priority you want — and whether `comfy-pr-bot` / `cloud-code-bot[bot]` should even be treated as "bot" for this purpose (their PRs are automated release/CI bumps with no human requester, so the fallback will just find nothing and fall through harmlessly, but flagging in case you'd rather scope this to `claude[bot]` only). ## Testing - `bun run typecheck` (tsgo) — clean - `bun run lint` (oxlint) — 0 warnings/errors on changed files (pre-existing warnings elsewhere in the repo are unrelated) - `bun run fmt` (oxfmt) — clean - `bun test app/tasks/gh-frontend-backport-checker/ lib/notion/` — 107 pass, 0 fail - Added path-filtering, bot-detection, and attribution-regex tests to `index.spec.ts`, including fixtures built from the real bodies of PR #14206 and #15342. - Added `lib/notion/people.spec.ts` (new — no prior test file existed) covering `findGithubUsernameByPersonName`, mocking the Notion client per the pattern already used in `app/tasks/gh-priority-sync/index.spec.ts`. No live Slack/Notion/GitHub calls were made by this change; nothing was posted to Slack or Notion as part of this work. --- _Generated by [Claude Code](https://claude.ai/code/session_01V2TbWrYdYkEfK5qx8m5MXx)_ --- <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:38 +00:00