Files
fusion/packages/engine/src/reviewer.ts
gsxdsm ee00d9f1b7 FN-5928: enforce surface enumeration for bug-fix invariants
Require bug-fix specs and reviews to enumerate affected surfaces and reject repro-only regression coverage.

- add a required `## Surface Enumeration` section to triage prompt templates and bug-fix planning guidance
- tighten reviewer guidance to block missing surface enumeration and repro-only regression tests
- document the canonical surface checklist in `docs/testing.md` and cover the new wording with prompt/reviewer tests

Files changed:
 AGENTS.md                                         |  6 ++--
 docs/testing.md                                   |  9 +++++
 packages/core/src/__tests__/agent-prompts.test.ts | 22 ++++++++++++
 packages/core/src/agent-prompts.ts                | 16 +++++++++
 packages/engine/src/__tests__/reviewer.test.ts    | 17 ++++++++++
 packages/engine/src/__tests__/triage.test.ts      | 41 ++++++++++++++++++++---
 packages/engine/src/reviewer.ts                   |  3 ++
 packages/engine/src/triage.ts                     | 24 +++++++++++++
 8 files changed, 131 insertions(+), 7 deletions(-)

Fusion-Task-Id: FN-5928
Fusion-Task-Lineage: 717ddcbe-f3a6-4589-ad90-4e640f7a9ff2
2026-06-02 21:57:21 -07:00

