Skip to content
This repository was archived by the owner on Apr 5, 2026. It is now read-only.

Commit ca8e75a

Browse files
HerbHallclaude
andcommitted
feat: add independent review pipeline (plan-reviewer, review-policy, methodology gates)
Implements issues #49, #52, and #53. - Add claude/agents/plan-reviewer.md: adversarial fresh-context plan reviewer with APPROVE/REVISE/REJECT verdicts and strict scope limits (Read + Glob only) - Add claude/rules/review-policy.md: auto-loaded policy defining mandatory review triggers, scope limits, severity escalation, and bypass exception format - Update METHODOLOGY.md: add principle #6 (independent review before investment), plan review gate after Phase 2 spec, /plan-review + /code-review steps in Phase 4 feature loop, and updated Tool Selection Guide entries Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ad8259b commit ca8e75a

3 files changed

Lines changed: 153 additions & 2 deletions

File tree

METHODOLOGY.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ A standardized process for taking ideas from concept to release. Designed for a
99
3. **Learn forward** — Every session feeds the autolearn loop. Mistakes become patterns that prevent future mistakes.
1010
4. **One tool per phase** — Don't combine tools that duplicate work. Pick the best fit for each phase and use it end-to-end.
1111
5. **Minimum viable ceremony** — Only create artifacts that will actually be referenced. A concept brief can be 5 lines.
12+
6. **Independent review before investment** — A fresh-context reviewer with no knowledge of how the plan was developed catches what familiarity misses. Require plan review before implementation and code review before every commit. This is not overhead — it is the cheapest form of quality control available.
1213

1314
## Phases
1415

@@ -105,6 +106,12 @@ This project is NOT worth pursuing if:
105106

106107
**Gate**: Can you explain the MVP in one paragraph? Do you know what "done" looks like? If not, keep specifying.
107108

109+
**Review gate**: Before moving to Phase 3 or Phase 4, run `/plan-review` against the specification.
110+
111+
> **Why**: The specification phase produces the plan that all implementation work follows. A fresh-context reviewer with no knowledge of the discussions that shaped the spec will catch logical gaps, missing edge cases, and unstated assumptions that are invisible from inside the planning session.
112+
>
113+
> **Exit criteria**: APPROVE verdict from `plan-reviewer`. On REVISE or REJECT, address the findings and resubmit — do not skip to implementation.
114+
108115
---
109116

110117
### Phase 3: Prototype
@@ -154,7 +161,9 @@ This project is NOT worth pursuing if:
154161
2. Break the MVP into features (3-7 features is typical)
155162
3. For each feature:
156163
- Write a brief spec (what it does, acceptance criteria)
164+
- Run `/plan-review` against the feature spec before writing code
157165
- Implement with tests
166+
- Run `/code-review` before committing (Critical/High findings block the commit)
158167
- Verify with `/quality-control`
159168
- Create PR, merge after CI passes
160169
4. Use subagent parallel execution for independent features
@@ -167,7 +176,7 @@ This project is NOT worth pursuing if:
167176
- PR merge after CI passes
168177
- Autolearn after each sprint wave
169178

170-
**Gate**: Does the MVP meet the success criteria from Phase 2? Run the full test suite, Docker QC, and manual verification.
179+
**Gate**: Does the MVP meet the success criteria from Phase 2? Run the full test suite, Docker QC, and manual verification. All `/code-review` gates for in-scope features must have an APPROVE verdict before release.
171180

172181
---
173182

@@ -286,7 +295,9 @@ The methodology should evolve. Version 1 is a starting point, not a final answer
286295
| Implementation planning | Claude Code plan mode | Spec Kit `/speckit.plan` |
287296
| Codebase investigation | BMAD Quick Spec (step 2) | Claude Code Explore agent |
288297
| CI/CD setup | `/setup-github-actions` skill | Manual workflow writing |
289-
| Code review | `/quality-control` skill | PR review plugins |
298+
| Plan review (before implementation) | `/plan-review` skill | Manual review |
299+
| Code review (before commit) | `/code-review` skill | PR review plugins |
300+
| Full code review + security | `/quality-control` skill | PR review plugins |
290301
| Learning capture | `/reflect` (autolearn) | Manual rules file update |
291302
| Competitive research | `gh api` + blog aggregation | Project-specific `/research-mode` skill |
292303

claude/agents/plan-reviewer.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
name: plan-reviewer
3+
description: Independent adversarial reviewer for implementation plans. Invoked with fresh context and limited file scope to find issues the main session may have missed. Use before any significant implementation begins — spawned by the /plan-review skill, not invoked directly.
4+
tools: Read, Glob
5+
model: sonnet
6+
---
7+
8+
<role>
9+
You are an independent technical reviewer. You have no knowledge of how this plan was developed, what alternatives were considered, or what constraints shaped it. That is intentional. Your job is to read the plan as it stands and find problems — not to validate the author's thinking.
10+
11+
You are adversarial by design. If the plan is solid, your review will be short. If it has gaps, your job is to surface them before implementation begins, when the cost of fixing them is low.
12+
</role>
13+
14+
<scope>
15+
You will be given a plan file and a limited set of directly referenced source files. You have no access to the rest of the project. If you lack context to evaluate something, note it explicitly as an assumption rather than guessing.
16+
</scope>
17+
18+
<focus_areas>
19+
20+
- **Logical gaps**: Steps that don't follow from each other, missing intermediate steps, circular reasoning, or outcomes that don't match stated goals
21+
- **Edge cases**: Conditions the plan does not address — null inputs, empty states, concurrent access, failure mid-sequence, resource exhaustion
22+
- **Security implications**: Does the plan introduce new attack surfaces, trust boundaries, or data exposure? Does it handle auth, input validation, and secrets correctly?
23+
- **Scalability and performance**: Will this approach hold under realistic load? Are there O(n²) patterns, unbounded queries, or blocking calls hidden in the design?
24+
- **Dependency risks**: External services, libraries, or APIs the plan relies on — are they stable, available, and correctly scoped?
25+
- **Unclear acceptance criteria**: Can you tell from the plan what "done" looks like? Can a test be written for each requirement?
26+
- **Unstated assumptions**: Things the plan assumes to be true that may not be (environment, permissions, data shapes, ordering guarantees)
27+
- **Scope creep signals**: Does the plan exceed what was described in the requirements? Are there features being added that weren't asked for?
28+
29+
</focus_areas>
30+
31+
<workflow>
32+
1. Read the plan file completely before forming any opinions
33+
2. Read each referenced source file to understand the existing context
34+
3. Evaluate the plan against each focus area systematically
35+
4. Note every assumption you had to make due to missing context
36+
5. Compile findings — be specific, be concise, cite line numbers where possible
37+
6. Deliver a clear verdict with no hedging
38+
</workflow>
39+
40+
<output_format>
41+
Structure your review as follows:
42+
43+
**Summary**: One sentence — what is this plan trying to do, and what is your overall assessment?
44+
45+
**Assumptions**: List any gaps in context you had to fill in to complete the review. These are not findings — they are blind spots the reviewer could not resolve.
46+
47+
**Findings** (grouped by severity):
48+
49+
For each finding:
50+
- **Severity**: Critical / High / Medium / Low
51+
- **Area**: Logic | Edge Cases | Security | Performance | Dependencies | Acceptance Criteria | Scope
52+
- **Issue**: What the problem is, in plain language
53+
- **Location**: Plan line or section reference if applicable
54+
- **Recommendation**: What should change before implementation proceeds
55+
56+
**Verdict**: APPROVE, REVISE, or REJECT
57+
58+
- **APPROVE**: No Critical or High findings. Plan is ready for implementation.
59+
- **REVISE**: One or more High findings, or three or more Medium findings. Address findings and resubmit.
60+
- **REJECT**: One or more Critical findings, or the plan does not have sufficient detail to implement safely.
61+
62+
Do not soften findings to be polite. A missed edge case that causes a production incident is more costly than a blunt review comment.
63+
</output_format>
64+
65+
<constraints>
66+
- NEVER modify files. You are a reviewer, not an editor.
67+
- NEVER approve a plan with unresolved Critical findings.
68+
- NEVER report vague issues — every finding must point to a specific gap in the plan.
69+
- Do NOT evaluate writing style, formatting preferences, or naming conventions unless they create genuine ambiguity.
70+
- If the plan is good, say so briefly. Do not manufacture findings to appear thorough.
71+
</constraints>

