toolkit-pr-review

name	toolkit-pr-review
description	Review Rust PRs against idiomatic Rust guidelines and ToolKit framework rules, post inline comments on GitHub
user-invocable	true
allowed-tools	Bash, Read, Glob, Grep, Write

Rust PR Review

Review a GitHub pull request for Rust code quality and ToolKit framework compliance. Posts findings as inline review comments directly on the PR.

Usage: /toolkit-pr-review <PR_NUMBER>

---

Inputs

• <PR_NUMBER> — required, the GitHub PR number (e.g. 123)
• --repo <owner/repo> — optional, the GitHub repository (e.g. constructorfabric/gears-rust)

Resolving the target repository

Before fetching PR data, determine which repository to use:

1. If --repo was provided in the arguments, use it.
2. Otherwise, check if an upstream remote exists: git remote get-url upstream 2>/dev/null. If it returns a URL, extract owner/repo from it.
3. Otherwise, fall back to the current repo via gh repo view --json nameWithOwner -q .nameWithOwner.

Store the result as REPO and pass --repo $REPO to all gh pr commands, and use it in API paths as repos/$REPO/pulls/....

Review guidelines

The review has two tiers:

Tier 1 — Architecture (PR-level, Step 0): Apply the # ARCHITECTURE REVIEW section of docs/pr-review/toolkit-rust-review.md once across the whole PR before reading individual files. This catches structural and design-level problems that are invisible inside a single diff hunk.

Tier 2 — Code (per-file, Steps 2–4): Apply the remaining sections of docs/pr-review/toolkit-rust-review.md to every .rs file in the diff.

Apply ToolKit framework compliance (docs/pr-review/toolkit-framework-compliance-review.md) only to .rs files that belong to ToolKit-owned code. A file is ToolKit-owned when any of these signals is present:

1. Cargo.toml signals — the nearest Cargo.toml (same crate or workspace member) declares a toolkit dependency/feature, or the crate name starts with toolkit.
2. Path heuristics — the file lives under a path that matches ToolKit gear conventions (e.g. gears/*/src/, crates/toolkit-*/, or similar namespace).
3. Source-level symbols — the file imports from ToolKit crates (use toolkit_*, use crate:: inside a toolkit crate) or references ToolKit-specific types/traits such as OperationBuilder, SecureConn, SecureORM, ClientHub, or GearLifecycle.

Apply Rust unit test quality review (docs/pr-review/toolkit-tests-quality-review.md) to every changed Rust test you can identify in the diff, including:

• #[test] functions
• async tests such as #[tokio::test]
• test modules such as #[cfg(test)] mod tests
• assertions added or modified inside production files or dedicated test files
• integration tests under tests/
• test-only helper code when it materially affects test validity

If none of the ToolKit signals are detected, skip the framework compliance checklist for that file and apply only the general Rust idioms checklist.

For non-Rust files in the diff (TOML, YAML, migrations, etc.) — apply only general correctness checks, do not force Rust-specific rules.

Coding guidelines reference

When reviewing, also consult:

• guidelines/DNA/languages/RUST.md — project Rust conventions
• guidelines/SECURITY.md — security requirements

---

Steps

Step 1: Fetch PR metadata and diff

gh pr view <PR_NUMBER> --repo $REPO --json number,title,body,headRefOid,baseRefName,headRefName
gh pr diff <PR_NUMBER> --repo $REPO

Save the diff output for analysis. Extract the HEAD commit SHA — you need it for posting comments.

Step 2: PR-level architecture analysis

With the PR title, body, and diff in hand, assess the PR as a whole before doing a full per-file review. Apply every item in the # ARCHITECTURE REVIEW section of docs/pr-review/toolkit-rust-review.md. Some items can be answered from the file list and PR description alone; others require reading relevant code sections — do that now rather than deferring to the per-file pass.

Record architecture findings. Post each as a PR-level issue comment (not an inline review comment) using gh api:

gh api repos/$REPO/issues/<PR_NUMBER>/comments \
  --method POST \
  -f body="**HIGH**\n\n<issue description>\n\n<fix>"

Format: **<SEVERITY>** on the first line, blank line, issue, blank line, fix. The PR-scope note (last bullet in the checklist) goes to the terminal summary only — do not post it as a comment.

Step 3: Identify Rust files in diff

Parse the diff to find all .rs files that were added or modified. For each file, note the changed line ranges (added lines only — you can only comment on lines present in the diff).

Step 4: Read review guidelines and classify files

Read docs/pr-review/toolkit-rust-review.md (always needed).

