Skip to content

Grant PR board-sync callers GITHUB_TOKEN write - #1132

Merged
jhelferty-nv merged 2 commits into
mainfrom
chore/pr-board-sync-github-token-permissions
Sep 3, 2026
Merged

Grant PR board-sync callers GITHUB_TOKEN write#1132
jhelferty-nv merged 2 commits into
mainfrom
chore/pr-board-sync-github-token-permissions

Conversation

@jhelferty-nv

Copy link
Copy Markdown
Contributor

Summary

  • Grant issues: write and pull-requests: write on the slangpy jobs that call shader-slang/slang/.github/workflows/pr-board-sync.yml.
  • That lets the reusable workflow post repo-scoped writes (assignees, review requests, assignment comments) as github-actions[bot] via GITHUB_TOKEN instead of SLANG_PR_BOT_TOKEN.
  • Leave pr-review-fork-bridge.yml at permissions: {}; it only relays fork-PR reviews and does not write to the repo or board.

Org-team reads and ProjectsV2 writes still use the PAT. This is safe to merge before shader-slang/slang#12888: current slang master still sets permissions: {} on the reusable workflow and authenticates every call with the PAT, so these grants are inert until that PR lands.

Test plan

  • Confirm Actionlint / workflow YAML checks pass
  • After slang#12888 is on master, run a slangpy PR through board-sync and confirm assignment comments / assignee changes appear as github-actions[bot], not the PAT owner
  • Confirm fork-PR reviews still flow bridge → apply and that the bridge job does not gain write permissions

Repo-scoped writes in shader-slang/slang's reusable workflow now use
github-actions[bot] via GITHUB_TOKEN, so slangpy callers must pass those
permissions through rather than inheriting an empty grant.
@jhelferty-nv
jhelferty-nv requested a review from a team as a code owner September 2, 2026 20:41
@jhelferty-nv
jhelferty-nv requested review from bmillsNV and removed request for a team September 2, 2026 20:41
@jhelferty-nv jhelferty-nv self-assigned this Sep 2, 2026
@jhelferty-nv
jhelferty-nv removed the request for review from bmillsNV September 2, 2026 20:42
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: e2ff1d00-1119-4c8b-944e-3f65380c97ed

📥 Commits

Reviewing files that changed from the base of the PR and between 47f06a1 and 695dd6d.

📒 Files selected for processing (5)
  • .github/workflows/pr-checks-complete.yml
  • .github/workflows/pr-commit-status.yml
  • .github/workflows/pr-maintenance.yml
  • .github/workflows/pr-review-fork-apply.yml
  • .github/workflows/pr-sweep-nightly.yml

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The PR adds job-level issues: write and pull-requests: write permissions to five board-sync workflows. Existing PAT-based configuration remains in place for organization-team and ProjectsV2 operations.

Changes

Board-sync repository permissions

Layer / File(s) Summary
Enable repository-scoped writes
.github/workflows/pr-checks-complete.yml, .github/workflows/pr-commit-status.yml, .github/workflows/pr-maintenance.yml, .github/workflows/pr-review-fork-apply.yml, .github/workflows/pr-sweep-nightly.yml
Each board-sync job now declares issues: write and pull-requests: write permissions. PAT-based configuration remains for operations that use it.

Merge Risk: ⚪ Minimal · up to ab142

This change grants limited repository write permissions to board-sync jobs while leaving the fork-review bridge without write access; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: granting GITHUB_TOKEN write permissions to PR board-sync callers.
Description check ✅ Passed The description accurately explains the permission changes, affected workflows, authentication behavior, exclusions, and test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (5 skipped: 5 unsupported.)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jhelferty-nv
jhelferty-nv enabled auto-merge (squash) September 3, 2026 16:12
@jhelferty-nv
jhelferty-nv merged commit 8ae4d67 into main Sep 3, 2026
16 checks passed
@jhelferty-nv
jhelferty-nv deleted the chore/pr-board-sync-github-token-permissions branch September 3, 2026 16:28
pdeayton-nv pushed a commit to pdeayton-nv/slang that referenced this pull request Sep 3, 2026
## Motivation

