Skills review-rust
Comprehensive Rust code review with optional parallel agents
git clone https://github.com/openclaw/skills
T=$(mktemp -d) && git clone --depth=1 https://github.com/openclaw/skills "$T" && mkdir -p ~/.claude/skills && cp -r "$T/skills/anderskev/review-rust" ~/.claude/skills/openclaw-skills-review-rust && rm -rf "$T"
T=$(mktemp -d) && git clone --depth=1 https://github.com/openclaw/skills "$T" && mkdir -p ~/.openclaw/skills && cp -r "$T/skills/anderskev/review-rust" ~/.openclaw/skills/openclaw-skills-review-rust && rm -rf "$T"
skills/anderskev/review-rust/SKILL.mdRust Code Review
Arguments
: Spawn specialized subagents per technology area--parallel- Path: Target directory (default: current working directory)
Step 1: Identify Changed Files
git diff --name-only $(git merge-base HEAD main)..HEAD | grep -E '\.rs$'
Step 2: Check Rust Edition and MSRV
# Check Cargo.toml for edition and rust-version grep -E 'edition|rust-version' Cargo.toml # Check workspace members if workspace grep -A 20 '\[workspace\]' Cargo.toml
Edition 2024 awareness (requires MSRV 1.85+):
If
edition = "2024" is detected, the following behavioral changes apply throughout the review:
is deny by default — unsafe operations insideunsafe_op_in_unsafe_fn
MUST use explicitunsafe fn
blocksunsafe {}
blocks must beextern "C" {}unsafe extern "C" {}
and#[no_mangle]
must be#[export_name]
and#[unsafe(no_mangle)]#[unsafe(export_name)]
captures ALL in-scope lifetimes by default (RPIT lifetime capture change); use-> impl Trait
for precise capture+ use<'a>
is a reserved keyword — code using it as an identifier must usegenr#gen
(never type) falls back to!
instead of!
— may change behavior of inferred types()- Temporaries in
conditions and tail expressions are dropped earlier than in edition 2021if let
now implementsBox<[T]>IntoIterator
Record the detected edition — it affects severity calibration in Steps 3, 8, and the verification protocol.
Step 3: Verify Linter Status
CRITICAL: Run clippy and check BEFORE flagging style or correctness issues. Do NOT flag issues that clippy or the compiler already catches.
cargo clippy --all-targets --all-features -- -D warnings 2>&1 | head -50 cargo clippy -- -D clippy::perf 2>&1 | head -20 cargo check --all-targets 2>&1 | head -50
Edition 2024 note: Edition 2024 promotes several previously-warn lints to deny (notably
unsafe_op_in_unsafe_fn). If clippy or cargo check already reports edition-related errors, do not duplicate those as review findings — instead note that the author must fix compiler errors first.
Step 4: Detect Technologies
# Detect tokio async runtime grep -r "tokio" --include="Cargo.toml" -l | head -3 # Detect axum web framework grep -r "axum" --include="Cargo.toml" -l | head -3 # Detect sqlx database grep -r "sqlx" --include="Cargo.toml" -l | head -3 # Detect serde serialization grep -r "serde" --include="Cargo.toml" -l | head -3 # Detect thiserror / anyhow grep -r "thiserror\|anyhow" --include="Cargo.toml" -l | head -3 # Detect tracing grep -r "tracing" --include="Cargo.toml" -l | head -3 # Check for test files in diff git diff --name-only $(git merge-base HEAD main)..HEAD | grep -E '((^|/)(test|tests)/.*\.rs$)|(_test\.rs$)' # Check for unsafe code in diff git diff $(git merge-base HEAD main)..HEAD -- '*.rs' | grep -c 'unsafe' # Detect async fn in traits (no async-trait crate needed since Rust 1.75) grep -r "async-trait" --include="Cargo.toml" -l | head -3 # Detect LazyLock/LazyCell usage (replaces once_cell/lazy_static since 1.80) grep -r "once_cell\|lazy_static" --include="Cargo.toml" -l | head -3 # Detect #[expect] lint attribute usage (stable since 1.81) git diff $(git merge-base HEAD main)..HEAD -- '*.rs' | grep -c '#\[expect(' # Detect macro definitions in diff git diff $(git merge-base HEAD main)..HEAD -- '*.rs' | grep -cE 'macro_rules!|#\[proc_macro|#\[derive\(' # Detect FFI code in diff git diff $(git merge-base HEAD main)..HEAD -- '*.rs' | grep -cE 'extern "C"|#\[no_mangle\]|#\[repr\(C\)\]|bindgen|#\[unsafe\(no_mangle\)\]'
Modern Rust detection notes:
- If
is a dependency but the project uses edition 2024 or MSRV >= 1.75, flag as Informational — nativeasync-trait
in traits is available andasync fn
can likely be removed.async-trait - If
oronce_cell
is a dependency but MSRV >= 1.80, flag as Informational —lazy_static
andstd::sync::LazyLock
are stable replacements.std::cell::LazyCell - If
is used where#[allow(...)]
would be better (MSRV >= 1.81), note as Minor —#[expect(...)]
warns when the suppressed lint no longer fires, keeping suppressions clean.#[expect]
Step 5: Load Verification Protocol
Load
beagle-rust:review-verification-protocol skill and keep its checklist in mind throughout the review.
Step 6: Load Skills
Use the
Skill tool to load each applicable skill (e.g., Skill(skill: "beagle-rust:rust-code-review")).
Always load:
beagle-rust:rust-code-review
Conditionally load based on detection:
| Condition | Skill |
|---|---|
| Tokio detected | |
| Axum detected | |
| sqlx detected | |
| Serde detected | |
| Test files changed | |
| Macro definitions in diff | |
| FFI code detected (extern, repr(C), bindgen) | |
Step 7: Review
Sequential (default):
- Load applicable skills
- Review core Rust quality (ownership, error handling, unsafe, traits)
- Review detected technology areas
- Consolidate findings
Parallel (--parallel flag):
- Detect all technologies upfront
- Spawn one subagent per technology area with
toolTask - Each agent loads its skill and reviews its domain
- Wait for all agents
- Consolidate findings
Step 8: Verify Findings
Before reporting any issue:
- Re-read the actual code (not just diff context)
- For "unused" claims - did you search all references across the workspace?
- For "missing" claims - did you check trait definitions, derive macros, and
gated code?#[cfg] - For "unnecessary clone" - did you verify the borrow checker allows a reference?
- For "unsafe" issues - did you check the safety comments and surrounding invariants?
- Remove any findings that are style preferences, not actual issues
Edition 2024 verification rules: 7. Do NOT flag
unsafe {} blocks inside unsafe fn as unnecessary — they are REQUIRED in edition 2024
8. Do NOT flag unsafe extern "C" as unusual syntax — it is REQUIRED in edition 2024
9. Do NOT flag #[unsafe(no_mangle)] or #[unsafe(export_name)] as unusual — they are REQUIRED in edition 2024
10. For -> impl Trait returns, verify whether implicit lifetime capture is intentional — in edition 2024 all in-scope lifetimes are captured by default; suggest + use<'a> only when narrower capture is needed
11. For code using Box<[T]> in iterator contexts, remember IntoIterator is now available in edition 2024 — do not flag .iter() on boxed slices as the only approach
12. If temporaries in if let or tail expressions cause borrow issues, consider whether edition 2024's earlier drop semantics are the root cause
Step 9: Review Convergence
Single-Pass Completeness
You MUST report ALL issues across ALL categories (ownership, error handling, async, types, tests, security, performance) in a single review pass. Do not hold back issues for later rounds.
Before submitting findings, ask yourself:
- "If all my recommended fixes are applied, will I find NEW issues in the fixed code?"
- "Am I requesting new code (tests, types, modules) that will itself need review?"
If yes to either: include those anticipated downstream issues NOW, in this review, so the author can address everything at once.
Scope Rules
- Review ONLY the code in the diff and directly related existing code
- Do NOT request new features, test infrastructure, or architectural changes that didn't exist before the diff
- If test coverage is missing, flag it as ONE Minor issue ("Missing test coverage for X, Y, Z") — do NOT specify implementation details
- Doc comments, naming issues are Minor unless they affect public API contracts
- Do NOT request adding new dependencies (e.g., proptest, mockall, criterion)
Fix Complexity Budget
Fixes to existing code should be flagged at their real severity regardless of size.
However, requests for net-new code that didn't exist before the diff must be classified as Informational:
- Adding a new dependency
- Creating entirely new modules, files, or test suites
- Extracting new traits or abstractions
- Adding benchmark suites
These are improvement suggestions for the author to consider in future work, not review blockers.
Iteration Policy
If this is a re-review after fixes were applied:
- ONLY verify that previously flagged issues were addressed correctly
- Do NOT introduce new findings unrelated to the previous review's issues
- Accept Minor/Nice-to-Have issues that weren't fixed — do not re-flag them
- The goal of re-review is VERIFICATION, not discovery
Output Format
## Review Summary [1-2 sentence overview of findings] ## Issues ### Critical (Blocking) 1. [FILE:LINE] ISSUE_TITLE - Issue: Description of what's wrong - Why: Why this matters (unsound unsafe, data race, panic, security) - Fix: Specific recommended fix ### Major (Should Fix) 2. [FILE:LINE] ISSUE_TITLE - Issue: ... - Why: ... - Fix: ... ### Minor (Nice to Have) N. [FILE:LINE] ISSUE_TITLE - Issue: ... - Why: ... - Fix: ... ### Informational (For Awareness) N. [FILE:LINE] SUGGESTION_TITLE - Suggestion: ... - Rationale: ... ## Good Patterns - [FILE:LINE] Pattern description (preserve this) ## Verdict Ready: Yes | No | With fixes 1-N (Critical/Major only; Minor items are acceptable) Rationale: [1-2 sentences]
Rules
- Load skills BEFORE reviewing (not after)
- Number every issue sequentially (1, 2, 3...)
- Include FILE:LINE for each issue
- Separate Issue/Why/Fix clearly
- Categorize by actual severity
- Run clippy before flagging style issues
- Run verification after fixes
- Report ALL issues in a single pass — do not hold back findings for later iterations
- Re-reviews verify previous fixes ONLY — no new discovery
- Requests for net-new code (new modules, dependencies, test suites) are Informational, not blocking
- The Verdict ignores Minor and Informational items — only Critical and Major block approval
Post-Fix Verification
After fixes are applied, run:
cargo check --all-targets cargo clippy --all-targets --all-features -- -D warnings cargo test --all-targets
All checks must pass before approval.