TapTap
5c8970c64f
CI / lint (pull_request) Successful in 1m29s
CI / sanitizers (address) (pull_request) Skipped
CI / sanitizers (undefined) (pull_request) Skipped
CI / fuzz-build (pull_request) Skipped
CI / coverage (pull_request) Skipped
CI / valgrind (pull_request) Skipped
CI / build-and-test (pull_request) Successful in 1m45s
The agent and skill definitions had drifted badly from the current codebase and tooling, repeating the same class of bug as the benchmark tool (references to nonexistent scripts and invented flags): - Replace the removed `python3 test.py` with the real integration command (`python3 -m pytest tests/integration/ -n 4 --dist=load -m "not setpriv"`) across agents and skills. - Fix `feature-scout`'s fabricated CLI flag list (--host, --server-mode, --use-* etc.) using the authoritative src/client/usage.c flags. - Fix `perf-analyst` benchmark flags (-m -c -> -j -z) and point at benchmark/bench.py instead of stale numbers. - Correct `code-explainer` (no getopt_long; --sendfile not -f) and version drift in the release skill (1.1.0 -> 2.20.0). - Replace GitHub/`gh` workflows with Gitea/`tea` (PRs target dev; issues via tea; branch strategy updated in all agents). - Use the built-in `-DSANITIZER=address|thread` CMake option instead of hand-rolled -fsanitize flags. - Add `-p 8080 --allow-unauthenticated` to plain-TCP server examples. - Merge the redundant security-screener into security-auditor; drop the duplicate (16 agents remain). Repo hygiene: gitignore `root/` and `test_partial_install_tmp/`, remove the empty leftover trees, delete the tracked scratch scripts tmux.sh and to_one_file.py, and note the compile_commands.json symlink in README.
137 lines
3.7 KiB
Markdown
137 lines
3.7 KiB
Markdown
---
|
|
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 dev..HEAD --oneline
|
|
```
|
|
|
|
### Step 2: Get changed files
|
|
|
|
```bash
|
|
git diff dev --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
|
|
|
|
**Security**
|
|
- No `strcpy`/`strcat`/`sprintf` — use `snprintf` with bounds
|
|
- `malloc` size calculations don't overflow
|
|
- Path traversal prevention (`..` in filenames)
|
|
- No fixed-size stack buffers for unbounded input
|
|
- TLS error codes checked after `SSL_read`/`SSL_write`
|
|
- No hardcoded certificates, keys, or credentials
|
|
- Received file permissions validated (no SUID/SGID injection)
|
|
- Denial of service: bounded memory, malformed messages handled
|
|
|
|
**Performance Impact**
|
|
- Unnecessary memory copies in hot paths
|
|
- Excessive malloc/free in tight loops
|
|
- Missing `sendfile()` opportunity for large files
|
|
- Compression level appropriate for use case
|
|
- Queue sizing appropriate for workload
|
|
|
|
### Step 5: Categorize findings
|
|
|
|
For each issue:
|
|
1. **File:line** — exact location
|
|
2. **Severity** — critical / warning / style
|
|
3. **Category** — memory / thread / protocol / security / performance / 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
|