[PR #68] [MERGED] fix(pr-size): mint bot token with pull-requests:write and degrade on runtime 403 (BE-4385) #77

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-4385-pr-size-pr-write

📝 Commits (2)

2cd82ff fix(pr-size): mint bot token with pull-requests:write and degrade on runtime 403 (BE-4385)
7454a68 fix(pr-size): soften post-failed diagnosis to point at upsert log (BE-4385)

📊 Changes

1 file changed (+24 additions, -7 deletions)

View changed files

📝 .github/workflows/pr-size.yml (+24 -7)

📄 Description

ELI-5

When a pull request is too big, a bot leaves a comment saying so. To leave that comment the bot needs a key with the right permission. We were minting the key with the "Issues" permission, but a comment on a pull request actually needs the "Pull requests" permission — GitHub treats PR comments through the same endpoint as issue comments, and for GitHub Apps that endpoint checks the Pull requests scope. So the comment 403'd on every PR. This swaps the requested scope to pull-requests: write, and also makes the comment step fail-soft: if posting still can't happen (e.g. the app isn't granted the permission yet on that install), the check stays green and the full size report still shows up in the job's step summary instead of turning the check red.

What changed

.github/workflows/pr-size.yml, the comment job:

Mint bot token — request permission-pull-requests: write instead of permission-issues: write. The issue-comments API on a pull request checks the app's Pull requests permission for GitHub Apps, so that is the one scope required (read is implied for the list-comments GET). issues:write was both wrong (didn't grant PR-comment access) and, if kept alongside, an ungranted extra that 422s the mint — this workflow only ever comments on pull requests.
Corrected the scope comment above that input to explain the issues-comments-on-a-PR / Pull-requests-permission relationship.
Hardened "Upsert sticky size comment" with continue-on-error: true and id: upsert. Design intent already stated in the file is that the comment path "must degrade to status + summary, never get a red check"; today only the mint step was guarded, so a runtime 403 (proven live on Jul-24 after an Issues grant) escaped it and reddened the check. "Note degraded mode" now also fires on steps.upsert.outcome == 'failure' with a message pointing at the likely missing Pull requests permission and the step-summary report.

Why it's safe / notable

Ships safely before the org-admin grants Pull requests to the Cloud Code Bot app: an ungranted permission makes the mint 422, which is already tolerated (continue-on-error on the mint step) and degrades to a green check with no comment — i.e. the pre-2026-07-24 behavior that unblocks merges. Once the grant lands, comments post correctly.
Riskiest line is steps.upsert.outcome == 'failure' in the degraded-mode gate. This relies on GitHub Actions semantics: outcome reflects the step result before continue-on-error is applied (which only rewrites conclusion to success), so a tolerated 403 still reads as outcome == 'failure'. This mirrors the existing steps.bot.outcome == 'success' guard on the already-tolerated mint step. When the upsert step is skipped (bot mint didn't succeed), its outcome is skipped, not failure, so that path correctly falls through to the existing creds-absent/mint-failed message.
No new uses: were added, so all actions stay SHA-pinned (zizmor clean). actionlint passes (it runs shellcheck over the embedded run scripts).
The header "Caller pattern" comment does not mention token scopes, so no change was needed there. test-pr-size.yml and scripts/check-pr-size/ contain no assertions on the mint permissions or the comment-job step layout (they cover the Go counting logic only), so nothing to update. groom.yml's permission-issues uses are unrelated — groom files GitHub issues, not PR comments — and were left untouched.

Verification

actionlint .github/workflows/pr-size.yml → clean.
Post-merge: the bump-pr-size-callers.yml path filter includes .github/workflows/pr-size.yml, so the merge should fire the caller-bump run and open the SHA-bump PR in the caller repo (the delivery vehicle, tracked by the sibling cloud ticket).

This is not a capability-denial change (it adds tolerance and degrades gracefully), so the negative-claim falsification discipline does not apply.

_{🔄 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/68 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/24/2026 **Status:** ✅ Merged **Merged:** 7/26/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-4385-pr-size-pr-write` --- ### 📝 Commits (2) - [`2cd82ff`](https://github.com/Comfy-Org/github-workflows/commit/2cd82ffb72a6161c7d7184848adceff258a867ab) fix(pr-size): mint bot token with pull-requests:write and degrade on runtime 403 (BE-4385) - [`7454a68`](https://github.com/Comfy-Org/github-workflows/commit/7454a688b8e08e67eeb6adb8b232be526ad53c21) fix(pr-size): soften post-failed diagnosis to point at upsert log (BE-4385) ### 📊 Changes **1 file changed** (+24 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/pr-size.yml` (+24 -7) </details> ### 📄 Description ## ELI-5 When a pull request is too big, a bot leaves a comment saying so. To leave that comment the bot needs a key with the right permission. We were minting the key with the "Issues" permission, but a comment on a *pull request* actually needs the "Pull requests" permission — GitHub treats PR comments through the same endpoint as issue comments, and for GitHub Apps that endpoint checks the Pull requests scope. So the comment 403'd on every PR. This swaps the requested scope to `pull-requests: write`, and also makes the comment step fail-soft: if posting still can't happen (e.g. the app isn't granted the permission yet on that install), the check stays green and the full size report still shows up in the job's step summary instead of turning the check red. ## What changed `.github/workflows/pr-size.yml`, the `comment` job: 1. **Mint bot token** — request `permission-pull-requests: write` instead of `permission-issues: write`. The issue-comments API *on a pull request* checks the app's Pull requests permission for GitHub Apps, so that is the one scope required (read is implied for the list-comments GET). `issues:write` was both wrong (didn't grant PR-comment access) and, if kept alongside, an ungranted extra that 422s the mint — this workflow only ever comments on pull requests. 2. **Corrected the scope comment** above that input to explain the issues-comments-on-a-PR / Pull-requests-permission relationship. 3. **Hardened "Upsert sticky size comment"** with `continue-on-error: true` and `id: upsert`. Design intent already stated in the file is that the comment path "must degrade to status + summary, never get a red check"; today only the *mint* step was guarded, so a runtime 403 (proven live on Jul-24 after an Issues grant) escaped it and reddened the check. "Note degraded mode" now also fires on `steps.upsert.outcome == 'failure'` with a message pointing at the likely missing Pull requests permission and the step-summary report. ## Why it's safe / notable - Ships safely **before** the org-admin grants Pull requests to the Cloud Code Bot app: an ungranted permission makes the mint 422, which is already tolerated (`continue-on-error` on the mint step) and degrades to a green check with no comment — i.e. the pre-2026-07-24 behavior that unblocks merges. Once the grant lands, comments post correctly. - Riskiest line is `steps.upsert.outcome == 'failure'` in the degraded-mode gate. This relies on GitHub Actions semantics: `outcome` reflects the step result *before* `continue-on-error` is applied (which only rewrites `conclusion` to `success`), so a tolerated 403 still reads as `outcome == 'failure'`. This mirrors the existing `steps.bot.outcome == 'success'` guard on the already-tolerated mint step. When the upsert step is *skipped* (bot mint didn't succeed), its outcome is `skipped`, not `failure`, so that path correctly falls through to the existing creds-absent/mint-failed message. - No new `uses:` were added, so all actions stay SHA-pinned (zizmor clean). `actionlint` passes (it runs shellcheck over the embedded run scripts). - The header "Caller pattern" comment does not mention token scopes, so no change was needed there. `test-pr-size.yml` and `scripts/check-pr-size/` contain no assertions on the mint permissions or the comment-job step layout (they cover the Go counting logic only), so nothing to update. `groom.yml`'s `permission-issues` uses are unrelated — groom files GitHub *issues*, not PR comments — and were left untouched. ## Verification - `actionlint .github/workflows/pr-size.yml` → clean. - Post-merge: the `bump-pr-size-callers.yml` path filter includes `.github/workflows/pr-size.yml`, so the merge should fire the caller-bump run and open the SHA-bump PR in the caller repo (the delivery vehicle, tracked by the sibling cloud ticket). This is not a capability-denial change (it *adds* tolerance and degrades gracefully), so the negative-claim falsification discipline does not apply. --- <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:59 +00:00