Add pr-review and pr-build skills
- pr-review: read-only code review of PR branches, outputs findings report - pr-build: builds, tests, and fixes PR branches, commits fixes
This commit is contained in:
@@ -0,0 +1,107 @@
|
||||
---
|
||||
name: pr-build
|
||||
description: Builds and tests a pull request branch, fixing compilation errors and test failures. Use when the user says "build PR", "fix PR build", "run PR build", or wants to compile and test a PR branch.
|
||||
---
|
||||
|
||||
# PR Build Skill
|
||||
|
||||
Builds, tests, and fixes a pull request branch. This skill CAN edit files, commit, and push.
|
||||
|
||||
## Workflow
|
||||
|
||||
### Step 1: Identify the PR branch
|
||||
|
||||
If the user specifies a PR number, check it out:
|
||||
```bash
|
||||
tea pr checkout <number>
|
||||
```
|
||||
|
||||
If already on a PR branch, verify with:
|
||||
```bash
|
||||
git branch --show-current
|
||||
git log main..HEAD --oneline
|
||||
```
|
||||
|
||||
### Step 2: Clean build
|
||||
|
||||
```bash
|
||||
rm -rf build
|
||||
cmake -B build -S . 2>&1
|
||||
cmake --build build -j$(nproc) 2>&1
|
||||
```
|
||||
|
||||
Capture both stdout and stderr.
|
||||
|
||||
### Step 3: Handle build failures
|
||||
|
||||
If the build fails, read the error output carefully. Common issues:
|
||||
|
||||
**Missing include / undefined reference:**
|
||||
- Check if the new `.c` file is in the right `file(GLOB ...)` directory
|
||||
- Check if the new `.h` file is included properly
|
||||
- Check if CMakeLists.txt needs updating (new target, new source file, new dependency)
|
||||
|
||||
**Type errors / implicit declarations:**
|
||||
- Check function signatures match between `.h` and `.c`
|
||||
- Check struct field names and types
|
||||
|
||||
**Linker errors:**
|
||||
- Check if all required libraries are linked in CMakeLists.txt
|
||||
- Check if all source files are included in the target
|
||||
|
||||
Use the cmake-expert agent to diagnose and fix CMake issues.
|
||||
|
||||
### Step 4: Run unit tests
|
||||
|
||||
If build succeeds:
|
||||
```bash
|
||||
./build/tests
|
||||
```
|
||||
|
||||
### Step 5: Handle test failures
|
||||
|
||||
If tests fail:
|
||||
- Read the test output carefully
|
||||
- Check which test function failed and the assertion line
|
||||
- Read the test source file and the module being tested
|
||||
- Use the test-writer agent to investigate and fix
|
||||
|
||||
### Step 6: Run integration tests (optional)
|
||||
|
||||
```bash
|
||||
python3 test.py
|
||||
```
|
||||
|
||||
This runs the integration + benchmark suite. It takes longer — only run if the user asks or if unit tests pass.
|
||||
|
||||
### Step 7: Fix and commit
|
||||
|
||||
If fixes were needed:
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "Fix build: <brief description of what was fixed>"
|
||||
git push
|
||||
```
|
||||
|
||||
### Step 8: Report results
|
||||
|
||||
Print a summary:
|
||||
|
||||
```
|
||||
=== PR BUILD SUMMARY ===
|
||||
Branch: <branch-name>
|
||||
Build: [PASS/FAIL]
|
||||
Unit tests: [PASS/FAIL] (<passed>/<total>)
|
||||
Integration tests: [PASS/FAIL/SKIPPED]
|
||||
|
||||
Fixes applied: <count>
|
||||
<list of fixes with commit hashes>
|
||||
```
|
||||
|
||||
## Rules
|
||||
- DO edit source files and CMakeLists.txt to fix issues
|
||||
- DO commit and push fixes
|
||||
- Always build from clean state (rm -rf build)
|
||||
- Read error messages carefully before fixing
|
||||
- Don't change functionality — only fix build/test issues
|
||||
- Preserve existing code style when making fixes
|
||||
@@ -0,0 +1,119 @@
|
||||
---
|
||||
name: pr-review
|
||||
description: Reviews a pull request for bugs, memory safety, thread safety, and style issues. Use when the user says "review PR", "review this PR", "review pull request", or wants a code review of changes.
|
||||
---
|
||||
|
||||
# PR Review Skill
|
||||
|
||||
Read-only code review of a pull request branch. Produces a report — does NOT edit files.
|
||||
|
||||
## Workflow
|
||||
|
||||
### Step 1: Identify the PR branch
|
||||
|
||||
If the user specifies a PR number, check it out:
|
||||
```bash
|
||||
tea pr checkout <number>
|
||||
```
|
||||
|
||||
If already on a PR branch, verify with:
|
||||
```bash
|
||||
git branch --show-current
|
||||
git log main..HEAD --oneline
|
||||
```
|
||||
|
||||
### Step 2: Get changed files
|
||||
|
||||
```bash
|
||||
git diff main --name-only -- '*.c' '*.h'
|
||||
```
|
||||
|
||||
This gives the list of C source and header files changed in the PR.
|
||||
|
||||
### Step 3: Read all changed files
|
||||
|
||||
Use the Read tool to read every changed `.c` and `.h` file. Read full files — don't skip any.
|
||||
|
||||
### Step 4: Review each file
|
||||
|
||||
For each changed file, review for:
|
||||
|
||||
**Memory Safety**
|
||||
- Every `malloc`/`calloc` has a matching `free` on all code paths (including error paths)
|
||||
- No use-after-free (pointers used after `*_destroy()` is called)
|
||||
- No double-free
|
||||
- Null checks after allocation before use
|
||||
- Correct buffer sizes (strlen + 1 for null terminators)
|
||||
- `Data` objects created/destroyed properly
|
||||
|
||||
**Thread Safety**
|
||||
- Shared state accessed under mutex
|
||||
- No race conditions on queue operations
|
||||
- Condition variable signals under lock
|
||||
- No deadlock potential (consistent lock ordering)
|
||||
- `done` flags checked properly in consumer loops
|
||||
|
||||
**Protocol Safety**
|
||||
- `send_n_data` / `receive_n_data` return values checked
|
||||
- Status codes validated before use
|
||||
- Config serialization handles partial reads
|
||||
|
||||
**Logic Errors**
|
||||
- Off-by-one in loops/buffers
|
||||
- Incorrect size calculations
|
||||
- Wrong enum values or comparisons
|
||||
- Missing break statements in switch
|
||||
|
||||
**Error Handling**
|
||||
- Resources freed on error paths (no leaks)
|
||||
- Functions return appropriate error values
|
||||
- Error messages are useful
|
||||
|
||||
### Step 5: Categorize findings
|
||||
|
||||
For each issue:
|
||||
1. **File:line** — exact location
|
||||
2. **Severity** — critical / warning / style
|
||||
3. **Category** — memory / thread / protocol / logic / error
|
||||
4. **Description** — what's wrong and how to fix it
|
||||
|
||||
### Step 6: Output report
|
||||
|
||||
Print a formatted summary:
|
||||
|
||||
```
|
||||
=== PR REVIEW SUMMARY ===
|
||||
Branch: <branch-name>
|
||||
Files reviewed: <count>
|
||||
Issues found: <count>
|
||||
|
||||
CRITICAL: <count>
|
||||
WARNING: <count>
|
||||
STYLE: <count>
|
||||
|
||||
=== ISSUES ===
|
||||
[1] src/shared/compression.c:42 — CRITICAL (memory)
|
||||
Potential leak: data returned from data_compress() not freed on error path
|
||||
Fix: Add data_destroy(compressed) before return false
|
||||
|
||||
...
|
||||
|
||||
=== VERDICT ===
|
||||
[PASS] No critical issues found
|
||||
— or —
|
||||
[FAIL] <N> critical issues must be fixed before merge
|
||||
```
|
||||
|
||||
### Step 7: Optional PR comment
|
||||
|
||||
If the user wants to post the review as a PR comment:
|
||||
```bash
|
||||
tea pr comment <number> --comment "<review report>"
|
||||
```
|
||||
|
||||
## Rules
|
||||
- Do NOT edit any source files
|
||||
- Do NOT run builds or tests
|
||||
- Do NOT commit or push
|
||||
- Report ALL issues — don't filter or minimize
|
||||
- Be specific about line numbers and fix suggestions
|
||||
Reference in New Issue
Block a user