From 92d70cff5bf1f12844d9eb48fdc711a99dfe2e47 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 31 Mar 2026 17:22:34 -0700 Subject: [PATCH] feat: add review-work and ai-slop-remover as built-in skills, add remove-ai-slops command Embed user-level skills into the plugin's built-in system so they ship with the product rather than requiring per-user configuration. - review-work: 5-agent parallel post-implementation review orchestrator - ai-slop-remover: per-file AI-generated code smell detector and remover - /remove-ai-slops: command that orchestrates parallel ai-slop-remover runs --- src/config/schema/commands.ts | 1 + .../builtin-commands/commands.test.ts | 72 +++ src/features/builtin-commands/commands.ts | 11 + .../templates/remove-ai-slops.ts | 89 +++ src/features/builtin-commands/types.ts | 2 +- src/features/builtin-skills/skills.test.ts | 54 +- src/features/builtin-skills/skills.ts | 4 +- .../builtin-skills/skills/ai-slop-remover.ts | 145 +++++ src/features/builtin-skills/skills/index.ts | 2 + .../builtin-skills/skills/review-work.ts | 536 ++++++++++++++++++ 10 files changed, 906 insertions(+), 10 deletions(-) create mode 100644 src/features/builtin-commands/templates/remove-ai-slops.ts create mode 100644 src/features/builtin-skills/skills/ai-slop-remover.ts create mode 100644 src/features/builtin-skills/skills/review-work.ts diff --git a/src/config/schema/commands.ts b/src/config/schema/commands.ts index 967254538..714580729 100644 --- a/src/config/schema/commands.ts +++ b/src/config/schema/commands.ts @@ -8,6 +8,7 @@ export const BuiltinCommandNameSchema = z.enum([ "refactor", "start-work", "stop-continuation", + "remove-ai-slops", ]) export type BuiltinCommandName = z.infer diff --git a/src/features/builtin-commands/commands.test.ts b/src/features/builtin-commands/commands.test.ts index c6927bc70..668027368 100644 --- a/src/features/builtin-commands/commands.test.ts +++ b/src/features/builtin-commands/commands.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from "bun:test" import { loadBuiltinCommands } from "./commands" import { HANDOFF_TEMPLATE } from "./templates/handoff" +import { REMOVE_AI_SLOPS_TEMPLATE } from "./templates/remove-ai-slops" import type { BuiltinCommandName } from "./types" describe("loadBuiltinCommands", () => { @@ -60,6 +61,77 @@ describe("loadBuiltinCommands", () => { }) }) +describe("loadBuiltinCommands — remove-ai-slops", () => { + test("should include remove-ai-slops command in loaded commands", () => { + //#given + const disabledCommands: BuiltinCommandName[] = [] + + //#when + const commands = loadBuiltinCommands(disabledCommands) + + //#then + expect(commands["remove-ai-slops"]).toBeDefined() + expect(commands["remove-ai-slops"].name).toBe("remove-ai-slops") + }) + + test("should exclude remove-ai-slops when disabled", () => { + //#given + const disabledCommands: BuiltinCommandName[] = ["remove-ai-slops"] + + //#when + const commands = loadBuiltinCommands(disabledCommands) + + //#then + expect(commands["remove-ai-slops"]).toBeUndefined() + }) + + test("should include remove-ai-slops template content in command template", () => { + //#given - no disabled commands + + //#when + const commands = loadBuiltinCommands() + + //#then + expect(commands["remove-ai-slops"].template).toContain(REMOVE_AI_SLOPS_TEMPLATE) + }) + + test("should have correct description for remove-ai-slops", () => { + //#given - no disabled commands + + //#when + const commands = loadBuiltinCommands() + + //#then + expect(commands["remove-ai-slops"].description).toContain("AI-generated code smells") + }) +}) + +describe("REMOVE_AI_SLOPS_TEMPLATE", () => { + test("should include phase structure", () => { + //#given - the template string + + //#when / #then + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("Identify Changed Files") + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("Parallel AI Slop Removal") + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("Critical Review") + }) + + test("should reference ai-slop-remover skill", () => { + //#given - the template string + + //#when / #then + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("ai-slop-remover") + }) + + test("should include safety verification checklist", () => { + //#given - the template string + + //#when / #then + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("Safety Verification") + expect(REMOVE_AI_SLOPS_TEMPLATE).toContain("Behavior Preservation") + }) +}) + describe("HANDOFF_TEMPLATE", () => { test("should include session reading instruction", () => { //#given - the template string diff --git a/src/features/builtin-commands/commands.ts b/src/features/builtin-commands/commands.ts index 0802eb9aa..e3b0bb52d 100644 --- a/src/features/builtin-commands/commands.ts +++ b/src/features/builtin-commands/commands.ts @@ -6,6 +6,7 @@ import { STOP_CONTINUATION_TEMPLATE } from "./templates/stop-continuation" import { REFACTOR_TEMPLATE } from "./templates/refactor" import { START_WORK_TEMPLATE } from "./templates/start-work" import { HANDOFF_TEMPLATE } from "./templates/handoff" +import { REMOVE_AI_SLOPS_TEMPLATE } from "./templates/remove-ai-slops" const BUILTIN_COMMAND_DEFINITIONS: Record> = { "init-deep": { @@ -77,6 +78,16 @@ $ARGUMENTS template: ` ${STOP_CONTINUATION_TEMPLATE} `, + }, + "remove-ai-slops": { + description: "(builtin) Remove AI-generated code smells from branch changes and critically review the results", + template: ` +${REMOVE_AI_SLOPS_TEMPLATE} + + + +$ARGUMENTS +`, }, handoff: { description: "(builtin) Create a detailed context summary for continuing work in a new session", diff --git a/src/features/builtin-commands/templates/remove-ai-slops.ts b/src/features/builtin-commands/templates/remove-ai-slops.ts new file mode 100644 index 000000000..2d2155549 --- /dev/null +++ b/src/features/builtin-commands/templates/remove-ai-slops.ts @@ -0,0 +1,89 @@ +export const REMOVE_AI_SLOPS_TEMPLATE = `# Remove AI Slops Command + +## What this command does +Analyzes all files changed in the current branch (compared to parent commit), removes AI-generated code smells in parallel, then critically reviews the changes to ensure safety and behavior preservation. Fixes any issues found during review. + +## Step 0: Task Planning + +Use TodoWrite to create the task list: +1. Get changed files from branch +2. Run ai-slop-remover on each file in parallel +3. Critically review all changes +4. Fix any issues found + +## Role Definition +You are a senior code quality engineer specialized in identifying and removing AI-generated code patterns while preserving original functionality. You have deep expertise in code review, refactoring safety, and behavioral preservation. + +## Process + +### Phase 1: Identify Changed Files +Execute the following command to get all changed files in the current branch: +\\\`\\\`\\\`bash +git diff $(git merge-base main HEAD)..HEAD --name-only +\\\`\\\`\\\` + +### Phase 2: Parallel AI Slop Removal +For each changed file, spawn an agent in parallel using the Task tool with the ai-slop-remover skill: + +\\\`\\\`\\\` +task(category="quick", load_skills=["ai-slop-remover"], run_in_background=true, description="Remove AI slops from {filename}", prompt="Remove AI slops from: {file_path}") +\\\`\\\`\\\` + +**CRITICAL**: Launch ALL agents in a SINGLE message with multiple Task tool calls for maximum parallelism. + +### Phase 3: Critical Review +After all ai-slop-remover agents complete, perform a critical review with the following checklist: + +**Safety Verification**: +- [ ] No functional logic was accidentally removed +- [ ] All error handling is preserved +- [ ] Type hints remain correct and complete +- [ ] Import statements are still valid +- [ ] No breaking changes to public APIs + +**Behavior Preservation**: +- [ ] Return values unchanged +- [ ] Side effects unchanged +- [ ] Exception behavior unchanged +- [ ] Edge case handling preserved + +**Code Quality**: +- [ ] Removed changes are genuinely AI slop (not intentional patterns) +- [ ] Remaining code follows project conventions +- [ ] No orphaned code or dead references + +### Phase 4: Fix Issues +If any issues are found during critical review: +1. Identify the specific problem +2. Explain why it's a problem +3. Use git checkout to revert the changes from ai-slop-remover +4. If remaining ai-slops are found after reverting, remove them by editing the file yourself - with parallel tool calls, per-file +5. Verify the fix doesn't introduce new issues + +## Output Format + +### Summary Report +\\\`\\\`\\\` +## AI Slop Removal Summary + +### Files Processed +- file1.py: X changes +- file2.py: Y changes + +### Critical Review Results +- Safety: PASS/FAIL +- Behavior: PASS/FAIL +- Quality: PASS/FAIL + +### Issues Found & Fixed +1. [Issue description] -> [Fix applied] + +### Final Status +[CLEAN / ISSUES FIXED / REQUIRES ATTENTION] +\\\`\\\`\\\` + +## Quality Assurance +- NEVER remove code that serves a functional purpose +- ALWAYS verify changes compile/parse correctly +- ALWAYS preserve test coverage +- If uncertain about a change, err on the side of keeping the original code` diff --git a/src/features/builtin-commands/types.ts b/src/features/builtin-commands/types.ts index 0c2624f12..47a803379 100644 --- a/src/features/builtin-commands/types.ts +++ b/src/features/builtin-commands/types.ts @@ -1,6 +1,6 @@ import type { CommandDefinition } from "../claude-code-command-loader" -export type BuiltinCommandName = "init-deep" | "ralph-loop" | "cancel-ralph" | "ulw-loop" | "refactor" | "start-work" | "stop-continuation" | "handoff" +export type BuiltinCommandName = "init-deep" | "ralph-loop" | "cancel-ralph" | "ulw-loop" | "refactor" | "start-work" | "stop-continuation" | "handoff" | "remove-ai-slops" export interface BuiltinCommandConfig { disabled_commands?: BuiltinCommandName[] diff --git a/src/features/builtin-skills/skills.test.ts b/src/features/builtin-skills/skills.test.ts index 59a4198d1..afbca82de 100644 --- a/src/features/builtin-skills/skills.test.ts +++ b/src/features/builtin-skills/skills.test.ts @@ -61,7 +61,7 @@ describe("createBuiltinSkills", () => { expect(agentBrowserSkill!.template).toContain("agent-browser snapshot") }) - test("always includes frontend-ui-ux and git-master skills", () => { + test("always includes frontend-ui-ux, git-master, review-work, and ai-slop-remover skills", () => { // given - both provider options // when @@ -72,10 +72,12 @@ describe("createBuiltinSkills", () => { for (const skills of [defaultSkills, agentBrowserSkills]) { expect(skills.find((s) => s.name === "frontend-ui-ux")).toBeDefined() expect(skills.find((s) => s.name === "git-master")).toBeDefined() + expect(skills.find((s) => s.name === "review-work")).toBeDefined() + expect(skills.find((s) => s.name === "ai-slop-remover")).toBeDefined() } }) - test("returns exactly 4 skills regardless of provider", () => { + test("returns exactly 6 skills regardless of provider", () => { // given // when @@ -83,8 +85,8 @@ describe("createBuiltinSkills", () => { const agentBrowserSkills = createBuiltinSkills({ browserProvider: "agent-browser" }) // then - expect(defaultSkills).toHaveLength(4) - expect(agentBrowserSkills).toHaveLength(4) + expect(defaultSkills).toHaveLength(6) + expect(agentBrowserSkills).toHaveLength(6) }) test("should exclude playwright when it is in disabledSkills", () => { @@ -99,7 +101,9 @@ describe("createBuiltinSkills", () => { expect(skills.map((s) => s.name)).toContain("frontend-ui-ux") expect(skills.map((s) => s.name)).toContain("git-master") expect(skills.map((s) => s.name)).toContain("dev-browser") - expect(skills.length).toBe(3) + expect(skills.map((s) => s.name)).toContain("review-work") + expect(skills.map((s) => s.name)).toContain("ai-slop-remover") + expect(skills.length).toBe(5) }) test("should exclude multiple skills when they are in disabledSkills", () => { @@ -114,13 +118,15 @@ describe("createBuiltinSkills", () => { expect(skills.map((s) => s.name)).not.toContain("git-master") expect(skills.map((s) => s.name)).toContain("frontend-ui-ux") expect(skills.map((s) => s.name)).toContain("dev-browser") - expect(skills.length).toBe(2) + expect(skills.map((s) => s.name)).toContain("review-work") + expect(skills.map((s) => s.name)).toContain("ai-slop-remover") + expect(skills.length).toBe(4) }) test("should return an empty array when all skills are disabled", () => { // #given const options = { - disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "dev-browser"]), + disabledSkills: new Set(["playwright", "frontend-ui-ux", "git-master", "dev-browser", "review-work", "ai-slop-remover"]), } // #when @@ -138,7 +144,39 @@ describe("createBuiltinSkills", () => { const skills = createBuiltinSkills(options) // #then - expect(skills.length).toBe(4) + expect(skills.length).toBe(6) + }) + + test("review-work skill has correct structure", () => { + // #given - default options + + // #when + const skills = createBuiltinSkills() + const reviewWork = skills.find((s) => s.name === "review-work") + + // #then + expect(reviewWork).toBeDefined() + expect(reviewWork!.description).toContain("review") + expect(reviewWork!.template).toContain("5-Agent Parallel Review Orchestrator") + expect(reviewWork!.template).toContain("Goal & Constraint Verification") + expect(reviewWork!.template).toContain("QA") + expect(reviewWork!.template).toContain("Code Quality") + expect(reviewWork!.template).toContain("Security") + expect(reviewWork!.template).toContain("Context Mining") + }) + + test("ai-slop-remover skill has correct structure", () => { + // #given - default options + + // #when + const skills = createBuiltinSkills() + const aiSlopRemover = skills.find((s) => s.name === "ai-slop-remover") + + // #then + expect(aiSlopRemover).toBeDefined() + expect(aiSlopRemover!.description).toContain("AI-generated code smells") + expect(aiSlopRemover!.template).toContain("DETECTION CRITERIA") + expect(aiSlopRemover!.template).toContain("SAFETY RULES") }) test("returns playwright-cli skill when browserProvider is 'playwright-cli'", () => { diff --git a/src/features/builtin-skills/skills.ts b/src/features/builtin-skills/skills.ts index d0405f600..484d3adf4 100644 --- a/src/features/builtin-skills/skills.ts +++ b/src/features/builtin-skills/skills.ts @@ -8,6 +8,8 @@ import { frontendUiUxSkill, gitMasterSkill, devBrowserSkill, + reviewWorkSkill, + aiSlopRemoverSkill, } from "./skills/index" export interface CreateBuiltinSkillsOptions { @@ -27,7 +29,7 @@ export function createBuiltinSkills(options: CreateBuiltinSkillsOptions = {}): B browserSkill = playwrightSkill } - const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill] + const skills = [browserSkill, frontendUiUxSkill, gitMasterSkill, devBrowserSkill, reviewWorkSkill, aiSlopRemoverSkill] if (!disabledSkills) { return skills diff --git a/src/features/builtin-skills/skills/ai-slop-remover.ts b/src/features/builtin-skills/skills/ai-slop-remover.ts new file mode 100644 index 000000000..33660c500 --- /dev/null +++ b/src/features/builtin-skills/skills/ai-slop-remover.ts @@ -0,0 +1,145 @@ +import type { BuiltinSkill } from "../types" + +export const aiSlopRemoverSkill: BuiltinSkill = { + name: "ai-slop-remover", + description: + "Removes AI-generated code smells from a SINGLE file while preserving functionality. For multiple files, call in PARALLEL per file.", + template: `You are an expert code refactorer specializing in removing AI-generated "slop" patterns while STRICTLY preserving functionality. + +**INPUT**: Exactly ONE file path. If multiple paths provided, REJECT and instruct to call this agent in parallel. + +--- + +## DETECTION CRITERIA (Specific) + +### 1. Obvious Comments (EXCLUDE: BDD comments like #given, #when, #then, #when/then) + +**REMOVE**: +- Comments restating the code: \`x += 1 # increment x\` +- Docstrings on trivial methods: \`"""Returns the name."""\` for \`def get_name(): return self.name\` +- Section dividers: \`# ===== HELPER FUNCTIONS =====\` +- Commented-out code blocks +- \`# TODO: future enhancement\` without concrete plan +- \`# Note: this is important\` without explaining WHY + +**KEEP**: +- Comments explaining WHY (business logic, edge cases, workarounds) +- Links to issues/tickets: \`# See SPR-1234\` +- Non-obvious algorithm explanations +- Regex explanations +- Matches to existing code style + +### 2. Over-Defensive Code + +**REMOVE**: +- Null checks for values that CANNOT be None (e.g., Django request in view) +- \`if x is not None and x.attr is not None:\` when x is guaranteed +- Try-except around code that can't raise (e.g., dict literal access) +- \`isinstance()\` checks for statically typed parameters +- Default values for required parameters: \`def foo(x: str = "")\` when empty string is invalid +- Backward-compat shims: \`_old_name = new_name # deprecated\` +- \`# removed\` or \`# deleted\` comments for removed code +- Re-exports of unused items +- Verbose, duplicated, or redundant code / test cases + +**KEEP**: +- Validation at system boundaries (user input, external API responses) +- Error handling for I/O operations +- Null checks for nullable DB fields +- assertions in test code to matching type expectations + +### 3. Spaghetti Nesting (2+ levels deep) + +**REFACTOR**: +- Nested if-else chains -> early returns / guard clauses +- \`if x: if y: if z:\` -> \`if not x: return\` / \`if not y: return\` +- Nested loops with conditionals -> extract to helper OR use comprehensions +- Complex ternary \`a if b else (c if d else e)\` -> explicit if-else + +--- + +## PROCESS + +### Step 1: Read & Analyze +Read the file. Identify ALL slop instances with line numbers. + +### Step 2: Deep Consideration (CRITICAL) +For EACH identified issue, think: +- **Functionality Impact**: Will removing this change behavior? If ANY doubt, SKIP. +- **Test Coverage**: Are there tests that might break? If uncertain, SKIP. +- **Context Dependency**: Is this "slop" actually necessary for this specific codebase? (e.g., defensive code for known flaky external API) +- **Readability Trade-off**: Will removal make code LESS readable? If yes, SKIP. + +**RULE**: When in doubt, DO NOT CHANGE. False negatives are better than breaking code. + +### Step 3: Execute Changes +Make changes using Edit tool. One logical change at a time. + +### Step 4: Detailed Report + +**OUTPUT FORMAT**: + +\`\`\` +## AI Slop Removed: {filename} + +### Analysis Summary +- Total issues found: N +- Issues fixed: M +- Issues skipped (safety): K + +### Changes Made + +#### Change 1: [Category] Line X-Y +**Before**: [original code snippet] +**After**: [modified code snippet] +**Why this is slop**: [Explain why this pattern is problematic] +**Why safe to remove**: [Explain why functionality is preserved] +**Impact**: None - purely cosmetic improvement + +--- + +### Skipped Issues (Preserved for Safety) + +#### Skipped 1: Line X +**Reason**: [Why you chose not to change this] + +### Summary +- Removed N obvious comments +- Simplified M defensive patterns +- Flattened K nested structures +- Preserved L patterns that looked like slop but serve purpose +\`\`\` + +--- + +## SAFETY RULES + +1. **NEVER remove error handling for I/O, network, or file operations** +2. **NEVER simplify validation for user input or external data** +3. **NEVER change public API signatures** +4. **NEVER remove type hints (even redundant-looking ones)** +5. **If a pattern appears in multiple places, it might be intentional - ASK before bulk removal** +6. **Preserve all BDD test comments (#given, #when, #then)** + +When finished, your report should be detailed enough that a reviewer can understand EXACTLY what changed and feel confident the changes are safe. + +--- + +## WHEN NO SLOP FOUND + +If the file is clean, report: + +\`\`\` +## AI Slop Analysis: {filename} + +### Result: No AI Slop Detected + +This file is clean. Here's why: + +**Comments**: N comments found, all explain WHY not WHAT +**Defensive Code**: Null checks present are appropriate (e.g., checks external API response) +**Code Structure**: Maximum nesting depth acceptable, early returns used appropriately + +**Conclusion**: This code appears to be human-written or well-reviewed AI code. No changes needed. +\`\`\``, +} diff --git a/src/features/builtin-skills/skills/index.ts b/src/features/builtin-skills/skills/index.ts index 073930865..414e81002 100644 --- a/src/features/builtin-skills/skills/index.ts +++ b/src/features/builtin-skills/skills/index.ts @@ -3,3 +3,5 @@ export { playwrightCliSkill } from "./playwright-cli" export { frontendUiUxSkill } from "./frontend-ui-ux" export { gitMasterSkill } from "./git-master" export { devBrowserSkill } from "./dev-browser" +export { reviewWorkSkill } from "./review-work" +export { aiSlopRemoverSkill } from "./ai-slop-remover" diff --git a/src/features/builtin-skills/skills/review-work.ts b/src/features/builtin-skills/skills/review-work.ts new file mode 100644 index 000000000..73958b04b --- /dev/null +++ b/src/features/builtin-skills/skills/review-work.ts @@ -0,0 +1,536 @@ +import type { BuiltinSkill } from "../types" + +export const reviewWorkSkill: BuiltinSkill = { + name: "review-work", + description: + "Post-implementation review orchestrator. Launches 5 parallel background sub-agents: Oracle (goal/constraint verification), Oracle (code quality), Oracle (security), unspecified-high (hands-on QA execution), unspecified-high (context mining from GitHub/git/Slack/Notion). All must pass for review to pass. MUST USE after completing any significant implementation work. Triggers: 'review work', 'review my work', 'review changes', 'QA my work', 'verify implementation', 'check my work', 'validate changes', 'post-implementation review'.", + template: `# Review Work — 5-Agent Parallel Review Orchestrator + +Launch 5 specialized sub-agents in parallel to review completed implementation work from every angle. All 5 must pass for the review to pass. If even ONE fails, the review fails. + +The 5 agents cover complementary concerns — together they form a comprehensive review that no single reviewer could match: + +| # | Agent | Type | Role | Focus Level | +|---|-------|------|------|-------------| +| 1 | Goal Verifier | Oracle | Did we build what was asked? | MAIN | +| 2 | QA Executor | unspecified-high | Does it actually work? | MAIN | +| 3 | Code Reviewer | Oracle | Is the code well-written? | MAIN | +| 4 | Security Auditor | Oracle | Is it secure? | SUB | +| 5 | Context Miner | unspecified-high | Did we miss any context? | MAIN | + +--- + +## Phase 0: Gather Review Context + +Before launching agents, collect these inputs. Extract from conversation history first — the user's original request, constraints discussed, and decisions made are usually already in the thread. Only ask if truly missing. + + + +- **GOAL**: The original objective. What was the user trying to achieve? Pull from the initial request in this conversation. +- **CONSTRAINTS**: Rules, requirements, or limitations. Tech stack restrictions, performance targets, API contracts, design patterns to follow, backward compatibility needs. +- **BACKGROUND**: Why this work was needed. Business context, user stories, related systems, prior decisions that informed the approach. +- **CHANGED_FILES**: Auto-collect via \`git diff --name-only HEAD~1\` or against the appropriate base (branch point, specific commit). +- **DIFF**: Auto-collect via \`git diff HEAD~1\` or against the appropriate base. +- **FILE_CONTENTS**: Read the full content of each changed file (not just the diff). Oracle agents cannot read files — they need full context in the prompt. +- **RUN_COMMAND**: How to start/run the application. Check \`package.json\` scripts, \`Makefile\`, \`docker-compose.yml\`, or ask the user. + + + + +**NEVER CHECKOUT A PR BRANCH IN THE MAIN WORKTREE. ALWAYS CREATE A NEW GIT WORKTREE (\`git worktree add\`) AND WORK THERE. THIS PREVENTS CONTAMINATING THE USER'S WORKING DIRECTORY WITH UNRELATED BRANCH STATE.** + +**Auto-collection sequence:** + +\`\`\`bash +# 1. Get changed files +git diff --name-only HEAD~1 # or: git diff --name-only main...HEAD + +# 2. Get diff +git diff HEAD~1 # or: git diff main...HEAD + +# 3. Detect run command +# Check package.json -> "scripts.dev" or "scripts.start" +# Check Makefile -> default target +# Check docker-compose.yml -> services +\`\`\` + +For GOAL, CONSTRAINTS, BACKGROUND — review the full conversation history. The user's original message almost always contains the goal. Constraints often emerge during discussion. If anything critical is ambiguous, ask ONE focused question — not a checklist. + +--- + +## Phase 1: Launch 5 Agents + +Launch ALL 5 in a single turn. Every agent uses \`run_in_background=true\`. No sequential launches. No waiting between them. + +**Oracle agents receive everything in the prompt** (they cannot read files or run commands). Include DIFF + FILE_CONTENTS + all context directly in the prompt text. + +**unspecified-high agents are autonomous** — they can read files, run commands, and use tools. Give them goals and pointers, not raw content dumps. + +--- + +### Agent 1: Goal & Constraint Verification (Oracle) — MAIN + +This agent answers: "Did we build exactly what was asked, within the rules we were given?" + +\`\`\` +task( + subagent_type="oracle", + run_in_background=true, + load_skills=[], + description="Verify implementation against original goal and constraints", + prompt=""" +GOAL & CONSTRAINT VERIFICATION + + +{GOAL — paste the user's original request and any clarifications} + + + +{CONSTRAINTS — every rule, requirement, or limitation discussed} + + + +{BACKGROUND — why this work was needed, broader context} + + + +{CHANGED_FILES — list of modified file paths} + + + +{FILE_CONTENTS — full content of every changed file, clearly delimited per file} + + + +{DIFF — the actual git diff} + + +Review whether this implementation correctly and completely achieves the stated goal within the given constraints. Be obsessively thorough — the point of this review is to catch what the implementer missed. + +REVIEW CHECKLIST: + +1. **Goal Completeness**: Break the goal into every sub-requirement (explicit AND implied). For each, mark ACHIEVED / MISSED / PARTIAL. Missing even one implied requirement that a reasonable engineer would have addressed = PARTIAL at minimum. + +2. **Constraint Compliance**: List every constraint. For each, verify compliance with specific code evidence. A constraint violated = automatic FAIL. + +3. **Requirement Gaps**: Requirements the user clearly wanted but didn't spell out. Things implied by the goal or background that a thoughtful engineer would have included. + +4. **Over-Engineering**: Anything added that wasn't requested — unnecessary abstractions, extra features, premature optimizations, speculative generality. Flag these as scope creep. + +5. **Edge Cases**: Given the goal, what inputs or scenarios would break this? Trace through at least 5 edge cases mentally. + +6. **Behavioral Correctness**: Walk through the code logic for 3+ representative scenarios. Does the code actually produce the expected behavior in each case? + +OUTPUT FORMAT: +PASS or FAIL +HIGH / MEDIUM / LOW +1-3 sentence overall assessment + + For each sub-requirement: + - [ACHIEVED/MISSED/PARTIAL] Requirement description + - Evidence: specific code reference or gap + + + For each constraint: + - [ACHIEVED/MISSED] Constraint description — evidence + + + - [PASS/FAIL/WARN] Category: Description + - File: path (line range if applicable) + - Evidence: specific code or logic reference + +Issues that MUST be fixed. Empty if PASS. +""") +\`\`\` + +--- + +### Agent 2: QA via App Execution (unspecified-high) — MAIN + +This agent answers: "Does it actually work when you run it?" + +The QA agent follows a structured process: brainstorm scenarios exhaustively first, then self-review and augment, then create a task list, then execute systematically. + +\`\`\` +task( + category="unspecified-high", + run_in_background=true, + load_skills=["playwright", "dev-browser"], + description="QA by actually running and using the application", + prompt=""" +QA — HANDS-ON APP EXECUTION + + +{GOAL} + + + +{CONSTRAINTS} + + + +{CHANGED_FILES} + + + +{RUN_COMMAND — how to start the application, or "unknown" if not determined} + + +You are a QA engineer. Your job is to RUN the application and verify it works through hands-on testing. You do not review code — you test behavior. + +MANDATORY PROCESS (follow in order): + +### Step 1: Scenario Brainstorm + +Before touching the app, write down EVERY test scenario you can think of. Be exhaustive. Think about: + +- **Happy paths**: The primary use cases this implementation enables. What's the main thing the user wanted to do? +- **Boundary conditions**: Empty inputs, maximum-length inputs, zero values, negative numbers, special characters, unicode, very large datasets. +- **Error paths**: Invalid inputs, network failures, missing files, permission denied, timeout conditions. +- **Regression scenarios**: Existing features that touch the same code paths. Things that worked before and must still work. +- **State transitions**: What happens when you do things out of order? Rapid repeated actions? Concurrent usage? +- **UX scenarios** (if applicable): Layout on different sizes, keyboard navigation, screen reader compatibility, loading states, error messages. +- **Integration points**: Does this feature interact with external services, databases, or other modules? Test those boundaries. + +Write each scenario as a one-liner with expected behavior. Aim for 15-30 scenarios minimum. + +### Step 2: Scenario Augmentation + +Review your scenario list with fresh eyes. For each scenario, ask: +- "What could go wrong here that I haven't considered?" +- "What would a malicious or careless user do?" +- "What environmental conditions could affect this?" (disk full, slow network, expired tokens) + +Add at least 5 more scenarios from this reflection. Group scenarios by priority: P0 (must pass), P1 (should pass), P2 (nice to pass). + +### Step 3: Create Task List + +Convert your augmented scenario list into a structured task list (use TaskCreate/TaskUpdate or your todo system). Each task = one test scenario with: +- Test name +- Steps to execute +- Expected result +- Priority (P0/P1/P2) + +### Step 4: Execute Systematically + +Work through the task list in priority order (P0 first). For each test: + +1. Execute the test steps +2. Record actual result +3. Compare with expected result +4. Mark PASS or FAIL +5. If FAIL: capture evidence (screenshot, terminal output, error message) +6. Mark the task complete + +**Execution guidance by app type:** +- **Web app**: Use playwright/dev-browser to navigate, click, fill forms, verify visual output. +- **CLI tool**: Run commands with various arguments, pipe inputs, check exit codes and output. +- **Library/SDK**: Write and execute a test script that imports and exercises the public API. +- **Backend API**: Use curl/httpie to hit endpoints with various payloads, verify response codes and bodies. +- **Mobile/Desktop**: If not directly runnable, write integration tests and execute them. + +If the app cannot be started (build failure), that's an immediate FAIL — no need to continue. + +### Step 5: Compile Results + +OUTPUT FORMAT: +PASS or FAIL +HIGH / MEDIUM / LOW +1-3 sentence overall assessment + + Total scenarios: N + P0: X tested, Y passed + P1: X tested, Y passed + P2: X tested, Y passed + + + For each test: + - [PASS/FAIL] Test name (Priority) + - Steps: What you did + - Expected: What should happen + - Actual: What actually happened + - Evidence: Screenshot path or terminal output snippet (if FAIL) + +P0 or P1 failures only. Empty if PASS. +""") +\`\`\` + +--- + +### Agent 3: Code Quality Review (Oracle) — MAIN + +This agent answers: "Is the code well-written, maintainable, and consistent with the codebase?" + +\`\`\` +task( + subagent_type="oracle", + run_in_background=true, + load_skills=[], + description="Review overall code quality, patterns, and architecture", + prompt=""" +CODE QUALITY REVIEW + + +{CHANGED_FILES} + + + +{FILE_CONTENTS — full content of changed files AND neighboring files that show existing patterns} + + + +{DIFF} + + + +{BACKGROUND} + + +You are a senior staff engineer conducting a code review. Your standard: "Would I approve this PR without comments?" + +REVIEW DIMENSIONS (examine each): + +1. **Correctness**: Logic errors, off-by-one, null/undefined handling, race conditions, resource leaks, unhandled promise rejections. + +2. **Pattern Consistency**: Does new code follow the codebase's established patterns? Compare with the neighboring files provided. Introducing a new pattern where one already exists = finding. + +3. **Naming & Readability**: Clear variable/function/type names? Self-documenting code? Would another engineer understand this without explanation? + +4. **Error Handling**: Errors properly caught, logged, and propagated? No empty catch blocks? No swallowed errors? User-facing errors are helpful? + +5. **Type Safety**: Any \`as any\`, \`@ts-ignore\`, \`@ts-expect-error\`? Proper generic usage? Correct type narrowing? (If TypeScript/typed language) + +6. **Performance**: N+1 queries? Unnecessary re-renders? Blocking I/O on hot paths? Memory leaks? Unbounded growth? + +7. **Abstraction Level**: Right level of abstraction? No copy-paste duplication? But also no premature over-abstraction? + +8. **Testing**: New behaviors covered by tests? Tests are meaningful, not just coverage padding? Test names describe scenarios? + +9. **API Design**: Public interfaces clean and consistent with existing APIs? Breaking changes flagged? + +10. **Tech Debt**: Does this introduce new tech debt? Or create coupling that will be painful to change? + +Categorize each finding by severity: +- **CRITICAL**: Will cause bugs, data loss, or crashes in production +- **MAJOR**: Significant quality issue that should be fixed before merge +- **MINOR**: Improvement worth making but not blocking +- **NITPICK**: Style preference, optional + +OUTPUT FORMAT: +PASS or FAIL +HIGH / MEDIUM / LOW +1-3 sentence overall assessment + + - [CRITICAL/MAJOR/MINOR/NITPICK] Category: Description + - File: path (line range) + - Current: what the code does now + - Suggestion: how to improve + +CRITICAL and MAJOR items only. Empty if PASS. +""") +\`\`\` + +--- + +### Agent 4: Security Review (Oracle) — SUB + +This agent answers: "Are there security vulnerabilities in these changes?" + +This is supplementary — it focuses exclusively on security. It does NOT comment on code style, architecture, or functionality unless those directly create a security risk. + +\`\`\` +task( + subagent_type="oracle", + run_in_background=true, + load_skills=[], + description="Security-focused review of implementation changes", + prompt=""" +SECURITY REVIEW (supplementary) + + +{CHANGED_FILES} + + + +{FILE_CONTENTS — full content of changed files} + + + +{DIFF} + + +You are a security engineer. Review this diff exclusively for security vulnerabilities and anti-patterns. Ignore code style, naming, architecture — unless it directly creates a security risk. + +SECURITY CHECKLIST: + +1. **Input Validation**: User inputs sanitized? SQL injection, XSS, command injection, SSRF vectors? +2. **Auth & AuthZ**: Authentication checks where needed? Authorization verified for each action? Privilege escalation paths? +3. **Secrets & Credentials**: Hardcoded secrets, API keys, tokens in code or config? Secrets in logs? +4. **Data Exposure**: Sensitive data in logs? PII in error messages? Over-exposed API responses? +5. **Dependencies**: New dependencies added? Known CVEs? Suspicious or unnecessary packages? +6. **Cryptography**: Proper algorithms? No custom crypto? Secure random? Proper key management? +7. **File & Path**: Path traversal? Unsafe file operations? Symlink following? +8. **Network**: CORS configured correctly? Rate limiting? TLS enforced? Certificate validation? +9. **Error Leakage**: Stack traces exposed to users? Internal details in error responses? +10. **Supply Chain**: Lockfile updated consistently? Dependency pinning? + +OUTPUT FORMAT: +PASS or FAIL +CRITICAL / HIGH / MEDIUM / LOW / NONE +1-3 sentence overall assessment + + - [CRITICAL/HIGH/MEDIUM/LOW] Category: Description + - File: path (line range) + - Risk: What could an attacker do? + - Remediation: Specific fix + +CRITICAL and HIGH items only. Empty if PASS. +""") +\`\`\` + +--- + +### Agent 5: Context Mining (unspecified-high) — MAIN + +This agent answers: "Did we miss any context that should have informed this implementation?" + +\`\`\` +task( + category="unspecified-high", + run_in_background=true, + load_skills=["git-master"], + description="Mine all accessible contexts for missed requirements or background knowledge", + prompt=""" +CONTEXT MINING — MISSED REQUIREMENTS & BACKGROUND + + +{GOAL} + + + +{CONSTRAINTS} + + + +{CHANGED_FILES} + + + +{BACKGROUND} + + +You are an investigator. Your mission: search every accessible information source to find context that should have informed this implementation but might have been missed. The question: "Is there something we should have known but didn't?" + +SOURCES TO SEARCH (use every available tool): + +1. **Git History** (ALWAYS search): + - \`git log --oneline -20 -- {each changed file}\` — recent changes and their reasons + - \`git blame {critical sections}\` — who wrote what and when + - \`git log --all --grep="{keywords from goal}"\` — related commits + - Look for reverted commits, TODO/FIXME/HACK comments in history + +2. **GitHub** (if \`gh\` CLI available): + - \`gh issue list --search "{keywords}"\` — related open/closed issues + - \`gh pr list --search "{keywords}" --state all\` — related PRs and their review comments + - Check if any issue is specifically linked to this work + - Look at review comments on past PRs touching these files + +3. **Communication Channels** (if MCP tools available): + - Slack: search for messages mentioning the feature, file names, or related keywords + - Notion: search for design docs, RFCs, ADRs related to this feature + - Discord: relevant discussions + +4. **Codebase Cross-References** (ALWAYS search): + - Files that import or reference the changed modules + - Tests that might need updating due to behavior changes + - Documentation (README, docs/, comments) that references changed behavior + - Config files that might need corresponding updates + - Related features in the same domain + +WHAT TO LOOK FOR: + +- Requirements mentioned in issues/PRs that the implementation misses +- Past decisions explaining WHY code was written a certain way — and whether new changes respect those reasons +- Related systems or features affected by these changes +- Warnings from previous developers (PR review comments, inline TODOs, commit messages) +- Migration or deprecation notes that affect the changed code +- Design decisions documented outside the codebase (Notion, Slack, ADRs) + +OUTPUT FORMAT: +PASS or FAIL +HIGH / MEDIUM / LOW +1-3 sentence overall assessment + + - [SEARCHED/SKIPPED] Source name — what was searched (or why it wasn't accessible) + + + For each discovery: + - Source: Where found (git commit abc123, GitHub issue #42, Slack message, etc.) + - Finding: What was found + - Relevance: How it relates to the current work + - Impact: [BLOCKING / IMPORTANT / FYI] + +Requirements the implementation should address but doesn't. Empty if none. +BLOCKING items only. Empty if PASS. +""") +\`\`\` + +--- + +## Phase 2: Wait & Collect + +After launching all 5 agents in one turn, **end your response**. Wait for system notifications as each agent completes. + +As each completes, collect via \`background_output(task_id="...")\`. Store each verdict: + +| Agent | Verdict | Notes | +|-------|---------|-------| +| 1. Goal Verification | pending | — | +| 2. QA Execution | pending | — | +| 3. Code Quality | pending | — | +| 4. Security | pending | — | +| 5. Context Mining | pending | — | + +Do NOT deliver the final report until ALL 5 have completed. + +--- + +## Phase 3: Deliver Verdict + + + +ALL 5 agents returned PASS → **REVIEW PASSED** +ANY agent returned FAIL → **REVIEW FAILED — criteria not met** + + + +Compile the final report in this format: + +\`\`\`markdown +# Review Work — Final Report + +## Overall Verdict: PASSED / FAILED + +| # | Review Area | Agent Type | Verdict | Confidence | +|---|------------|------------|---------|------------| +| 1 | Goal & Constraint Verification | Oracle | PASS/FAIL | HIGH/MED/LOW | +| 2 | QA Execution | unspecified-high | PASS/FAIL | HIGH/MED/LOW | +| 3 | Code Quality | Oracle | PASS/FAIL | HIGH/MED/LOW | +| 4 | Security (supplementary) | Oracle | PASS/FAIL | Severity | +| 5 | Context Mining | unspecified-high | PASS/FAIL | HIGH/MED/LOW | + +## Blocking Issues +[Aggregated from all agents — deduplicated, prioritized] + +## Key Findings +[Top 5-10 most important findings across all agents, grouped by theme] + +## Recommendations +[If FAILED: exactly what to fix, in priority order] +[If PASSED: non-blocking suggestions worth considering] +\`\`\` + +If FAILED — be specific. The user should know exactly what to fix and in what order. No vague "consider improving X" — state the problem, the file, and the fix. + +If PASSED — keep it short. Highlight any non-blocking suggestions, but don't turn a passing review into a lecture.`, +}