ci(review): let the reviewer read the files it reviews - #510
Merged
Conversation
When the review job fails, the job log reports only the aggregate result — is_error: true alongside a permission_denials_count — and never names the tool that was denied. That is the one fact needed to fix it, and it lives in the execution output file, which is written to the runner temp dir and discarded with the runner. Diagnosing the current failures ruled out three plausible causes and confirmed none: auth is fine (the run initializes and bills), quota is fine (it spent real budget across 4 turns), and the action version is unchanged (the v1 tag has not moved since before the last green run). The denial itself is all that is left, and it is the part not recorded. Upload it unconditionally. Re-triggering a run is cheap but reproducing one is not: which tool the model reaches for varies per PR, so a failure not captured when it happens may not recur on demand. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG
The upload added in the parent commit produced no artifact on the very
first failing run it was meant to capture. The job log showed the step
running with an empty path:
with:
name: claude-review-execution-log
if-no-files-found: warn
A step that fails never sets its outputs, so
steps.claude-review.outputs.execution_file is empty in exactly the case
this upload exists for — it would only ever have captured the runs that
did not need capturing.
Fall back to the action's documented temp path, which is where it writes
the file regardless of outcome.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG
✅ Performance Benchmark ResultsStatus: No significant regressions detected Benchmark ComparisonThresholds
See benchmarking guide for details. |
Core's allowlist granted seven gh subcommands and no file access. The review could fetch the diff but never open the file around it — and CLAUDE.md asks it to verify paths, function names and call-site counts cited in doc changes, none of which a diff answers. Reaching for an unlisted tool is a denial, and a denial ends the run as a failure, so the omission cost whole reviews rather than degrading them. What points at this rather than at credentials or quota: across the three repos that run this workflow, client and docs recovered on their own while core kept failing, and core is the only one whose allowlist omits Read. client lists it; docs sets no allowlist at all and gets permissive defaults. Read, Grep and Glob are read-only over the checked-out tree, which the job already has via contents: read — and which it can already see in full through gh pr diff. This grants no reach it did not have. Replaces the execution-log capture this branch previously carried. That approach could not work: the action prints "Log saved to $RUNNER_TEMP/claude-execution-output.json" but leaves nothing there, and a search of RUNNER_TEMP on a green run turned up no such file. Shipping a step that always warns "no files found" is worse than shipping nothing, because it reads as captured on the next failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG
adnaan
changed the title
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core's review bot has been failing while the same workflow recovered on its own in the other two repos. This grants it file access.
The evidence
Across the three repos running this workflow, the split is clean:
Readin allowlistCore is the only repo whose allowlist omits
Read, and the only one still failing. Its list granted sevenghsubcommands and no file access at all — the review could fetch the diff but never open the file around it. Since CLAUDE.md asks the bot to verify paths, function names and call-site counts cited in doc changes, and none of that is answerable from a diff, it reaches forRead, gets denied, and a denial ends the run as a failure. The omission cost whole reviews rather than degrading them.What this rules out
Three plausible causes, all eliminated:
"model": "claude-sonnet-5") and bill real budget. The emptyANTHROPIC_API_KEYin the env dump is a red herring — this workflow authenticates viaclaude_code_oauth_token, so that variable is expected to be blank. I initially misread it as the cause.@v1tag, but that tag last moved at2026-07-19T02:57:33Z, hours before the last green run. Same version both succeeded and failed.Scope
Read,GrepandGlobare read-only over the checked-out tree, which the job already has viacontents: readand can already see in full throughgh pr diff. This grants no reach it didn't have.What this branch previously carried
An execution-log capture step, to make the denial self-identifying. Removed, because it could not work: the action prints
Log saved to $RUNNER_TEMP/claude-execution-output.jsonbut leaves nothing there — a search ofRUNNER_TEMPon a green run found no such file, only runner scratch scripts. A step that always warns "no files found" is worse than no step, since it reads as captured on the next failure.The companion PRs in client and docs are being closed for the same reason.
Verification
This PR is its own test: if core's
claude-reviewposts a review here, the diagnosis holds.🤖 Generated with Claude Code
https://claude.ai/code/session_01Ui2cwpeGkrUfRt8rh2FgGG