Skip to content

Commit c7eddc9

Browse files
Overhaul review skill (#661)
Rewrite /review around task() for a single lens and spawn_agent / wait_agents for multi-lens reviews. Pin critique / neckbeard / greybeard routing and the no-fix rule in catalog coverage. Closes CL-7038
1 parent cde1a48 commit c7eddc9

2 files changed

Lines changed: 28 additions & 7 deletions

File tree

plugins/corbits-skills/skills/review/SKILL.md

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,20 @@ argument-hint: "[paths | PR | diff | hygiene | architecture]"
88

99
You are Skywalker. This skill is a slash command (`/review`) and is also loadable with `use_skill("review")`. Do not implement fixes. Do not write product patches to "just quickly" address findings. Do not post GitHub review comments under a Claude (or any other vendor) identity.
1010

11-
Spawn a director. Pass the operator's scope — paths, PR, branch, or diff — as the brief. Report that director's Summary / Findings / Blockers / Paths.
11+
Classify the lens, spawn the matching director(s), wait for reports, synthesize. Findings only — never land fixes in this recipe.
1212

1313
## Routing
1414

15-
- **Default** (correctness, completeness, brief adherence, defects with evidence): `task(agent="critique")`
16-
- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `task(agent="neckbeard")`
17-
- **Architecture-only** (structure, boundaries, approach): `task(agent="greybeard")`
15+
- **Default** (correctness, completeness, brief adherence, defects with evidence): `critique`
16+
- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `neckbeard`
17+
- **Architecture-only** (structure, boundaries, approach): `greybeard`
1818

19-
If the operator did not say hygiene-only or architecture-only, spawn critique. Do not spawn all three unless they asked for a wider review.
19+
If the operator did not say hygiene-only or architecture-only, spawn critique alone. Do not spawn all three unless they asked for a wider review.
20+
21+
## Fleet
22+
23+
- **One lens:** `task(agent="<director>")` — blocking single spawn; prefer this when only one worker is needed.
24+
- **Wider review** (operator asked for more than one lens): `spawn_agent(agent="<director>", …)` once per lens in the same turn; record each returned `agent_id`, then `wait_agents` on those ids.
2025

2126
Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`, and `agent`.
2227

@@ -27,9 +32,9 @@ Include whatever the operator gave you, plus enough for a scoped review:
2732
- Paths, PR number/URL, or branch to review
2833
- Base for comparison when known (`git diff <base>...HEAD`); if the base is unclear, ask rather than guessing `main`
2934
- That only the operator's scope is in scope — pre-existing issues outside the diff are out of lane
30-
- Do not implement fixes; findings only, with evidence
35+
- Do not implement fixes; findings only, with evidence (`path:line`)
3136
- Signal over noise: skip hypotheticals and style nits that do not affect correctness, readability, or maintainability (neckbeard is the exception when hygiene was requested)
3237

3338
## After the report
3439

35-
Synthesize. Do not land fixes. If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill.
40+
Synthesize Summary / Findings / Blockers / Paths for the operator. Do not land fixes. If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill.

tests/unit/corbits-skills-catalog.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,22 @@ test("style skill is guidance, not ceremony or tool-contract restatement", async
119119
expect(skill).not.toContain("## Acknowledgment");
120120
});
121121

122+
test("review skill routes critique/neckbeard/greybeard via task or spawn_agent/wait_agents", async () => {
123+
const skill = await Bun.file(join(pluginRoot, "skills/review/SKILL.md")).text();
124+
expect(skill).toContain("task(agent=");
125+
expect(skill).toContain("spawn_agent");
126+
expect(skill).toContain("wait_agents");
127+
expect(skill).toContain("returned `agent_id`");
128+
expect(skill).toContain("critique");
129+
expect(skill).toContain("neckbeard");
130+
expect(skill).toContain("greybeard");
131+
expect(skill).toContain("Do not implement fixes");
132+
expect(skill).toContain("Findings only");
133+
expect(skill).not.toContain('task(agent="critique")');
134+
expect(skill).not.toContain('task(agent="neckbeard")');
135+
expect(skill).not.toContain('task(agent="greybeard")');
136+
});
137+
122138
test("create-issue is Linear-first without restated MCP tool contracts", async () => {
123139
const skill = await Bun.file(join(pluginRoot, "skills/create-issue/SKILL.md")).text();
124140
expect(skill).toContain("mcp__linear__");

0 commit comments

Comments
 (0)