Skip to content

Commit 471fe32

Browse files
obraarittr
authored andcommitted
Lift superpowers:code-reviewer agent into the requesting-code-review skill
The plugin had a single named agent (`agents/code-reviewer.md`) used by two skills, while every other reviewer/implementer subagent in the repo is dispatched as `general-purpose` with the prompt template living alongside its skill. That asymmetry had no upside and several costs: - Two sources of truth for the code review checklist (the agent file and `requesting-code-review/code-reviewer.md`), both drifting independently. - `Codex` users could not use the named agent directly; the codex-tools reference doc had a workaround section explaining how to flatten the named agent into a `worker` dispatch. - No third-party reliance on `superpowers:code-reviewer` inside this repo. Changes: - Merge `agents/code-reviewer.md` (persona + checklist) and `skills/requesting-code-review/code-reviewer.md` (placeholder template) into a single self-contained Task-dispatch template, matching the shape of `implementer-prompt.md`, `spec-reviewer-prompt.md`, etc. - Update `skills/requesting-code-review/SKILL.md` and `skills/subagent-driven-development/code-quality-reviewer-prompt.md` to dispatch `Task (general-purpose)` instead of the named agent. - Drop the now-obsolete "Named agent dispatch" workaround sections from `codex-tools.md` and `copilot-tools.md` — superpowers no longer ships any named agents, so those instructions documented nothing. - Delete `agents/code-reviewer.md` and the empty `agents/` directory. Tier 3 coverage for the change: a new behavioral test `tests/claude-code/test-requesting-code-review.sh` plants real bugs (SQL injection, plaintext password handling, credential logging) into a tiny project, runs the actual `requesting-code-review` skill against the working tree, and asserts the dispatched reviewer flags every planted issue at Critical/Important severity and refuses to approve the diff. Verified end-to-end on this branch: - The new test passes (5/5 assertions; reviewer caught all planted bugs and several others). - The existing SDD integration test still passes (7/7 subagents dispatched, all as `general-purpose`; spec compliance still rejects extra features; produced code is correct). - Session JSONLs confirm zero remaining `superpowers:code-reviewer` dispatches anywhere in the SDD pipeline.
1 parent 4c7c544 commit 471fe32

9 files changed

Lines changed: 344 additions & 203 deletions

File tree

agents/code-reviewer.md

Lines changed: 0 additions & 48 deletions
This file was deleted.

skills/requesting-code-review/SKILL.md

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: Use when completing tasks, implementing major features, or before m
55

66
# Requesting Code Review
77

8-
Dispatch superpowers:code-reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work.
8+
Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work.
99

1010
**Core principle:** Review early, review often.
1111

