[PR #18] [MERGED] feat(assign-reviewers): expertise-aware load-balanced reviewer assignment workflow (BE-1943) #26

New issue

Closed

opened 2026-10-26 22:35:31 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:31 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/18
Author: @mattmillerai
Created: 6/30/2026
Status: ✅ Merged
Merged: 7/3/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-1943-assign-reviewers

📝 Commits (1)

c354055 feat(assign-reviewers): add expertise-aware load-balanced reviewer assignment workflow (BE-1943)

📊 Changes

2 files changed (+359 additions, -0 deletions)

View changed files

➕ .github/workflows/assign-reviewers.yml (+358 -0)
📝 README.md (+1 -0)

📄 Description

ELI-5

When you open a PR, this new reusable workflow looks at which files you changed,
checks a little config file in your repo that says "people who know area X are
A and B," and automatically asks the right folks to review — while spreading the
load so nobody gets buried, and occasionally tapping someone newer so they get
review reps. It never breaks your PR if anything goes sideways; worst case it
just doesn't assign anyone.

What this adds

A new reusable workflow assign-reviewers.yml (on: workflow_call) that
auto-requests expertise-aware, load-balanced PR reviewers with new-folk
randomization. Structurally modeled on cursor-review-auto-label.yml
(workflow_call header-comment style, vars-driven config, app-token step,
draft/skip handling, defensive "never fail the PR" logging).

Contract

Inputs: reviewer_config_path (default .github/reviewers.yml),
num_reviewers (number, default 2), skip_label (default skip-auto-assign).
Secret: CLOUD_CODE_BOT_PRIVATE_KEY (required); uses vars.APP_ID for the app token.
Caller vars: REVIEWER_GROWTH_POOL, REVIEWER_LOAD_CAP, REVIEWER_EXCLUDE.

Job logic (app-token step → single github-script step): early-exit on
draft / bot author / skip label / already-has-reviewers → list changed paths →
read+parse reviewers.yml from the PR head SHA → match globs to candidate
experts (else default_pool) → drop author + excludes → rank ascending by open
review load (steer off at/over REVIEWER_LOAD_CAP) → deterministic per-PR
new-folk swap from REVIEWER_GROWTH_POOL → request top num_reviewers via the
app-token octokit (try/catch, never fails the PR).

Permissions: job-level contents: read only. Reviewer requests go through
the app token, so no pull-requests: write on GITHUB_TOKEN — which also dodges
the fork-PR read-only-token failure from #12.

Acceptance criteria

✅ Workflow parses (yamllint clean) and is pin-check clean
(zizmor --offline reports no findings — same single sub-threshold
concurrency-limits note the existing cursor-review-auto-label.yml carries).
✅ actions/create-github-app-token and actions/github-script pinned to full
SHAs with version comments (matching detect-unreviewed-merge.yml /
bump-cursor-review-callers.yml style).
✅ README Workflows-table row + full header-comment block (inputs/secrets/vars
- caller example).
⏳ "Move/cut the v1 tag" is a post-merge step (a tag can't point at an
unmerged SHA) — left for the merger per the README Versioning section.

Judgment calls

YAML + glob without dependencies. github-script does not bundle js-yaml
or minimatch, so I wrote a small glob→RegExp matcher and a YAML parser scoped
to the documented reviewers.yml schema (both flow [a, b] and block - a
sequences). Both are covered by a 32-case unit suite I ran against the helpers
extracted from the committed workflow (not just standalone copies). The
schema is documented in the header comment; any parse failure is caught and
degrades to a no-op.
No actions/checkout. I fetch the config via the contents API instead, so
the ticket's checkout option isn't used — hence only the two actions I actually
use are SHA-pinned.
Head-SHA with a base-SHA fallback. Config is read from the PR head SHA
(so a PR's own config edits apply), falling back to the base SHA when the head
isn't resolvable via the base repo's contents API (fork PRs). The base copy is
also the trusted one, so a fork can't inject reviewers by editing the config.
Load query scope uses org:${owner} (the caller's org) rather than a
hardcoded org:Comfy-Org, so the workflow is genuinely reusable.
Single-expert guard. When only one expert is available and the new-folk
swap fires, the newcomer is added rather than replacing the sole expert, so we
don't trade away coverage; the final slice(0, num_reviewers) still caps the count.

Out of scope

No consumer wiring (that's the cloud caller child ticket per the ticket).

_{🔄 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/github-workflows/pull/18 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 6/30/2026 **Status:** ✅ Merged **Merged:** 7/3/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-1943-assign-reviewers` --- ### 📝 Commits (1) - [`c354055`](https://github.com/Comfy-Org/github-workflows/commit/c354055e91a60f778a2149ab983a0a312f6d8d60) feat(assign-reviewers): add expertise-aware load-balanced reviewer assignment workflow (BE-1943) ### 📊 Changes **2 files changed** (+359 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `.github/workflows/assign-reviewers.yml` (+358 -0) 📝 `README.md` (+1 -0) </details> ### 📄 Description ## ELI-5 When you open a PR, this new reusable workflow looks at which files you changed, checks a little config file in your repo that says "people who know area X are A and B," and automatically asks the right folks to review — while spreading the load so nobody gets buried, and occasionally tapping someone newer so they get review reps. It never breaks your PR if anything goes sideways; worst case it just doesn't assign anyone. ## What this adds A new reusable workflow `assign-reviewers.yml` (`on: workflow_call`) that auto-requests expertise-aware, load-balanced PR reviewers with new-folk randomization. Structurally modeled on `cursor-review-auto-label.yml` (workflow_call header-comment style, `vars`-driven config, app-token step, draft/skip handling, defensive "never fail the PR" logging). **Contract** - Inputs: `reviewer_config_path` (default `.github/reviewers.yml`), `num_reviewers` (number, default 2), `skip_label` (default `skip-auto-assign`). - Secret: `CLOUD_CODE_BOT_PRIVATE_KEY` (required); uses `vars.APP_ID` for the app token. - Caller `vars`: `REVIEWER_GROWTH_POOL`, `REVIEWER_LOAD_CAP`, `REVIEWER_EXCLUDE`. **Job logic** (app-token step → single `github-script` step): early-exit on draft / bot author / skip label / already-has-reviewers → list changed paths → read+parse `reviewers.yml` from the PR head SHA → match globs to candidate experts (else `default_pool`) → drop author + excludes → rank ascending by open review load (steer off at/over `REVIEWER_LOAD_CAP`) → deterministic per-PR new-folk swap from `REVIEWER_GROWTH_POOL` → request top `num_reviewers` via the app-token octokit (try/catch, never fails the PR). **Permissions:** job-level `contents: read` only. Reviewer requests go through the app token, so no `pull-requests: write` on `GITHUB_TOKEN` — which also dodges the fork-PR read-only-token failure from #12. ## Acceptance criteria - ✅ Workflow parses (yamllint clean) and is pin-check clean (`zizmor --offline` reports no findings — same single sub-threshold `concurrency-limits` note the existing `cursor-review-auto-label.yml` carries). - ✅ `actions/create-github-app-token` and `actions/github-script` pinned to full SHAs with version comments (matching `detect-unreviewed-merge.yml` / `bump-cursor-review-callers.yml` style). - ✅ README Workflows-table row + full header-comment block (inputs/secrets/vars + caller example). - ⏳ "Move/cut the `v1` tag" is a **post-merge** step (a tag can't point at an unmerged SHA) — left for the merger per the README Versioning section. ## Judgment calls - **YAML + glob without dependencies.** `github-script` does not bundle `js-yaml` or `minimatch`, so I wrote a small glob→RegExp matcher and a YAML parser scoped to the documented `reviewers.yml` schema (both flow `[a, b]` and block `- a` sequences). Both are covered by a 32-case unit suite I ran against the helpers **extracted from the committed workflow** (not just standalone copies). The schema is documented in the header comment; any parse failure is caught and degrades to a no-op. - **No `actions/checkout`.** I fetch the config via the contents API instead, so the ticket's checkout option isn't used — hence only the two actions I actually use are SHA-pinned. - **Head-SHA with a base-SHA fallback.** Config is read from the PR head SHA (so a PR's own config edits apply), falling back to the base SHA when the head isn't resolvable via the base repo's contents API (fork PRs). The base copy is also the trusted one, so a fork can't inject reviewers by editing the config. - **Load query scope** uses `org:${owner}` (the caller's org) rather than a hardcoded `org:Comfy-Org`, so the workflow is genuinely reusable. - **Single-expert guard.** When only one expert is available and the new-folk swap fires, the newcomer is added rather than replacing the sole expert, so we don't trade away coverage; the final `slice(0, num_reviewers)` still caps the count. ## Out of scope No consumer wiring (that's the cloud caller child ticket per the ticket). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:31 +00:00