From 3f3ff61c71dd18a27cb3b3c94af2793e9fec0879 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 14:27:53 -0700 Subject: [PATCH] Overhaul review skill 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 --- plugins/corbits-skills/skills/review/SKILL.md | 19 ++++++++++++------- tests/unit/corbits-skills-catalog.test.ts | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/plugins/corbits-skills/skills/review/SKILL.md b/plugins/corbits-skills/skills/review/SKILL.md index 9e37cf9e2..92732451f 100644 --- a/plugins/corbits-skills/skills/review/SKILL.md +++ b/plugins/corbits-skills/skills/review/SKILL.md @@ -8,15 +8,20 @@ argument-hint: "[paths | PR | diff | hygiene | architecture]" 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. -Spawn a director. Pass the operator's scope — paths, PR, branch, or diff — as the brief. Report that director's Summary / Findings / Blockers / Paths. +Classify the lens, spawn the matching director(s), wait for reports, synthesize. Findings only — never land fixes in this recipe. ## Routing -- **Default** (correctness, completeness, brief adherence, defects with evidence): `task(agent="critique")` -- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `task(agent="neckbeard")` -- **Architecture-only** (structure, boundaries, approach): `task(agent="greybeard")` +- **Default** (correctness, completeness, brief adherence, defects with evidence): `critique` +- **Hygiene-only** (nits, naming, lint, pedantry with receipts): `neckbeard` +- **Architecture-only** (structure, boundaries, approach): `greybeard` -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. +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. + +## Fleet + +- **One lens:** `task(agent="")` — blocking single spawn; prefer this when only one worker is needed. +- **Wider review** (operator asked for more than one lens): `spawn_agent(agent="", …)` once per lens in the same turn; record each returned `agent_id`, then `wait_agents` on those ids. Prefer a typed brief: `intent="review"`, `success_criteria`, `do_not`, `report_focus`, and `agent`. @@ -27,9 +32,9 @@ Include whatever the operator gave you, plus enough for a scoped review: - Paths, PR number/URL, or branch to review - Base for comparison when known (`git diff ...HEAD`); if the base is unclear, ask rather than guessing `main` - That only the operator's scope is in scope — pre-existing issues outside the diff are out of lane -- Do not implement fixes; findings only, with evidence +- Do not implement fixes; findings only, with evidence (`path:line`) - Signal over noise: skip hypotheticals and style nits that do not affect correctness, readability, or maintainability (neckbeard is the exception when hygiene was requested) ## After the report -Synthesize. Do not land fixes. If the operator then wants repairs, that is a later `/implement` or `use_skill("dispatch")` — not this skill. +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. diff --git a/tests/unit/corbits-skills-catalog.test.ts b/tests/unit/corbits-skills-catalog.test.ts index bd540fdd6..0247170d1 100644 --- a/tests/unit/corbits-skills-catalog.test.ts +++ b/tests/unit/corbits-skills-catalog.test.ts @@ -119,6 +119,22 @@ test("style skill is guidance, not ceremony or tool-contract restatement", async expect(skill).not.toContain("## Acknowledgment"); }); +test("review skill routes critique/neckbeard/greybeard via task or spawn_agent/wait_agents", async () => { + const skill = await Bun.file(join(pluginRoot, "skills/review/SKILL.md")).text(); + expect(skill).toContain("task(agent="); + expect(skill).toContain("spawn_agent"); + expect(skill).toContain("wait_agents"); + expect(skill).toContain("returned `agent_id`"); + expect(skill).toContain("critique"); + expect(skill).toContain("neckbeard"); + expect(skill).toContain("greybeard"); + expect(skill).toContain("Do not implement fixes"); + expect(skill).toContain("Findings only"); + expect(skill).not.toContain('task(agent="critique")'); + expect(skill).not.toContain('task(agent="neckbeard")'); + expect(skill).not.toContain('task(agent="greybeard")'); +}); + test("create-issue is Linear-first without restated MCP tool contracts", async () => { const skill = await Bun.file(join(pluginRoot, "skills/create-issue/SKILL.md")).text(); expect(skill).toContain("mcp__linear__");