Skip to content

ci: fix /claude-review workflow — pre-approve tools to resolve 22 permission denials#1987

Merged
chongchonghe merged 4 commits into
developmentfrom
chong/github/claude-review
Jun 23, 2026
Merged

ci: fix /claude-review workflow — pre-approve tools to resolve 22 permission denials#1987
chongchonghe merged 4 commits into
developmentfrom
chong/github/claude-review

Conversation

@chongchonghe

@chongchonghe chongchonghe commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes the /claude-review workflow that was failing with 22 permission denials on every trigger.

When a PR comment contains /claude-review, the claude-code-review.yml workflow runs Claude Code in headless CI mode. Without pre-approved tools in .claude/settings.json, Claude's every tool call hits a permission denial. With the default --max-turns 30, all 30 turns are consumed by denials before Claude can do any work, resulting in error_max_turns.

Root cause: .claude/ is listed in .gitignore, so no settings.json was ever committed. Claude Code in CI had no pre-approved tools.

Fix:

  1. Update .gitignore to exclude only the files within .claude/ (keeping settings.json trackable via !/.claude/settings.json exception).
  2. Add .claude/settings.json with read-only tool permissions pre-approved: Read, git diff/log/show/blame/ls-files, gh pr view/diff, gh issue view, find, grep, ls, wc.
  3. Enable show_full_output: true in the workflow so future failures are visible in CI logs rather than hidden.

Validation: After this fix, a workflow_dispatch test on PR #1987 completed in 10 turns with 3 remaining denials (vs. 22 denials + max-turns failure before). The 3 remaining denials will be identifiable via show_full_output: true after merging and retesting with an issue_comment trigger.

Note on testing: The workflow_dispatch path cannot post a sticky comment on a specific PR (the action doesn't expose the pr_number input to its comment-posting logic for workflow_dispatch events). The primary trigger — issue_comment containing /claude-review — will work correctly after this is merged to development, since the action restores .claude from the base branch.

Related issues

N/A

Checklist

Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an x inside the square brackets [ ] in the Markdown source below:

  • I have added a description (see above).
  • I have added a link to any related issues (if applicable; see above).
  • I have read the Contributing Guide.
  • I have added tests for any new physics that this PR adds to the code.
  • (For quokka-astro org members) I have manually triggered the GPU tests with the magic comment /azp run.

@chongchonghe

Copy link
Copy Markdown
Contributor Author

/claude-review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a dummy comment in the CMakeLists.txt file for the Advection problem to test the /claude-review workflow. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

…iew workflow

The claude-code-review.yml workflow was hitting 22 permission denials per run
because Claude Code had no pre-approved tools in headless CI. Pre-approve
read-only tools (Read, git, gh, find, grep, ls) to allow the review to proceed.

Also enable show_full_output to make future failures easier to diagnose, and
update .gitignore to track .claude/settings.json while continuing to ignore
all other files in .claude/.
…s.txt change

show_full_output: true makes Claude's tool calls visible in CI logs, which is
needed to identify the remaining 3 permission denials. Removes the throwaway
comment that was only used to trigger the test PR.
@chongchonghe chongchonghe changed the title Test: /claude-review workflow trigger ci: fix /claude-review workflow — pre-approve tools to resolve 22 permission denials Jun 23, 2026
@chongchonghe chongchonghe marked this pull request as ready for review June 23, 2026 01:38
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. CI github_actions Pull requests that update GitHub Actions code labels Jun 23, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d48e0aee5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/claude-code-review.yml
Comment thread .claude/settings.json
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 23, 2026
@chongchonghe chongchonghe enabled auto-merge June 23, 2026 02:18
@chongchonghe chongchonghe added this pull request to the merge queue Jun 23, 2026
Merged via the queue into development with commit 5655af2 Jun 23, 2026
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI github_actions Pull requests that update GitHub Actions code lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants