familienarchiv/.claude/skills/review-pr/SKILL.md

name, description

name	description
review-pr	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 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 — rules the code must obey (esp. §4 Do-Not-Touch)
• .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 — 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)