@@ -29,16 +29,15 @@ BASE_SHA=$(git rev-parse HEAD~1) # or origin/main
2929
HEAD_SHA=$(git rev-parse HEAD)
3030
```
3131

32-
**2. Dispatch code-reviewer subagent:**
32+
**2. Dispatch code reviewer subagent:**
3333

34-
Use Task tool with superpowers:code-reviewer type, fill template at `code-reviewer.md`
34+
Use Task tool with `general-purpose` type, fill template at `code-reviewer.md`
3535

3636
**Placeholders:**
37-
- `{WHAT_WAS_IMPLEMENTED}` - What you just built
37+
- `{DESCRIPTION}` - Brief summary of what you built
3838
- `{PLAN_OR_REQUIREMENTS}` - What it should do
3939
- `{BASE_SHA}` - Starting commit
4040
- `{HEAD_SHA}` - Ending commit
41-
- `{DESCRIPTION}` - Brief summary
4241

4342
**3. Act on feedback:**
4443
- Fix Critical issues immediately
@@ -56,12 +55,11 @@ You: Let me request code review before proceeding.
5655
BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}')
5756
HEAD_SHA=$(git rev-parse HEAD)
5857
59-
[Dispatch superpowers:code-reviewer subagent]
60-
WHAT_WAS_IMPLEMENTED: Verification and repair functions for conversation index
58+
[Dispatch code reviewer subagent]
59+
DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types
6160
PLAN_OR_REQUIREMENTS: Task 2 from docs/superpowers/plans/deployment-plan.md
6261
BASE_SHA: a7981ec
6362
HEAD_SHA: 3df7661
64-
DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types
6563
6664
[Subagent returns]:
6765
Strengths: Clean architecture, real tests

skills/requesting-code-review/code-reviewer.md

Lines changed: 107 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,111 +1,133 @@
1-
# Code Review Agent
1+
# Code Reviewer Prompt Template
22

3-
You are reviewing code changes for production readiness.
3+
Use this template when dispatching a code reviewer subagent.
44

5-
**Your task:**
6-
1. Review {WHAT_WAS_IMPLEMENTED}
7-
2. Compare against {PLAN_OR_REQUIREMENTS}
8-
3. Check code quality, architecture, testing
9-
4. Categorize issues by severity
10-
5. Assess production readiness
5+
**Purpose:** Review completed work against requirements and code quality standards before it cascades into more work.
116

12-
## What Was Implemented
7+
```
8+
Task tool (general-purpose):
9+
description: "Review code changes"
10+
prompt: |
11+
You are a Senior Code Reviewer with expertise in software architecture,
12+
design patterns, and best practices. Your job is to review completed work
13+
against its plan or requirements and identify issues before they cascade.
1314
14-
{DESCRIPTION}
15+
## What Was Implemented
1516
16-
## Requirements/Plan
17+
{DESCRIPTION}
1718
18-
{PLAN_REFERENCE}
19+
## Requirements / Plan
1920
20-
## Git Range to Review
21+
{PLAN_OR_REQUIREMENTS}
2122
22-
**Base:** {BASE_SHA}
23-
**Head:** {HEAD_SHA}
23+
## Git Range to Review
2424
25-
```bash
26-
git diff --stat {BASE_SHA}..{HEAD_SHA}
27-
git diff {BASE_SHA}..{HEAD_SHA}
28-
```
25+
**Base:** {BASE_SHA}
26+
**Head:** {HEAD_SHA}
2927
30-
## Review Checklist
31-
32-
**Code Quality:**
33-
- Clean separation of concerns?
34-
- Proper error handling?
35-
- Type safety (if applicable)?
36-
- DRY principle followed?
37-
- Edge cases handled?
38-
39-
**Architecture:**
40-
- Sound design decisions?
41-
- Scalability considerations?
42-
- Performance implications?
43-
- Security concerns?
44-
45-
**Testing:**
46-
- Tests actually test logic (not mocks)?
47-
- Edge cases covered?
48-
- Integration tests where needed?
49-
- All tests passing?
50-
51-
**Requirements:**
52-
- All plan requirements met?
53-
- Implementation matches spec?
54-
- No scope creep?
55-
- Breaking changes documented?
56-
57-
**Production Readiness:**
58-
- Migration strategy (if schema changes)?
59-
- Backward compatibility considered?
60-
- Documentation complete?
61-
- No obvious bugs?
62-
63-
## Output Format
28+
```bash
29+
git diff --stat {BASE_SHA}..{HEAD_SHA}
30+
git diff {BASE_SHA}..{HEAD_SHA}
31+
```
6432
65-
### Strengths
66-
[What's well done? Be specific.]
33+
## What to Check
6734
68-
### Issues
35+
**Plan alignment:**
36+
- Does the implementation match the plan / requirements?
37+
- Are deviations justified improvements, or problematic departures?
38+
- Is all planned functionality present?
6939
70-
#### Critical (Must Fix)
71-
[Bugs, security issues, data loss risks, broken functionality]
40+
**Code quality:**
41+
- Clean separation of concerns?
42+
- Proper error handling?
43+
- Type safety where applicable?
44+
- DRY without premature abstraction?
45+
- Edge cases handled?
7246
73-
#### Important (Should Fix)
74-
[Architecture problems, missing features, poor error handling, test gaps]
47+
**Architecture:**
48+
- Sound design decisions?
49+
- Reasonable scalability and performance?
50+
- Security concerns?
51+
- Integrates cleanly with surrounding code?
7552
76-
#### Minor (Nice to Have)
77-
[Code style, optimization opportunities, documentation improvements]
53+
**Testing:**
54+
- Tests verify real behavior, not mocks?
55+
- Edge cases covered?
56+
- Integration tests where they matter?
57+
- All tests passing?
7858
79-
**For each issue:**
80-
- File:line reference
81-
- What's wrong
82-
- Why it matters
83-
- How to fix (if not obvious)
59+
**Production readiness:**
60+
- Migration strategy if schema changed?
61+
- Backward compatibility considered?
62+
- Documentation complete?
63+
- No obvious bugs?
8464
85-
### Recommendations
86-
[Improvements for code quality, architecture, or process]
65+
## Calibration
8766
88-
### Assessment
67+
Categorize issues by actual severity. Not everything is Critical.
68+
Acknowledge what was done well before listing issues — accurate praise
69+
helps the implementer trust the rest of the feedback.
70+
71+
If you find significant deviations from the plan, flag them specifically
72+
so the implementer can confirm whether the deviation was intentional.
73+
If you find issues with the plan itself rather than the implementation,
74+
say so.
75+
76+
## Output Format
77+
78+
### Strengths
79+
[What's well done? Be specific.]
80+
81+
### Issues
8982
90-
**Ready to merge?** [Yes/No/With fixes]
83+
#### Critical (Must Fix)
84+
[Bugs, security issues, data loss risks, broken functionality]
9185
92-
**Reasoning:** [Technical assessment in 1-2 sentences]
86+
#### Important (Should Fix)
87+
[Architecture problems, missing features, poor error handling, test gaps]
9388
94-
## Critical Rules
89+
#### Minor (Nice to Have)
90+
[Code style, optimization opportunities, documentation polish]
91+
92+
For each issue:
93+
- File:line reference
94+
- What's wrong
95+
- Why it matters
96+
- How to fix (if not obvious)
97+
98+
### Recommendations
99+
[Improvements for code quality, architecture, or process]
100+
101+
### Assessment
102+
103+
**Ready to merge?** [Yes | No | With fixes]
104+
105+
**Reasoning:** [1-2 sentence technical assessment]
106+
107+
## Critical Rules
108+
109+
**DO:**
110+
- Categorize by actual severity
111+
- Be specific (file:line, not vague)
112+
- Explain WHY each issue matters
113+
- Acknowledge strengths
114+
- Give a clear verdict
115+
116+
**DON'T:**
117+
- Say "looks good" without checking
118+
- Mark nitpicks as Critical
119+
- Give feedback on code you didn't actually read
120+
- Be vague ("improve error handling")
121+
- Avoid giving a clear verdict
122+
```
95123

96-
**DO:**
97-
- Categorize by actual severity (not everything is Critical)
98-
- Be specific (file:line, not vague)
99-
- Explain WHY issues matter
100-
- Acknowledge strengths
101-
- Give clear verdict
124+
**Placeholders:**
125+
- `{DESCRIPTION}` — brief summary of what was built
126+
- `{PLAN_OR_REQUIREMENTS}` — what it should do (plan file path, task text, or requirements)
127+
- `{BASE_SHA}` — starting commit
128+
- `{HEAD_SHA}` — ending commit
102129

103-
**DON'T:**
104-
- Say "looks good" without checking
105-
- Mark nitpicks as Critical
106-
- Give feedback on code you didn't review
107-
- Be vague ("improve error handling")
108-
- Avoid giving a clear verdict
130+
**Reviewer returns:** Strengths, Issues (Critical / Important / Minor), Recommendations, Assessment
109131

110132
## Example Output
111133

skills/subagent-driven-development/code-quality-reviewer-prompt.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@ Use this template when dispatching a code quality reviewer subagent.
77
**Only dispatch after spec compliance review passes.**
88

99
```
10-
Task tool (superpowers:code-reviewer):
10+
Task tool (general-purpose):
1111
Use template at requesting-code-review/code-reviewer.md
1212
13-
WHAT_WAS_IMPLEMENTED: [from implementer's report]
13+
DESCRIPTION: [task summary, from implementer's report]
1414
PLAN_OR_REQUIREMENTS: Task N from [plan-file]
1515
BASE_SHA: [commit before task]
1616
HEAD_SHA: [current commit]
17-
DESCRIPTION: [task summary]
1817
```
1918

2019
**In addition to standard code quality concerns, the reviewer should check:**

skills/using-superpowers/references/codex-tools.md

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Skills use Claude Code tool names. When you encounter these in a skill, use your
44

55
| Skill references | Codex equivalent |
66
|-----------------|------------------|
7-
| `Task` tool (dispatch subagent) | `spawn_agent` (see [Named agent dispatch](#named-agent-dispatch)) |
7+
| `Task` tool (dispatch subagent) | `spawn_agent` (see [Subagent dispatch requires multi-agent support](#subagent-dispatch-requires-multi-agent-support)) |
88
| Multiple `Task` calls (parallel) | Multiple `spawn_agent` calls |
99
| Task returns result | `wait_agent` |
1010
| Task completes automatically | `close_agent` to free slot |
@@ -29,53 +29,6 @@ waiting as `wait`. Current Codex uses `wait_agent` for spawned agents. The
2929
`wait` name now belongs to code-mode `exec/wait`, which resumes a yielded exec
3030
cell by `cell_id`; it is not the spawned-agent result tool.
3131

32-
## Named agent dispatch
33-
34-
Claude Code skills reference named agent types like `superpowers:code-reviewer`.
35-
Codex does not have a named agent registry — `spawn_agent` creates generic agents
36-
from built-in roles (`default`, `explorer`, `worker`).
37-
38-
When a skill says to dispatch a named agent type:
39-
40-
1. Find the agent's prompt file (e.g., `agents/code-reviewer.md` or the skill's
41-
local prompt template like `code-quality-reviewer-prompt.md`)
42-
2. Read the prompt content
43-
3. Fill any template placeholders (`{BASE_SHA}`, `{WHAT_WAS_IMPLEMENTED}`, etc.)
44-
4. Spawn a `worker` agent with the filled content as the `message`
45-
46-
| Skill instruction | Codex equivalent |
47-
|-------------------|------------------|
48-
| `Task tool (superpowers:code-reviewer)` | `spawn_agent(agent_type="worker", message=...)` with `code-reviewer.md` content |
49-
| `Task tool (general-purpose)` with inline prompt | `spawn_agent(message=...)` with the same prompt |
50-
51-
### Message framing
52-
53-
The `message` parameter is user-level input, not a system prompt. Structure it
54-
for maximum instruction adherence:
55-
56-
```
57-
Your task is to perform the following. Follow the instructions below exactly.
58-
59-
<agent-instructions>
60-
[filled prompt content from the agent's .md file]
61-
</agent-instructions>
62-
63-
Execute this now. Output ONLY the structured response following the format
64-
specified in the instructions above.
65-
```
66-
67-
- Use task-delegation framing ("Your task is...") rather than persona framing ("You are...")
68-
- Wrap instructions in XML tags — the model treats tagged blocks as authoritative
69-
- End with an explicit execution directive to prevent summarization of the instructions
70-
71-
### When this workaround can be removed
72-
73-
This approach compensates for Codex not yet exposing plugin-packaged custom
74-
agents as named `spawn_agent` targets. OpenAI plugin examples can include
75-
plugin-level `agents/` directories, but skills still need to read those prompts
76-
and spawn a built-in agent role. When Codex exposes plugin agents as callable
77-
named agent types, this manual prompt-loading workaround can be removed.
78-
7932
## Environment Detection
8033

8134
Skills that create worktrees or finish branches should detect their

0 commit comments

Comments
 (0)