Skip to content

Claude fix #1259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbryngelson merged 3 commits into from
Feb 24, 2026
Merged

Claude fix #1259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d22f0f3
Delete .claude/settings.json
sbryngelson Feb 24, 2026
9f3f0a1
Refactor Claude Code Review workflow configuration
sbryngelson Feb 24, 2026
5bd3c35
Enhance Claude Code Review workflow configuration
sbryngelson Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions .claude/settings.json
View file
Open in desktop

This file was deleted.

66 changes: 36 additions & 30 deletions .github/workflows/claude-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,68 +10,74 @@ jobs:
permissions:
contents: read
pull-requests: write
issues: read
issues: write
actions: read
id-token: write
Comment on lines 10 to 15
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

issues: write expands the workflow token scope compared to the previous issues: read. If this job only needs to post PR comments, keep this permission read-only (or omit it) and remove gh issue comment:* from the allowed tools; otherwise, please justify why issue write access is required in a pull_request_target workflow that runs with secrets on forked PRs.

Copilot uses AI. Check for mistakes.

steps:
- name: Checkout PR head (fork)
# IMPORTANT: checkout BASE repo only (safe on forks)
- name: Checkout base repo (safe)
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 1

- name: Run Claude Code Review
id: claude-review
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ github.token }}

plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
plugins: 'code-review@claude-code-plugins'
plugin_marketplaces: "https://github.com/anthropics/claude-code.git"
plugins: "code-review@claude-code-plugins"

claude_args: >
--dangerouslyDisableSandbox
--dangerously-skip-permissions
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The --dangerously-skip-permissions flag is risky in a pull_request_target workflow because it can undermine the whole point of restricting capabilities via --allowedTools. Consider removing this flag and relying on a minimal allowedTools set plus least-privilege GitHub token permissions to reduce prompt-injection blast radius.

Suggested change
--dangerously-skip-permissions

Copilot uses AI. Check for mistakes.
--max-turns 10
--allowedTools
"Bash(gh pr view:*)"
"Bash(gh pr diff:*)"
"Bash(gh pr comment:*)"
"Bash(gh api repos/*/pulls/*:*)"
"Bash(gh api repos/*/issues/*/comments:*)"
"Bash(gh pr list:*)"
"Bash(gh pr status:*)"
"Bash(gh issue comment:*)"
"Bash(gh api:*)"
"Bash(cat:*)"
"Bash(ls:*)"
"Bash(grep:*)"
"Bash(find:*)"
"Bash(sed:*)"
"Bash(awk:*)"
"Bash(head:*)"
"Bash(tail:*)"
"Bash(wc:*)"
"Bash(sort:*)"
"Bash(uniq:*)"
"Bash(cut:*)"
"Bash(xargs:*)"
"Bash(jq:*)"
"Bash(python3:*)"
Comment on lines 36 to +58
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The current --allowedTools list is very broad (gh api:* plus python3:* and many shell text utilities). In a pull_request_target job with secrets, a prompt-injected PR can use python3 (or other tooling) to read environment variables and leak tokens via PR comments/logs. Tighten this to only the exact gh commands/endpoints and parsing tools required (prefer jq over python3 here), and avoid wildcard gh api:* if possible.

Copilot uses AI. Check for mistakes.

prompt: |
/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}
You are running in pull_request_target. DO NOT execute or inspect the fork's checked-out code.
Review ONLY via GitHub API/gh commands.

Always use the numeric PR form with --repo, e.g.:
- gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} ...
- gh pr diff ${{ github.event.pull_request.number }} --repo ${{ github.repository }} ...
- gh pr comment ${{ github.event.pull_request.number }} --repo ${{ github.repository }} ...
Always use numeric PR form with --repo:
- gh pr view ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --json files,title,body
- gh pr diff ${{ github.event.pull_request.number }} --repo ${{ github.repository }}

If CLAUDE.md exists in the base repo checkout, read it with:
- cat CLAUDE.md
Prefer jq/python3 for JSON parsing instead of shell loops.

Output requirements (even if no issues):
Copy link
Contributor

@codeant-ai codeant-ai bot Feb 24, 2026

Choose a reason for hiding this comment

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

Suggestion: Update the workflow prompt so that the Claude reviewer explicitly reads and follows MFC's contribution and GPU documentation, and focuses on correctness and the documented PR-pattern triggers rather than style, as required by the project's AI review rules. [custom_rule]

Severity Level: Minor ⚠️

Suggested change
Output requirements (even if no issues):
Before reviewing code, fetch and skim these MFC-specific docs from the base repository:
- docs/documentation/contributing.md (especially "Common Pitfalls", "What to Review", and PR-pattern triggers)
- docs/documentation/gpuParallelization.md (GPU macro API and GPU rules)
Prioritize correctness, numerical and physics issues, MPI/GPU correctness, precision mixing, and memory issues over formatting or lint-only concerns, which are enforced by pre-commit/CI.
Why it matters? ⭐

The project's AI review rules (provided in the prompt for this task) require reviewers to prioritize correctness, numerical/physics/MPI/GPU issues, and to consult the repo's contribution and GPU documentation (docs/documentation/contributing.md and docs/documentation/gpuParallelization.md). The proposed improved prompt explicitly instructs the automated reviewer to read those MFC docs and to emphasize correctness-related checks over style (which is already enforced by pre-commit). This directly enforces a documented custom rule and is therefore a substantive, beneficial change rather than a cosmetic tweak. The improved YAML is syntactically valid and clearly narrows the review focus to the repository's stated priorities.

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** .github/workflows/claude-code-review.yml
**Line:** 72:72
**Comment:**
	*Custom Rule: Update the workflow prompt so that the Claude reviewer explicitly reads and follows MFC's contribution and GPU documentation, and focuses on correctness and the documented PR-pattern triggers rather than style, as required by the project's AI review rules.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

- Start with: files changed count + list up to 15 changed file paths
- Then: a short summary of what the PR changes (3–6 bullets)
- Then: findings:
- If issues: list them with file path + line numbers when possible
- If no issues: list at least 3 concrete "improvement opportunities" with file paths
- Files changed count + list up to 15 file paths
- Summary (3–6 bullets)
- Findings:
- If issues: include file + line numbers when possible
- If no issues: at least 3 concrete improvement opportunities with file paths

Post the results as ONE top-level PR comment titled "Claude Code Review".
If you cannot access the diff/files, say exactly what is blocked.
Post ONE top-level PR comment titled "Claude Code Review".
If posting a PR comment is blocked, write the full review to the GitHub Actions job summary instead.

IMPORTANT:
- Do NOT stop just because a previous "Claude Code Review" comment exists.
- If a prior Claude review exists, post a NEW comment titled "Claude Code Review (updated)" that includes:
- current head SHA: ${{ github.event.pull_request.head.sha }}
- files changed count + up to 15 file paths
- 3–6 bullet summary of changes
- findings or at least 3 improvement opportunities with file paths
Post as ONE top-level PR comment.

additional_permissions: |
actions: read
Loading