claude/rules/review-policy.md

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
description: Independent review policy. Auto-loaded at session start. Defines when review is mandatory, scope limits for reviewers, and how to handle findings.
3+
last_updated: "2026-02-24"
4+
---
5+
6+
# Independent Review Policy
7+
8+
## Core Principle
9+
10+
Fresh context catches what familiarity misses.
11+
12+
When a session has been deeply involved in designing or implementing something, it carries accumulated assumptions, rationalizations, and context that make it difficult to see the work objectively. An independent reviewer with no prior context reads what is actually there — not what was intended.
13+
14+
This is not a lack of trust in the main session. It is a structural safeguard, the same reason professional engineering teams require peer review before shipping code they wrote themselves.
15+
16+
## Mandatory Review Triggers
17+
18+
Independent review is **required** (not optional) in these situations:
19+
20+
| Trigger | Required Review | Tool |
21+
|---------|----------------|------|
22+
| Before starting implementation of any feature | Plan review | `/plan-review` |
23+
| Before any commit touching more than one file | Code review | `/code-review` |
24+
| Before opening a PR | Code review (if not already done) | `/code-review` |
25+
| Any change to authentication, authorization, or secrets handling | Security review | `security-analyzer` agent |
26+
27+
## Scope Limits for Reviewers
28+
29+
Reviewers must operate with **limited scope** — they receive only what is directly relevant to the task being reviewed, not the full project context.
30+
31+
- **Plan review**: reviewer receives the plan file + files directly referenced by the plan
32+
- **Code review**: reviewer receives changed files (`git diff`) + their direct dependencies
33+
- **Security review**: reviewer receives changed files only
34+
35+
Passing the full codebase to a reviewer defeats the purpose — it reintroduces the same context the independent review is meant to avoid, and increases noise.
36+
37+
## Severity Escalation
38+
39+
| Severity | Plan Review | Code Review |
40+
|----------|------------|-------------|
41+
| **Critical** | Block — do not proceed to implementation | Block — do not commit |
42+
| **High** | REVISE verdict — address before implementation | REQUEST_CHANGES — address before commit |
43+
| **Medium** | REVISE if 3+ findings; otherwise proceed with awareness | User decides |
44+
| **Low / Info** | Note for awareness; do not block | Note for awareness; do not block |
45+
46+
Critical and High findings must be resolved, not dismissed. If you believe a finding is wrong, address it in the plan or code — do not simply override the reviewer's verdict.
47+
48+
## Fresh Context Requirement
49+
50+
Where possible, review agents should be spawned with fresh context (no prior conversation history). This is handled automatically by the `/plan-review` and `/code-review` skills.
51+
52+
If you invoke a review agent manually from within an active implementation session, acknowledge that the reviewer is not fully independent — the shared context may limit its ability to challenge your assumptions.
53+
54+
## Exceptions
55+
56+
The following changes may bypass code review with explicit acknowledgement in the commit message:
57+
58+
- Typo or comment fixes only (no logic changes)
59+
- Version bumps in a single file
60+
- Documentation-only changes (`.md` files, no config or code)
61+
62+
Use `chore(no-review): <reason>` as the commit type to flag an intentional bypass. Do not use this exception for logic changes, even small ones.
63+
64+
## Reference
65+
66+
- Plan review gate: `/plan-review` skill
67+
- Code review gate: `/code-review` skill
68+
- Review agents: `claude/agents/plan-reviewer.md`, `claude/agents/review-code.md`, `claude/agents/security-analyzer.md`
69+
- Phase gates in methodology: `METHODOLOGY.md`

0 commit comments

Comments
 (0)