[PR #36] [MERGED] feat: reusable PR-size cap workflow with per-repo inputs #47

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-3481-pr-size-reusable

📝 Commits (4)

d2e53fb feat: add reusable PR-size cap workflow with per-repo inputs
132efea Merge remote-tracking branch 'origin/main' into matt/be-3481-pr-size-reusable
2eecbcd fix(check-pr-size): cap the deleted-file blob read in contentGenerated
c413115 Merge origin/main into matt/be-3481-pr-size-reusable

📊 Changes

8 files changed (+1836 additions, -0 deletions)

View changed files

➕ .github/workflows/pr-size.yml (+303 -0)
➕ .github/workflows/test-pr-size.yml (+55 -0)
📝 README.md (+1 -0)
➕ scripts/check-pr-size/go.mod (+3 -0)
➕ scripts/check-pr-size/main.go (+367 -0)
➕ scripts/check-pr-size/main_test.go (+397 -0)
➕ scripts/check-pr-size/size.go (+298 -0)
➕ scripts/check-pr-size/size_test.go (+412 -0)

📄 Description

ELI-5

Big PRs are hard to review well, so we have a CI check (built for the cloud repo) that fails a PR when it changes too many lines of hand-written code — generated files and lockfiles don't count, and a label lets a legitimately big change through. This PR moves that check into this shared-workflows repo so any Comfy-Org repo can turn it on with a few lines of config, picking its own line cap and its own list of "don't count these" files, and can start in a warn-only mode that reports without blocking.

What

.github/workflows/pr-size.yml — reusable (workflow_call) PR-size cap, same topology as cursor-review.yml: callers add a thin SHA-pinned wrapper; the counting logic lives here.
- Inputs, all optional: max_lines (1000), mode (enforce | warn — warn reports and comments on overage but never fails the check, for gradual rollout), bypass_label (oversized-ok), extra_lockfiles, extra_generated_globs, comment (true), bot_app_id, workflows_ref.
- Secrets: BOT_APP_PRIVATE_KEY (pass-through; callers use the existing vars.APP_ID + bot-key pattern). When credentials are absent or comment: false, the workflow degrades gracefully to check status + step summary — no comment job failure.
- Two-job security split preserved from the original: the size job runs against PR code with contents: read and builds the checker from this repo's pinned checkout (no PR-authored code executes in it); the comment job holds the bot write token, checks out no PR code, and receives the report as an artifact.
scripts/check-pr-size/ — the Go module (diff parsing, generated-file classification, cap evaluation), extended with --mode, --bypass-label, --extra-lockfiles, and --extra-generated-globs, plus an over_cap flag handed to the comment job so warn mode still comments. The base-ref-only linguist-generated reading (a PR cannot mark its own files generated) and the Go // Code generated ... DO NOT EDIT. marker rule carry over unchanged; the marker rule simply never matches in non-Go repos.
.github/workflows/test-pr-size.yml — CI for the module: gofmt, go vet, go test on change (mirrors test-cursor-review-scripts.yml).
README table row.

Verification

go test ./..., go vet, gofmt -l — all clean (49 tests incl. new tables for extras parsing, glob semantics, warn/enforce exit contract, report rendering).
End-to-end smoke against a throwaway git repo: enforce-over → exit 1 + over_cap=true; warn-over → exit 0 + over_cap=true; bypass → exit 0 + over_cap=false; extras exclude lockfiles/globs; invalid --mode → exit 2; GITHUB_STEP_SUMMARY + GITHUB_OUTPUT writes verified.
zizmor (v1.27.0): no findings on both new workflows (actions hash-pinned, app token scoped to issues/pull-requests write). actionlint: clean — it caught that the secrets context is unavailable in step-level if, so the credentials-present test is evaluated in job env instead.

Judgment calls

Dropped the source's persist-credentials: true + token-drop dance. The original fetched the base branch with persisted credentials, then stripped the token before running the (PR-authored) checker. Here the checker is trusted (built from this repo) and fetch-depth: 0 already fetches all refs during the authenticated checkout, so the job diffs base.sha...head.sha directly with persist-credentials: false throughout — equivalent counts (three-dot uses the merge-base), simpler, and zizmor-clean.
Comment posting keys off an over_cap flag carried in the report artifact rather than the size job's result, because in warn mode the job is green on overage but must still comment. The flag rides the artifact (not job outputs) so it carries identically whether the size job passed or failed. A crash before rendering leaves any existing sticky comment untouched instead of overwriting it with an empty body — a small robustness improvement over the source.
git clean -ffdx on the tool checkout before building, so files a PR pre-seeds under _pr_size_tool/ (untracked in the tool repo, which checkout does not remove) can't be compiled into the checker.
Glob semantics for extra_generated_globs are documented in the input description (* segment-local, ** cross-segment, ? single char; slash-less patterns match base names at any depth) and pinned by table tests.
The upstream source of this port is still in review on its home repo; if it changes before this merges, this port should be re-synced (the sticky-comment marker  is kept identical so existing comments carry over when that repo flips to a thin caller).

Out of scope (tracked separately): registering pr-size callers in the bump dispatcher, flipping the origin repo to a thin caller, and per-repo onboarding (cap selection, bypass-label creation, branch protection).

_{🔄 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/36 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/18/2026 **Status:** ✅ Merged **Merged:** 7/21/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-3481-pr-size-reusable` --- ### 📝 Commits (4) - [`d2e53fb`](https://github.com/Comfy-Org/github-workflows/commit/d2e53fb8b12d5c5d148eddd7f309bfa716fc2355) feat: add reusable PR-size cap workflow with per-repo inputs - [`132efea`](https://github.com/Comfy-Org/github-workflows/commit/132efea4aa9211d7694d28ba4f00b0000b1f9589) Merge remote-tracking branch 'origin/main' into matt/be-3481-pr-size-reusable - [`2eecbcd`](https://github.com/Comfy-Org/github-workflows/commit/2eecbcdeb0489537696776e3db2a6a54b2277c43) fix(check-pr-size): cap the deleted-file blob read in contentGenerated - [`c413115`](https://github.com/Comfy-Org/github-workflows/commit/c413115b17147b4348e6e84cf2273bfd0c75460f) Merge origin/main into matt/be-3481-pr-size-reusable ### 📊 Changes **8 files changed** (+1836 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `.github/workflows/pr-size.yml` (+303 -0) ➕ `.github/workflows/test-pr-size.yml` (+55 -0) 📝 `README.md` (+1 -0) ➕ `scripts/check-pr-size/go.mod` (+3 -0) ➕ `scripts/check-pr-size/main.go` (+367 -0) ➕ `scripts/check-pr-size/main_test.go` (+397 -0) ➕ `scripts/check-pr-size/size.go` (+298 -0) ➕ `scripts/check-pr-size/size_test.go` (+412 -0) </details> ### 📄 Description ## ELI-5 Big PRs are hard to review well, so we have a CI check (built for the cloud repo) that fails a PR when it changes too many lines of hand-written code — generated files and lockfiles don't count, and a label lets a legitimately big change through. This PR moves that check into this shared-workflows repo so *any* Comfy-Org repo can turn it on with a few lines of config, picking its own line cap and its own list of "don't count these" files, and can start in a warn-only mode that reports without blocking. ## What - **`.github/workflows/pr-size.yml`** — reusable (`workflow_call`) PR-size cap, same topology as `cursor-review.yml`: callers add a thin SHA-pinned wrapper; the counting logic lives here. - Inputs, all optional: `max_lines` (1000), `mode` (`enforce` | `warn` — warn reports and comments on overage but never fails the check, for gradual rollout), `bypass_label` (`oversized-ok`), `extra_lockfiles`, `extra_generated_globs`, `comment` (true), `bot_app_id`, `workflows_ref`. - Secrets: `BOT_APP_PRIVATE_KEY` (pass-through; callers use the existing `vars.APP_ID` + bot-key pattern). When credentials are absent or `comment: false`, the workflow degrades gracefully to check status + step summary — no comment job failure. - Two-job security split preserved from the original: the size job runs against PR code with `contents: read` and **builds the checker from this repo's pinned checkout** (no PR-authored code executes in it); the comment job holds the bot write token, checks out no PR code, and receives the report as an artifact. - **`scripts/check-pr-size/`** — the Go module (diff parsing, generated-file classification, cap evaluation), extended with `--mode`, `--bypass-label`, `--extra-lockfiles`, and `--extra-generated-globs`, plus an `over_cap` flag handed to the comment job so warn mode still comments. The base-ref-only `linguist-generated` reading (a PR cannot mark its own files generated) and the Go `// Code generated ... DO NOT EDIT.` marker rule carry over unchanged; the marker rule simply never matches in non-Go repos. - **`.github/workflows/test-pr-size.yml`** — CI for the module: `gofmt`, `go vet`, `go test` on change (mirrors `test-cursor-review-scripts.yml`). - README table row. ## Verification - `go test ./...`, `go vet`, `gofmt -l` — all clean (49 tests incl. new tables for extras parsing, glob semantics, warn/enforce exit contract, report rendering). - End-to-end smoke against a throwaway git repo: enforce-over → exit 1 + `over_cap=true`; warn-over → exit 0 + `over_cap=true`; bypass → exit 0 + `over_cap=false`; extras exclude lockfiles/globs; invalid `--mode` → exit 2; `GITHUB_STEP_SUMMARY` + `GITHUB_OUTPUT` writes verified. - `zizmor` (v1.27.0): no findings on both new workflows (actions hash-pinned, app token scoped to `issues`/`pull-requests` write). `actionlint`: clean — it caught that the `secrets` context is unavailable in step-level `if`, so the credentials-present test is evaluated in job `env` instead. ## Judgment calls - **Dropped the source's `persist-credentials: true` + token-drop dance.** The original fetched the base branch with persisted credentials, then stripped the token before running the (PR-authored) checker. Here the checker is trusted (built from this repo) and `fetch-depth: 0` already fetches all refs during the authenticated checkout, so the job diffs `base.sha...head.sha` directly with `persist-credentials: false` throughout — equivalent counts (three-dot uses the merge-base), simpler, and zizmor-clean. - **Comment posting keys off an `over_cap` flag carried in the report artifact** rather than the size job's result, because in `warn` mode the job is green on overage but must still comment. The flag rides the artifact (not job outputs) so it carries identically whether the size job passed or failed. A crash before rendering leaves any existing sticky comment untouched instead of overwriting it with an empty body — a small robustness improvement over the source. - **`git clean -ffdx` on the tool checkout before building**, so files a PR pre-seeds under `_pr_size_tool/` (untracked in the tool repo, which checkout does not remove) can't be compiled into the checker. - Glob semantics for `extra_generated_globs` are documented in the input description (`*` segment-local, `**` cross-segment, `?` single char; slash-less patterns match base names at any depth) and pinned by table tests. - The upstream source of this port is still in review on its home repo; if it changes before this merges, this port should be re-synced (the sticky-comment marker `` is kept identical so existing comments carry over when that repo flips to a thin caller). Out of scope (tracked separately): registering pr-size callers in the bump dispatcher, flipping the origin repo to a thin caller, and per-repo onboarding (cap selection, bypass-label creation, branch protection). --- <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:42 +00:00