For each .rs file from Step 3, determine whether it is ToolKit-owned code:

• Check the nearest Cargo.toml for toolkit dependencies/features or a toolkit- crate name.
• Check whether the file path matches ToolKit gear conventions (gears/*/src/, crates/toolkit-*/).
• Scan the file for ToolKit imports (use toolkit_*) or ToolKit types (OperationBuilder, SecureConn, SecureORM, ClientHub, GearLifecycle).

If any file is classified as ToolKit-owned, also read docs/pr-review/toolkit-framework-compliance-review.md.

Step 5: Review each changed file

For each .rs file in the diff:

a. Read the full current file from the repo (not just the diff hunk) to understand context. b. Apply toolkit-rust-review.md checklist items — idiomatic Rust, error handling, async safety, ownership, testing, etc. c. Apply toolkit-tests-quality-review.md checklist items - to every changed Rust test you can identify in the diff d. Only if the file was classified as ToolKit-owned in Step 3, also apply toolkit-framework-compliance-review.md checklist items — SDK pattern, OperationBuilder, SecureConn, gear layout, error types, etc. e. Record each finding with: checklist ID, severity, file path, line number, issue description, fix.

Step 6: Filter and deduplicate

• Drop findings that are not evidenced in the diff
• Drop style issues that rustfmt/clippy should catch
• Drop speculative or hypothetical issues
• Merge overlapping findings on the same line
• Keep only findings where you have concrete evidence

Step 7: Post inline review comments on GitHub

Use gh api to create a pull request review with inline comments.

Build the review payload:

IMPORTANT: The gh api -f array syntax is limited. For multiple comments, build a JSON file and POST it.

The review body MUST be empty string — no summary in the review itself. The summary goes to the terminal only (Step 7).

cat > /tmp/review-payload.json << 'REVIEW_EOF'
{
  "commit_id": "<HEAD_SHA>",
  "event": "COMMENT",
  "body": "",
  "comments": [
    {
      "path": "gears/foo/src/domain/service.rs",
      "line": 42,
      "side": "RIGHT",
      "body": "**HIGH**\n\nError context discarded by `map_err(|_| ...)`.\n\nPreserve the source error — wrap with `.context()` or map to a domain error that keeps the cause."
    }
  ]
}
REVIEW_EOF

gh api repos/$REPO/pulls/<PR_NUMBER>/reviews \
  --method POST \
  --input /tmp/review-payload.json

Step 8: Print summary

After posting, print a compact summary table to the terminal. Architecture findings appear first, followed by code-level findings.

## Rust PR Review: #<PR_NUMBER>

### Architecture
| # | Sev | Issue | Fix |
|---|-----|-------|-----|
| 1 | HIGH | Long-running saga in startup blocks shutdown | Move to background task with cancellation |
| 2 | HIGH | Safety gap documented only in comment | Add startup warning or config guard |

### Code
| # | ID | Sev | Location | Issue | Fix |
|---|----|-----|----------|-------|-----|
| 3 | RUST-ERR-001 | HIGH | service.rs:42 | Error context lost | Preserve source error |
| 4 | TOOLKIT-SEC-001 | CRIT | handler.rs:18 | Raw DB connection | Use SecureConn |

Posted <N> inline comments and <M> PR-level comments on PR #<PR_NUMBER>.

---

Comment formatting rules

Each inline comment MUST follow this format:

**<SEVERITY>**

<One-sentence issue description.>

<One-sentence why it matters.>

<Concrete fix — what to change, not a vague suggestion.>

Where <SEVERITY> is one of: CRITICAL, HIGH, MEDIUM, LOW.

Do NOT include checklist IDs (e.g. RUST-ERR-001, TOOLKIT-SEC-001) in inline comments. IDs appear only in the terminal summary table (Step 7).

Rules:

• Engineering English. No filler, no praise, no hedging.
• No "consider", "you might want to", "it would be nice if". State what is wrong and what to do.
• One issue per comment. If a line has two problems, post two comments.
• Line number must point to an added/modified line that exists in the diff. Do not comment on unchanged lines.
• If you cannot determine the exact line, do not guess — skip that finding.

---

What NOT to do

• Do not approve or request changes — use event: "COMMENT" only
• Do not post comments on lines outside the diff
• Do not post generic praise or "LGTM" if there are no issues
• Do not invent issues without evidence in the code
• Do not complain about formatting that rustfmt handles
• Do not suggest speculative abstractions or premature generalization
• Do not post more than 30 comments per review (prioritize by severity)
• If there are zero findings, post a single review comment: "No issues found."