c160ab3223
The Gitea issue body is the single source of truth for a spec; the only per-feature artifact in git is the RTM row (REQ-ID -> issue # -> test). Drops per-feature spec.md/tasks.md/checklist files from the workflow (the _example stays as a template/reference). Updates the guide, ADR-041, AGENTS.md, CLAUDE.md, templates, the RTM (adds an Issue column), the implement/review-pr skills, and replaces the file-spec CI jobs with an rtm-check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
96 lines
4.8 KiB
Markdown
96 lines
4.8 KiB
Markdown
---
|
|
name: review-pr
|
|
description: Multi-persona SDD code review of a Gitea PR. Each persona pairs its .claude/personas/ identity with its .specify/personas/ checklist, verifies the diff against the constitution and the feature spec's REQ-NNN (every requirement implemented and tested), and posts findings as a separate Gitea comment.
|
|
---
|
|
|
|
# Multi-Persona PR Review (SDD)
|
|
|
|
You will perform a thorough multi-persona code review of the given PR and post each persona's
|
|
findings as a **separate comment**. Under SDD, the review verifies the diff against two
|
|
contracts: the project [constitution](../../../.specify/constitution.md) and the feature's
|
|
spec (the linked **Gitea issue body** — every `REQ-NNN` must be implemented **and** covered by a test).
|
|
|
|
## Argument
|
|
|
|
The user provides a Gitea PR URL, e.g.:
|
|
`http://heim-nas:3005/marcel/familienarchiv/pulls/160`
|
|
|
|
Parse it to extract `owner`, `repo`, and `pull_number`.
|
|
|
|
## Step 0 — Load the SDD ground truth
|
|
|
|
Read before reviewing:
|
|
- [`.specify/constitution.md`](../../../.specify/constitution.md) — rules the code must obey (esp. §4 Do-Not-Touch)
|
|
- [`.specify/AGENTS.md`](../../../.specify/AGENTS.md) — constraints
|
|
- The feature's spec — the **Gitea issue** the PR closes (`Closes #n`). Read its body for the
|
|
`REQ-NNN` requirements, acceptance criteria, inline API stub, and any STRIDE/threat notes.
|
|
- [`.specify/rtm.md`](../../../.specify/rtm.md) — the requirement→test→status matrix
|
|
|
|
## Step 1 — Gather PR context
|
|
|
|
Use the Gitea MCP tools to collect:
|
|
1. PR metadata (title, description, base/head branch) via `pull_request_read`
|
|
2. The list of changed files
|
|
3. The full content of every changed file at the head commit via `get_file_contents`
|
|
|
|
Read ALL changed files completely before starting. Do not skip files.
|
|
|
|
## Step 2 — Read every persona (identity + checklist)
|
|
|
|
Adopt each persona's voice from `.claude/personas/`; apply its review lens. For the SDD
|
|
personas, also re-read the matching `.specify/personas/` checklist — at PR time the same
|
|
checklist items are verified against the **code** rather than the spec.
|
|
|
|
| Persona | Identity (`.claude/personas/`) | Checklist (`.specify/personas/`) | PR-time focus |
|
|
|---|---|---|---|
|
|
| Requirements Engineer | `req_engineer.md` | `requirements-engineer.md` | Traceability: every `REQ-NNN` implemented; RTM updated |
|
|
| Developer (Felix Brandt) | `developer.md` | `developer.md` | Clean code, layering, generate:api run, ErrorCode four-site |
|
|
| Tester | `tester.md` | — (uses identity) | Test quality: each REQ has a real failing-first test; edge cases; levels right |
|
|
| Security (Nora "NullX") | `security_expert.md` | `security.md` | authn/authz, IDOR, mass-assignment, `{@html}`, secrets/PII |
|
|
| DevOps | `devops.md` | `devops.md` | migration rollback, env vars, CI guards intact, artifact pin |
|
|
| UI/UX | `ui_expert.md` | `ui-ux.md` | states, i18n, a11y, design tokens |
|
|
| Architect | `architect.md` | `architect.md` | boundaries, ADR present for irreversible choices, no superseded-ADR violation |
|
|
|
|
## Step 3 — Write each review
|
|
|
|
For each persona, write a review that:
|
|
|
|
- Opens with a one-line verdict: **✅ Approved**, **⚠️ Approved with concerns**, or **🚫 Changes requested**
|
|
- Lists concrete findings with file paths and line references; cite the constitution rule
|
|
(e.g. "violates §2.4 — `updatedBy` bound from request body") or the `REQ-NNN` at issue
|
|
- Distinguishes **blockers** (must fix) from **suggestions** (nice to have)
|
|
- **Requirements Engineer specifically** produces a traceability table — for each `REQ-NNN`:
|
|
is it implemented? is there a test? is `rtm.md` updated to `Done`? Any unimplemented or
|
|
untested REQ is a blocker. Any code behavior with no backing requirement is flagged
|
|
(scope creep — should it be a new REQ, or removed?).
|
|
- A constitution **Do-Not-Touch** violation (edited generated file, edited shipped migration,
|
|
edited an Accepted ADR, bumped the artifact action past v3, weakened a CI guard) is always
|
|
a blocker.
|
|
|
|
```
|
|
### 🔐 Security — PR Review
|
|
|
|
**Verdict: ⚠️ Approved with concerns**
|
|
|
|
### Blockers
|
|
- `UserAvatarController.java:42` — REQ-009's 403 path has no test (constitution §2.8)
|
|
|
|
### Suggestions
|
|
- ...
|
|
```
|
|
|
|
## Step 4 — Post comments
|
|
|
|
Post each persona's review as a **separate comment** via the Gitea MCP `issue_write` tool
|
|
(issues and PRs share the comment API). Post all personas; if one has nothing to flag, post a
|
|
brief "LGTM" naming what they checked.
|
|
|
|
## Step 5 — Report back
|
|
|
|
Summarize to the user:
|
|
- Each persona's verdict and the overall verdict (worst-case wins: any "Changes requested" → overall "Changes requested")
|
|
- The full list of blockers, grouped by persona
|
|
- **Traceability status:** which `REQ-NNN` are implemented+tested vs. missing, and whether
|
|
`rtm.md` is in sync
|
|
- Any constitution Do-Not-Touch violations (called out explicitly)
|