Commit ec54b17be68c

Vincent Demeester <vincent@sbr.pm>
2026-03-26 15:21:00
refactor: dynamically discover reviewer agents in review.ts
Replaced hardcoded focus area map with runtime discovery of reviewer-*.md agents from ~/.pi/agent/agents/. Adding a new reviewer (e.g. reviewer-go.md) now automatically appears in the /review focus selector without code changes. Removed unused diffContainsNixFiles and loadReviewRubric.
1 parent 0b9eb59
Changed files (1)
dots
pi
agent
extensions
dots/pi/agent/extensions/review.ts
@@ -32,7 +32,7 @@ import type { ExtensionAPI, ExtensionContext, ExtensionCommandContext } from "@m
 import { DynamicBorder, BorderedLoader } from "@mariozechner/pi-coding-agent";
 import { Container, type SelectItem, SelectList, Text, Key } from "@mariozechner/pi-tui";
 import path from "node:path";
-import { promises as fs } from "node:fs";
+import { promises as fs, readdirSync, readFileSync } from "node:fs";
 import { execSync } from "node:child_process";
 
 // =============================================================================
@@ -382,76 +382,76 @@ const FOLDER_REVIEW_PROMPT =
 	"Review the code in the following paths: {paths}. This is a snapshot review (not a diff). Read the files directly in these paths and provide prioritized, actionable findings.";
 
 // =============================================================================
-// Review Focus Areas (dispatched to specialized subagents)
+// Review Focus Areas (dynamically discovered from reviewer agents)
 // =============================================================================
 
