[PR #112] [MERGED] fix(pr-risk): grant the token permissions the grade job actually needs #121

New issue

Closed

opened 2026-10-26 22:36:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:36:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/112
Author: @mattmillerai
Created: 8/3/2026
Status: ✅ Merged
Merged: 8/3/2026
Merged by: @mattmillerai

Base: main ← Head: matt/pr-risk-token-permissions

📝 Commits (2)

4914cd6 fix(pr-risk): grant the token permissions the grade job actually needs
2bb77fa fix(pr-risk): document the full caller grant + surface why a token failure failed

📊 Changes

6 files changed (+164 additions, -23 deletions)

View changed files

📝 .github/workflows/pr-risk.yml (+36 -7)
📝 README.md (+1 -1)
📝 scripts/pr-risk/apply-risk-label.sh (+31 -11)
📝 scripts/pr-risk/grade-pr-risk.sh (+37 -4)
📝 scripts/pr-risk/tests/test_apply_risk_label.sh (+29 -0)
📝 scripts/pr-risk/tests/test_grade_pr_risk.sh (+30 -0)

📄 Description

ELI5

The risk grader shipped asking for a key ring that opens the wrong doors: it could write on issues but not on pull requests (which is where its one label actually goes), and it couldn't look at the workflow-runs shelf its own self-exclusion trick reads from. First real run: it couldn't read the PR, graded "ungraded", then couldn't even apply the "ungraded" sticker. This swaps in the two keys it actually needs — same lock-out from everything else.

Motivation

The first live consumer enrollment (comfy-cloud-mcp-server#1093) failed on both halves of the grade job in one run, which means every consumer would fail identically and the shadow pilot cannot start until this lands. The hermetic suites stub gh, so token-permission reality was never exercisable in CI — a live consumer run was always going to be the first test of the grants, and it was.

Provenance

Authored by: interactive session
From: first live run of the pilot enrollment comfy-cloud-mcp-server#1093 (job 91778171341)
Verified: shellcheck -x on all four scripts: clean; tests/test_grade_pr_risk.sh: 46 passed, 0 failed; tests/test_apply_risk_label.sh: 19 passed, 0 failed. Root causes read from the failing job log: "PR unreadable via the API after 4 attempt(s)" with Checks/PullRequests/Statuses: read granted (the GraphQL query's checkSuite -> workflowRun hop is an Actions resource), then gh: Resource not accessible by integration (HTTP 403) on POST /repos/.../issues/1093/labels.
Deviations: none — the token-permission path stays uncovered by the hermetic suites (they stub gh by design); verification of the fix itself is the pilot PR's re-run, tracked in the test plan

Reviewer context

Type: bug fix — two missing token grants in the reusable workflow's grade job, plus the matching caller-pattern doc and script comments
Slots into: the pr-risk shadow pilot from #111; unblocks first consumer comfy-cloud-mcp-server#1093
Review needed by: the pilot enrollment PR is red on this and its shadow-period clock starts when it can grade

Summary

actions: read added: the rollup GraphQL walks CheckRun -> checkSuite -> workflowRun (self-exclusion keys on workflowRun.databaseId), workflow runs are an Actions resource, and GraphQL fails the WHOLE query on one unreadable field — so the miss surfaced as "PR unreadable", not as a partial record
pull-requests: read -> write: the labels endpoint is dual-mapped — labeling an issue rides issues: write, labeling a PR rides pull-requests: write (same reason actions/labeler requires it); issues: write stays for the repo-side risk:* label creation on first use
The no-cascade property is untouched: the label still rides the plain GITHUB_TOKEN, which cannot fire labeled triggers regardless of which permission bit authorized the write
Caller pattern in the workflow header updated to match; consumers must mirror the new grants (caller permissions intersect the called job's)

Test plan

shellcheck + both hermetic suites green in CI
After merge: bump comfy-cloud-mcp-server#1093's caller to the new SHA with the widened grants and confirm the grade job goes green and applies exactly one risk:* label (expected: risk:R3 — it touches .github/workflows/)

_{🔄 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/112 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/3/2026 **Status:** ✅ Merged **Merged:** 8/3/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/pr-risk-token-permissions` --- ### 📝 Commits (2) - [`4914cd6`](https://github.com/Comfy-Org/github-workflows/commit/4914cd68d0617b4622a028fba27c839f05348c80) fix(pr-risk): grant the token permissions the grade job actually needs - [`2bb77fa`](https://github.com/Comfy-Org/github-workflows/commit/2bb77fa20684c3678c266f4ebc1a7dbcb1cebc92) fix(pr-risk): document the full caller grant + surface why a token failure failed ### 📊 Changes **6 files changed** (+164 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/pr-risk.yml` (+36 -7) 📝 `README.md` (+1 -1) 📝 `scripts/pr-risk/apply-risk-label.sh` (+31 -11) 📝 `scripts/pr-risk/grade-pr-risk.sh` (+37 -4) 📝 `scripts/pr-risk/tests/test_apply_risk_label.sh` (+29 -0) 📝 `scripts/pr-risk/tests/test_grade_pr_risk.sh` (+30 -0) </details> ### 📄 Description ## ELI5 The risk grader shipped asking for a key ring that opens the wrong doors: it could write on issues but not on pull requests (which is where its one label actually goes), and it couldn't look at the workflow-runs shelf its own self-exclusion trick reads from. First real run: it couldn't read the PR, graded "ungraded", then couldn't even apply the "ungraded" sticker. This swaps in the two keys it actually needs — same lock-out from everything else. ## Motivation The first live consumer enrollment (comfy-cloud-mcp-server#1093) failed on both halves of the grade job in one run, which means every consumer would fail identically and the shadow pilot cannot start until this lands. The hermetic suites stub `gh`, so token-permission reality was never exercisable in CI — a live consumer run was always going to be the first test of the grants, and it was. ## Provenance - **Authored by:** interactive session - **From:** first live run of the pilot enrollment comfy-cloud-mcp-server#1093 (job 91778171341) - **Verified:** `shellcheck -x` on all four scripts: clean; `tests/test_grade_pr_risk.sh`: 46 passed, 0 failed; `tests/test_apply_risk_label.sh`: 19 passed, 0 failed. Root causes read from the failing job log: "PR unreadable via the API after 4 attempt(s)" with `Checks/PullRequests/Statuses: read` granted (the GraphQL query's `checkSuite -> workflowRun` hop is an Actions resource), then `gh: Resource not accessible by integration (HTTP 403)` on `POST /repos/.../issues/1093/labels`. - **Deviations:** none — the token-permission path stays uncovered by the hermetic suites (they stub `gh` by design); verification of the fix itself is the pilot PR's re-run, tracked in the test plan ## Reviewer context - **Type:** bug fix — two missing token grants in the reusable workflow's grade job, plus the matching caller-pattern doc and script comments - **Slots into:** the pr-risk shadow pilot from #111; unblocks first consumer comfy-cloud-mcp-server#1093 - **Review needed by:** the pilot enrollment PR is red on this and its shadow-period clock starts when it can grade ## Summary - `actions: read` added: the rollup GraphQL walks `CheckRun -> checkSuite -> workflowRun` (self-exclusion keys on `workflowRun.databaseId`), workflow runs are an Actions resource, and GraphQL fails the WHOLE query on one unreadable field — so the miss surfaced as "PR unreadable", not as a partial record - `pull-requests: read` -> `write`: the labels endpoint is dual-mapped — labeling an issue rides `issues: write`, labeling a PR rides `pull-requests: write` (same reason `actions/labeler` requires it); `issues: write` stays for the repo-side `risk:*` label creation on first use - The no-cascade property is untouched: the label still rides the plain `GITHUB_TOKEN`, which cannot fire `labeled` triggers regardless of which permission bit authorized the write - Caller pattern in the workflow header updated to match; consumers must mirror the new grants (caller permissions intersect the called job's) ## Test plan - [ ] shellcheck + both hermetic suites green in CI - [ ] After merge: bump comfy-cloud-mcp-server#1093's caller to the new SHA with the widened grants and confirm the grade job goes green and applies exactly one `risk:*` label (expected: `risk:R3` — it touches `.github/workflows/`) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:36:28 +00:00