diff --git a/README.md b/README.md index 121c050c0f..89a0935aec 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,11 @@ defaults: workspace: worktree notifiers: [desktop] +# AO-local code reviewer backend (`ao review run`). Optional — defaults to the +# worker agent when it has a reviewer adapter, otherwise codex. +review: + agent: claude-code # claude-code | codex (or `command:` for a custom shell reviewer) + projects: my-app: repo: owner/my-app @@ -168,6 +173,8 @@ reactions: CI fails → agent gets the logs and fixes it. Reviewer requests changes → agent addresses them. PR approved with green CI → you get a notification to merge. +The AO-local reviewer (`ao review run`) picks its backend in this order: a `--command` flag, then `projects..review`, then the global `review` block, then your worker agent (`defaults.agent`) if it ships a reviewer adapter, and finally codex. Set `review.agent: claude-code` to review with Claude Code on hosts without Codex/OpenAI credentials. + Keep the `$schema` line so editors can autocomplete and validate against [`schema/config.schema.json`](schema/config.schema.json). See [`agent-orchestrator.yaml.example`](agent-orchestrator.yaml.example) for the full reference, or run `ao config-help` for the complete schema. diff --git a/agent-orchestrator.yaml.example b/agent-orchestrator.yaml.example index eecf3dd490..b8b44d48c5 100644 --- a/agent-orchestrator.yaml.example +++ b/agent-orchestrator.yaml.example @@ -42,6 +42,14 @@ defaults: workspace: worktree # worktree | clone notifiers: [desktop] # desktop | slack | discord | webhook | composio | openclaw +# AO-local code reviewer backend (`ao review run`). Optional. +# Resolution precedence: `--command` flag > project.review > this block > +# worker agent (defaults.agent, when it has a reviewer adapter) > codex. +# `agent` and `command` are mutually exclusive. +# review: +# agent: claude-code # claude-code | codex +# # command: "bash ~/.config/agent-orchestrator/claude-reviewer.sh main" + # Installer-managed external plugins (optional) # plugins: # - name: owasp-auditor @@ -108,6 +116,10 @@ projects: # agentConfig: # model: gpt-5-codex + # Per-project reviewer backend override (see top-level `review:` for keys) + # review: + # agent: claude-code + # Inline rules included in every agent prompt for this project # agentRules: | # Always run tests before pushing. diff --git a/packages/cli/src/commands/review.ts b/packages/cli/src/commands/review.ts index 488b24032e..1a70fcf8d5 100644 --- a/packages/cli/src/commands/review.ts +++ b/packages/cli/src/commands/review.ts @@ -1,7 +1,6 @@ import chalk from "chalk"; import type { Command } from "commander"; import { - createShellCodeReviewRunner, createCodeReviewStore, executeCodeReviewRun, loadConfig, @@ -91,7 +90,14 @@ function getNextQueuedRun( } export function registerReview(program: Command): void { - const review = program.command("review").description("Manage AO-local reviewer runs"); + const review = program + .command("review") + .description( + "Manage AO-local reviewer runs. The reviewer backend resolves in order: " + + "--command flag > project review config > global review config > worker agent > codex. " + + "Configure a persistent backend with `review.agent` (claude-code|codex) or " + + "`review.command` globally, or under `projects..review` per project.", + ); review .command("run") @@ -100,7 +106,10 @@ export function registerReview(program: Command): void { .option("--summary ", "Summary to store on the review run") .option("--status ", "Initial run status (defaults to queued)") .option("--execute", "Execute the review run immediately") - .option("--command ", "Shell command to execute as the reviewer") + .option( + "--command ", + "Shell command to execute as the reviewer (overrides review config)", + ) .option("--json", "Output as JSON") .action( async ( @@ -128,11 +137,7 @@ export function registerReview(program: Command): void { if (opts.execute || opts.command) { run = await executeCodeReviewRun( - { - config, - sessionManager, - ...(opts.command ? { runReviewer: createShellCodeReviewRunner(opts.command) } : {}), - }, + { config, sessionManager, reviewCommand: opts.command }, { projectId: run.projectId, runId: run.id }, ); } @@ -164,7 +169,10 @@ export function registerReview(program: Command): void { .description("Execute a queued AO-local reviewer run") .argument("[project]", "Project ID (searches all projects if omitted)") .option("--run ", "Review run ID or reviewer session ID") - .option("--command ", "Shell command to execute as the reviewer") + .option( + "--command ", + "Shell command to execute as the reviewer (overrides review config)", + ) .option("--force", "Execute even if the run is not queued") .option("--json", "Output as JSON") .action( @@ -194,7 +202,7 @@ export function registerReview(program: Command): void { config, sessionManager, force: opts.force, - ...(opts.command ? { runReviewer: createShellCodeReviewRunner(opts.command) } : {}), + reviewCommand: opts.command, }, { projectId: target.projectId, runId: target.run.id }, ); diff --git a/packages/cli/src/lib/config-instruction.ts b/packages/cli/src/lib/config-instruction.ts index a526f9a5ba..29f6e5a4f0 100644 --- a/packages/cli/src/lib/config-instruction.ts +++ b/packages/cli/src/lib/config-instruction.ts @@ -38,6 +38,15 @@ defaults: worker: agent: claude-code # Optional override for worker sessions +# ── AO-local reviewer backend (optional) ─────────────────────────── +# Backend for 'ao review run'. Precedence: --command flag > project.review > +# this block > worker agent (defaults.agent, if it has an adapter) > codex. +# 'agent' and 'command' are mutually exclusive. + +review: + agent: claude-code # claude-code | codex + # command: "bash ~/.config/agent-orchestrator/claude-reviewer.sh main" + # ── Installer-managed marketplace plugins (optional) ─────────────── # External plugins are declared here. Built-ins do not need entries. diff --git a/packages/core/src/__tests__/code-review-backend.test.ts b/packages/core/src/__tests__/code-review-backend.test.ts new file mode 100644 index 0000000000..557fdad6c6 --- /dev/null +++ b/packages/core/src/__tests__/code-review-backend.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from "vitest"; +import { + buildClaudeCodeReviewArgs, + parseReviewerOutput, + resolveCodeReviewRunner, + runClaudeCodeReview, + runCodexCodeReview, + SUPPORTED_REVIEW_AGENTS, +} from "../code-review-manager.js"; +import type { OrchestratorConfig, ProjectConfig, ReviewConfig } from "../types.js"; + +function makeConfig(overrides: { + defaultAgent?: string; + review?: ReviewConfig; + projectReview?: ReviewConfig; +}): { config: OrchestratorConfig; project: ProjectConfig } { + const project: ProjectConfig = { + name: "App", + path: "/tmp/app", + defaultBranch: "main", + sessionPrefix: "app", + ...(overrides.projectReview ? { review: overrides.projectReview } : {}), + }; + + const config: OrchestratorConfig = { + configPath: "/tmp/ao/agent-orchestrator.yaml", + readyThresholdMs: 300_000, + defaults: { + runtime: "tmux", + agent: overrides.defaultAgent ?? "claude-code", + workspace: "worktree", + notifiers: [], + }, + projects: { app: project }, + notifiers: {}, + notificationRouting: { urgent: [], action: [], warning: [], info: [] }, + reactions: {}, + ...(overrides.review ? { review: overrides.review } : {}), + }; + + return { config, project }; +} + +describe("resolveCodeReviewRunner precedence", () => { + it("uses the explicit --command flag above everything else", () => { + const { config, project } = makeConfig({ + defaultAgent: "claude-code", + review: { agent: "codex" }, + projectReview: { agent: "codex" }, + }); + const runner = resolveCodeReviewRunner({ config, project, command: "echo hi" }); + // Shell-command runners are fresh closures, distinct from the agent adapters. + expect(runner).not.toBe(runClaudeCodeReview); + expect(runner).not.toBe(runCodexCodeReview); + }); + + it("prefers the per-project review config over the global review config", () => { + const { config, project } = makeConfig({ + defaultAgent: "claude-code", + review: { agent: "claude-code" }, + projectReview: { agent: "codex" }, + }); + expect(resolveCodeReviewRunner({ config, project })).toBe(runCodexCodeReview); + }); + + it("prefers the global review config over the worker-agent fallback", () => { + const { config, project } = makeConfig({ + defaultAgent: "claude-code", + review: { agent: "codex" }, + }); + expect(resolveCodeReviewRunner({ config, project })).toBe(runCodexCodeReview); + }); + + it("falls back to the worker agent when it has a known reviewer adapter", () => { + const { config, project } = makeConfig({ defaultAgent: "claude-code" }); + expect(resolveCodeReviewRunner({ config, project })).toBe(runClaudeCodeReview); + }); + + it("falls back to Codex when the worker agent has no reviewer adapter", () => { + const { config, project } = makeConfig({ defaultAgent: "aider" }); + expect(resolveCodeReviewRunner({ config, project })).toBe(runCodexCodeReview); + }); + + it("preserves Codex behavior when the worker agent is already codex", () => { + const { config, project } = makeConfig({ defaultAgent: "codex" }); + expect(resolveCodeReviewRunner({ config, project })).toBe(runCodexCodeReview); + }); + + it("resolves review.command to a shell runner", () => { + const { config, project } = makeConfig({ review: { command: "echo hi" } }); + const runner = resolveCodeReviewRunner({ config, project }); + expect(runner).not.toBe(runClaudeCodeReview); + expect(runner).not.toBe(runCodexCodeReview); + }); + + it("throws a clear error for an unsupported review.agent", () => { + const { config, project } = makeConfig({ review: { agent: " collaborator" } }); + expect(() => resolveCodeReviewRunner({ config, project })).toThrowError( + new RegExp(`Unsupported review.agent.*${SUPPORTED_REVIEW_AGENTS.join(", ")}`), + ); + }); + + it("throws for an unsupported per-project review.agent", () => { + const { config, project } = makeConfig({ projectReview: { agent: "nope" } }); + expect(() => resolveCodeReviewRunner({ config, project })).toThrow(/Unsupported review\.agent/); + }); +}); + +describe("buildClaudeCodeReviewArgs", () => { + it("builds the expected read-only argv with the prompt", () => { + expect(buildClaudeCodeReviewArgs("REVIEW PROMPT")).toEqual([ + "-p", + "REVIEW PROMPT", + "--permission-mode", + "bypassPermissions", + "--output-format", + "text", + ]); + }); +}); + +describe("parseReviewerOutput (claude-code style responses)", () => { + const findings = [ + { + severity: "error" as const, + title: "Null deref", + body: "`user` may be undefined.", + filePath: "src/app.ts", + startLine: 10, + endLine: 12, + confidence: 0.9, + }, + ]; + + it("round-trips a plain JSON object response", () => { + const raw = JSON.stringify({ findings }); + const parsed = parseReviewerOutput(raw); + expect(parsed).toHaveLength(1); + expect(parsed[0]).toMatchObject({ + severity: "error", + title: "Null deref", + filePath: "src/app.ts", + startLine: 10, + endLine: 12, + }); + }); + + it("round-trips a markdown-fenced JSON response", () => { + const raw = ["Here is my review:", "```json", JSON.stringify({ findings }), "```"].join("\n"); + const parsed = parseReviewerOutput(raw); + expect(parsed).toHaveLength(1); + expect(parsed[0]?.title).toBe("Null deref"); + }); + + it("treats an empty findings array as no findings", () => { + expect(parseReviewerOutput('{"findings":[]}')).toEqual([]); + }); +}); diff --git a/packages/core/src/code-review-manager.ts b/packages/core/src/code-review-manager.ts index 756cc3d1c1..70ddeee26e 100644 --- a/packages/core/src/code-review-manager.ts +++ b/packages/core/src/code-review-manager.ts @@ -27,9 +27,11 @@ import { SessionNotFoundError, type OrchestratorConfig, type ProjectConfig, + type ReviewConfig, type Session, type SessionManager, } from "./types.js"; +import { resolveAgentSelection } from "./agent-selection.js"; import { getShell, isWindows, killProcessTree } from "./platform.js"; const REVIEW_COMMAND_TIMEOUT_MS = 10 * 60_000; @@ -207,7 +209,14 @@ export interface ExecuteCodeReviewRunOptions { sessionManager: SessionManager; storeFactory?: (projectId: string) => CodeReviewStore; prepareWorkspace?: PrepareCodeReviewWorkspace; + /** + * Explicit reviewer override. When provided it wins over all configuration + * (used by tests and any caller that fully controls the runner). When + * omitted, the backend is resolved via {@link resolveCodeReviewRunner}. + */ runReviewer?: CodeReviewRunner; + /** Shell command from the `--command` flag (highest configuration precedence). */ + reviewCommand?: string; now?: () => Date; force?: boolean; } @@ -761,6 +770,102 @@ export async function runCodexCodeReview( } } +export function buildClaudeCodeReviewArgs(prompt: string): string[] { + return ["-p", prompt, "--permission-mode", "bypassPermissions", "--output-format", "text"]; +} + +export async function runClaudeCodeReview( + context: CodeReviewRunnerContext, +): Promise { + const prompt = buildDefaultReviewPrompt(context); + const args = buildClaudeCodeReviewArgs(prompt); + + try { + const { stdout, stderr } = await execFileWithClosedStdin("claude", args, { + cwd: context.workspacePath, + timeout: REVIEW_COMMAND_TIMEOUT_MS, + maxBuffer: REVIEW_COMMAND_MAX_BUFFER, + env: process.env, + shell: isWindows(), + }); + return { rawOutput: stdout.trim() || stderr.trim() }; + } catch (error) { + const details = + error instanceof Error && "stderr" in error && typeof error.stderr === "string" + ? error.stderr.trim() + : error instanceof Error + ? error.message + : String(error); + throw new Error(`Claude Code review failed: ${details}`, { cause: error }); + } +} + +/** + * Agent plugins that ship a built-in reviewer adapter. Keyed by the plugin's + * manifest name (matches `defaults.agent` / `review.agent` values). + */ +const REVIEWER_ADAPTERS: Readonly> = { + "claude-code": runClaudeCodeReview, + codex: runCodexCodeReview, +}; + +export const SUPPORTED_REVIEW_AGENTS: readonly string[] = Object.keys(REVIEWER_ADAPTERS); + +function reviewerFromConfig(review: ReviewConfig | undefined): CodeReviewRunner | undefined { + if (!review) return undefined; + if (review.command) return createShellCodeReviewRunner(review.command); + if (review.agent) { + const adapter = REVIEWER_ADAPTERS[review.agent]; + if (!adapter) { + throw new Error( + `Unsupported review.agent "${review.agent}". Valid values: ${SUPPORTED_REVIEW_AGENTS.join(", ")}.`, + ); + } + return adapter; + } + return undefined; +} + +/** + * Resolve the reviewer backend for a run using the documented precedence: + * + * 1. explicit `command` (the `--command` CLI flag) + * 2. per-project `project.review` (command or agent) + * 3. global `config.review` (command or agent) + * 4. the project's worker agent, when it has a known reviewer adapter + * 5. Codex (final fallback, preserving the original behavior) + * + * Throws when a configured `review.agent` names an agent without an adapter, + * so misconfiguration fails fast before any review run state is mutated. + */ +export function resolveCodeReviewRunner({ + config, + project, + command, +}: { + config: OrchestratorConfig; + project: ProjectConfig; + command?: string; +}): CodeReviewRunner { + if (command) return createShellCodeReviewRunner(command); + + const projectReviewer = reviewerFromConfig(project.review); + if (projectReviewer) return projectReviewer; + + const globalReviewer = reviewerFromConfig(config.review); + if (globalReviewer) return globalReviewer; + + const workerAgent = resolveAgentSelection({ + role: "worker", + project, + defaults: config.defaults, + }).agentName; + const workerAdapter = workerAgent ? REVIEWER_ADAPTERS[workerAgent] : undefined; + if (workerAdapter) return workerAdapter; + + return runCodexCodeReview; +} + function defaultReviewSummary(session: Session, source: CodeReviewRequestSource): string { const sourceLabel = source === "cli" ? "CLI" : source === "web" ? "dashboard" : "automation"; return `Review requested from ${sourceLabel} for ${session.id}.`; @@ -902,7 +1007,8 @@ export async function executeCodeReviewRun( sessionManager, storeFactory = createCodeReviewStore, prepareWorkspace = prepareGitReviewerWorkspace, - runReviewer = runCodexCodeReview, + runReviewer, + reviewCommand, now = () => new Date(), force = false, }: ExecuteCodeReviewRunOptions, @@ -913,6 +1019,11 @@ export async function executeCodeReviewRun( throw new Error(`Unknown project: ${projectId}`); } + // Resolve the backend up front so a misconfigured `review.agent` fails fast, + // before any run is claimed or marked `preparing`. + const reviewer = + runReviewer ?? resolveCodeReviewRunner({ config, project, command: reviewCommand }); + const store = storeFactory(projectId); const claimed = await withReviewRunExecutionLock(store, runId, async () => { const executableRun = getExecutableRun(store, runId, force); @@ -946,7 +1057,7 @@ export async function executeCodeReviewRun( now(), ); const baseRef = session.pr?.baseBranch?.trim() || project.defaultBranch; - const result = await runReviewer({ config, project, session, run, workspacePath, baseRef }); + const result = await reviewer({ config, project, session, run, workspacePath, baseRef }); const findings = result.findings ?? parseReviewerOutput(result.rawOutput ?? ""); for (const finding of findings) { diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 145a7b2cb5..c8017fd3a4 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -244,6 +244,16 @@ const RoleAgentConfigSchema = z }) .optional(); +const ReviewConfigSchema = z + .object({ + agent: z.string().optional(), + command: z.string().optional(), + }) + .strict() + .refine((value) => !(value.agent && value.command), { + message: "review.agent and review.command are mutually exclusive", + }); + const ProjectConfigSchema = z.object({ name: z.string().optional(), repo: z.string().optional(), @@ -266,6 +276,7 @@ const ProjectConfigSchema = z.object({ agentConfig: AgentSpecificConfigSchema.default({}), orchestrator: RoleAgentConfigSchema, worker: RoleAgentConfigSchema, + review: ReviewConfigSchema.optional(), reactions: z.record(ReactionConfigSchema.partial()).optional(), agentRules: z.string().optional(), agentRulesFile: z.string().optional(), @@ -366,6 +377,7 @@ const OrchestratorConfigSchema = z.object({ defaults: DefaultPluginsSchema.default({}), plugins: z.array(InstalledPluginConfigSchema).default([]), dashboard: DashboardConfigSchema.optional(), + review: ReviewConfigSchema.optional(), projects: z.record( z .string() diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4ca21c112c..e0c5eb404a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -70,14 +70,18 @@ export { CodeReviewNoOpenFindingsError, CodeReviewRunNotExecutableError, CodeReviewRunNotFoundError, + buildClaudeCodeReviewArgs, createShellCodeReviewRunner, executeCodeReviewRun, formatCodeReviewFindingsForAgent, markOutdatedCodeReviewRunsForSession, parseReviewerOutput, prepareGitReviewerWorkspace, + resolveCodeReviewRunner, + runClaudeCodeReview, runCodexCodeReview, sendCodeReviewFindingsToAgent, + SUPPORTED_REVIEW_AGENTS, triggerCodeReviewForSession, } from "./code-review-manager.js"; export type { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 86850228e6..e56a57d47c 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1359,6 +1359,20 @@ export interface ObservabilityConfig { } /** Top-level orchestrator configuration (from agent-orchestrator.yaml) */ +/** + * Configuration for the AO-local code reviewer backend. + * + * `agent` and `command` are mutually exclusive: + * - `agent`: name of a registered agent plugin with a reviewer adapter + * (currently "claude-code" or "codex"). + * - `command`: an explicit shell command run in the reviewer workspace whose + * stdout is parsed for findings (same contract as `ao review run --command`). + */ +export interface ReviewConfig { + agent?: string; + command?: string; +} + export interface OrchestratorConfig { /** Optional JSON Schema hint for editor autocomplete/validation. */ "$schema"?: string; @@ -1411,6 +1425,9 @@ export interface OrchestratorConfig { /** Dashboard UI configuration */ dashboard?: DashboardConfig; + /** Global AO-local reviewer backend configuration */ + review?: ReviewConfig; + /** Notification channel configs */ notifiers: Record; @@ -1576,6 +1593,9 @@ export interface ProjectConfig { worker?: RoleAgentConfig; + /** Per-project AO-local reviewer backend override */ + review?: ReviewConfig; + /** Per-project reaction overrides */ reactions?: Record>; diff --git a/schema/config.schema.json b/schema/config.schema.json index cf4dd56cc2..33e7290489 100644 --- a/schema/config.schema.json +++ b/schema/config.schema.json @@ -57,6 +57,9 @@ "dashboard": { "$ref": "#/$defs/dashboardConfig" }, + "review": { + "$ref": "#/$defs/reviewConfig" + }, "projects": { "type": "object", "description": "Project configs keyed by project ID.", @@ -287,6 +290,25 @@ } } }, + "reviewConfig": { + "type": "object", + "additionalProperties": false, + "description": "AO-local reviewer backend. `agent` and `command` are mutually exclusive.", + "properties": { + "agent": { + "type": "string", + "enum": ["claude-code", "codex"], + "description": "Registered agent plugin with a reviewer adapter." + }, + "command": { + "type": "string", + "description": "Explicit shell command run in the reviewer workspace; its stdout is parsed for findings." + } + }, + "not": { + "required": ["agent", "command"] + } + }, "projectConfig": { "type": "object", "additionalProperties": false, @@ -355,6 +377,9 @@ "worker": { "$ref": "#/$defs/roleAgentConfig" }, + "review": { + "$ref": "#/$defs/reviewConfig" + }, "reactions": { "type": "object", "additionalProperties": {