-type ReviewFocus = "general" | "security" | "performance" | "nix" | "full";
-
-const FOCUS_AGENT_MAP: Record<Exclude<ReviewFocus, "full">, string> = {
-	general: "reviewer",
-	security: "reviewer-security",
-	performance: "reviewer-performance",
-	nix: "reviewer-nix",
-};
-
-const FOCUS_DESCRIPTIONS: Record<ReviewFocus, { label: string; description: string }> = {
-	general: { label: "General", description: "Bugs, logic errors, maintainability, code smells" },
-	security: { label: "Security", description: "OWASP, injection, auth, secrets, untrusted input" },
-	performance: { label: "Performance", description: "Complexity, allocations, caching, concurrency" },
-	nix: { label: "Nix", description: "Nix idioms, module patterns, eval cost, reproducibility" },
-	full: { label: "Full review", description: "All focus areas in parallel" },
-};
-
-// Rubric is loaded from the CodeReview skill at ~/.config/claude/skills/CodeReview/rubric.md
-const RUBRIC_PATH = path.join(
-	process.env.HOME || process.env.USERPROFILE || "",
-	".config", "claude", "skills", "CodeReview", "rubric.md",
-);
-
-async function loadReviewRubric(): Promise<string> {
-	try {
-		return await fs.readFile(RUBRIC_PATH, "utf-8");
-	} catch {
-		// Fallback: minimal rubric if skill file is missing
-		return "# Review Guidelines\n\nReview the code for bugs, security issues, performance problems, and maintainability. Tag findings with priority [P0]-[P3]. Provide file paths and line numbers.";
-	}
+interface ReviewerAgent {
+	/** Focus key, e.g. "general", "security", "go" */
+	focus: string;
+	/** Agent name from frontmatter, e.g. "reviewer-security" */
+	agent: string;
+	/** Human-readable label, e.g. "Security" */
+	label: string;
+	/** Description from agent frontmatter */
+	description: string;
 }
 
 /**
- * Detect whether the diff contains .nix files
+ * Discover reviewer agents from ~/.pi/agent/agents/reviewer*.md
+ * The base `reviewer.md` is treated as "general".
+ * Agents named `reviewer-<focus>.md` become focus areas.
+ * Descriptions are read from the agent frontmatter.
  */
-async function diffContainsNixFiles(pi: ExtensionAPI, target: ReviewTarget): Promise<boolean> {
+function discoverReviewerAgents(): ReviewerAgent[] {
+	const home = process.env.HOME || process.env.USERPROFILE || "";
+	const agentsDir = path.join(home, ".pi", "agent", "agents");
+
+	let entries: string[];
 	try {
-		let result: { stdout: string; code: number };
-		switch (target.type) {
-			case "baseBranch":
-				result = await pi.exec("git", ["diff", "--name-only", target.branch]);
-				break;
-			case "commit":
-				result = await pi.exec("git", ["diff", "--name-only", `${target.sha}^`, target.sha]);
-				break;
-			case "pullRequest": {
-				const execOpts = target.cloneDir ? { cwd: target.cloneDir } : undefined;
-				result = await pi.exec("git", ["diff", "--name-only", target.baseBranch], execOpts);
-				break;
-			}
-			case "uncommitted":
-				result = await pi.exec("git", ["diff", "--name-only", "HEAD"]);
-				break;
-			case "folder":
-				// Check if any of the paths contain .nix files
-				for (const p of target.paths) {
-					const findResult = await pi.exec("find", [p, "-name", "*.nix", "-type", "f", "-maxdepth", "3"]);
-					if (findResult.code === 0 && findResult.stdout.trim()) return true;
-				}
-				return false;
-			default:
-				return false;
-		}
-		return result.code === 0 && /\.nix$/m.test(result.stdout);
+		entries = readdirSync(agentsDir).filter(
+			(f) => f.startsWith("reviewer") && f.endsWith(".md"),
+		);
 	} catch {
-		return false;
+		return [];
 	}
+
+	const agents: ReviewerAgent[] = [];
+	for (const filename of entries.sort()) {
+		const filePath = path.join(agentsDir, filename);
+		let content: string;
+		try {
+			content = readFileSync(filePath, "utf-8");
+		} catch {
+			continue;
+		}
+
+		// Parse YAML frontmatter
+		const fmMatch = content.match(/^---\n([\s\S]*?)\n---/);
+		if (!fmMatch) continue;
+
+		const fm = fmMatch[1];
+		const nameMatch = fm.match(/^name:\s*(.+)$/m);
+		const descMatch = fm.match(/^description:\s*(.+)$/m);
+		if (!nameMatch) continue;
+
+		const agentName = nameMatch[1].trim();
+		const description = descMatch ? descMatch[1].trim() : agentName;
+
+		// Derive focus key: "reviewer" โ†’ "general", "reviewer-security" โ†’ "security"
+		let focus: string;
+		let label: string;
+		if (filename === "reviewer.md") {
+			focus = "general";
+			label = "General";
+		} else {
+			focus = filename.replace(/^reviewer-/, "").replace(/\.md$/, "");
+			label = focus.charAt(0).toUpperCase() + focus.slice(1);
+		}
+
+		agents.push({ focus, agent: agentName, label, description });
+	}
+
+	return agents;
 }
 
 async function loadProjectReviewGuidelines(cwd: string): Promise<string | null> {
@@ -1072,19 +1072,32 @@ export default function reviewExtension(pi: ExtensionAPI) {
 	}
 
 	/**
-	 * Show focus area selector for the review
+	 * Show focus area selector for the review.
+	 * Dynamically discovers reviewer-*.md agents from ~/.pi/agent/agents/.
+	 * Returns the focus key ("general", "security", "full", etc.) or null if cancelled.
 	 */
-	async function showFocusSelector(ctx: ExtensionContext, hasNixFiles: boolean): Promise<ReviewFocus | null> {
-		const focusOptions: ReviewFocus[] = ["general", "security", "performance"];
-		if (hasNixFiles) focusOptions.push("nix");
-		focusOptions.push("full");
+	async function showFocusSelector(ctx: ExtensionContext): Promise<{ focus: string; agents: ReviewerAgent[] } | null> {
+		// Discover all reviewer agents fresh each time (allows adding agents mid-session)
+		const allAgents = discoverReviewerAgents();
 
-		const items: SelectItem[] = focusOptions.map((focus) => ({
-			value: focus,
-			label: FOCUS_DESCRIPTIONS[focus].label,
-			description: FOCUS_DESCRIPTIONS[focus].description,
+		if (allAgents.length === 0) {
+			ctx.ui.notify("No reviewer agents found in ~/.pi/agent/agents/", "error");
+			return null;
+		}
+
+		const items: SelectItem[] = allAgents.map((ra) => ({
+			value: ra.focus,
+			label: ra.label,
+			description: ra.description,
 		}));
 
+		// Add "full" option at the end
+		items.push({
+			value: "full",
+			label: "Full review",
+			description: `All ${allAgents.length} reviewers in parallel`,
+		});
+
 		const result = await ctx.ui.custom<string | null>((tui, theme, _kb, done) => {
 			const container = new Container();
 			container.addChild(new DynamicBorder((str) => theme.fg("accent", str)));
@@ -1119,7 +1132,8 @@ export default function reviewExtension(pi: ExtensionAPI) {
 			};
 		});
 
-		return result as ReviewFocus | null;
+		if (!result) return null;
+		return { focus: result, agents: allAgents };
 	}
 
 	/**
@@ -1333,7 +1347,7 @@ export default function reviewExtension(pi: ExtensionAPI) {
 	/**
 	 * Execute the review
 	 */
-	async function executeReview(ctx: ExtensionCommandContext, target: ReviewTarget, useFreshSession: boolean, focus: ReviewFocus): Promise<void> {
+	async function executeReview(ctx: ExtensionCommandContext, target: ReviewTarget, useFreshSession: boolean, focus: string, reviewerAgents: ReviewerAgent[]): Promise<void> {
 		// Check if we're already in a review
 		if (reviewOriginId) {
 			ctx.ui.notify("Already in a review. Use /end-review to finish first.", "warning");
@@ -1417,7 +1431,9 @@ export default function reviewExtension(pi: ExtensionAPI) {
 			taskContext += repoRules;
 		}
 
-		const focusLabel = FOCUS_DESCRIPTIONS[focus].label;
+		// Look up the selected agent (or all agents for "full")
+		const selectedAgent = reviewerAgents.find((ra) => ra.focus === focus);
+		const focusLabel = focus === "full" ? "Full" : (selectedAgent?.label ?? focus);
 		const modeHint = useFreshSession ? " (fresh session)" : "";
 		ctx.ui.notify(`Starting ${focusLabel} review: ${hint}${modeHint}`, "info");
 
@@ -1438,19 +1454,13 @@ export default function reviewExtension(pi: ExtensionAPI) {
 
 		// Dispatch to subagent(s) based on focus area
 		if (focus === "full") {
-			// Determine which agents to run in parallel
-			const hasNix = await diffContainsNixFiles(pi, target);
-			const agents: Array<Exclude<ReviewFocus, "full">> = ["general", "security", "performance"];
-			if (hasNix) agents.push("nix");
-
-			const agentNames = agents.map((f) => FOCUS_AGENT_MAP[f]);
-			const taskDescriptions = agents.map((f) => `${FOCUS_DESCRIPTIONS[f].label}-focused review: ${taskContext}`);
-
-			const subagentTasks = agents.map((f, i) => ({
-				agent: agentNames[i],
-				task: taskDescriptions[i],
+			// Run all discovered reviewer agents in parallel
+			const subagentTasks = reviewerAgents.map((ra) => ({
+				agent: ra.agent,
+				task: `${ra.label}-focused review: ${taskContext}`,
 			}));
 
+			const focusNames = reviewerAgents.map((ra) => ra.focus).join("/");
 			const tasksJson = JSON.stringify(subagentTasks);
 			const subagentPrompt = `Run a full code review using parallel subagents. Dispatch the following reviewers using the subagent tool in parallel mode:
 
@@ -1458,24 +1468,24 @@ ${tasksJson}
 
 After all reviewers complete, present a **consolidated review report**:
 1. Group findings by file, deduplicating overlapping issues (keep the most specific)
-2. Note which reviewer (general/security/performance${hasNix ? "/nix" : ""}) flagged each issue
+2. Note which reviewer (${focusNames}) flagged each issue
 3. Use the highest priority tag when reviewers disagree
 4. Provide a unified verdict: "correct" if no P0/P1 issues, "needs attention" otherwise`;
 
 			pi.sendUserMessage(subagentPrompt);
-		} else {
+		} else if (selectedAgent) {
 			// Single focused review โ€” dispatch to one subagent
-			const agentName = FOCUS_AGENT_MAP[focus];
-
 			const subagentPrompt = `Run a ${focusLabel.toLowerCase()}-focused code review using the subagent tool.
 
-Dispatch to the \`${agentName}\` agent with this task:
+Dispatch to the \`${selectedAgent.agent}\` agent with this task:
 
 ${taskContext}
 
 Present the subagent's findings directly to the user.`;
 
 			pi.sendUserMessage(subagentPrompt);
+		} else {
+			ctx.ui.notify(`No reviewer agent found for focus "${focus}"`, "error");
 		}
 	}
 
@@ -1695,10 +1705,9 @@ Present the subagent's findings directly to the user.`;
 				}
 
 				// Select review focus area
-				const hasNixFiles = await diffContainsNixFiles(pi, target);
-				const focus = await showFocusSelector(ctx, hasNixFiles);
+				const focusResult = await showFocusSelector(ctx);
 
-				if (!focus) {
+				if (!focusResult) {
 					if (fromSelector) {
 						target = null;
 						continue;
@@ -1731,7 +1740,7 @@ Present the subagent's findings directly to the user.`;
 				}
 				// If messageCount === 0, useFreshSession stays false (current session mode)
 
-				await executeReview(ctx, target, useFreshSession, focus);
+				await executeReview(ctx, target, useFreshSession, focusResult.focus, focusResult.agents);
 				return;
 			}
 		},