PR board sync used `SLANG_PR_BOT_TOKEN` for repo writes: assignment
comments, assignees, and review requests. That token is a fine-grained
PAT owned by a human. GitHub auto-subscribes whoever comments on a
thread, so the PAT owner became a watcher of every PR the workflow
greeted. There is no comment-API flag to skip that subscription.

## Proposed solution

Keep the PAT for what `GITHUB_TOKEN` cannot do: org team reads (Source
classification) and org ProjectsV2 writes (the shared board). Route
every repo-scoped write through the run's `GITHUB_TOKEN` so the visible
actor is `github-actions[bot]`:

- PR and linked-issue assignees
- Community-author co-assignment
- Reviewer requests and ignored-reviewer removals
- The assignment comment

Callers grant `issues: write` and `pull-requests: write` on the calling
job. The reusable workflow declares no `permissions:` block of its own:
requesting more than the caller granted would fail the run at startup
for every un-updated cross-repo caller.

Assignment and reviewer writes fall back to the PAT on a 403 so existing
callers keep working during rollout. Comments never fall back: posting
as the PAT would subscribe its owner again.

Companion caller update:
[shader-slang/slangpy#1132](shader-slang/slangpy#1132).

## Change summary

| Area | Change |
| --- | --- |
| `pr-board-sync.yml` | `actionsWrite` posts repo writes with
`GITHUB_TOKEN`; PAT fallback for assignees/reviewers only |
| Caller workflows in this repo | Job-level `issues: write` +
`pull-requests: write` |
| `pr-board-sync-templates/` | Same grants and docs for cross-repo
copy-me callers |
| `pr-write-identity.test.js` | Covers `GITHUB_TOKEN` routing, PAT
fallback, and no-fallback comments |
| `pr-board-sync.md` | Documents the identity split and caller
permission contract |

## Concepts and vocabulary

- **`GITHUB_TOKEN`**: per-run Actions token; comments and assigns as
`github-actions[bot]`.
- **`SLANG_PR_BOT_TOKEN`**: org-level fine-grained PAT used for org
teams and ProjectsV2, and as a rollout fallback for assignment/reviewer
writes.
- **Caller ceiling**: a reusable workflow's `GITHUB_TOKEN` cannot have
more permission than the calling job granted.

## Process report

Consider a Community PR with no owner yet. Board sync needs to assign
someone, optionally request reviewers, comment that it did so, classify
Source from org team membership, and set Status on the shared ProjectsV2
board.

The last two of those are org-scoped. `GITHUB_TOKEN` has no Members or
org Projects access, so they stay on the PAT. Switching them would break
Source classification and the board.

The first three are repo-scoped REST writes. They used the same PAT
Octokit as the org calls, which is why the comment subscribed a human.
`actions/github-script` is still bound to the PAT for those org calls,
so repo writes go through a raw `fetch` with `github.token` instead of
that Octokit.

The reusable workflow does not declare write permissions itself. GitHub
would reject un-updated callers at startup (`requesting pull-requests:
write, but is only allowed none`). Inheritance plus PAT fallback for
assignees/reviewers keeps board sync alive; comments are skipped until
the caller grants `issues: write`.

That input shape is intentional: a caller that has not granted write yet
is a valid rollout state, not a malformed request.

## Test plan

- [ ] `node .github/scripts/pr-write-identity.test.js` (and the existing
assign/signal/classify tests)
- [ ] On merge, open or wait for an unassigned Community PR and confirm
the assignment comment is from `github-actions[bot]`, not the PAT owner
- [ ] Confirm the PAT owner is not newly subscribed to that PR
- [ ] Confirm assignee and requested reviewer still land
- [ ] Confirm the ProjectsV2 card still gets Source/Status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants