feat(decopilot): add tool approval workflow

.cursor/skills/plan-critique/SKILL.md → .cursor/skills/review-plan/SKILL.md

@@ -1,9 +1,9 @@
 ---
-name: plan-critique
+name: review-plan
 description: Spawn parallel subagents to criticize implementation plans from multiple perspectives (duplication, correctness, security, performance, testing, architecture, scope), then improve the plan based on feedback. Use when reviewing a plan before implementation or when stress-testing a plan for gaps.
 ---
 
-# Plan Critique
+# Review Plan
 
 Spawn parallel subagents, each critiquing a plan from a single perspective. Synthesize results into actionable feedback, then improve the plan and document decisions.
 
@@ -13,6 +13,7 @@ Spawn parallel subagents, each critiquing a plan from a single perspective. Synt
 - After drafting a plan in superpowers:writing-plans
 - When a plan feels risky or complex
 - When you want diverse criticism without one reviewer dominating
+- When a plan references specific frameworks/APIs and you want to verify against official docs
 
 ## Critique Perspectives
 
@@ -27,6 +28,7 @@ Dispatch one subagent per perspective. Each critic is independent.
 | **Testing** | Coverage strategy, edge cases, integration needs |
 | **Architecture** | Fit with existing patterns, dependencies, layering |
 | **Scope** | YAGNI, scope creep, unnecessary features |
+| **Documentation** | *(Conditional)* API docs, guides, best practices alignment (when plan mentions docs, frameworks, or external APIs) |
 
 ## Workflow
 
@@ -38,6 +40,15 @@ Ensure the plan is in context (read the file or paste it). Note the plan path fo
 
 For each perspective, spawn a subagent with a focused prompt. Use the Task tool with the critic template below.
 
+**Standard critics (always dispatch these 7):**
+- Duplication, Correctness, Security, Performance, Testing, Architecture, Scope
+
+**Documentation critic (dispatch if):**
+- Plan mentions specific frameworks, libraries, or external APIs
+- Plan references documentation or docs lookup
+- Plan involves integrating with third-party services
+- You need to verify best practices from official sources
+
 **Template per critic:**
 
 ```
@@ -48,7 +59,7 @@ Critique the implementation plan from the [{PERSPECTIVE}] perspective.
 **Your role:** You are a critical reviewer focused ONLY on [{PERSPECTIVE}]. Be skeptical. Find gaps, risks, and oversights.
 
 **Check:**
-[PERSPECTIVE-SPECIFIC CHECKS - see references/critic-prompts.md]
+[PERSPECTIVE-SPECIFIC CHECKS - see below]
 
 **Output format:**
 - **Issues found:** (Blocker / Important / Minor)
@@ -57,7 +68,34 @@ Critique the implementation plan from the [{PERSPECTIVE}] perspective.
 - **Verdict:** Ready / Needs changes / High risk
 ```
 
-For full prompts per perspective, see [references/critic-prompts.md](references/critic-prompts.md).
+#### Documentation Critic Template (when applicable)
+
+```
+Review the implementation plan against official documentation and best practices.
+
+**Plan:** [path or paste plan content]
+
+**Your role:** You are a documentation expert. Look up official docs, guides, and best practices for the technologies mentioned in the plan.
+
+**Steps:**
+1. Identify frameworks, libraries, APIs, or services mentioned in the plan
+2. Search for and read relevant official documentation
+3. Check if the plan's approach aligns with recommended patterns
+4. Identify any deprecated methods, better alternatives, or missing configurations
+
+**Check:**
+- Does the plan use recommended/current APIs and patterns?
+- Are there official best practices the plan should follow?
+- Are there important configuration options or setup steps missing?
+- Are there newer or better approaches documented?
+- Are there known gotchas or warnings in the docs?
+
+**Output format:**
+- **Issues found:** (Blocker / Important / Minor) - cite specific docs
+- **Missing considerations:** What the docs recommend but plan doesn't address
+- **Recommendations:** Specific improvements with doc references
+- **Verdict:** Ready / Needs changes / High risk
+```
 
 ### 3. Synthesize Results
 
@@ -71,7 +109,7 @@ When all critics return:
 ### 4. Output Format
 
 ```markdown
-# Plan Critique Summary
+# Review Plan Summary
 
 ## Blockers
 [Must address before implementation]
@@ -137,7 +175,9 @@ Add to the plan after the improvement pass:
 **Adapted:** [Item] – [How you addressed it differently]
 ```
 
-## Example
+## Examples
+
+### Example 1: Standard Critique (7 critics)
 
 ```
 Plan: .cursor/plans/tasks_feature_implementation_a2a999e8.plan.md
@@ -158,10 +198,33 @@ Dispatch 7 subagents in parallel:
 [Add Critique Decisions section]
 ```
 
+### Example 2: With Documentation Lookup (8 critics)
+
+```
+Plan: .cursor/plans/react_query_integration_b3f441a2.plan.md
+(Plan mentions "React Query", "TanStack Query", and "data fetching patterns")
+
+Dispatch 8 subagents in parallel:
+- Task("Critique from Duplication perspective...")
+- Task("Critique from Correctness perspective...")
+- Task("Critique from Security perspective...")
+- Task("Critique from Performance perspective...")
+- Task("Critique from Testing perspective...")
+- Task("Critique from Architecture perspective...")
+- Task("Critique from Scope perspective...")
+- Task("Review against React Query official docs...") ← Documentation critic
+
+[Synthesize when all return] → Critique Summary (includes doc-based recommendations)
+
+[Improve plan] → Adopt/Reject/Adapt each item
+
+[Add Critique Decisions section]
+```
+
 ## Integration
 
 - **superpowers:writing-plans** – Use after drafting to critique before implementation
-- **plan-with-critique** – Runs plan first, then invokes this skill
+- **plan-with-critique** – Runs plan first, then invokes this skill (review-plan)
 - **superpowers:dispatching-parallel-agents** – Same pattern (independent domains)
 
 ## Red Flags
@@ -173,3 +236,5 @@ Dispatch 7 subagents in parallel:
 - **Don't** skip the Critique Decisions section (it clarifies your reasoning)
 - **Don't** let critics override project conventions (e.g., CLAUDE.md patterns)
 - **Don't** make the plan worse by over-incorporating feedback (e.g., scope creep from "add more")
+- **Don't** skip the documentation critic when plan references specific frameworks/APIs/libraries (docs often reveal better approaches or missing steps)
+- **Don't** dispatch documentation critic for generic plans without specific tech mentioned (wastes time)

.cursor/skills/review-critique/SKILL.md → .cursor/skills/review-pr/SKILL.md

@@ -1,9 +1,9 @@
 ---
-name: review-critique
+name: review-pr
 description: Analyze the git diff between the current branch and main from multiple perspectives (duplication, correctness, security, performance, testing, architecture, scope) using parallel subagents, then produce a remediation plan for issues found. Use when reviewing branch changes before merge, after implementation, or when the user asks to critique or review current code changes.
 ---
 
-# Review Critique
+# Review PR
 
 Spawn parallel subagents to critique the git diff of the current branch (vs main) from independent perspectives. Synthesize findings into a prioritized remediation plan.
 
@@ -92,7 +92,7 @@ When all critics return:
 ### 4. Critique Summary
 
 ```markdown
-# Review Critique Summary
+# Review PR Summary
 
 **Branch:** {branch_name}
 **Files changed:** {count}
@@ -169,6 +169,6 @@ Transform findings into an actionable plan. Group by file or concern, not by cri
 
 ## Integration
 
-- **plan-critique** — Similar pattern but for plans; use review-critique for code
+- **review-plan** — Similar pattern but for plans; use review-pr for code
 - **superpowers:verification-before-completion** — Run after remediation to confirm fixes
-- **superpowers:finishing-a-development-branch** — Use review-critique before finishing
+- **superpowers:finishing-a-development-branch** — Use review-pr before finishing

apps/mesh/src/api/routes/decopilot/built-in-tools/agent-search.ts

@@ -49,6 +49,7 @@ const description =
 
 export interface AgentSearchParams {
   organization: OrganizationScope;
+  needsApproval?: boolean;
 }
 
 const AGENT_SEARCH_ANNOTATIONS = {
@@ -69,12 +70,13 @@ export function createAgentSearchTool(
   params: AgentSearchParams,
   ctx: MeshContext,
 ) {
-  const { organization } = params;
+  const { organization, needsApproval } = params;
 
   return tool({
     description,
     inputSchema: zodSchema(AgentSearchInputSchema),
     outputSchema: zodSchema(AgentSearchOutputSchema),
+    needsApproval,
     execute: async ({ search_term }, options) => {
       const startTime = performance.now();
       try {

apps/mesh/src/api/routes/decopilot/built-in-tools/index.ts

@@ -7,6 +7,7 @@
 
 import type { MeshContext, OrganizationScope } from "@/core/mesh-context";
 import type { UIMessageStreamWriter } from "ai";
+import { toolNeedsApproval, type ToolApprovalLevel } from "../helpers";
 import { createAgentSearchTool } from "./agent-search";
 import { createSubtaskTool } from "./subtask";
 import { userAskTool } from "./user-ask";
@@ -16,6 +17,7 @@ export interface BuiltinToolParams {
   modelProvider: ModelProvider;
   organization: OrganizationScope;
   models: ModelsConfig;
+  toolApprovalLevel?: ToolApprovalLevel;
 }
 
 /**
@@ -28,14 +30,31 @@ export function getBuiltInTools(
   params: BuiltinToolParams,
   ctx: MeshContext,
 ) {
-  const { modelProvider, organization, models } = params;
+  const {
+    modelProvider,
+    organization,
+    models,
+    toolApprovalLevel = "none",
+  } = params;
   return {
     user_ask: userAskTool,
     subtask: createSubtaskTool(
       writer,
-      { modelProvider, organization, models },
+      {
+        modelProvider,
+        organization,
+        models,
+        needsApproval: toolNeedsApproval(toolApprovalLevel, false),
+      },
+      ctx,
+    ),
+    agent_search: createAgentSearchTool(
+      writer,
+      {
+        organization,
+        needsApproval: toolNeedsApproval(toolApprovalLevel, true),
+      },
       ctx,
     ),
-    agent_search: createAgentSearchTool(writer, { organization }, ctx),
   } as const;
 }

apps/mesh/src/api/routes/decopilot/built-in-tools/subtask.ts

@@ -63,6 +63,7 @@ export interface SubtaskParams {
   modelProvider: ModelProvider;
   organization: OrganizationScope;
   models: ModelsConfig;
+  needsApproval?: boolean;
 }
 
 const SUBTASK_ANNOTATIONS = {
@@ -104,11 +105,12 @@ export function createSubtaskTool(
   params: SubtaskParams,
   ctx: MeshContext,
 ) {
-  const { modelProvider, organization, models } = params;
+  const { modelProvider, organization, models, needsApproval } = params;
 
   return tool({
     description: SUBTASK_DESCRIPTION,
     inputSchema: zodSchema(SubtaskInputSchema),
+    needsApproval,
     execute: async function* (
       { prompt, agent_id },
       { abortSignal, toolCallId },
@@ -137,7 +139,7 @@ export function createSubtaskTool(
       );
 
       // ── 3. Load tools, excluding ones that shouldn't nest ──────────
-      const mcpTools = await toolsFromMCP(mcpClient);
+      const mcpTools = await toolsFromMCP(mcpClient, writer);
       const subagentTools = Object.fromEntries(
         Object.entries(mcpTools).filter(
           ([name]) => !SUBAGENT_EXCLUDED_TOOLS.includes(name),

apps/mesh/src/api/routes/decopilot/helpers.test.ts

@@ -0,0 +1,56 @@
+/**
+ * Tests for Decopilot Helper Functions
+ */
+
+import { describe, expect, test } from "bun:test";
+import { toolNeedsApproval, type ToolApprovalLevel } from "./helpers";
+
+describe("toolNeedsApproval", () => {
+  describe('approval level: "yolo"', () => {
+    const level: ToolApprovalLevel = "yolo";
+
+    test("returns false when readOnlyHint is true", () => {
+      expect(toolNeedsApproval(level, true)).toBe(false);
+    });
+
+    test("returns false when readOnlyHint is false", () => {
+      expect(toolNeedsApproval(level, false)).toBe(false);
+    });
+
+    test("returns false when readOnlyHint is undefined", () => {
+      expect(toolNeedsApproval(level, undefined)).toBe(false);
+    });
+  });
+
+  describe('approval level: "none"', () => {
+    const level: ToolApprovalLevel = "none";
+
+    test("returns true when readOnlyHint is true", () => {
+      expect(toolNeedsApproval(level, true)).toBe(true);
+    });
+
+    test("returns true when readOnlyHint is false", () => {
+      expect(toolNeedsApproval(level, false)).toBe(true);
+    });
+
+    test("returns true when readOnlyHint is undefined", () => {
+      expect(toolNeedsApproval(level, undefined)).toBe(true);
+    });
+  });
+
+  describe('approval level: "readonly"', () => {
+    const level: ToolApprovalLevel = "readonly";
+
+    test("returns false when readOnlyHint is true (auto-approve)", () => {
+      expect(toolNeedsApproval(level, true)).toBe(false);
+    });
+
+    test("returns true when readOnlyHint is false (requires approval)", () => {
+      expect(toolNeedsApproval(level, false)).toBe(true);
+    });
+
+    test("returns true when readOnlyHint is undefined (requires approval)", () => {
+      expect(toolNeedsApproval(level, undefined)).toBe(true);
+    });
+  });
+});