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 <noreply@anthropic.com>
This commit is contained in:
@@ -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..<branch>
|
||||
git diff main...<branch> --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...<branch>` (full diff) and filter in the prompt, or
|
||||
2. Have Tyre read source files directly from the branch:
|
||||
`git show origin/<branch>:<path>`
|
||||
|
||||
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/<branch>:<path>` 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/<branch>:<path>`
|
||||
- Request structured verdict: APPROVE or REQUEST_CHANGES with file-specific comments
|
||||
|
||||
### 4. Present results
|
||||
|
||||
Format the combined review as a table per reviewer:
|
||||
|
||||
```
|
||||
## Review: <branch> -> 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.
|
||||
Reference in New Issue
Block a user