From dead4361a5dbeb3e2b8da1cd690c4de90beba8a5 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 11 Feb 2026 17:37:24 +0100 Subject: [PATCH] feat(skills): add review-pr skill for dual-agent PR review Spawns Hoshe (code quality) and Tyre (architecture) in parallel to review branch diffs against main. Includes Gitea tea CLI integration, vendor/generated file exclusion patterns, and practical tips learned from first review run on server and client boilerplate PRs. Co-Authored-By: Claude Opus 4.6 --- .claude/skills/review-pr/SKILL.md | 153 ++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 .claude/skills/review-pr/SKILL.md diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 000000000..a8032e5e3 --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,153 @@ +--- +name: review-pr +description: > + Review a branch diff with dual agents before merge. Use when the user says + "review-pr", "review this PR", "review this branch", or invokes /review-pr. + Spawns Hoshe (code quality) and Tyre (architecture) in parallel to review + the diff against main. Reports approve/reject with inline comments. +user-invocable: true +allowed-tools: Bash, Read, Grep, Glob, Task +--- + +# PR Review Skill + +Dual-agent review of a branch diff against main. Hoshe reviews code quality, +Tyre reviews architecture. Both must approve for a clean review. + +## Workflow + +### 1. Determine the branch + +If the user provided a branch name as argument, use it. Otherwise use the +current branch (`git branch --show-current`). If on `main`, ask the user +which branch to review. + +To list open PRs on Gitea: +```bash +tea pr list --login schweitz --repo jpmschweitzer/settled-reach --state open --output simple +``` + +Fetch remote branches first: +```bash +git fetch --all +``` + +### 2. Generate the diff + +```bash +git log --oneline main.. +git diff main... --stat +``` + +If the diff is empty, report "No changes to review" and stop. + +**Exclude generated/vendor files from the review diff.** Common exclusions: +- `Cargo.lock` (auto-generated) +- `client/addons/gdUnit4/` (vendor test framework) +- `*.uid` (Godot-generated) +- `db/commonwealth.db` (binary) + +Three-dot diff with pathspec exclusions is unreliable. Instead, either: +1. Use `git diff main...` (full diff) and filter in the prompt, or +2. Have Tyre read source files directly from the branch: + `git show origin/:` + +For large diffs (>1000 lines of source), provide **source files** rather than +raw diff to reviewers — cleaner context, better reviews. Read files with +`git show origin/:` and include them in the prompt. + +### 3. Spawn both reviewers in parallel + +Use the Task tool to spawn **two agents simultaneously** in a single message. +Use `model: sonnet` for both — sufficient for review, saves cost. + +**Agent 1 — Hoshe (Code Quality)** +- `subagent_type`: `hoshe` +- `model`: `sonnet` +- Prompt: Include source code and commit log. Ask Hoshe to review for: + - Correctness and bug risks + - Error handling gaps + - Test coverage (are new features tested?) + - Code style and clarity + - Security concerns (OWASP top 10, injection risks) + - Performance issues +- Request structured verdict: APPROVE or REQUEST_CHANGES with file-specific comments + +**Agent 2 — Tyre (Architecture)** +- `subagent_type`: `tyre` +- `model`: `sonnet` +- Prompt: Include source code and commit log. Tell Tyre to read the relevant + `decisions/*.md` files first, then review for: + - Architectural consistency with project decisions + - API/interface design quality + - Dependency and coupling concerns + - Scalability implications + - Whether the change respects non-negotiable baselines (D-010, D-012) +- Tyre can read files directly from the branch using `git show origin/:` +- Request structured verdict: APPROVE or REQUEST_CHANGES with file-specific comments + +### 4. Present results + +Format the combined review as a table per reviewer: + +``` +## Review: -> main + +### Hoshe (Code Quality): [APPROVE | REQUEST_CHANGES] +[Summary] +| # | File | Severity | Issue | +|---|------|----------|-------| +| 1 | path:line | critical/warning/suggestion | description | + +### Tyre (Architecture): [APPROVE | REQUEST_CHANGES] +[Summary] +| # | File | Severity | Issue | +|---|------|----------|-------| +| 1 | path:line | critical/warning/suggestion | description | + +### Verdict: [APPROVED | CHANGES REQUESTED] +``` + +The overall verdict is APPROVED only if **both** reviewers approve. + +## Prompt template for reviewers + +Use this structure when constructing the agent prompts (adapt as needed): + +``` +Review the following branch diff for merge into main. + +Branch: {branch} +Commits: +{commit_log} + +Diff stats: +{diff_stat} + +[Source files or diff here — exclude vendor/generated code] + +Review focus: {focus_area} + +Respond with: +1. Verdict: APPROVE or REQUEST_CHANGES +2. Summary: 2-3 sentence overall assessment +3. Comments: List of specific issues, each with: + - File path and approximate location + - Severity: critical / warning / suggestion + - Description of the issue +If no issues found, say APPROVE with a brief positive summary. +``` + +## Tips from practice + +- **Vendor code**: Explicitly note vendor code in the prompt so reviewers focus + on project code. Mention it as a separate architectural concern (should it be + gitignored? submoduled?). +- **Large PRs**: For PRs touching many files, provide file-by-file source code + rather than a single massive diff. Reviewers give better feedback. +- **Multiple PRs**: When reviewing several PRs, spawn all reviewers in one + parallel batch (4 agents for 2 PRs). This is faster than sequential. +- **Tyre reads decisions**: Always tell Tyre to read the relevant `decisions/*.md` + files — this grounds the review in project-specific architectural choices. +- **Binary/DB files**: Exclude binary files from the diff. Note them in the + prompt as "also changed" if relevant.