998 lines
41 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
// port-4040-allowlist: this file embeds the "never kill port 4040" rule in the reviewer prompt.
/**
* Reviewer — spawns a separate pi agent to review a worker's plan or code.
*
* Replicates taskplane's cross-model review pattern:
* - Worker calls fn_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 type { TaskStore, TaskComment, AgentPromptsConfig, Settings } from "@fusion/core";
import { buildReviewerMemoryInstructions, resolveAgentPrompt, resolvePersistAgentThinkingLog, resolveAgentMemoryInclusionMode } from "@fusion/core";
import { recordRetry } from "./retry-burned-logger.js";
import { describeModel, promptWithFallback } from "./pi.js";
import { isContextLimitError } from "./context-limit-detector.js";
import { createResolvedAgentSession, extractRuntimeHint } from "./agent-session-helpers.js";
import { buildSessionSkillContext } from "./session-skill-context.js";
import { AgentLogger } from "./agent-logger.js";
import { reviewerLog } from "./logger.js";
import { checkSessionError } from "./usage-limit-detector.js";
import {
resolveAgentInstructions,
buildPluginPromptSection,
} from "./agent-instructions.js";
import { buildPromptLayers, collapsePromptLayers } from "./prompt-layers.js";
import { createFallbackModelObserver } from "./fallback-model-observer.js";
import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js";
import { createMemoryGetTool, createMemorySearchTool, createWebFetchTool } from "./agent-tools.js";
export const REVIEWER_SYSTEM_PROMPT = `You are an independent code and plan reviewer.
## Your Role
You are an objective quality gate for plans, code, and specs.
You are neither the implementor's advocate nor adversary: your job is evidence-based assessment that protects delivery quality.
You provide quality assessment for task implementations. You have full read
access to the codebase and can run commands to inspect code.
## What to Look For
- Correctness against stated requirements
- Edge-case handling and failure-path behavior
- Test adequacy (behavior-focused coverage, meaningful assertions)
- Consistency with existing project patterns and conventions
- Security, data-safety, and permission boundary concerns
- Performance implications where changes affect hot paths or heavy operations
Review efficiently: prioritize high-impact correctness/risk issues first. Do not spend blocking attention on style nits when substantive defects exist.
## 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
Concrete examples:
- APPROVE: implementation satisfies outcomes; only optional cleanup or minor wording suggestions remain.
- REVISE: a required behavior is missing, tests are insufficient for changed behavior, or a likely regression exists.
- RETHINK: the approach conflicts with architecture/task goals such that incremental edits are unlikely to rescue it.
**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
- Code outside the task's File Scope is deleted, removed, or gutted (out-of-scope removal)
- Existing functionality is removed without a corresponding changeset explaining the removal
- Code changes were made outside the assigned task worktree, unless the path is an expected exception such as project memory or task attachments
### Do NOT issue REVISE for
- STATUS/formatting preferences
- Splitting outcome checkboxes into implementation sub-steps
- Necessary fixes outside the initial File Scope when they are required to restore green lint, tests, build, or typecheck and do not delete/gut unrelated functionality
- 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]
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
### Suggestions
- [Optional improvements, not blocking]
\`\`\`
## Spec Review Format
\`\`\`markdown
## Spec Review: [Task ID]
### Verdict: [APPROVE | REVISE | RETHINK]
### Summary
[2-3 sentence assessment of the specification quality]
### Issues Found
1. **[Severity: critical/important/minor]** — [Description and suggested fix]
### Criteria Assessment
- **Mission clarity:** [Clear, unambiguous mission statement?]
- **Step specificity:** [Steps have verifiable, concrete outcomes?]
- **File scope accuracy:** [All affected files listed? No extras?]
- **Dependency correctness:** [Dependencies exist and are appropriate?]
- **Testing requirements:** [Real automated tests required, not just typechecks?]
- **Surface enumeration:** [For bug-fix specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? Missing or incomplete coverage is a blocking REVISE.]
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
- **Dangling task-document references:** [No \`.fusion/tasks/<id>/<file>\` path is cited in Context, Steps, or File Scope unless the file exists or is explicitly created as a \`(new)\` artifact in this spec. References to nonexistent task-local artifacts are a blocking REVISE.]
- **Sizing & review level:** [Size and review level appropriate for the work?]
- **Subtask breakdown:** [Only flag genuinely oversized specs (12+ implementation steps, OR 5+ truly independent deliverables that could ship separately). Do NOT flag a coherent vertical change just because it touches multiple packages. When borderline, prefer leaving the task whole.]
- **User comment coverage:** [Were all user comments addressed? Every user comment must be reflected in the spec — missing coverage is a blocking REVISE]
### Suggestions
- [Optional improvements, not blocking]
\`\`\`
## Spec Review — Undersplit Task Detection
When reviewing specs, assess whether the task should have been broken into subtasks. The bar for splitting is high — most tasks should remain whole. Coordination overhead (worktrees, dependency wiring, merge sequencing) is real, so splitting must clearly pay for itself.
**Default position:** do NOT flag undersplit. Reach for it only when the spec is genuinely oversized.
**Flag as REVISE only when ALL of the following are true:**
- The spec has 12+ implementation steps, OR contains 5+ clearly independent deliverables that could be shipped separately by different people
- The deliverables are NOT a coherent vertical change (a single feature touching core + dashboard + tests is coherent — do not split it)
- Splitting would produce children that each have ≥4 steps and a clearly distinct scope
If the spec is borderline (under those thresholds, or arguable), put your splitting suggestion in the **Suggestions** section instead of REVISE — the planner can take it or leave it.
**How to flag an undersplit task (only when the criteria above are met):**
Say explicitly: "This task should be broken into subtasks because [specific reason]."
Recommend the number of child tasks (2-5) and what each should cover.
Instruct the planner to:
1. Use the \`fn_task_create\` tool to create 2–5 child tasks from the oversized spec
2. Do NOT write a parent PROMPT.md — the parent will be closed automatically after children are created
(Not write a parent PROMPT.md is also unacceptable.)
3. Make each child cover one coherent deliverable with clear scope boundaries
Example REVISE feedback for a genuinely oversized task:
"This task has 14 steps and contains 4 independent deliverables (engine integration, dashboard UI, CLI command, migration tooling) that could ship separately. Use fn_task_create to split into: (1) engine logic, (2) dashboard UI, (3) CLI integration, (4) migration tooling. Do not write a parent PROMPT."
**Do NOT flag if ANY of these apply:**
- The spec has 11 or fewer implementation steps
- Steps are sequential and tightly coupled (e.g., a pipeline where each step depends on the previous)
- The task is a vertical change touching multiple packages for one coherent feature (typical in this monorepo)
- The task is a bug fix, regardless of how many files it touches
- Splitting would create coordination overhead that exceeds the benefit
## 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.
## Test Quality Review
When reviewing tests, check that they verify observable behavior and regression risk (not only implementation trivia).
Flag REVISE when key edge cases or failure modes for changed behavior are untested.
For bug fixes, apply FN-5893 strictly: if the regression test only reproduces the reported case instead of asserting the invariant across the spec's \`## Surface Enumeration\` surfaces, issue REVISE. Use the motivating recurrences (FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751) as concrete examples of why repro-only coverage is insufficient.
## Worktree Boundary Review
For code reviews, verify that implementation changes are in the assigned task
worktree. The review request includes the current worktree path. Inspect git
state and recent commits from that worktree, and treat changes outside it as a
blocking REVISE unless they are expected project-root state such as
\`.fusion/memory/\` files, task attachments, or other explicitly documented
Fusion metadata. If you see edits or commits in the primary project checkout
instead of the task worktree, call that out directly and ask the worker to move
the changes into the assigned worktree.
## 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)
- **NEVER kill processes on port 4040.** Port 4040 is the production dashboard. If you need to test server endpoints, start a server on a different port (\`--port 0\` for random). If port 4040 is occupied, use a different port — do NOT kill the occupant. Issue REVISE if the executor kills or attempts to kill processes on port 4040.
`;
export type ReviewType = "plan" | "code" | "spec";
export type ReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
export interface ReviewResult {
verdict: ReviewVerdict;
review: string;
summary: string;
}
export interface ReviewOptions {
onText?: (delta: string) => void;
/** Default model provider (e.g. "anthropic"). When set with `defaultModelId`, overrides the reviewer's model selection. */
defaultProvider?: string;
/** Default model ID within the provider (e.g. "claude-sonnet-4-5"). When set with `defaultProvider`, overrides the reviewer's model selection. */
defaultModelId?: string;
/** Task-level validator model provider override. When both provider and modelId are set, takes precedence over project/global lanes. */
taskValidatorProvider?: string;
/** Task-level validator model ID override. When both provider and modelId are set, takes precedence over project/global lanes. */
taskValidatorModelId?: string;
/** Project-level validator model provider override. Takes precedence over global validator lane. */
projectValidatorProvider?: string;
/** Project-level validator model ID override. Takes precedence over global validator lane. */
projectValidatorModelId?: string;
/** Global validator lane provider. Takes precedence over project default override + execution defaults. */
globalValidatorProvider?: string;
/** Global validator lane model ID. Takes precedence over project default override + execution defaults. */
globalValidatorModelId?: string;
/** Project-level default provider override, used when validator lanes are absent. */
projectDefaultOverrideProvider?: string;
/** Project-level default model override, used when validator lanes are absent. */
projectDefaultOverrideModelId?: string;
/** Fallback model provider used when the primary reviewer model hits a retryable provider-side error. */
fallbackProvider?: string;
/** Fallback model ID used with `fallbackProvider`. */
fallbackModelId?: string;
/** Project-level validator fallback provider override. Takes precedence over global fallback. */
projectValidatorFallbackProvider?: string;
/** Project-level validator fallback model ID override. Takes precedence over global fallback. */
projectValidatorFallbackModelId?: string;
/** Default thinking effort level for the reviewer agent session. */
defaultThinkingLevel?: string;
/** Task store for persisting agent log entries. When provided with `taskId`, enables full conversation logging. */
store?: TaskStore;
/** Task ID for agent log persistence. Required alongside `store`. */
taskId?: string;
/** Optional reviewer agent id for retry-burn telemetry. */
agentId?: string;
/** Optional task title for fallback-used notification context. */
taskTitle?: string;
/** Task with optional assignedAgentId for skill selection. */
task?: { assignedAgentId?: string | null };
/** User comments on the task (author === "user"). For spec reviews, the reviewer explicitly checks that every comment is addressed. */
userComments?: TaskComment[];
/** Agent prompt configuration for resolving custom reviewer prompts. */
agentPrompts?: AgentPromptsConfig;
/** AgentStore for resolving per-agent custom instructions. */
agentStore?: import("@fusion/core").AgentStore;
/** Project root directory for resolving relative instructionsPath files. */
rootDir?: string;
/** Project settings used for backend-aware memory tools and instructions. */
settings?: Settings;
/** Plugin runner for runtime selection. When provided, enables plugin runtime lookup. */
pluginRunner?: import("./plugin-runner.js").PluginRunner;
/**
* Fired immediately after the reviewer's `AgentSession` is created. The
* caller can register the session in a per-task subagent map so that the
* session can be disposed when the parent task moves out of `in-progress`,
* is paused, or the engine globally pauses. Without this hook, reviewer
* sessions outlive their parent task on a stop signal.
*/
onSessionCreated?: (session: import("@earendil-works/pi-coding-agent").AgentSession) => void;
/**
* Fired in a `finally` block after the reviewer is fully done (or aborted).
* Pair with `onSessionCreated` to deregister from the subagent map.
*/
onSessionEnded?: (session: import("@earendil-works/pi-coding-agent").AgentSession) => 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> {
// Pause gate: do not spawn a reviewer subprocess while the engine is paused.
// Re-read settings from the store so a stale `options.settings` snapshot can't
// leak a reviewer past a pause that flipped on after the parent agent started.
let liveSettings: Settings | undefined = options.settings;
if (options.store) {
try {
liveSettings = await options.store.getSettings();
} catch {
// Fall back to the snapshot — better to spawn than crash on a transient store error.
}
}
if (liveSettings?.globalPause || liveSettings?.enginePaused) {
const reason = liveSettings.globalPause ? "Global pause" : "Engine paused";
reviewerLog.log(
`${taskId}: ${reviewType} review for Step ${stepNumber} skipped — ${reason} active`,
);
if (options.store && options.taskId) {
try {
await options.store.logEntry(
options.taskId,
`${reviewType} review skipped — ${reason} active`,
);
} catch {
// best-effort
}
}
return {
verdict: "UNAVAILABLE",
review: `${reason} active — reviewer not spawned. Stop calling fn_review_* and exit cleanly; the parent task will resume after unpause.`,
summary: `Skipped: ${reason}`,
};
}
const request = buildReviewRequest(
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
);
const agentLogger = options.store && options.taskId
? new AgentLogger({
store: options.store,
taskId: options.taskId,
agent: "reviewer",
onAgentText: options.onText
? (_id, delta) => options.onText!(delta)
: undefined,
persistAgentToolOutput: liveSettings?.persistAgentToolOutput,
// Reviewer sessions are task-scoped ephemeral workers.
persistAgentThinkingLog: resolvePersistAgentThinkingLog(liveSettings, { ephemeral: true }),
})
: null;
const validatorProvider = options.taskValidatorProvider && options.taskValidatorModelId
? options.taskValidatorProvider
: (options.projectValidatorProvider && options.projectValidatorModelId
? options.projectValidatorProvider
: (options.globalValidatorProvider && options.globalValidatorModelId
? options.globalValidatorProvider
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
? options.projectDefaultOverrideProvider
: options.defaultProvider)));
const validatorModelId = options.taskValidatorProvider && options.taskValidatorModelId
? options.taskValidatorModelId
: (options.projectValidatorProvider && options.projectValidatorModelId
? options.projectValidatorModelId
: (options.globalValidatorProvider && options.globalValidatorModelId
? options.globalValidatorModelId
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
? options.projectDefaultOverrideModelId
: options.defaultModelId)));
const validatorFallbackProvider = options.projectValidatorFallbackProvider && options.projectValidatorFallbackModelId
? options.projectValidatorFallbackProvider
: options.fallbackProvider;
const validatorFallbackModelId = options.projectValidatorFallbackProvider && options.projectValidatorFallbackModelId
? options.projectValidatorFallbackModelId
: options.fallbackModelId;
let reviewerInstructions = "";
if (options.agentStore && options.rootDir) {
try {
const agents = await options.agentStore.listAgents({ role: "reviewer" });
for (const agent of agents) {
if (agent.instructionsText || agent.instructionsPath) {
const memoryMode = resolveAgentMemoryInclusionMode({ agent, globalSettings: options.settings }).mode;
reviewerInstructions = await resolveAgentInstructions(agent, options.rootDir, undefined, memoryMode);
break;
}
}
} catch {
// Graceful fallback
}
}
const reviewerBasePrompt = resolveAgentPrompt("reviewer", options.agentPrompts) || REVIEWER_SYSTEM_PROMPT;
const memorySection = options.rootDir && options.settings?.memoryEnabled !== false
? buildReviewerMemoryInstructions(options.rootDir, options.settings)
: "";
const reviewerPluginContributions = buildPluginPromptSection(
"reviewer",
options.pluginRunner,
);
if (reviewerPluginContributions) {
reviewerLog.log(`applied plugin prompt contributions for reviewer surface`);
}
const layers = buildPromptLayers({
basePrompt: reviewerBasePrompt,
agentInstructions: reviewerInstructions,
memorySection,
pluginContributions: reviewerPluginContributions,
});
const reviewerSystemPromptFinal = collapsePromptLayers(layers);
let skillContext = undefined;
if (options.agentStore && options.rootDir) {
try {
skillContext = await buildSessionSkillContext({
agentStore: options.agentStore,
task: options.task ?? {},
sessionPurpose: "reviewer",
projectRootDir: options.rootDir,
pluginRunner: options.pluginRunner,
});
} catch {
// Graceful fallback - no skill selection
}
}
const assignedAgentId = options.task?.assignedAgentId ?? null;
const agentStore = options.agentStore;
const memoryAgent =
options.rootDir
&& agentStore
&& assignedAgentId
&& typeof (agentStore as { getAgent?: unknown }).getAgent === "function"
? await agentStore.getAgent(assignedAgentId).catch(() => null)
: null;
const memoryTools = options.rootDir && options.settings?.memoryEnabled !== false
? [
createMemorySearchTool(options.rootDir, options.settings, memoryAgent ? {
agentMemory: {
agentId: memoryAgent.id,
agentName: memoryAgent.name,
memory: memoryAgent.memory,
},
} : undefined),
createMemoryGetTool(options.rootDir, options.settings, memoryAgent ? {
agentMemory: {
agentId: memoryAgent.id,
agentName: memoryAgent.name,
memory: memoryAgent.memory,
},
} : undefined),
]
: undefined;
class ReviewerPauseAbortError extends Error {
constructor(public readonly reason: string) {
super(`reviewer aborted: ${reason}`);
}
}
const activeSessions = new Set<import("@earendil-works/pi-coding-agent").AgentSession>();
let reviewText = "";
const endSession = (session: import("@earendil-works/pi-coding-agent").AgentSession) => {
if (!activeSessions.delete(session)) {
return;
}
session.dispose();
options.onSessionEnded?.(session);
};
const buildPauseUnavailableResult = async (reason: string): Promise<ReviewResult> => {
reviewerLog.log(
`${taskId}: ${reviewType} review for Step ${stepNumber} aborted before spawn — ${reason} active`,
);
if (options.store && options.taskId) {
await options.store.logEntry(
options.taskId,
`${reviewType} review aborted before spawn — ${reason} active`,
).catch(() => undefined);
}
return {
verdict: "UNAVAILABLE",
review: `${reason} active — reviewer not spawned. Stop calling fn_review_* and exit cleanly; the parent task will resume after unpause.`,
summary: `Skipped: ${reason}`,
};
};
const createReviewerSession = async (
overrides?: { forceProvider?: string; forceModelId?: string },
): Promise<import("@earendil-works/pi-coding-agent").AgentSession> => {
const runAuditor = options.store
? createRunAuditor(options.store, {
runId: generateSyntheticRunId("reviewer", options.taskId ?? "review"),
agentId: options.agentId ?? "reviewer",
taskId: options.taskId,
phase: "review",
source: "reviewer",
})
: undefined;
const { session } = await createResolvedAgentSession({
sessionPurpose: "reviewer",
runtimeHint: extractRuntimeHint(memoryAgent?.runtimeConfig),
pluginRunner: options.pluginRunner,
cwd,
systemPrompt: reviewerSystemPromptFinal,
systemPromptLayers: layers,
tools: "readonly",
customTools: [createWebFetchTool(), ...(memoryTools ?? [])],
onText: agentLogger ? agentLogger.onText : (delta) => options.onText?.(delta),
onThinking: agentLogger?.onThinking,
onToolStart: agentLogger?.onToolStart,
onToolEnd: agentLogger?.onToolEnd,
defaultProvider: overrides?.forceProvider ?? validatorProvider,
defaultModelId: overrides?.forceModelId ?? validatorModelId,
fallbackProvider: validatorFallbackProvider,
fallbackModelId: validatorFallbackModelId,
defaultThinkingLevel: options.defaultThinkingLevel,
runAuditor,
settings: options.settings,
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
taskId: options.taskId,
taskTitle: options.taskTitle,
onFallbackModelUsed: createFallbackModelObserver({
agent: "reviewer",
label: "reviewer",
store: options.store,
taskId: options.taskId,
taskTitle: options.taskTitle,
}),
beforeSpawnSession: async () => {
if (!options.store) return;
let finalSettings: Settings | undefined;
try {
finalSettings = await options.store.getSettings();
} catch {
return;
}
if (finalSettings?.globalPause || finalSettings?.enginePaused) {
const reason = finalSettings.globalPause ? "Global pause" : "Engine paused";
throw new ReviewerPauseAbortError(reason);
}
},
});
const reviewerModelDesc = describeModel(session);
const reviewerModelMarker = `Reviewer using model: ${reviewerModelDesc}`;
reviewerLog.log(`${taskId}: reviewer using model ${reviewerModelDesc}`);
if (options.store && options.taskId) {
await options.store.logEntry(options.taskId, reviewerModelMarker);
await options.store.appendAgentLog(options.taskId, reviewerModelMarker, "text", undefined, "reviewer").catch(() => undefined);
}
activeSessions.add(session);
options.onSessionCreated?.(session);
session.subscribe((event) => {
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
reviewText += event.assistantMessageEvent.delta;
}
});
return session;
};
const runReviewPrompt = async (
session: import("@earendil-works/pi-coding-agent").AgentSession,
prompt: string,
): Promise<void> => {
await promptWithFallback(session, prompt);
checkSessionError(session);
};
const runAttempt = async (
attemptRequest: string,
sessionOptions?: { forceProvider?: string; forceModelId?: string },
): Promise<{ verdict: ReviewVerdict; summary: string; review: string }> => {
reviewText = "";
let session: import("@earendil-works/pi-coding-agent").AgentSession;
try {
session = await createReviewerSession(sessionOptions);
} catch (err) {
if (err instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(err.reason);
}
throw err;
}
try {
try {
await runReviewPrompt(session, attemptRequest);
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
if (!isContextLimitError(errorMessage)) {
throw err;
}
const retryLogMessage = reviewType === "code"
? "code review hit context limit — retrying with compacted request"
: `${reviewType} review hit context limit — retrying with compacted request`;
reviewerLog.warn(`${taskId}: ${retryLogMessage}`);
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
await options.store.logEntry(options.taskId, retryLogMessage).catch(() => undefined);
const taskForRetry = await options.store.getTask(options.taskId);
await recordRetry({
store: options.store,
settings: retrySettings,
task: taskForRetry,
category: "reviewerContext",
role: "reviewer",
agentId: options.agentId,
});
}
reviewText = "";
const reducedRequest = buildReducedReviewRequest(
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
);
try {
await runReviewPrompt(session, reducedRequest);
} catch (retryErr: unknown) {
if (!isReviewerSessionReuseError(retryErr)) {
throw retryErr;
}
endSession(session);
try {
session = await createReviewerSession(sessionOptions);
} catch (recreateErr) {
if (recreateErr instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(recreateErr.reason);
}
throw recreateErr;
}
await runReviewPrompt(session, reducedRequest);
}
}
} finally {
if (agentLogger) {
await agentLogger.flush();
}
for (const activeSession of [...activeSessions]) {
endSession(activeSession);
}
}
const verdict = extractVerdict(reviewText);
const summary = extractSummary(reviewText);
return { verdict, review: reviewText, summary };
};
const fallbackReviewRequest = `${request}\n\nIMPORTANT: Respond with exactly one of: APPROVE | REVISE | RETHINK on a line starting with "Verdict:".`;
const logFallbackRetry = async (reason: string, mode: string): Promise<void> => {
const message = `${reviewType} review retry with fallback model after ${reason} (${mode})`;
reviewerLog.warn(`${taskId}: ${message}`);
if (options.store && options.taskId) {
await options.store.logEntry(options.taskId, message).catch(() => undefined);
}
};
const hasConfiguredFallback = Boolean(validatorFallbackProvider && validatorFallbackModelId);
const retrySettings = liveSettings ?? options.settings;
const resetReviewerFallbackRetryCount = async (): Promise<void> => {
if (!options.store || !options.taskId || typeof options.store.updateTask !== "function") {
return;
}
await options.store.updateTask(options.taskId, { reviewerFallbackRetryCount: 0 }).catch(() => undefined);
};
let firstAttempt: { verdict: ReviewVerdict; summary: string; review: string };
try {
firstAttempt = await runAttempt(request);
} catch (err) {
if (hasConfiguredFallback) {
await logFallbackRetry("reviewer error", `${validatorFallbackProvider}/${validatorFallbackModelId}`);
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
const taskForRetry = await options.store.getTask(options.taskId);
await recordRetry({
store: options.store,
settings: retrySettings,
task: taskForRetry,
category: "reviewerFallback",
role: "reviewer",
agentId: options.agentId,
});
}
try {
const fallbackResult = await runAttempt(request, {
forceProvider: validatorFallbackProvider,
forceModelId: validatorFallbackModelId,
});
if (fallbackResult.verdict !== "UNAVAILABLE") {
await resetReviewerFallbackRetryCount();
}
return fallbackResult;
} catch {
throw err;
}
}
await logFallbackRetry("reviewer error", "same-model strict prompt");
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
const taskForRetry = await options.store.getTask(options.taskId);
await recordRetry({
store: options.store,
settings: retrySettings,
task: taskForRetry,
category: "reviewerFallback",
role: "reviewer",
agentId: options.agentId,
});
}
try {
const fallbackResult = await runAttempt(fallbackReviewRequest);
if (fallbackResult.verdict !== "UNAVAILABLE") {
await resetReviewerFallbackRetryCount();
}
return fallbackResult;
} catch {
throw err;
}
}
if (firstAttempt.verdict !== "UNAVAILABLE") {
await resetReviewerFallbackRetryCount();
return firstAttempt;
}
if (hasConfiguredFallback) {
await logFallbackRetry("UNAVAILABLE verdict", `${validatorFallbackProvider}/${validatorFallbackModelId}`);
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
const taskForRetry = await options.store.getTask(options.taskId);
await recordRetry({
store: options.store,
settings: retrySettings,
task: taskForRetry,
category: "reviewerFallback",
role: "reviewer",
agentId: options.agentId,
});
}
const fallbackResult = await runAttempt(request, {
forceProvider: validatorFallbackProvider,
forceModelId: validatorFallbackModelId,
});
if (fallbackResult.verdict !== "UNAVAILABLE") {
await resetReviewerFallbackRetryCount();
}
return fallbackResult;
}
await logFallbackRetry("UNAVAILABLE verdict", "same-model strict prompt");
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
const taskForRetry = await options.store.getTask(options.taskId);
await recordRetry({
store: options.store,
settings: retrySettings,
task: taskForRetry,
category: "reviewerFallback",
role: "reviewer",
agentId: options.agentId,
});
}
const fallbackResult = await runAttempt(fallbackReviewRequest);
if (fallbackResult.verdict !== "UNAVAILABLE") {
await resetReviewerFallbackRetryCount();
}
return fallbackResult;
}
function isReviewerSessionReuseError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error);
return /prompt is in progress|session (?:is )?(?:closed|disposed|ended)|conversation already active/i.test(message);
}
function extractPromptSection(promptContent: string, sectionName: string): string {
const heading = `## ${sectionName}`;
const start = promptContent.indexOf(heading);
if (start === -1) {
return "";
}
const afterHeading = start + heading.length;
const nextH2 = promptContent.indexOf("\n## ", afterHeading);
const nextH1 = promptContent.indexOf("\n# ", afterHeading);
const endCandidates = [nextH2, nextH1].filter((value) => value !== -1);
const end = endCandidates.length > 0 ? Math.min(...endCandidates) : promptContent.length;
return promptContent.slice(start, end).trim();
}
function summarizePromptSteps(promptContent: string): string {
const stepTitles = Array.from(promptContent.matchAll(/^### Step \d+:.*$/gm), (match) => match[0].trim());
if (stepTitles.length === 0) {
return "";
}
return ["## Steps", ...stepTitles].join("\n");
}
function buildReducedTaskPromptSummary(promptContent: string): string {
const firstSectionIndex = promptContent.indexOf("\n## ");
const header = (firstSectionIndex === -1 ? promptContent : promptContent.slice(0, firstSectionIndex)).trim();
const sections = [
header,
extractPromptSection(promptContent, "Mission"),
extractPromptSection(promptContent, "Dependencies"),
extractPromptSection(promptContent, "File Scope"),
summarizePromptSteps(promptContent),
"_... additional PROMPT.md sections omitted after context-limit retry ..._",
].filter(Boolean);
return sections.join("\n\n").trim();
}
function buildReducedReviewRequest(
taskId: string,
stepNumber: number,
stepName: string,
reviewType: ReviewType,
promptContent: string,
cwd: string,
baseline?: string,
): string {
return buildReviewRequest(
taskId,
stepNumber,
stepName,
reviewType,
buildReducedTaskPromptSummary(promptContent),
cwd,
baseline,
undefined,
);
}
function buildReviewRequest(
taskId: string,
stepNumber: number,
stepName: string,
reviewType: ReviewType,
promptContent: string,
cwd: string,
baseline?: string,
userComments?: TaskComment[],
): string {
const parts = [
`Review request for task ${taskId}, Step ${stepNumber}: ${stepName}`,
`Review type: **${reviewType}**`,
"",
"## Task PROMPT.md",
"```markdown",
promptContent,
"```",
"",
];
if (reviewType === "spec") {
parts.push(
"## What to review",
"Evaluate this PROMPT.md specification for completeness and quality.",
"Assess against the spec quality criteria: mission clarity, step specificity/verifiability,",
"file scope accuracy, dependency correctness, testing requirements, documentation completeness,",
"dangling task-document references, and appropriate sizing/review level.",
"For tasks integrating third-party tools, also verify canonical upstream repo URL, docs URL, release/download URL, binary/CLI name, and checksum or explicit upstream-pending-verification marker are present.",
"",
"Read relevant source files to verify the spec references real files, functions, and patterns.",
"Check that steps have concrete, verifiable outcomes — not vague instructions.",
"Ensure testing requirements demand real automated tests with assertions.",
);
// Add user comment coverage check for spec reviews
if (userComments && userComments.length > 0) {
parts.push(
"",
"## User Comment Coverage (MANDATORY)",
"",
"The following user comments were posted on this task. You MUST verify that the spec addresses **every** comment. If any user comment is not reflected or addressed in the PROMPT.md, issue a REVISE verdict.",
"",
);
for (const comment of userComments) {
const date = comment.updatedAt || comment.createdAt;
parts.push(`- **[${date}]** ${comment.text}`);
}
parts.push(
"",
"Check each comment above against the spec content. Missing coverage for any user comment is a blocking issue.",
);
}
} else 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.",
"",
"## Worktree Boundary",
`Assigned task worktree: \`${cwd}\``,
"Verify that implementation changes are in this worktree. If you find changes or commits in the primary project checkout or any other path, issue REVISE unless the outside path is an expected project-root exception such as .fusion/memory/ files, task attachments, or explicitly documented Fusion metadata.",
"",
);
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 {
// Strategy 1: Look for a JSON verdict block (structured output)
// Matches: ```json\n{"verdict": "APPROVE"}\n``` or inline {"verdict":"REVISE"}
const jsonMatch = review.match(
/\{\s*"verdict"\s*:\s*"(APPROVE|REVISE|RETHINK)"\s*\}/i,
);
if (jsonMatch) {
reviewerLog.log(`Verdict extracted via JSON block: ${jsonMatch[1].toUpperCase()}`);
return jsonMatch[1].toUpperCase() as ReviewVerdict;
}
// Strategy 2: Look for verdict in a heading line (### Verdict: APPROVE, **Verdict: REVISE**)
// Only match lines that START with a verdict pattern to avoid matching keywords in body text
const headingMatch = review.match(
/^[>\s]*(?:###?\s*|[*_]{1,2})Verdict[:\s]*[*_]{0,2}\s*(APPROVE|REVISE|RETHINK)\b/im,
);
if (headingMatch) {
return headingMatch[1].toUpperCase() as ReviewVerdict;
}
// Strategy 3: Standalone verdict line like "Verdict: APPROVE" or "Decision: REVISE"
const lineFallback = review.match(
/^[>\s]*(?:verdict|decision)\s*[-:]\s*(APPROVE|REVISE|RETHINK)\b/im,
);
if (lineFallback) {
return lineFallback[1].toUpperCase() as ReviewVerdict;
}
reviewerLog.warn(`Could not extract verdict from review (${review.length} chars). Returning UNAVAILABLE.`);
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);
}