feat: cross-model review via review_step tool
- reviewer.ts: spawns separate pi agent with read-only tools and reviewer system prompt (taskplane's review format/verdicts) - Executor registers review_step as a custom tool on the worker session - Worker calls review_step(step, type, step_name) at step boundaries based on review level (0=none, 1=plan, 2=plan+code, 3=full) - Reviewer returns APPROVE/REVISE/RETHINK with structured feedback - REVISE feedback returned inline to worker for immediate action - Review calls logged to task via hai task log
This commit is contained in:
@@ -2,7 +2,10 @@ import { execSync } from "node:child_process";
|
||||
import { join } from "node:path";
|
||||
import { existsSync } from "node:fs";
|
||||
import type { TaskStore, Task, TaskDetail } from "@hai/core";
|
||||
import { Type } from "@mariozechner/pi-ai";
|
||||
import { createHaiAgent } from "./pi.js";
|
||||
import { reviewStep, type ReviewResult } from "./reviewer.js";
|
||||
import type { ToolDefinition } from "@mariozechner/pi-coding-agent";
|
||||
|
||||
const EXECUTOR_SYSTEM_PROMPT = `You are a task execution agent for "hai", an AI-orchestrated task board.
|
||||
|
||||
@@ -152,11 +155,17 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
// Create pi agent session in the worktree
|
||||
// Build the review_step tool for cross-model review
|
||||
const reviewStepTool = this.createReviewStepTool(
|
||||
task.id, worktreePath, detail.prompt,
|
||||
);
|
||||
|
||||
// Create pi agent session in the worktree with review_step tool
|
||||
const { session } = await createHaiAgent({
|
||||
cwd: worktreePath,
|
||||
systemPrompt: EXECUTOR_SYSTEM_PROMPT,
|
||||
tools: "coding",
|
||||
customTools: [reviewStepTool],
|
||||
onText: (delta) => this.options.onAgentText?.(task.id, delta),
|
||||
onToolStart: (name) => this.options.onAgentTool?.(task.id, name),
|
||||
});
|
||||
@@ -207,6 +216,120 @@ export class TaskExecutor {
|
||||
console.log(`[executor] Worktree created: ${path}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create the review_step tool that the worker calls at step boundaries.
|
||||
* Spawns a separate reviewer pi session with read-only tools.
|
||||
*/
|
||||
private createReviewStepTool(
|
||||
taskId: string,
|
||||
worktreePath: string,
|
||||
promptContent: string,
|
||||
): ToolDefinition {
|
||||
const store = this.store;
|
||||
const options = this.options;
|
||||
|
||||
return {
|
||||
name: "review_step",
|
||||
label: "Review Step",
|
||||
description:
|
||||
"Spawn a reviewer agent to evaluate your plan or code for a step. " +
|
||||
"Returns APPROVE, REVISE, RETHINK, or UNAVAILABLE. " +
|
||||
"Call at step boundaries based on the task's review level. " +
|
||||
"Skip reviews for Step 0 (Preflight) and the final documentation step.",
|
||||
parameters: Type.Object({
|
||||
step: Type.Number({ description: "Step number to review" }),
|
||||
type: Type.Union(
|
||||
[Type.Literal("plan"), Type.Literal("code")],
|
||||
{ description: 'Review type: "plan" or "code"' },
|
||||
),
|
||||
step_name: Type.String({ description: "Name of the step being reviewed" }),
|
||||
baseline: Type.Optional(
|
||||
Type.String({
|
||||
description:
|
||||
"Git commit SHA for code review diff baseline. " +
|
||||
"Capture HEAD before starting a step and pass it here.",
|
||||
}),
|
||||
),
|
||||
}),
|
||||
execute: async (_toolCallId, params) => {
|
||||
const { step, type: reviewType, step_name, baseline } = params;
|
||||
|
||||
console.log(
|
||||
`[reviewer] ${taskId}: ${reviewType} review for Step ${step} (${step_name})`,
|
||||
);
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`${reviewType} review requested for Step ${step} (${step_name})`,
|
||||
);
|
||||
|
||||
try {
|
||||
const result = await reviewStep(
|
||||
worktreePath,
|
||||
taskId,
|
||||
step,
|
||||
step_name,
|
||||
reviewType,
|
||||
promptContent,
|
||||
baseline,
|
||||
{
|
||||
onText: (delta) => options.onAgentText?.(taskId, delta),
|
||||
},
|
||||
);
|
||||
|
||||
// Log the result
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`${reviewType} review Step ${step}: ${result.verdict}`,
|
||||
result.summary,
|
||||
);
|
||||
|
||||
console.log(
|
||||
`[reviewer] ${taskId}: Step ${step} ${reviewType} → ${result.verdict}`,
|
||||
);
|
||||
|
||||
// Format response for the worker
|
||||
let text: string;
|
||||
switch (result.verdict) {
|
||||
case "APPROVE":
|
||||
text = "APPROVE";
|
||||
break;
|
||||
case "REVISE":
|
||||
text = `REVISE\n\n${result.review}`;
|
||||
break;
|
||||
case "RETHINK":
|
||||
text = `RETHINK\n\n${result.review}`;
|
||||
break;
|
||||
default:
|
||||
text = "UNAVAILABLE — reviewer did not produce a usable verdict.";
|
||||
}
|
||||
|
||||
return {
|
||||
content: [{ type: "text" as const, text }],
|
||||
details: {},
|
||||
};
|
||||
} catch (err: any) {
|
||||
console.error(
|
||||
`[reviewer] ${taskId}: review failed: ${err.message}`,
|
||||
);
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`${reviewType} review failed: ${err.message}`,
|
||||
);
|
||||
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: `UNAVAILABLE — reviewer error: ${err.message}`,
|
||||
},
|
||||
],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async cleanup(taskId: string): Promise<void> {
|
||||
const worktreePath = this.activeWorktrees.get(taskId);
|
||||
if (!worktreePath) return;
|
||||
|
||||
@@ -2,4 +2,5 @@ export { TriageProcessor, type TriageProcessorOptions } from "./triage.js";
|
||||
export { TaskExecutor, type TaskExecutorOptions } from "./executor.js";
|
||||
export { Scheduler, type SchedulerOptions } from "./scheduler.js";
|
||||
export { aiMergeTask, type MergerOptions } from "./merger.js";
|
||||
export { reviewStep, type ReviewType, type ReviewVerdict, type ReviewResult, type ReviewOptions } from "./reviewer.js";
|
||||
export { createHaiAgent, type AgentOptions, type AgentResult } from "./pi.js";
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
SessionManager,
|
||||
SettingsManager,
|
||||
type AgentSession,
|
||||
type ToolDefinition,
|
||||
} from "@mariozechner/pi-coding-agent";
|
||||
|
||||
export interface AgentResult {
|
||||
@@ -25,6 +26,7 @@ export interface AgentOptions {
|
||||
cwd: string;
|
||||
systemPrompt: string;
|
||||
tools?: "coding" | "readonly";
|
||||
customTools?: ToolDefinition[];
|
||||
onText?: (delta: string) => void;
|
||||
onToolStart?: (name: string) => void;
|
||||
onToolEnd?: (name: string, isError: boolean) => void;
|
||||
@@ -62,6 +64,7 @@ export async function createHaiAgent(options: AgentOptions): Promise<AgentResult
|
||||
modelRegistry,
|
||||
resourceLoader,
|
||||
tools,
|
||||
customTools: options.customTools,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settingsManager,
|
||||
});
|
||||
|
||||
256
packages/engine/src/reviewer.ts
Normal file
256
packages/engine/src/reviewer.ts
Normal file
@@ -0,0 +1,256 @@
|
||||
/**
|
||||
* Reviewer — spawns a separate pi agent to review a worker's plan or code.
|
||||
*
|
||||
* Replicates taskplane's cross-model review pattern:
|
||||
* - Worker calls review_step(step, type) during execution
|
||||
* - A separate reviewer agent is spawned with read-only tools
|
||||
* - Reviewer writes a structured verdict: APPROVE, REVISE, or RETHINK
|
||||
* - Verdict + feedback is returned to the worker
|
||||
*/
|
||||
|
||||
import { createHaiAgent } from "./pi.js";
|
||||
|
||||
const REVIEWER_SYSTEM_PROMPT = `You are an independent code and plan reviewer.
|
||||
|
||||
You provide quality assessment for task implementations. You have full read
|
||||
access to the codebase and can run commands to inspect code.
|
||||
|
||||
## Verdict Criteria
|
||||
|
||||
- **APPROVE** — Step will achieve its stated outcomes. Minor suggestions go in
|
||||
the Suggestions section but do NOT block progress. If your only findings are
|
||||
minor or suggestion-level, verdict is APPROVE.
|
||||
- **REVISE** — Step will fail, produce incorrect results, or miss a stated
|
||||
requirement without fixes. Use ONLY for issues that would cause the worker to
|
||||
redo work later.
|
||||
- **RETHINK** — Approach is fundamentally wrong. Explain why and suggest an
|
||||
alternative.
|
||||
|
||||
### APPROVE vs REVISE
|
||||
|
||||
**APPROVE** when:
|
||||
- The approach will work, but you see a cleaner alternative
|
||||
- Documentation style could improve
|
||||
- You'd suggest additional tests but core coverage is adequate
|
||||
|
||||
**REVISE** when:
|
||||
- A requirement from PROMPT.md will not be met
|
||||
- A bug or regression is introduced
|
||||
- A critical edge case is unhandled and would cause runtime failure
|
||||
- Backward compatibility is broken without migration
|
||||
|
||||
### Do NOT issue REVISE for
|
||||
- STATUS/formatting preferences
|
||||
- Splitting outcome checkboxes into implementation sub-steps
|
||||
- Suggestions that improve quality but aren't required for correctness
|
||||
|
||||
## Plan Review Format
|
||||
|
||||
\`\`\`markdown
|
||||
## Plan Review: [Step Name]
|
||||
|
||||
### Verdict: [APPROVE | REVISE | RETHINK]
|
||||
|
||||
### Summary
|
||||
[2-3 sentence assessment]
|
||||
|
||||
### Issues Found
|
||||
1. **[Severity: critical/important/minor]** — [Description and suggested fix]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Code Review Format
|
||||
|
||||
\`\`\`markdown
|
||||
## Code Review: [Step Name]
|
||||
|
||||
### Verdict: [APPROVE | REVISE | RETHINK]
|
||||
|
||||
### Summary
|
||||
[2-3 sentence assessment]
|
||||
|
||||
### Issues Found
|
||||
1. **[File:Line]** [Severity] — [Description and fix]
|
||||
|
||||
### Pattern Violations
|
||||
- [Deviations from project standards]
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Plan Granularity
|
||||
|
||||
When reviewing plans, assess whether the approach achieves the step's OUTCOMES —
|
||||
not whether every function and parameter is listed.
|
||||
|
||||
Good plan: identifies key behavioral changes, calls out risks, has a testing strategy.
|
||||
Do NOT demand function-level implementation checklists.
|
||||
|
||||
## Rules
|
||||
|
||||
- Be specific — reference actual files and line numbers
|
||||
- Be constructive — suggest fixes, not just problems
|
||||
- Be proportional — don't block on style nits
|
||||
- Output your review as plain text (not to a file)
|
||||
`;
|
||||
|
||||
export type ReviewType = "plan" | "code";
|
||||
export type ReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
|
||||
|
||||
export interface ReviewResult {
|
||||
verdict: ReviewVerdict;
|
||||
review: string;
|
||||
summary: string;
|
||||
}
|
||||
|
||||
export interface ReviewOptions {
|
||||
onText?: (delta: string) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a reviewer agent to evaluate a worker's plan or code for a step.
|
||||
*/
|
||||
export async function reviewStep(
|
||||
cwd: string,
|
||||
taskId: string,
|
||||
stepNumber: number,
|
||||
stepName: string,
|
||||
reviewType: ReviewType,
|
||||
promptContent: string,
|
||||
baseline?: string,
|
||||
options: ReviewOptions = {},
|
||||
): Promise<ReviewResult> {
|
||||
// Build the review request
|
||||
const request = buildReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
|
||||
);
|
||||
|
||||
// Spawn a reviewer agent with read-only tools
|
||||
const { session } = await createHaiAgent({
|
||||
cwd,
|
||||
systemPrompt: REVIEWER_SYSTEM_PROMPT,
|
||||
tools: "readonly",
|
||||
onText: (delta) => options.onText?.(delta),
|
||||
});
|
||||
|
||||
let reviewText = "";
|
||||
|
||||
// Capture the reviewer's full text output
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
||||
reviewText += event.assistantMessageEvent.delta;
|
||||
}
|
||||
});
|
||||
|
||||
try {
|
||||
await session.prompt(request);
|
||||
} finally {
|
||||
session.dispose();
|
||||
}
|
||||
|
||||
// Extract verdict from the review text
|
||||
const verdict = extractVerdict(reviewText);
|
||||
const summary = extractSummary(reviewText);
|
||||
|
||||
return { verdict, review: reviewText, summary };
|
||||
}
|
||||
|
||||
function buildReviewRequest(
|
||||
taskId: string,
|
||||
stepNumber: number,
|
||||
stepName: string,
|
||||
reviewType: ReviewType,
|
||||
promptContent: string,
|
||||
cwd: string,
|
||||
baseline?: string,
|
||||
): string {
|
||||
const parts = [
|
||||
`Review request for task ${taskId}, Step ${stepNumber}: ${stepName}`,
|
||||
`Review type: **${reviewType}**`,
|
||||
"",
|
||||
"## Task PROMPT.md",
|
||||
"```markdown",
|
||||
promptContent,
|
||||
"```",
|
||||
"",
|
||||
];
|
||||
|
||||
if (reviewType === "plan") {
|
||||
parts.push(
|
||||
"## What to review",
|
||||
`The worker is about to implement Step ${stepNumber} (${stepName}).`,
|
||||
"Assess whether the step's checkboxes will achieve the stated outcomes.",
|
||||
"Read relevant source files to understand the current codebase state.",
|
||||
"Check for risks, missing edge cases, and gaps in the plan.",
|
||||
);
|
||||
} else {
|
||||
parts.push(
|
||||
"## What to review",
|
||||
`The worker has implemented Step ${stepNumber} (${stepName}).`,
|
||||
"Review the code changes for correctness, patterns, and test coverage.",
|
||||
"",
|
||||
);
|
||||
if (baseline) {
|
||||
parts.push(
|
||||
"To see the changes for this step, run:",
|
||||
`\`\`\`bash`,
|
||||
`git diff ${baseline}..HEAD`,
|
||||
`\`\`\``,
|
||||
);
|
||||
} else {
|
||||
parts.push(
|
||||
"To see recent changes, run:",
|
||||
"```bash",
|
||||
"git diff HEAD~1",
|
||||
"```",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
parts.push(
|
||||
"",
|
||||
"## Instructions",
|
||||
"1. Read the relevant source files",
|
||||
"2. Assess the work against the task requirements",
|
||||
"3. Output your review using the format from your system prompt",
|
||||
"4. Be specific with file paths and line numbers",
|
||||
);
|
||||
|
||||
return parts.join("\n");
|
||||
}
|
||||
|
||||
function extractVerdict(review: string): ReviewVerdict {
|
||||
// Look for "### Verdict: APPROVE" or similar patterns
|
||||
const verdictMatch = review.match(
|
||||
/###?\s*Verdict[:\s]*(APPROVE|REVISE|RETHINK)/i,
|
||||
);
|
||||
if (verdictMatch) {
|
||||
return verdictMatch[1].toUpperCase() as ReviewVerdict;
|
||||
}
|
||||
|
||||
// Fallback: look for the word anywhere in the text
|
||||
const upper = review.toUpperCase();
|
||||
if (upper.includes("RETHINK")) return "RETHINK";
|
||||
if (upper.includes("REVISE")) return "REVISE";
|
||||
if (upper.includes("APPROVE")) return "APPROVE";
|
||||
|
||||
return "UNAVAILABLE";
|
||||
}
|
||||
|
||||
function extractSummary(review: string): string {
|
||||
const summaryMatch = review.match(
|
||||
/###?\s*Summary[:\s]*([\s\S]*?)(?=###|$)/i,
|
||||
);
|
||||
if (summaryMatch) {
|
||||
return summaryMatch[1].trim().slice(0, 500);
|
||||
}
|
||||
// Fallback: first paragraph
|
||||
const lines = review.split("\n").filter((l) => l.trim());
|
||||
return lines.slice(0, 3).join(" ").slice(0, 300);
|
||||
}
|
||||
Reference in New Issue
Block a user