Skip to content

Restore pull-requests: write for claude-review - #91

Merged
d-morrison merged 1 commit into
mainfrom
claude/claude-review-write-permission
Aug 1, 2026
Merged

Restore pull-requests: write for claude-review#91
d-morrison merged 1 commit into
mainfrom
claude/claude-review-write-permission

Conversation

@d-morrison

Copy link
Copy Markdown
Member

Summary

  • Revert claude-review to the pull_request trigger #89 fixed the claude-review job's OIDC token-exchange failure by reverting the trigger from pull_request_target back to pull_request, and in the same change narrowed permissions.pull-requests from write to read.
  • That side effect silently breaks reviews on same-repo PRs: the job now runs to completion successfully (confirmed on PR #87, run 30681158599) but every attempt to actually post the review is denied — the job log shows "permission_denials_count": 8 and No buffered inline comments — so no review ever appears on the PR, even though the check shows green.
  • Restoring pull-requests: write is safe: this workflow uses the pull_request trigger (not pull_request_target), and GitHub always forces a read-only GITHUB_TOKEN for PRs from forks regardless of what permissions: declares. So fork PRs stay read-only either way — this change only restores write access for same-repo PRs, which is the common case in this repo and where reviews were silently being dropped.

Test plan


Generated by Claude Code

pull-requests was narrowed to read in #89 while fixing the OIDC
failure, but that also silently disables posting reviews on same-repo
PRs: the job runs and succeeds, but every write attempt is denied
(confirmed via permission_denials_count in the job log) and no review
ever appears. write is safe to restore here: this workflow uses the
pull_request trigger (not pull_request_target), and GitHub always
forces a read-only GITHUB_TOKEN for fork PRs regardless of what
permissions: declares, so fork PRs stay read-only either way.
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1-2-g6ad689f
Preview removed because the pull request was closed.
2026-08-01 04:04 UTC

@d-morrison

Copy link
Copy Markdown
Member Author

Confirming this, with evidence that strengthens the case: the bug is older than #89, so this is not a regression I introduced --- it is a defect I restored while claiming it was safe.

claude has never posted a review comment on this repository:

PR Date pull-requests claude comments
#78 2026-07-02 read 0
#79 2026-07-02 read 0
#80 2026-07-02 read 0
#86 2026-07-31 read 0

All four ran green. git log -L on the permissions line shows pull-requests: read was there from the workflow's first commit (0413ece), so every review this repo has ever run has been silently dropped at the posting step --- the failure mode you measured as permission_denials_count: 8.

My reasoning in #89 was wrong and is worth correcting explicitly. I wrote that restoring read was safe because "the action posts with its own app token, so the workflow token does not need write." The app token covers the action's own bookkeeping; the review itself is posted by Claude's tool calls, which run under GITHUB_TOKEN. I asserted that without testing it, and I also described the pre-#83 config as "the last version that produced successful reviews" --- it produced green checks and no reviews, which I would have caught by looking for a single claude comment on any earlier PR.

Worth noting for the record: Copilot reviewed #89 twice and repeated my premise back approvingly. Two clean reviews on a false claim, because both of us reasoned about which token ought to post rather than checking whether a review had ever appeared.

Your fork caveat is right --- GitHub forces a read-only GITHUB_TOKEN on fork PRs whatever permissions: declares, so this only affects same-repo PRs, and #90 stays open for the fork case.

One suggestion: this PR is the first chance to observe a genuine review on this repo. Its own claude-review run will still be under the old read permission (the head's workflow is what runs, and the fix is in that head --- so it should actually post). If a claude comment appears on this PR, that is the first one ever here, and worth saying so in the PR body.

Copy link
Copy Markdown
Member Author

One correction to the last point: this PR's own claude-review run did not get a genuine review, and the green check is misleading. The job log (run 30682869108) shows it was skipped outright by a GitHub-side guard:

##[warning]Skipping action due to workflow validation: Workflow validation failed.
The workflow file must exist and have identical content to the version on the
repository's default branch. ... your workflow will begin working once you
merge your PR.
Exiting due to workflow validation skip

Since this PR's diff is claude-code-review.yml itself, GitHub blocks claude-code-action from running with the modified permissions until the workflow file matches main — a sensible anti-privilege-escalation guard (a PR can't grant itself elevated permissions and exploit them in the same run). So the check is green because the job exited early, not because it reviewed anything and found nothing to say.

Practical effect: we won't get the first genuine confirmation from this PR. It'll come from the next PR opened/updated after this one merges to main.


Generated by Claude Code

@d-morrison

Copy link
Copy Markdown
Member Author

Withdrawing the suggestion at the end of my last comment. This PR cannot be the demonstration --- I should have predicted that from the mechanism I had just finished writing up.

Because this PR edits the review workflow, its head no longer matches main, so its own run skips at workflow validation and exits 0 before reaching any posting step. Run 30682869108 confirms it:

Skipping action due to workflow validation: Workflow validation failed...
Exiting due to workflow validation skip

So the 0 claude comments on this PR carry no information about the permission fix, in either direction. Any workflow-editing PR is structurally incapable of testing its own change --- the same reason #89 could not verify itself.

The real test is the first same-repo PR pushed after this merges, with the workflow untouched: #76, #86, or #87. A claude review comment appearing on one of those is the confirmation, and it would be this repository's first.

Worth stating plainly, since the two failure modes now stack: a green claude-review on this repo currently means one of three things --- a real review posted, a review silently denied at posting (read permission, what this PR fixes), or a skip at workflow validation (any workflow-editing PR). Only reading the log distinguishes them, and only one of the three is a review.

@d-morrison
d-morrison merged commit ffe8ac0 into main Aug 1, 2026
5 checks passed
@d-morrison
d-morrison deleted the claude/claude-review-write-permission branch August 1, 2026 04:03
d-morrison pushed a commit to imelainew/ucd-serg.github.io that referenced this pull request Aug 1, 2026
pull-requests: write (UCD-SERG#91) fixed the GITHUB_TOKEN's API scope, but
that's not what was blocking posting: permission_denials_count stayed
high (and grew) because Claude Code's own tool-permission gate, which
is separate from the token's API scope, was denying every attempt to
call `gh pr comment` and mcp__github_inline_comment__create_inline_comment
- the two mechanisms the code-review plugin actually uses to post a
review. Neither is in Claude Code's default allowed-tools set for a
non-interactive run, so every attempt was silently denied with nobody
present to approve the prompt.
d-morrison added a commit to imelainew/ucd-serg.github.io that referenced this pull request Aug 1, 2026
The code-review plugin gates all posting on a --comment argument:
"If --comment argument was NOT provided, stop here. Do not post any
GitHub comments." (step 7 of plugins/code-review/commands/code-review.md
in anthropics/claude-code.)

We have never passed it, so every review this repo has run has been
working as designed -- reviewing, printing to the job log, and posting
nothing behind a green check. claude has zero review comments across
UCD-SERG#78, UCD-SERG#79, UCD-SERG#80, UCD-SERG#86, UCD-SERG#87, UCD-SERG#92, UCD-SERG#93 and UCD-SERG#94.

This was not a permissions problem. UCD-SERG#91's pull-requests: write and
UCD-SERG#92/UCD-SERG#94's allowlist work were treating a symptom.

Closes UCD-SERG#96

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMmB6db4xpbuqcqaQKArZb
d-morrison added a commit that referenced this pull request Aug 24, 2026
- Stop dispatching a review for a fork PR. The reusable workflow's dispatch
  guard blocks forks, so dispatching posted ":mag: dispatched a review" and
  delivered nothing, with no red check to contradict it. Post an explicit
  fork notice linking #90 instead.
- Add `pull-requests: read` to dispatch-on-comment. It calls
  `gh api repos/../pulls/N`, and declaring any scope zeroes the unlisted ones.
- Document the three skip gates the reusable workflow adds (draft, fork,
  bot-sender) and the #84 red-to-gray interaction.
- Correct three comments that asserted capabilities this repo does not have
  until #100 lands: claude.yml cannot dispatch a review, and `@claude review`
  does not produce one.
- Restore the deleted rationale for `pull-requests: write` (#91) and for
  `--ref` on the dispatch.
- State the inherited upstream defaults (report-cost, use-ai-config,
  lab-manual, require-review) instead of accepting them silently.
- Document the exact accepted `/review` comment form.
- Narrow the gha#437 and `secrets: inherit` claims to what each source says.

Refs #101
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