ci: add Claude Code GitHub Action for on-demand reviews#551
ci: add Claude Code GitHub Action for on-demand reviews#551xdustinface wants to merge 1 commit intov0.42-devfrom
Conversation
Adds a workflow triggered by `@claude` mentions in issue comments, PR review comments, and reviews. Uses OAuth token for Max subscription. Includes Dash Core C++ and DIPs as reference checkouts so Claude can cross-reference protocol implementations during reviews, including P2P protocol compliance. Uses 3 parallel review agents (Rust systems, Dash protocol, code quality) with consolidated findings.
📝 WalkthroughWalkthroughA new GitHub Actions workflow is introduced that automates code review processes. The workflow triggers on issue comments, PR review comments, and issue assignments, executing a Claude Code Action with the Rust toolchain and detailed system prompts configured for multi-agent code review guidelines across Rust systems, protocol, and code quality standards. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/claude.yml:
- Around line 70-104: The system prompt block appended via
--append-system-prompt duplicates rules from AGENTS.md and omits the
non-consensus caveat for the "Dash Protocol Agent"; update the prompt text (the
--append-system-prompt content) to remove the duplicated "Code Style Rules" and
instead include a single sentence referencing AGENTS.md (e.g., "Follow the
authoritative guidelines in AGENTS.md") and add the missing caveat to the Dash
Protocol Agent section (e.g., "Note: These checks are not for consensus-critical
validation; do not rely on exact Dash Core consensus behavior"). Locate the
appended prompt string where "Dash Protocol Agent" and "Code Style Rules" are
defined and replace/adjust those paragraphs accordingly to reference AGENTS.md
and insert the caveat.
- Around line 49-50: The workflow step "Install Rust toolchain" currently pins
dtolnay/rust-toolchain@stable which diverges from rust-toolchain.toml and
doesn't ensure clippy/rustfmt components; update the step so the CI uses the
toolchain defined in rust-toolchain.toml (or auto-detect it) and installs
required components: either replace the action usage with a run step that
invokes rustup (so rustup reads rust-toolchain.toml and installs the exact
toolchain and components) or configure the dtolnay action to read the repo's
rust-toolchain.toml and explicitly install the rustfmt and clippy components
referenced there; locate the "Install Rust toolchain" step in the workflow and
apply one of these changes.
- Around line 57-67: The workflow's Bash permission patterns are too broad:
replace the wildcard entries Bash(cargo *), Bash(git *), and Bash(gh *) with
minimal allowed commands—allow only Bash(cargo check) and Bash(cargo clippy) for
cargo; allow only Bash(git log *), Bash(git show *), and Bash(git diff *) for
git; and remove Bash(gh *) entirely—so update the "permissions" -> "allow" list
to those explicit patterns to enforce least privilege.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab5f7fcb-b3e6-48e0-8e4d-6c1127526772
📒 Files selected for processing (1)
.github/workflows/claude.yml
| - name: Install Rust toolchain | ||
| uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
Toolchain version mismatch with rust-toolchain.toml.
The project pins Rust to version 1.92.0 in rust-toolchain.toml, but this workflow uses @stable. This divergence can cause inconsistent cargo clippy results between local development and Claude's review environment.
Additionally, the project's rust-toolchain.toml specifies rustfmt and clippy components, which aren't explicitly installed here.
Proposed fix: Let dtolnay/rust-toolchain auto-detect from rust-toolchain.toml
- name: Install Rust toolchain
- uses: dtolnay/rust-toolchain@stable
+ uses: dtolnay/rust-toolchain@master
+ with:
+ toolchain: 1.92.0
+ components: rustfmt, clippyAlternatively, if rust-toolchain.toml is present in the repo root, you can omit the toolchain input and use:
- name: Install Rust toolchain
run: rustup showThis will automatically install the toolchain specified in rust-toolchain.toml.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude.yml around lines 49 - 50, The workflow step
"Install Rust toolchain" currently pins dtolnay/rust-toolchain@stable which
diverges from rust-toolchain.toml and doesn't ensure clippy/rustfmt components;
update the step so the CI uses the toolchain defined in rust-toolchain.toml (or
auto-detect it) and installs required components: either replace the action
usage with a run step that invokes rustup (so rustup reads rust-toolchain.toml
and installs the exact toolchain and components) or configure the dtolnay action
to read the repo's rust-toolchain.toml and explicitly install the rustfmt and
clippy components referenced there; locate the "Install Rust toolchain" step in
the workflow and apply one of these changes.
| settings: | | ||
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash(cargo *)", | ||
| "Bash(rustup *)", | ||
| "Bash(git *)", | ||
| "Bash(gh *)" | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
anthropics claude-code-action permissions allow syntax documentation
💡 Result:
In Claude Code, tool permissions are configured in settings.json under:
{
"permissions": {
"allow": ["...rules..."],
"ask": ["...rules..."],
"deny": ["...rules..."]
}
}allow rule syntax is a list of strings in the form Tool or Tool(specifier); rules are evaluated deny → ask → allow, and the first match wins. Wildcards (*, and for paths also ** in gitignore-style patterns) are supported depending on the tool. Examples include Bash(npm run *), Read(./.env), and WebFetch(domain:example.com). [1]
The Claude Code Action lets you pass the same permissions either via the action’s settings input (inline JSON or a settings file path). Example inline settings include "permissions": { "allow": ["Bash","Read"], "deny": ["WebFetch"] }. [2] The action docs also show allowing specific bash commands/tools via claude_args (e.g., --allowedTools "Bash(npm install),Bash(npm run test),Edit,Replace"). [3]
Sources: Claude Code “Configure permissions” docs (rule syntax) [1], Claude Code settings docs (permissions in settings.json) [2], anthropics/claude-code-action configuration docs (using settings / --allowedTools) [3].
🏁 Script executed:
# Check the workflow file to understand its purpose and context
cat -n .github/workflows/claude.ymlRepository: dashpay/rust-dashcore
Length of output: 7112
Overly permissive Bash command patterns for git and gh violate the principle of least privilege.
The patterns Bash(git *) and Bash(gh *) allow all subcommands, including destructive operations like git push, git reset --hard, and gh pr merge. Based on the system prompt, Claude only needs to run cargo clippy and cargo check for analysis, and read git history via git log/show/diff. It has no reason to execute gh commands or modify the repository.
Consider restricting to explicitly needed operations:
Bash(cargo check)andBash(cargo clippy)instead ofBash(cargo *)Bash(git log *),Bash(git show *),Bash(git diff *)instead ofBash(git *)- Remove
Bash(gh *)entirely
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude.yml around lines 57 - 67, The workflow's Bash
permission patterns are too broad: replace the wildcard entries Bash(cargo *),
Bash(git *), and Bash(gh *) with minimal allowed commands—allow only Bash(cargo
check) and Bash(cargo clippy) for cargo; allow only Bash(git log *), Bash(git
show *), and Bash(git diff *) for git; and remove Bash(gh *) entirely—so update
the "permissions" -> "allow" list to those explicit patterns to enforce least
privilege.
| --append-system-prompt "## Code Review Guidelines | ||
|
|
||
| You are reviewing a Dash cryptocurrency protocol library written in Rust. This is security-sensitive code. | ||
|
|
||
| ### Review Process | ||
| When reviewing a PR, launch 3 independent subagents in parallel, each with a different perspective: | ||
| 1. Rust Systems Agent: focuses on correctness, error handling, type safety, memory safety, and compilation (runs cargo clippy/check). When the PR touches FFI code, also reviews C ABI safety, null pointer checks, and destroy/free pairing. | ||
| 2. Dash Protocol Agent: focuses on protocol compliance by cross-referencing changes against Dash Core C++ (.reference/dash/src/) and DIPs (.reference/dips/). Checks P2P message formats, serialization, consensus rules, and DIP adherence. When the PR does not touch protocol code, this agent should still verify the changes do not accidentally break protocol assumptions. | ||
| 3. Code Quality Agent: focuses on test coverage, code style rules (see below), visibility modifiers, import organization, duplication, and scope creep. | ||
| After all three agents complete, consolidate their findings into a single review. Deduplicate overlapping concerns, prioritize by severity, and post as one cohesive review. | ||
|
|
||
| ### Review Focus | ||
| - Correctness: verify logic, edge cases, and error handling | ||
| - Safety: check for memory safety issues especially in FFI boundaries (dash-spv-ffi, key-wallet-ffi) | ||
| - Security: flag any potential for private key leakage, command injection, or unsafe deserialization | ||
| - Code quality: verify proper error types (thiserror), no hardcoded values, correct visibility modifiers | ||
| - Tests: check that new code has adequate test coverage and that edge cases are tested | ||
| - Run cargo clippy and cargo check when reviewing Rust changes to catch compilation issues | ||
|
|
||
| ### Dash Protocol Reference | ||
| When the PR touches protocol-level code (transactions, blocks, masternodes, quorums, ChainLocks, InstantSend, special transactions, X11, P2P network messages, peer management, or any network protocol handling), cross-reference against: | ||
| - Dash Core C++ implementation at .reference/dash/src/ (especially net.cpp, net_processing.cpp, protocol.h for P2P) | ||
| - Dash Improvement Proposals at .reference/dips/ | ||
| Verify that the Rust implementation correctly follows the Dash Core P2P protocol: message formats, serialization, handshake sequences, version negotiation, inventory handling, and service flags must match the C++ reference. Flag any deviations from the protocol or the relevant DIPs. | ||
|
|
||
| ### Code Style Rules (enforce these strictly) | ||
| - Comments must document what code does, never what it fixed or replaced. No references to previous implementations or solved problems. | ||
| - Avoid numeric type suffixes (e.g., 1u32, 0usize) when the type is clear from context. | ||
| - Use the most restrictive visibility possible. Default to private (pub(crate), pub(super), or no modifier). Never use pub if pub(crate) suffices. | ||
| - All imports must be at the top of the file/module. Flag any inline fully-qualified paths (e.g., crate::foo::Bar::method()) when a top-level use import would work. | ||
| - Only add comments when they provide meaningful context that is not obvious from the code itself. Do not comment self-explanatory code or simple one-liners. | ||
| - No numbered comments or references to line numbers in comments. | ||
| - Reuse existing data structures and code. Flag duplication that could use existing types or helpers. | ||
| - Proper error handling is required. No temporary fixes, no swallowing errors, no unwrap() in non-test code without justification. | ||
| - Changes should be minimal and focused. Flag any unnecessary refactoring, feature additions, or scope creep beyond what the PR description states. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
System prompt duplicates AGENTS.md content without referencing the source file.
The coding style rules in this prompt largely duplicate content from AGENTS.md (lines 16-20). This creates a maintenance burden—updates to AGENTS.md won't automatically reflect here.
Additionally, AGENTS.md (lines 33-36) contains a critical caveat: "Not for consensus-critical validation; do not rely on exact Dash Core consensus behavior." This caveat is missing from the Dash Protocol Agent instructions, which could lead to over-reliance on Claude's protocol compliance checks.
Proposed fix: Reference AGENTS.md and add caveat
Add to the system prompt, near line 72:
+ IMPORTANT: Read AGENTS.md in the repository root for canonical coding guidelines and project conventions.
+
+ ### Critical Caveat
+ This library is NOT for consensus-critical validation. Do not rely on exact Dash Core consensus behavior. When cross-referencing against Dash Core C++, use it as a guide for protocol message formats and P2P behavior, not as a source of truth for consensus rules.
+
### Review ProcessThis ensures:
- Claude reads the authoritative guidelines from
AGENTS.md - The non-consensus-critical nature is clearly established
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude.yml around lines 70 - 104, The system prompt block
appended via --append-system-prompt duplicates rules from AGENTS.md and omits
the non-consensus caveat for the "Dash Protocol Agent"; update the prompt text
(the --append-system-prompt content) to remove the duplicated "Code Style Rules"
and instead include a single sentence referencing AGENTS.md (e.g., "Follow the
authoritative guidelines in AGENTS.md") and add the missing caveat to the Dash
Protocol Agent section (e.g., "Note: These checks are not for consensus-critical
validation; do not rely on exact Dash Core consensus behavior"). Locate the
appended prompt string where "Dash Protocol Agent" and "Code Style Rules" are
defined and replace/adjust those paragraphs accordingly to reference AGENTS.md
and insert the caveat.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #551 +/- ##
=============================================
- Coverage 65.97% 65.95% -0.02%
=============================================
Files 311 311
Lines 64586 64586
=============================================
- Hits 42611 42599 -12
- Misses 21975 21987 +12
|
1 participant
Adds a workflow triggered by
@claudementions in issue comments, PR review comments, and reviews. Uses OAuth token for Max subscription.Includes Dash Core C++ and DIPs as reference checkouts so Claude can cross-reference protocol implementations during reviews, including P2P protocol compliance. Uses 3 parallel review agents (Rust systems, Dash protocol, code quality) with consolidated findings.
Summary by CodeRabbit
New Features
Chores