[PR #51] [MERGED] harden pr-size reusable: batch check-attr, --source probe, git timeouts, comment-token scope #61

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/pr-size-checkattr-hardening

📝 Commits (6)

5959e36 perf(pr-size): batch check-attr + harden --source probe + bound git calls
8e9d787 fix(pr-size): scope comment token to issues:write + make minting best-effort
64c4803 fix(pr-size): build the tool with GOWORK=off for go.work-monorepo consumers
6a766ed fix(check-pr-size): harden git-timeout Wait + restore runGit stderr diagnostics
0b2b72d fix(pr-size): note degraded mode on artifact download failure too
a4e7e9e fix(pr-size): quote GOWORK "off" so Actions does not stringify it to false

📊 Changes

4 files changed (+344 additions, -46 deletions)

View changed files

📝 .github/workflows/pr-size.yml (+33 -5)
➕ scripts/check-pr-size/check-pr-size (+0 -0)
📝 scripts/check-pr-size/main.go (+184 -29)
📝 scripts/check-pr-size/main_test.go (+127 -12)

📄 Description

Hardens the reusable PR-size cap so every consumer inherits the improvements from one place. Three tool fixes + one workflow fix, all surfaced while migrating Comfy-Org/cloud onto this reusable (cloud#5335).

Tool: `scripts/check-pr-size/` (Go)

Ports two follow-ups that were open against cloud's now-deleted local copy — they belong here, in the shared tool:

BE-3247 (was cloud#5163) — checkAttrSourceSupported previously treated any probe error as "flag unsupported," so a transient error or unreadable base ref on a modern git would drop every base linguist-generated exclusion and print a misleading "your git is too old" note. It now inspects git stderr and only treats git's own unknown option/unknown switch wording as unsupported (LC_ALL=C forces canonical English). Fail direction stays conservative: any other failure keeps --source trusted and the read fails closed → over-count, never under.

BE-3248 (was cloud#5164) — attrGenerated (one subprocess per file) becomes attrGeneratedBatch: a single git check-attr -z --stdin pass. Every git call now runs under a 60s context.WithTimeout (runGitStdin + runGitCapped), so a wedged git can't stall the job.

The two cloud PRs each rewrote runGit; here they're unified into one runGitStdin (stdin + timeout + bounded stderr capture + C locale), and the deadline extends to the reusable's runGitCapped blob path too. Base-only reading (BE-3220) preserved.

Workflow: `pr-size.yml` comment-token scope

The comment job minted the bot token requesting issues:write and pull-requests:write, but the sticky-comment upsert only calls the issues comments API. On an installation not granted pull-requests:write (cloud's Cloud Code Bot), the mint 422s and the comment job fails — a red check from the best-effort comment path alone. Now:

request only permission-issues: write (exactly what's used);
continue-on-error on the mint so a consumer whose bot lacks the scope degrades to status + step summary, never a red check;
the degraded-mode note fires on a failed mint too, not just absent creds.

Testing

gofmt -l, go vet, go build, go test ./..., actionlint pr-size.yml all clean locally on git 2.50.1 (--source exercised, not skipped). New/updated tests: TestAttrGeneratedBatchMultiplePaths, TestIsUnknownFlagError, TestCheckAttrSourceSupported, TestCapWriter, TestAttrGeneratedSourceIsolatesBase (→ batch API).

After this merges

Re-pin cloud#5335's uses: + workflows_ref to the resulting SHA, then close cloud#5163 / cloud#5164.

🤖 Generated with 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/github-workflows/pull/51 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/21/2026 **Status:** ✅ Merged **Merged:** 7/21/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/pr-size-checkattr-hardening` --- ### 📝 Commits (6) - [`5959e36`](https://github.com/Comfy-Org/github-workflows/commit/5959e36ac03f37038885f0599da15076bf88922f) perf(pr-size): batch check-attr + harden --source probe + bound git calls - [`8e9d787`](https://github.com/Comfy-Org/github-workflows/commit/8e9d787b9326ba8637ddddfa3da2b48fb789ffde) fix(pr-size): scope comment token to issues:write + make minting best-effort - [`64c4803`](https://github.com/Comfy-Org/github-workflows/commit/64c4803691108b9504e39a3cfbb8ffb99aaa8f14) fix(pr-size): build the tool with GOWORK=off for go.work-monorepo consumers - [`6a766ed`](https://github.com/Comfy-Org/github-workflows/commit/6a766edbd143ad4667e69aaca48512836c288bc4) fix(check-pr-size): harden git-timeout Wait + restore runGit stderr diagnostics - [`0b2b72d`](https://github.com/Comfy-Org/github-workflows/commit/0b2b72dc7bb77b202e63d386a1f16bb12b49968f) fix(pr-size): note degraded mode on artifact download failure too - [`a4e7e9e`](https://github.com/Comfy-Org/github-workflows/commit/a4e7e9ea2559fd653562c053b121051b10a51051) fix(pr-size): quote GOWORK "off" so Actions does not stringify it to false ### 📊 Changes **4 files changed** (+344 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/pr-size.yml` (+33 -5) ➕ `scripts/check-pr-size/check-pr-size` (+0 -0) 📝 `scripts/check-pr-size/main.go` (+184 -29) 📝 `scripts/check-pr-size/main_test.go` (+127 -12) </details> ### 📄 Description Hardens the reusable PR-size cap so every consumer inherits the improvements from one place. Three tool fixes + one workflow fix, all surfaced while migrating `Comfy-Org/cloud` onto this reusable (cloud#5335). ## Tool: `scripts/check-pr-size/` (Go) Ports two follow-ups that were open against cloud's now-deleted local copy — they belong here, in the shared tool: **BE-3247** (was cloud#5163) — `checkAttrSourceSupported` previously treated **any** probe error as "flag unsupported," so a transient error or unreadable base ref on a modern git would drop every base `linguist-generated` exclusion and print a misleading "your git is too old" note. It now inspects git stderr and only treats git's own `unknown option`/`unknown switch` wording as unsupported (`LC_ALL=C` forces canonical English). Fail direction stays conservative: any other failure keeps `--source` trusted and the read fails closed → over-count, never under. **BE-3248** (was cloud#5164) — `attrGenerated` (one subprocess per file) becomes `attrGeneratedBatch`: a single `git check-attr -z --stdin` pass. Every git call now runs under a 60s `context.WithTimeout` (`runGitStdin` + `runGitCapped`), so a wedged git can't stall the job. The two cloud PRs each rewrote `runGit`; here they're unified into one `runGitStdin` (stdin + timeout + bounded stderr capture + C locale), and the deadline extends to the reusable's `runGitCapped` blob path too. Base-only reading (BE-3220) preserved. ## Workflow: `pr-size.yml` comment-token scope The comment job minted the bot token requesting `issues:write` **and** `pull-requests:write`, but the sticky-comment upsert only calls the **issues comments API**. On an installation not granted `pull-requests:write` (cloud's Cloud Code Bot), the mint 422s and the comment job fails — a red check from the best-effort comment path alone. Now: - request only `permission-issues: write` (exactly what's used); - `continue-on-error` on the mint so a consumer whose bot lacks the scope degrades to status + step summary, never a red check; - the degraded-mode note fires on a failed mint too, not just absent creds. ## Testing `gofmt -l`, `go vet`, `go build`, `go test ./...`, `actionlint pr-size.yml` all clean locally on **git 2.50.1** (`--source` exercised, not skipped). New/updated tests: `TestAttrGeneratedBatchMultiplePaths`, `TestIsUnknownFlagError`, `TestCheckAttrSourceSupported`, `TestCapWriter`, `TestAttrGeneratedSourceIsolatesBase` (→ batch API). ## After this merges Re-pin cloud#5335's `uses:` + `workflows_ref` to the resulting SHA, then close cloud#5163 / cloud#5164. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:50 +00:00