diff --git a/index.ts b/index.ts index 9fa77fc..a24fd68 100644 --- a/index.ts +++ b/index.ts @@ -15,11 +15,9 @@ import { import { ProgressTracker } from "./src/progress"; import { buildPlanPrompt } from "./src/prompts"; import { formatReflections } from "./src/reflection"; -import { - executeBatch, - SPINNER_FRAMES, - type SendChatMessage, -} from "./src/executor"; +import { verdictGlyph, verdictSummary, formatFindings } from "./src/review"; +import type { ReviewResult } from "./src/types"; +import { executeBatch, type SendChatMessage } from "./src/executor"; import { loadConfig, resolveTaskArg, @@ -243,7 +241,9 @@ async function executePlanBatches( sendChatMessage?: SendChatMessage, projectDir?: string, ): Promise { - // Write loop-active marker so widgets can be re-instantiated after a reload + // Write loop-active marker so a session reload can detect an interrupted + // loop and resume it (in-process agent sessions die on reload — the marker + // + progress.json in_progress tasks are the signal to re-run them). if (projectDir) { const allTaskIds = plan.batches.flatMap((b) => b.tasks.map((t) => t.id)); writeLoopActive(projectDir, { @@ -252,6 +252,9 @@ async function executePlanBatches( startedAt: new Date().toISOString(), taskIds: allTaskIds, prdKey: progress.getKey(), + autoCommit: config.execution.autoCommit, + autoReview: config.execution.autoReview, + saveReviews: config.execution.saveReviews, }); } @@ -371,6 +374,7 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { toolCalls?: Array<{ name: string; label: string }>; reviewText?: string; reviewPath?: string; + reviewResult?: ReviewResult; } | undefined; @@ -380,12 +384,34 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { // Header line — e.g. "✓ 05 · billing-subscriptions-trials (2m 14s)" lines.push(String(message.content)); - // Review body: in expanded mode render the full review text so long - // reviews aren't lost to the 500-char preview. In collapsed mode - // show a dim hint that the review is available via Ctrl+O (the - // header already carries a short tail + saved-path hint). - const hasReview = !!details?.reviewText; - if (hasReview && expanded && details!.reviewText) { + // Structured review: when we have a ReviewResult, render verdict + + // findings tree. In expanded mode show findings detail; collapsed + // shows the verdict summary + a hint to expand. + const hasReview = !!details?.reviewText || !!details?.reviewResult; + if (details?.reviewResult) { + const rv = details.reviewResult; + const glyph = verdictGlyph(rv.verdict); + const summary = verdictSummary(rv); + if (expanded) { + // Show verdict, summary, then findings tree, then raw text. + lines.push(` ${glyph} VERDICT: ${rv.verdict.toUpperCase()}`); + lines.push(` ${rv.summary}`); + if (rv.findings.length > 0) { + lines.push(` ${formatFindings(rv)}`); + } + if (details.reviewText) { + const body = details.reviewText.split("\n"); + for (const line of body) { + lines.push(` ${line}`); + } + } + } else { + const hint = details.reviewPath + ? `press Ctrl+O for full review · saved to ${details.reviewPath}` + : "press Ctrl+O for full review"; + lines.push(theme.fg("dim", ` ├── ${glyph} ${summary} · ${hint}`)); + } + } else if (hasReview && expanded && details!.reviewText) { const body = details!.reviewText.split("\n"); for (const line of body) { lines.push(` ${line}`); @@ -436,13 +462,15 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { }, ); - // ─── Reload detection: re-instantiate widgets when session reloads ────── + // ─── Reload detection: resume interrupted loops when session reloads ── // - // When the user types /reload while ralpi tasks are executing, the old - // ExtensionContext is torn down and widgets (created via ctx.ui.setWidget) - // disappear. This handler detects the reload, reads the persisted loop-active - // marker and progress.json, and re-creates live-status widgets that show - // task progress with spinner animation and tool calls from session files. + // ralpi runs task agent sessions in-process (createAgentSession), so they + // do NOT survive a /reload. When the new session starts, this handler + // reads the persisted loop-active marker + progress.json: if any task is + // still `in_progress`, the loop was interrupted mid-task and we resume it + // (resetting those tasks to pending so the DAG re-schedules them), using + // the mode + loop options snapshotted in loop-active.json so the resume is + // non-interactive. pi.on("session_start", async (event, ctx) => { if (event.reason !== "reload") return; @@ -455,89 +483,9 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { if (!loopState) return; // Load progress state - let abortPolling = false; const progressPath = path.join(projectDir, ".ralpi", "progress.json"); - const sessionsDir = path.join(projectDir, ".ralpi", "sessions"); - // Parse the task file to get task titles - const titleMap = new Map(); - try { - const project = parseTaskFile(loopState.taskFile); - for (const task of project.tasks) { - titleMap.set(task.id, task.title); - } - } catch { - // If parsing fails, just use IDs without titles - } - - /** Read recent tool calls from a task's session file. */ - const readRecentToolCalls = ( - taskId: string, - maxLines = 30, - ): Array<{ name: string; label: string }> => { - try { - const files = fs - .readdirSync(sessionsDir) - .filter((f) => f.startsWith(taskId + "-")) - .sort(); - if (files.length === 0) return []; - const sessionPath = path.join(sessionsDir, files[files.length - 1]); - const content = fs.readFileSync(sessionPath, "utf-8"); - const lines = content - .split("\n") - .filter((l) => l.trim()) - .slice(-maxLines); - const calls: Array<{ name: string; label: string }> = []; - for (const line of lines) { - try { - const event = JSON.parse(line); - if (event.type === "tool_execution_start") { - calls.push({ - name: event.toolName, - label: formatToolLabel(event.toolName, event.args), - }); - } - } catch { - // Skip malformed lines - } - } - return calls; - } catch { - return []; - } - }; - - /** - * Strip control characters and newlines from a display label so it - * does not break TUI layout (tree branches, text width calculation). - */ - function sanitizeLabel(s: string): string { - return s - .replace(/\r?\n/g, " ") - .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, "") - .trim(); - } - - /** Format a tool call argument into a short label. */ - function formatToolLabel(name: string, args: unknown): string { - const a = args as Record | undefined; - if (!a) return name; - if (name === "bash") - return sanitizeLabel(String(a.command ?? "").slice(0, 70)); - if (name === "write" || name === "read" || name === "edit") - return sanitizeLabel(String(a.path ?? "").slice(0, 60)); - if (name === "grep") - return sanitizeLabel( - `${a.pattern ?? "?"} — ${String(a.path ?? "").slice(0, 40)}`, - ); - if (name === "find") - return sanitizeLabel(`${a.path ?? "."} — ${a.glob ?? "*"}`); - if (name === "ls") - return sanitizeLabel(String(a.path ?? ".").slice(0, 60)); - return name; - } - - /** Re-read progress from disk (old tasks still writing to it). */ + /** Re-read progress from disk. */ const readTasks = (): Record | null => { try { const raw = fs.readFileSync(progressPath, "utf-8"); @@ -548,226 +496,89 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { } }; - // Early exit: if all tasks already finished during the reload, just clean up + // ralpi agent sessions run in-process (createAgentSession), so they do + // NOT survive a session reload. Any task left `in_progress` is therefore + // stalled — its agent died with the previous session. Detect that state + // and actively resume the loop instead of passively polling (which would + // spin forever waiting for a dead task to complete). const initialTasks = readTasks(); if (initialTasks) { - const remaining = Object.values(initialTasks).filter( - (t) => t.status === "in_progress", - ).length; - if (remaining === 0) { - ctx.ui.notify("All ralpi tasks completed during reload.", "info"); + const inProgressIds = Object.entries(initialTasks).flatMap(([id, t]) => + t.status === "in_progress" ? [id] : [], + ); + + if (inProgressIds.length === 0) { + // Nothing was mid-flight — loop either finished cleanly between + // the reload landing and this handler running, or was stopped + // between tasks. Clean up the stale marker and bail. + ctx.ui.notify( + "ralpi loop has no in-progress task to resume — marking complete.", + "info", + ); deleteLoopActive(projectDir); return; } - } - // Show a status notification for the reconnect - const taskCount = loopState.taskIds.length; - ctx.ui.notify( - `Reconnected to running ralpi execution (${taskCount} tasks, ${loopState.mode} mode)`, - "info", - ); + const taskCount = loopState.taskIds.length; + ctx.ui.notify( + `ralpi loop was interrupted by reload with ${inProgressIds.length} in-progress task(s). ` + + `Resuming execution (${taskCount} tasks, ${loopState.mode} mode)...`, + "info", + ); - // Shared state for the widget - let tickCount = 0; - const MAX_COLLAPSED = 3; - - if (loopState.mode === "parallel") { - // ── Parallel mode: single batch widget ── - const widgetKey = `ralpi-parallel-reconnect-${Date.now()}`; - let widgetTui: { requestRender(): void } | null = null; - - const buildBatchLines = (t: typeof ctx.ui.theme): string[] => { - const tasks = readTasks(); - if (!tasks) return [t.fg("dim", "(waiting for progress...)")]; - - const lines: string[] = []; - // Only show tasks that have started (in_progress, completed, failed). - // Pending/unstarted tasks are noise after a reload. - const sortedIds = [...loopState.taskIds].sort().filter((id) => { - const info = tasks[id]; - return info && info.status !== "pending"; + // Build the sendProgress wrapper so resumed task messages render the + // same expandable tool-call tree as an interactive run. + const sendProgress: SendChatMessage = ( + content: string, + meta?: { + toolCalls?: Array<{ name: string; label: string }>; + reviewText?: string; + reviewPath?: string; + reviewResult?: ReviewResult; + }, + ) => { + pi.sendMessage({ + customType: "ralpi-progress", + content, + display: true, + details: { + phase: "progress", + toolCalls: meta?.toolCalls, + reviewText: meta?.reviewText, + reviewPath: meta?.reviewPath, + reviewResult: meta?.reviewResult, + }, }); - - // If no tasks have started yet, show nothing — polling will pick up - // changes within 500ms. - if (sortedIds.length === 0) return [t.fg("dim", "(starting tasks...)")]; - - for (const id of sortedIds) { - const info = tasks[id]!; - const title = titleMap.get(id); - const header = title ? `${id} · ${title}` : id; - - // Status icon - if (info.status === "completed") { - lines.push(`${t.fg("success", "✓")} ${header}`); - } else if (info.status === "failed") { - lines.push(`${t.fg("error", "✗")} ${header}`); - } else if (info.status === "in_progress") { - const frame = t.fg( - "accent", - SPINNER_FRAMES[tickCount % SPINNER_FRAMES.length], - ); - lines.push(`${frame} ${header}`); - - // Show recent tool calls for active tasks - const toolCalls = readRecentToolCalls(id); - if (toolCalls.length > 0) { - if (toolCalls.length <= MAX_COLLAPSED) { - for (let i = 0; i < toolCalls.length; i++) { - const tc = toolCalls[i]; - const isLast = i === toolCalls.length - 1; - const branch = isLast ? " └── " : " ├── "; - lines.push( - `${branch}${t.fg("accent", `[${tc.name}]`)} ${tc.label}`, - ); - } - } else { - const shown = toolCalls.slice(-MAX_COLLAPSED); - const remaining = toolCalls.length - shown.length; - lines.push(t.fg("dim", ` ├── …${remaining} earlier`)); - for (let i = 0; i < shown.length; i++) { - const tc = shown[i]; - const isLast = i === shown.length - 1; - const branch = isLast ? " └── " : " ├── "; - lines.push( - `${branch}${t.fg("accent", `[${tc.name}]`)} ${tc.label}`, - ); - } - } - } - } - } - return lines; }; - ctx.ui.setWidget(widgetKey, (tui, t) => { - widgetTui = tui; - return { - render: () => buildBatchLines(t), - invalidate: () => widgetTui?.requestRender(), - }; - }); + // Load config from the project directory so model + thinking level + // resolve the same way the interactive command handler does. + const config = loadConfig(projectDir); - // 100ms tick: advances spinner frame every tick, refreshes - // progress + tool calls every 5 ticks (500ms). - const tickTimer = setInterval(() => { - if (abortPolling) return; - tickCount++; - widgetTui?.requestRender(); - - if (tickCount % 5 === 0) { - const tasks = readTasks(); - if (!tasks) return; - const activeCount = Object.values(tasks).filter( - (t) => t.status === "in_progress", - ).length; - if (activeCount === 0) { - clearInterval(tickTimer); - ctx.ui.setWidget(widgetKey, undefined); - deleteLoopActive(projectDir); - } - } - }, 100); - - // Clean up timer when extension is shut down - pi.on("session_shutdown", () => { - abortPolling = true; - clearInterval(tickTimer); - }); - } else { - // ── Sequential mode: per-task widget ── - const currentTaskId = loopState.taskIds.find((id) => { - const tasks = readTasks(); - return tasks?.[id]?.status === "in_progress"; - }); - - if (currentTaskId) { - const widgetKey = `ralpi-task-${currentTaskId}`; - let widgetTui: { requestRender(): void } | null = null; - - const buildLines = (t: typeof ctx.ui.theme): string[] => { - const tasks = readTasks(); - const info = tasks?.[currentTaskId]; - const title = titleMap.get(currentTaskId); - const header = title ? `${currentTaskId} · ${title}` : currentTaskId; - const lines: string[] = []; - - if (!info || info.status === "pending") { - return [t.fg("dim", "(starting task...)")]; - } - - if (info.status === "completed") { - lines.push(`${t.fg("success", "✓")} ${header}`); - } else if (info.status === "failed") { - lines.push(`${t.fg("error", "✗")} ${header}`); - } else if (info.status === "in_progress") { - const frame = t.fg( - "accent", - SPINNER_FRAMES[tickCount % SPINNER_FRAMES.length], - ); - lines.push(`${frame} ${header}`); - - // Show recent tool calls - const toolCalls = readRecentToolCalls(currentTaskId); - if (toolCalls.length > 0) { - const shown = toolCalls.slice(-MAX_COLLAPSED); - const remaining = toolCalls.length - shown.length; - if (remaining > 0) { - lines.push(t.fg("dim", ` ├── …${remaining} earlier`)); - } - for (let i = 0; i < shown.length; i++) { - const tc = shown[i]; - const isLast = i === shown.length - 1; - const branch = isLast ? " └── " : " ├── "; - lines.push( - `${branch}${t.fg("accent", `[${tc.name}]`)} ${tc.label}`, - ); - } - } - } - return lines; - }; - - ctx.ui.setWidget(widgetKey, (tui, t) => { - widgetTui = tui; - return { - render: () => buildLines(t), - invalidate: () => widgetTui?.requestRender(), - }; - }); - - const tickTimer = setInterval(() => { - if (abortPolling) return; - tickCount++; - widgetTui?.requestRender(); - - if (tickCount % 5 === 0) { - const tasks = readTasks(); - if (!tasks) return; - const status = tasks[currentTaskId]?.status; - if (status !== "in_progress") { - clearInterval(tickTimer); - // Keep widget visible a moment, then clean up - setTimeout(() => { - ctx.ui.setWidget(widgetKey, undefined); - deleteLoopActive(projectDir); - }, 3000); - } - } - }, 100); - - pi.on("session_shutdown", () => { - abortPolling = true; - clearInterval(tickTimer); - }); - } else { - // No task actively in progress — show a "resume" hint - ctx.ui.notify( - "No running task found. Use /ralpi resume to continue execution.", - "warning", + try { + await resumeLoop( + ctx, + loopState.taskFile, + projectDir, + loopState.prdKey, + sendProgress, + config.model ?? ctx.model, + pi.getThinkingLevel(), + { + mode: loopState.mode, + autoCommit: loopState.autoCommit ?? config.execution.autoCommit, + autoReview: loopState.autoReview ?? config.execution.autoReview, + saveReviews: loopState.saveReviews ?? config.execution.saveReviews, + skipFinalStatus: false, + }, ); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + ctx.ui.notify(`ralpi auto-resume failed: ${msg}`, "error"); + // Leave loop-active.json in place so the user can retry via + // /ralpi resume after addressing the underlying error. } + return; } }); @@ -781,7 +592,7 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { // Uses "ralpi-progress" customType with a "progress" phase so the // renderer omits the label prefix entirely (no [INFO] etc.). // Accepts an optional meta object with toolCalls for the expandable view, - // and reviewText/reviewPath for review messages so the expanded + // and reviewText/reviewPath/reviewResult for review messages so the expanded // (Ctrl+O) view can render the full review body without truncation. const sendProgress: SendChatMessage = ( content: string, @@ -789,6 +600,7 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { toolCalls?: Array<{ name: string; label: string }>; reviewText?: string; reviewPath?: string; + reviewResult?: ReviewResult; }, ) => { pi.sendMessage({ @@ -800,6 +612,7 @@ export default function ralpiLoopExtension(pi: ExtensionAPI): void { toolCalls: meta?.toolCalls, reviewText: meta?.reviewText, reviewPath: meta?.reviewPath, + reviewResult: meta?.reviewResult, }, }); }; @@ -997,6 +810,122 @@ async function handleRun( // ─── /ralpi resume ─────────────────────────────────────────────────────────── +/** + * Resume core: given a resolved task file, project dir, and PRD key, + * build the remaining plan and execute it. Used by both the explicit + * `/ralpi resume` command and the auto-resume on session reload. + * + * `mode` and loop options (`autoCommit`/`autoReview`/`saveReviews`) may be + * passed to skip interactive prompts — this is how a reload resumes + * non-interactively using the snapshot stored in loop-active.json. + * When omitted, the user is prompted as usual. + */ +async function resumeLoop( + ctx: ExtensionContext, + taskFile: string, + projectDir: string, + prdKey: string | undefined, + sendChatMessage: SendChatMessage | undefined, + parentModel: unknown, + parentThinkingLevel: unknown, + options?: { + mode?: ExecutionMode; + autoCommit?: boolean; + autoReview?: boolean; + saveReviews?: boolean; + skipFinalStatus?: boolean; + }, +): Promise { + const project = parseTaskFile(taskFile); + if (!Array.isArray(project.tasks)) { + throw new Error( + `Parsed project from ${taskFile} has invalid tasks: expected array, got ${typeof project.tasks}`, + ); + } + const config = loadConfig(projectDir); + config.model = parentModel ?? ctx.model; + config.thinkingLevel = parentThinkingLevel; + const progress = new ProgressTracker(projectDir, taskFile, prdKey); + + progress.setPaused(false); + + // Any task left `in_progress` died with the previous session (ralpi runs + // agents in-process). Reset them to `pending` so the DAG re-schedules + // them cleanly. Without this they'd still be re-run (they're not in the + // completed set), but the progress.json would carry a stale in_progress + // state during the rebuild window. + const resetIds = progress.resetInProgressToPending(); + if (resetIds.length > 0) { + // Keep the source-file checkboxes in sync so a later parse sees these + // tasks as `pending` rather than `in_progress`. + for (const id of resetIds) { + try { + updateTaskInFile(taskFile, id, "pending"); + } catch { + // Best-effort — progress.json is the source of truth for scheduling. + } + } + ctx.ui.notify( + `Reset stalled in-progress task(s) to pending: ${resetIds.join(", ")}`, + "info", + ); + } + + const completed = buildCompletedSet(progress, project); + const mode = + options?.mode ?? + (await selectExecutionMode(ctx, project, taskFile, config)); + + let autoCommit: boolean; + let autoReview: boolean; + let saveReviews: boolean; + if ( + options?.autoCommit !== undefined && + options?.autoReview !== undefined && + options?.saveReviews !== undefined + ) { + autoCommit = options.autoCommit; + autoReview = options.autoReview; + saveReviews = options.saveReviews; + } else { + const opt = await selectLoopOptions(ctx, config); + autoCommit = opt.autoCommit; + autoReview = opt.autoReview; + saveReviews = opt.saveReviews; + } + config.execution.autoCommit = autoCommit; + config.execution.autoReview = autoReview; + config.execution.saveReviews = saveReviews; + const plan = buildPlanByMode(mode, project, completed); + + // Print remaining batches before executing + const formattedPlan = formatExecutionPlan(plan); + if (mode === "parallel") { + ctx.ui.notify(`${formattedPlan}\n\nResuming parallel execution...`, "info"); + } else { + ctx.ui.notify( + `${formattedPlan}\n\nResuming sequential execution...`, + "info", + ); + } + + await executePlanBatches( + plan, + project, + taskFile, + config, + progress, + ctx, + mode, + sendChatMessage, + projectDir, + ); + + if (!options?.skipFinalStatus) { + ctx.ui.notify(formatProgressStatus(progress.getState()), "info"); + } +} + async function handleResume( ctx: ExtensionContext, args: string[], @@ -1042,54 +971,15 @@ async function handleResume( prdKey = selected.prdKey; } - const project = parseTaskFile(taskFile); - if (!Array.isArray(project.tasks)) { - throw new Error( - `Parsed project from ${taskFile} has invalid tasks: expected array, got ${typeof project.tasks}`, - ); - } - const config = loadConfig(projectDir); - config.model = parentModel ?? ctx.model; - config.thinkingLevel = parentThinkingLevel; - const progress = new ProgressTracker(projectDir, taskFile, prdKey); - - progress.setPaused(false); - - const completed = buildCompletedSet(progress, project); - const mode = await selectExecutionMode(ctx, project, taskFile, config); - const { autoCommit, autoReview, saveReviews } = await selectLoopOptions( + await resumeLoop( ctx, - config, - ); - config.execution.autoCommit = autoCommit; - config.execution.autoReview = autoReview; - config.execution.saveReviews = saveReviews; - const plan = buildPlanByMode(mode, project, completed); - - // Print remaining batches before executing - const formattedPlan = formatExecutionPlan(plan); - if (mode === "parallel") { - ctx.ui.notify(`${formattedPlan}\n\nResuming parallel execution...`, "info"); - } else { - ctx.ui.notify( - `${formattedPlan}\n\nResuming sequential execution...`, - "info", - ); - } - - await executePlanBatches( - plan, - project, taskFile, - config, - progress, - ctx, - mode, - sendChatMessage, projectDir, + prdKey, + sendChatMessage, + parentModel, + parentThinkingLevel, ); - - ctx.ui.notify(formatProgressStatus(progress.getState()), "info"); } // ─── /ralpi next ───────────────────────────────────────────────────────────── diff --git a/src/constants.ts b/src/constants.ts index c63378b..b7c5d9e 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -26,5 +26,10 @@ export const TASK_FILE_NAMES = [ export const REFLECTION_HEADER = "## REFLECTION"; export const REFLECTION_PATTERN = /##\s*REFLECTION\s*\n([\s\S]*?)(?=\n```|$)/i; +// Review verdict parsing +export const REVIEW_HEADER = "## REVIEW VERDICT"; +export const REVIEW_PATTERN = + /##\s*REVIEW\s+VERDICT\s*\n([\s\S]*?)(?=\n```|$)/i; + // Pi subprocess export const DEFAULT_PI_ARGS = ["--no-stream"] as const; diff --git a/src/executor.ts b/src/executor.ts index ae48d6e..53bcf2b 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -1,14 +1,31 @@ import { truncateToWidth } from "@earendil-works/pi-tui"; import * as path from "node:path"; -import type { Task, Project, Reflection, ToolUsage } from "./types"; +import type { + Task, + Project, + Reflection, + ToolUsage, + ReviewResult, +} from "./types"; import type { RalpiConfig } from "./types"; import type { ProgressTracker } from "./progress"; import type { ExtensionContext, ModelRuntime, } from "@earendil-works/pi-coding-agent"; -import { buildTaskPrompt, buildReviewPrompt, MAX_DIFF_BYTES } from "./prompts"; +import { + buildTaskPrompt, + buildReviewPrompt, + buildReviewPromptUncommitted, + MAX_DIFF_BYTES, +} from "./prompts"; import { extractReflection } from "./reflection"; +import { + extractReview, + saveReviewToFile as saveReviewJson, + verdictGlyph, + verdictSummary, +} from "./review"; import { runAgentSession, writeFileSafe, @@ -34,6 +51,8 @@ export type SendChatMessage = ( reviewText?: string; /** Saved file path when the review has been persisted to disk. */ reviewPath?: string; + /** Structured review result (when extractReview succeeded). */ + reviewResult?: ReviewResult; }, ) => void; @@ -168,6 +187,9 @@ export async function runTask( parallelState?: ParallelWidgetState, assignedModel?: unknown, batchRender?: () => void, + /** Review feedback from a rejected review — injected when re-executing + * a task in review-gated mode so the agent knows what to fix. */ + reviewFeedback?: ReviewResult, ): Promise<{ success: boolean; reflection?: Reflection; @@ -186,6 +208,7 @@ export async function runTask( project, depReflections, config.prompts.projectContext, + reviewFeedback, ); const taskHeader = `${task.id} · ${task.title}`; @@ -698,106 +721,241 @@ async function executeTask( ); if (result.success) { - // ── Auto-Commit: optionally trigger follow-up agent session for uncommitted changes ── let finalCommitMessages = result.commitMessages ?? []; let finalCommitSummary = result.commitSummary ?? ""; + let finalReview: ReviewResult | undefined; + let reviewRetries = 0; + + if (config.execution.autoCommit && config.execution.autoReview) { + // ── Review-gated commit: review FIRST, loop on reject, commit on pass ── + // The review examines uncommitted changes before the commit. If the + // verdict is "fail", the task is re-executed with the review feedback + // injected into the prompt (up to maxReviewRetries). Only when the + // review passes (or retries exhaust) does the commit session run. + const maxRetries = config.execution.maxReviewRetries; + let attempt = 0; - if (config.execution.autoCommit) { try { - if (hasUncommittedChanges(projectDir)) { + while (hasUncommittedChanges(projectDir)) { const status = getGitStatusPorcelain(projectDir); - let diff = getGitDiff(projectDir); - let diffNote = ""; - if (diff.length > MAX_DIFF_BYTES) { - diffNote = - "\n\n... (diff truncated: omitted " + - (diff.length - MAX_DIFF_BYTES).toLocaleString() + - " bytes; run `git diff` to view the full diff)"; - diff = diff.slice(0, MAX_DIFF_BYTES); - } - const commitPrompt = [ - `## Auto-Commit for Task ${task.id}: ${task.title}`, - "", - "The previous task is complete. There are uncommitted changes in the repository.", - "", - "Only commit changes you made while completing this task. Do not commit pre-existing changes, changes from other work, or files unrelated to this task.", - "Review the git status and diff below to identify which changes are from your work, and stage only those files.", - "", - "Stage only the files relevant to this task with `git add `, then create a meaningful git commit.", - "Use a descriptive commit message and follow conventional commits format.", - "Do NOT include the task number, task ID, or any ralpi task reference in the commit message. The commit message must describe only the work done — never mention the task ID (e.g. `task 03`, `#3`, etc.).", - "", - "### Current Changes (git status --porcelain)", - "```text", - status || "(no status output)", - "```", - "", - "### Current Tracked Diff (git diff)", - "```diff", - diff || "(no tracked diff output)", - diffNote, - "```", - ].join("\n"); + const reviewDiff = getGitDiff(projectDir); + if (!reviewDiff && !status) break; - // Resolve commit model (fall back to current task model) - const commitModel = - resolveModelSpec( - ctx.modelRegistry as - | { find(p: string, m: string): unknown } - | undefined, - config.execution.commitModel, - (msg) => ctx.ui.notify(msg, "warning"), - ) ?? currentModel; + const reviewPrompt = buildReviewPromptUncommitted( + task, + project, + status, + reviewDiff, + config.prompts.projectContext, + ); - // Build failover list: primary model first, then the rest of the pool. - const commitModels = buildFailoverModels(commitModel, roundRobin); + const reviewModel = resolveFollowUpModel( + ctx, + config.execution.reviewModel, + currentModel, + ); + const reviewModels = buildFailoverModels(reviewModel, roundRobin); - const { result: commitResult, toolCalls: commitToolCalls } = + const { result: reviewResult, toolCalls: reviewToolCalls } = await runFollowUpSession( ctx, config, - commitPrompt, + reviewPrompt, projectDir, - `commit for ${task.id} · ${task.title}`, - `commit-${task.id}`, - config.execution.commitTimeoutMs, - commitModels, + `review for ${task.id} · ${task.title}${ + attempt > 0 ? ` (attempt ${attempt + 1})` : "" + }`, + `review-${task.id}`, + config.execution.reviewTimeoutMs, + reviewModels, ); - if (commitResult.success) { - // Re-capture commits made during this follow-up session - const newCommits = captureGitCommits(projectDir); - if (newCommits.commitMessages.length > 0) { - finalCommitMessages = [ - ...finalCommitMessages, - ...newCommits.commitMessages, - ]; - finalCommitSummary = finalCommitSummary - ? `${finalCommitSummary}; ${newCommits.commitSummary}` - : newCommits.commitSummary; - } - sendChatMessage?.(`✓ commit for ${task.id} · ${task.title}`, { - toolCalls: commitToolCalls, - }); - } else { + if (!reviewResult.success) { sendChatMessage?.( - `~ commit for ${task.id} · ${task.title} — follow-up commit session failed: ${commitResult.error}`, - { toolCalls: commitToolCalls }, + `~ review for ${task.id} · ${task.title} — review session failed: ${reviewResult.error}`, + { toolCalls: reviewToolCalls }, ); + break; // commit what we have + } + + const reviewText = reviewResult.text.trim(); + const review = extractReview(reviewText, task.id, "uncommitted"); + finalReview = review ?? undefined; + + // Persist structured review JSON when opted in. + let reviewPath: string | undefined; + if (review && config.execution.saveReviews) { + reviewPath = saveReviewJson( + projectDir, + config.paths.reviewsDir, + review, + progress.getKey(), + ); + } + + if ( + review && + (review.verdict === "pass" || review.verdict === "warn") + ) { + // Review passed — proceed to commit. + const label = `${verdictGlyph(review.verdict)} ${verdictSummary(review)}`; + const savedHint = reviewPath ? ` · saved to ${reviewPath}` : ""; + sendChatMessage?.( + `⚑ review for ${task.id} · ${task.title} — ${label}${savedHint}`, + { + toolCalls: reviewToolCalls, + reviewText, + reviewPath, + reviewResult: review, + }, + ); + break; // good to commit + } + + // Review rejected (fail) or verdict not parsed. + if (review) { + sendChatMessage?.( + `⚑ review for ${task.id} · ${task.title} — ${verdictGlyph(review.verdict)} ${verdictSummary(review)}`, + { + toolCalls: reviewToolCalls, + reviewText, + reviewPath, + reviewResult: review, + }, + ); + } else { + const lines = reviewText.split("\n").filter((l) => l.trim()); + const tail = lines.slice(-3).join("\n"); + const savedHint = reviewPath ? ` · saved to ${reviewPath}` : ""; + sendChatMessage?.( + `⚑ review for ${task.id} · ${task.title} — verdict not found${savedHint}\n${tail}`, + { toolCalls: reviewToolCalls, reviewText, reviewPath }, + ); + } + + if (attempt >= maxRetries) { + // Retries exhausted. + if (config.execution.reviewBlockOnFail) { + sendChatMessage?.( + `✗ ${task.id} · ${task.title} — review rejected after ${maxRetries} retr${maxRetries === 1 ? "y" : "ies"} (reviewBlockOnFail)`, + ); + progress.markFailed( + task.id, + `Review rejected after ${maxRetries} re-execution attempt(s)`, + ); + try { + updateTaskInFile(project.sourcePath, task.id, "failed"); + } catch { + // Best-effort + } + roundRobin?.release(task.id); + return; + } + sendChatMessage?.( + `~ review for ${task.id} · ${task.title} — max retries (${maxRetries}) exhausted, committing anyway`, + ); + break; // commit what we have + } + + attempt++; + reviewRetries++; + sendChatMessage?.( + `↻ review for ${task.id} · ${task.title} — verdict ${review?.verdict ?? "unknown"}, re-executing with feedback (${attempt}/${maxRetries})...`, + ); + + // Re-execute the task with review feedback injected. + const fixResult = await runTask( + task, + project, + config, + depReflections, + ctx, + sendChatMessage, + projectDir, + parallelState, + currentModel, + batchRender, + review ?? undefined, + ); + + if (!fixResult.success) { + sendChatMessage?.( + `~ re-execution for ${task.id} · ${task.title} failed: ${fixResult.error}`, + ); + break; // commit what we have + } + + // Merge commit messages from the fix attempt. + finalCommitMessages = [ + ...finalCommitMessages, + ...(fixResult.commitMessages ?? []), + ]; + finalCommitSummary = finalCommitSummary + ? `${finalCommitSummary}; ${fixResult.commitSummary ?? ""}` + : (fixResult.commitSummary ?? ""); + // Loop back to review the updated changes. + } + + // ── Commit (after review passes or retries exhausted) ── + if (hasUncommittedChanges(projectDir)) { + const commitResult = await runCommitSession( + ctx, + config, + task, + projectDir, + currentModel, + roundRobin, + sendChatMessage, + ); + if (commitResult.success) { + finalCommitMessages = [ + ...finalCommitMessages, + ...commitResult.commitMessages, + ]; + finalCommitSummary = finalCommitSummary + ? `${finalCommitSummary}; ${commitResult.commitSummary}` + : commitResult.commitSummary; + } + } + } catch (error) { + sendChatMessage?.( + `~ review/commit for ${task.id} · ${task.title} — error: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } else if (config.execution.autoCommit) { + // ── Commit only (no review) — legacy path ── + try { + if (hasUncommittedChanges(projectDir)) { + const commitResult = await runCommitSession( + ctx, + config, + task, + projectDir, + currentModel, + roundRobin, + sendChatMessage, + ); + if (commitResult.success) { + finalCommitMessages = [ + ...finalCommitMessages, + ...commitResult.commitMessages, + ]; + finalCommitSummary = finalCommitSummary + ? `${finalCommitSummary}; ${commitResult.commitSummary}` + : commitResult.commitSummary; } } } catch (error) { - // Don't fail the task if auto-commit fails sendChatMessage?.( `~ commit for ${task.id} · ${task.title} — auto-commit error: ${ error instanceof Error ? error.message : String(error) }`, ); } - } - - // ── Auto-Review: optionally spawn a review agent to review the latest commit ── - if (config.execution.autoReview) { + } else if (config.execution.autoReview) { + // ── Review only (no commit) — reviews latest commit — legacy path ── try { const commitInfo = getLatestCommitDiff(projectDir); if (commitInfo && commitInfo.diff) { @@ -810,17 +968,11 @@ async function executeTask( config.prompts.projectContext, ); - // Resolve review model (fall back to current task model) - const reviewModel = - resolveModelSpec( - ctx.modelRegistry as - | { find(p: string, m: string): unknown } - | undefined, - config.execution.reviewModel, - (msg) => ctx.ui.notify(msg, "warning"), - ) ?? currentModel; - - // Build failover list: primary model first, then the rest of the pool. + const reviewModel = resolveFollowUpModel( + ctx, + config.execution.reviewModel, + currentModel, + ); const reviewModels = buildFailoverModels(reviewModel, roundRobin); const { result: reviewResult, toolCalls: reviewToolCalls } = @@ -837,35 +989,48 @@ async function executeTask( if (reviewResult.success) { const reviewText = reviewResult.text.trim(); + const review = extractReview( + reviewText, + task.id, + commitInfo.hash, + ); + finalReview = review ?? undefined; - // Persist the full review to disk when opted in at loop - // start. Mirrors the reflections layout so a repo can - // hold many loops without collisions: - // .ralpi/reviews//.md let reviewPath: string | undefined; - if (config.execution.saveReviews) { - reviewPath = saveReviewToFile( + if (review && config.execution.saveReviews) { + reviewPath = saveReviewJson( projectDir, - config, - task.id, - reviewText, + config.paths.reviewsDir, + review, progress.getKey(), ); } - // Post review as a chat message. The full body is - // passed via meta.reviewText so the expanded (Ctrl+O) - // view can render it without truncation; the collapsed - // content shows a short tail + a hint to expand. - const lines = reviewText.split("\n").filter((l) => l.trim()); - const tail = lines.slice(-3).join("\n"); - const savedHint = reviewPath - ? ` \u00b7 saved to ${reviewPath}` - : ""; - sendChatMessage?.( - `⚑ review for ${task.id} · ${task.title}${savedHint}\n${tail}`, - { toolCalls: reviewToolCalls, reviewText, reviewPath }, - ); + if (review) { + const label = `${verdictGlyph(review.verdict)} ${verdictSummary(review)}`; + const savedHint = reviewPath + ? ` · saved to ${reviewPath}` + : ""; + sendChatMessage?.( + `⚑ review for ${task.id} · ${task.title} — ${label}${savedHint}`, + { + toolCalls: reviewToolCalls, + reviewText, + reviewPath, + reviewResult: review, + }, + ); + } else { + const lines = reviewText.split("\n").filter((l) => l.trim()); + const tail = lines.slice(-3).join("\n"); + const savedHint = reviewPath + ? ` \u00b7 saved to ${reviewPath}` + : ""; + sendChatMessage?.( + `⚑ review for ${task.id} · ${task.title}${savedHint}\n${tail}`, + { toolCalls: reviewToolCalls, reviewText, reviewPath }, + ); + } } else { sendChatMessage?.( `~ review for ${task.id} · ${task.title} — review session failed: ${reviewResult.error}`, @@ -874,7 +1039,6 @@ async function executeTask( } } } catch (error) { - // Don't fail the task if auto-review fails sendChatMessage?.( `~ review for ${task.id} · ${task.title} — auto-review error: ${ error instanceof Error ? error.message : String(error) @@ -902,6 +1066,8 @@ async function executeTask( result.outputPreview, finalCommitMessages, finalCommitSummary, + finalReview, + reviewRetries, ); // Auto-update the PRD source file checkbox try { @@ -987,24 +1153,6 @@ function saveReflectionToFile( writeFileSafe(filePath, JSON.stringify(reflection, null, 2)); } -// ─── Save Review Output to File ───────────────────────────────────────────── -// Mirrors saveReflectionToFile's per-loop layout so a repo can hold many -// loops without collisions: .ralpi/reviews//.md - -function saveReviewToFile( - sourceDir: string, - config: RalpiConfig, - taskId: string, - reviewText: string, - prdKey: string, -): string { - const reviewsDir = path.join(sourceDir, config.paths.reviewsDir, prdKey); - ensureDir(reviewsDir); - const filePath = path.join(reviewsDir, `${taskId}.md`); - writeFileSafe(filePath, reviewText); - return filePath; -} - // ─── Follow-Up Sessions (Commit / Review) ───────────────────────────────────── /** @@ -1159,6 +1307,129 @@ function buildFailoverModels( // ─── Tool Call Formatting ──────────────────────────────────────────────── +/** + * Shorthand type for the model registry's find() shape. + */ +type ModelRegistryLike = { find(p: string, m: string): unknown }; + +/** + * Resolve a model spec for a follow-up session (commit/review), falling back + * to `currentModel` when the config field is blank or the registry can't + * resolve it. Warns via `ctx.ui.notify` on resolution failure. + */ +function resolveFollowUpModel( + ctx: ExtensionContext, + spec: string, + currentModel: unknown, +): unknown { + return ( + resolveModelSpec( + ctx.modelRegistry as ModelRegistryLike | undefined, + spec, + (msg) => ctx.ui.notify(msg, "warning"), + ) ?? currentModel + ); +} + +/** + * Run the auto-commit follow-up agent session. + * Returns the commit messages, summary, tool calls, and success flag. + */ +async function runCommitSession( + ctx: ExtensionContext, + config: RalpiConfig, + task: Task, + projectDir: string, + currentModel: unknown, + roundRobin: ModelRoundRobin | null | undefined, + sendChatMessage?: SendChatMessage, +): Promise<{ + commitMessages: string[]; + commitSummary: string; + toolCalls: ToolCallEntry[]; + success: boolean; +}> { + const status = getGitStatusPorcelain(projectDir); + let diff = getGitDiff(projectDir); + let diffNote = ""; + if (diff.length > MAX_DIFF_BYTES) { + diffNote = + "\n\n... (diff truncated: omitted " + + (diff.length - MAX_DIFF_BYTES).toLocaleString() + + " bytes; run `git diff` to view the full diff)"; + diff = diff.slice(0, MAX_DIFF_BYTES); + } + const commitPrompt = [ + `## Auto-Commit for Task ${task.id}: ${task.title}`, + "", + "The previous task is complete. There are uncommitted changes in the repository.", + "", + "Only commit changes you made while completing this task. Do not commit pre-existing changes, changes from other work, or files unrelated to this task.", + "Review the git status and diff below to identify which changes are from your work, and stage only those files.", + "", + "Stage only the files relevant to this task with `git add `, then create a meaningful git commit.", + "Use a descriptive commit message and follow conventional commits format.", + "Do NOT include the task number, task ID, or any ralpi task reference in the commit message. The commit message must describe only the work done — never mention the task ID (e.g. `task 03`, `#3`, etc.).", + "", + "### Current Changes (git status --porcelain)", + "```text", + status || "(no status output)", + "```", + "", + "### Current Tracked Diff (git diff)", + "```diff", + diff || "(no tracked diff output)", + diffNote, + "```", + ].join("\n"); + + const commitModel = resolveFollowUpModel( + ctx, + config.execution.commitModel, + currentModel, + ); + const commitModels = buildFailoverModels(commitModel, roundRobin); + + const { result: commitResult, toolCalls: commitToolCalls } = + await runFollowUpSession( + ctx, + config, + commitPrompt, + projectDir, + `commit for ${task.id} · ${task.title}`, + `commit-${task.id}`, + config.execution.commitTimeoutMs, + commitModels, + ); + + if (commitResult.success) { + const newCommits = captureGitCommits(projectDir); + const commitMessages = + newCommits.commitMessages.length > 0 ? newCommits.commitMessages : []; + const commitSummary = newCommits.commitSummary || ""; + sendChatMessage?.(`✓ commit for ${task.id} · ${task.title}`, { + toolCalls: commitToolCalls, + }); + return { + commitMessages, + commitSummary, + toolCalls: commitToolCalls, + success: true, + }; + } + + sendChatMessage?.( + `~ commit for ${task.id} · ${task.title} — follow-up commit session failed: ${commitResult.error}`, + { toolCalls: commitToolCalls }, + ); + return { + commitMessages: [], + commitSummary: "", + toolCalls: commitToolCalls, + success: false, + }; +} + /** * Strip control characters and newlines from a display label so it * does not break TUI layout (tree branches, text width calculation). diff --git a/src/progress.ts b/src/progress.ts index b77b77c..5f79479 100644 --- a/src/progress.ts +++ b/src/progress.ts @@ -6,6 +6,7 @@ import type { Task, Reflection, ToolUsage, + ReviewResult, } from "./types"; import { ensureDir } from "./utils"; @@ -174,6 +175,8 @@ export class ProgressTracker { outputPreview?: string, commitMessages?: string[], commitSummary?: string, + review?: ReviewResult, + reviewRetries?: number, ): void { const prd = this.getPRD(); this.ensureTask(prd, taskId); @@ -185,6 +188,9 @@ export class ProgressTracker { if (outputPreview) prd.tasks[taskId].outputPreview = outputPreview; if (commitMessages) prd.tasks[taskId].commitMessages = commitMessages; if (commitSummary) prd.tasks[taskId].commitSummary = commitSummary; + if (review) prd.tasks[taskId].review = review; + if (reviewRetries !== undefined) + prd.tasks[taskId].reviewRetries = reviewRetries; this.save(); } @@ -244,6 +250,26 @@ export class ProgressTracker { this.save(); } + /** Reset all `in_progress` tasks back to `pending`. + * + * Used after a session reload: in-process agent sessions die with the + * parent session, so any task left `in_progress` is actually stalled. + * Resetting ensures the DAG re-schedules it on the next resume. Returns + * the IDs that were reset. */ + resetInProgressToPending(): string[] { + const prd = this.getPRD(); + const reset: string[] = []; + for (const [id, info] of Object.entries(prd.tasks)) { + if (info.status === "in_progress") { + info.status = "pending"; + delete info.startedAt; + reset.push(id); + } + } + if (reset.length > 0) this.save(); + return reset; + } + /** Get the raw PRD state (for status display) */ getState(): PRDProgress { return this.getPRD(); diff --git a/src/prompts.ts b/src/prompts.ts index c2b43a5..c7e69a1 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -1,4 +1,4 @@ -import type { Task, Project, Reflection } from "./types"; +import type { Task, Project, Reflection, ReviewResult } from "./types"; import { readTaskSpec } from "./parser"; /** Maximum bytes of a commit diff embedded in a review/commit prompt. @@ -35,6 +35,9 @@ export function buildTaskPrompt( project: Project, depReflections: Reflection[], projectContext?: string, + /** Review feedback from a rejected review — injected when re-executing + * a task in review-gated mode so the agent knows what to fix. */ + reviewFeedback?: ReviewResult, ): string { const parts: string[] = []; @@ -130,6 +133,32 @@ export function buildTaskPrompt( parts.push(""); } + // ── Previous Review Feedback (re-execution only) ── + + if (reviewFeedback) { + parts.push("## Previous Review Feedback — FIX REQUIRED"); + parts.push( + "A review agent examined your previous attempt and rejected it.", + ); + parts.push(`Verdict: **${reviewFeedback.verdict.toUpperCase()}**`); + parts.push(`Summary: ${reviewFeedback.summary}`); + parts.push(""); + if (reviewFeedback.findings.length > 0) { + parts.push("You MUST address these findings:"); + for (const finding of reviewFeedback.findings) { + const loc = finding.file + ? finding.line + ? ` (${finding.file}:${finding.line})` + : ` (${finding.file})` + : ""; + parts.push(`- [${finding.severity}]${loc} ${finding.message}`); + } + parts.push(""); + } + parts.push("Fix every issue above. Do not re-introduce the same problems."); + parts.push(""); + } + // ── Reflection Instructions ── parts.push("## REFLECTION (REQUIRED)"); @@ -227,24 +256,141 @@ export function buildReviewPrompt( parts.push( "Review the commit above against the task description. Check for:", ); - parts.push( - "- **Correctness**: Does the implementation fulfill the task requirements?", - ); - parts.push("- **Completeness**: Are all aspects of the task addressed?"); - parts.push( - "- **Code quality**: Are there obvious bugs, anti-patterns, or issues?", - ); - parts.push( - "- **Missing changes**: Are there files that should have been modified but weren't?", - ); + parts.push(...reviewInstructions()); parts.push(""); parts.push( - "Provide a concise review with any issues found. If the commit looks good, say so explicitly.", + "Provide a concise review with any issues found. Your free-form prose", ); + parts.push("precedes the structured verdict block below."); + parts.push(...reviewVerdictBlock()); return parts.join("\n"); } +// ─── Uncommitted-Changes Review Prompt ────────────────────────────────────── + +/** + * Build a review prompt for uncommitted working-tree changes (pre-commit). + * Used in review-gated mode: the review runs BEFORE committing so a rejected + * review triggers a re-execution instead of a bad commit. + */ +export function buildReviewPromptUncommitted( + task: Task, + project: Project, + status: string, + diff: string, + projectContext?: string, +): string { + const parts: string[] = []; + + parts.push(`# Code Review (pre-commit): Task ${task.id}: ${task.title}`); + parts.push(""); + + // ── Task Description ── + + parts.push("## Task Description"); + if (task.description) { + parts.push(task.description); + } else { + parts.push(task.title); + } + parts.push(""); + + // ── Task Specification ── + + if (task.file) { + const spec = readTaskSpec(project.sourceDir, task.file); + if (spec) { + parts.push("## Task Specification"); + parts.push(`Full details from \`${task.file}\`:`); + parts.push(""); + parts.push(spec); + parts.push(""); + } + } + + // ── Uncommitted Changes Under Review ── + + parts.push("## Uncommitted Changes Under Review"); + parts.push( + "Review the working-tree changes below against the task description.", + ); + parts.push(""); + parts.push("### Current Changes (git status --porcelain)"); + parts.push("```text"); + parts.push(status || "(no status output)"); + parts.push("```"); + parts.push(""); + parts.push("### Current Tracked Diff (git diff)"); + parts.push("```diff"); + parts.push(truncateDiff(diff) || "(no tracked diff output)"); + parts.push("```"); + parts.push(""); + + // ── Project Context ── + + if (projectContext) { + parts.push("## Additional Context"); + parts.push(projectContext); + parts.push(""); + } + + // ── Review Instructions ── + + parts.push("## Review Instructions"); + parts.push( + "Review the uncommitted changes above against the task description. Check for:", + ); + parts.push(...reviewInstructions()); + parts.push(""); + parts.push( + "Provide a concise review with any issues found. Your free-form prose", + ); + parts.push("precedes the structured verdict block below."); + parts.push(...reviewVerdictBlock()); + + return parts.join("\n"); +} + +// ─── Shared Review Prompt Helpers ─────────────────────────────────────────── + +function reviewInstructions(): string[] { + return [ + "- **Correctness**: Does the implementation fulfill the task requirements?", + "- **Completeness**: Are all aspects of the task addressed?", + "- **Code quality**: Are there obvious bugs, anti-patterns, or issues?", + "- **Missing changes**: Are there files that should have been modified but weren't?", + ]; +} + +function reviewVerdictBlock(): string[] { + return [ + "## REVIEW VERDICT (REQUIRED)", + "End your response with a verdict block in EXACTLY this format:", + "", + "```", + "## REVIEW VERDICT", + "VERDICT: [pass | warn | fail]", + "SUMMARY: [1-2 sentence overall assessment]", + "FINDINGS:", + "- [blocker] file:line description (use severity: blocker|warning|nit|info)", + "- [warning] file:line description", + "```", + "", + "Verdict guidance:", + "- **pass**: the implementation fully satisfies the task requirements; no", + " action needed. Use an empty FINDINGS section (just the header).", + "- **warn**: the implementation is acceptable but has minor issues worth fixing", + " in a follow-up; not blocking.", + "- **fail**: the implementation does not satisfy the task, or has serious bugs", + " that must be fixed before proceeding.", + "", + "Each FINDINGS line uses the form `- [severity] [file:line] message`.", + "The `file:line` part is optional. Severity must be one of:", + "`blocker`, `warning`, `nit`, `info`.", + ]; +} + /** * Build the prompt for a dry-run / plan display */ diff --git a/src/review.ts b/src/review.ts new file mode 100644 index 0000000..c5a84f1 --- /dev/null +++ b/src/review.ts @@ -0,0 +1,211 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; +import type { ReviewResult, ReviewFinding, ReviewVerdict } from "./types"; +import { REVIEW_PATTERN } from "./constants"; +import { ensureDir, writeFileSafe } from "./utils"; + +// ─── Extract Structured Review ────────────────────────────────────────────── + +/** + * Extract a structured review verdict from the review agent's output text. + * Mirrors extractReflection() — parses a `## REVIEW VERDICT` block emitted at + * the end of the response. + * + * The raw text is preserved on the ReviewResult so the expanded (Ctrl+O) view + * can still render the full free-form prose. Returns null when no verdict + * block is found (caller falls back to free-form text handling). + */ +export function extractReview( + output: string, + taskId: string, + commitHash: string, +): ReviewResult | null { + const match = output.match(REVIEW_PATTERN); + if (!match) return null; + + const block = match[1]; + const verdict = extractVerdict(block); + if (!verdict) return null; // verdict is the one required field + + const summary = extractField(block, "SUMMARY") ?? ""; + const findings = extractFindings(block); + + return { + taskId, + verdict, + summary: summary || verdictLabel(verdict), + findings, + commitHash, + rawText: output.trim(), + timestamp: new Date().toISOString(), + }; +} + +function extractVerdict(block: string): ReviewVerdict | null { + const raw = extractField(block, "VERDICT"); + if (!raw) return null; + const v = raw.toLowerCase().trim(); + if (v === "pass" || v === "warn" || v === "fail") return v; + // Tolerate common synonyms + if (v === "warning" || v === "minor") return "warn"; + if (v === "fail" || v === "failing" || v === "blocker") return "fail"; + if (v === "ok" || v === "passing" || v === "approve") return "pass"; + return null; +} + +// Allowlisted static regexes — `field` is always a known literal, but we use +// a static map rather than string interpolation so there's no dynamic regex +// construction at all (`new RegExp` from a variable trips ReDoS linters). +const FIELD_PATTERNS: Record = { + VERDICT: /VERDICT:\s*(.+?)$/im, + SUMMARY: /SUMMARY:\s*(.+?)$/im, +}; + +function extractField(block: string, field: string): string | null { + const regex = FIELD_PATTERNS[field.toUpperCase()]; + if (!regex) return null; + const match = block.match(regex); + return match ? match[1].trim() : null; +} + +/** + * Parse FINDINGS: lines into structured ReviewFinding objects. + * Each finding line is expected as: + * - [severity] [file:line] message + * where severity is one of blocker|warning|nit|info. + * Falls back gracefully — an unparseable line becomes an info-severity + * finding with the raw line as the message. + */ +function extractFindings(block: string): ReviewFinding[] { + // Match the FINDINGS: header, then capture all following bullet lines. + const regex = /FINDINGS:\s*\n((?:[-*]\s+.+\n?)+)/i; + const match = block.match(regex); + if (!match) return []; + + const lines = match[1] + .split("\n") + .map((l) => l.replace(/^[-*]\s*/, "").trim()) + .filter(Boolean); + + const findings: ReviewFinding[] = []; + const severityRe = /^\[(blocker|warning|warn|nit|info)\]\s*(.*)$/i; + + for (const line of lines) { + const sm = line.match(severityRe); + if (sm) { + let sev = sm[1].toLowerCase(); + if (sev === "warn") sev = "warning"; + const rest = sm[2].trim(); + const { file, line: lineNum, message } = parseFileRef(rest); + findings.push({ + severity: sev as ReviewFinding["severity"], + file, + line: lineNum, + message, + }); + } else { + // No severity bracket — treat as info + const { file, line: lineNum, message } = parseFileRef(line); + findings.push({ severity: "info", file, line: lineNum, message }); + } + } + + return findings; +} + +/** Parse an optional `file:line` prefix from a finding message. */ +function parseFileRef(rest: string): { + file?: string; + line?: number; + message: string; +} { + const m = rest.match(/^([\w./-]+):(\d+)\s*[-—]?\s*(.*)$/); + if (m) { + return { file: m[1], line: Number(m[2]), message: m[3].trim() || rest }; + } + return { message: rest }; +} + +function verdictLabel(v: ReviewVerdict): string { + switch (v) { + case "pass": + return "Commit satisfies the task requirements."; + case "warn": + return "Commit passes with minor issues worth addressing."; + case "fail": + return "Commit does not satisfy the task requirements."; + } +} + +// ─── Save / Load Structured Reviews ───────────────────────────────────────── + +/** + * Save a structured review as JSON alongside (or instead of) the markdown + * body. Mirrors saveReflectionToFile's per-loop layout so a repo can hold + * many loops without collisions: + * .ralpi/reviews//.json + */ +export function saveReviewToFile( + sourceDir: string, + reviewsDir: string, + review: ReviewResult, + prdKey: string, +): string { + const dir = path.join(sourceDir, reviewsDir, prdKey); + ensureDir(dir); + const filePath = path.join(dir, `${review.taskId}.json`); + writeFileSafe(filePath, JSON.stringify(review, null, 2)); + return filePath; +} + +/** + * Load a structured review from disk. + */ +export function loadReview( + sourceDir: string, + reviewsDir: string, + taskId: string, + prdKey: string, +): ReviewResult | null { + const filePath = path.join(sourceDir, reviewsDir, prdKey, `${taskId}.json`); + if (!fs.existsSync(filePath)) return null; + try { + return JSON.parse(fs.readFileSync(filePath, "utf-8")) as ReviewResult; + } catch { + return null; + } +} + +// ─── Formatting ────────────────────────────────────────────────────────────── + +/** Verdict glyph for compact display in chat headers / widgets. */ +export function verdictGlyph(v: ReviewVerdict): string { + switch (v) { + case "pass": + return "✓"; + case "warn": + return "⚠"; + case "fail": + return "✗"; + } +} + +/** Short label: "PASS · 0 findings", "WARN · 2 findings", "FAIL · 3 findings" */ +export function verdictSummary(review: ReviewResult): string { + const n = review.findings.length; + const noun = n === 1 ? "finding" : "findings"; + return `${review.verdict.toUpperCase()} · ${n} ${noun}`; +} + +/** + * Format findings as an indented markdown tree for the expanded view. + */ +export function formatFindings(review: ReviewResult): string { + if (review.findings.length === 0) return "(no findings)"; + const lines: string[] = []; + for (const f of review.findings) { + const loc = f.file ? (f.line ? `${f.file}:${f.line}` : f.file) : ""; + lines.push(` - [${f.severity}]${loc ? ` ${loc}` : ""} — ${f.message}`); + } + return lines.join("\n"); +} diff --git a/src/types.ts b/src/types.ts index 35c7acd..a3356e2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -103,6 +103,37 @@ export interface Reflection { timestamp: string; } +// ─── Review Model ──────────────────────────────────────────────────────────── + +export type ReviewVerdict = "pass" | "warn" | "fail"; + +export interface ReviewFinding { + /** Severity of the finding */ + severity: "blocker" | "warning" | "nit" | "info"; + /** File path if applicable */ + file?: string; + /** Line number if applicable */ + line?: number; + /** Description of the issue */ + message: string; +} + +export interface ReviewResult { + taskId: string; + /** Overall verdict */ + verdict: ReviewVerdict; + /** 1-2 sentence overall assessment */ + summary: string; + /** Structured findings (empty when verdict is "pass") */ + findings: ReviewFinding[]; + /** Commit hash the review was performed against */ + commitHash: string; + /** Full free-form review text (preserved for display) */ + rawText: string; + /** ISO timestamp */ + timestamp: string; +} + export interface ToolUsage { read: number; write: number; @@ -117,6 +148,8 @@ export interface TaskProgressInfo { completedAt?: string; durationMs?: number; reflection?: Reflection; + /** Structured review result (when autoReview is enabled) */ + review?: ReviewResult; error?: string; /** Tool usage counts from parsed subprocess output */ toolUsage?: ToolUsage; @@ -126,6 +159,8 @@ export interface TaskProgressInfo { commitMessages?: string[]; /** Summary derived from git commits */ commitSummary?: string; + /** Number of review-fix re-execution attempts made (review-gated mode) */ + reviewRetries?: number; } export interface ProgressState { @@ -194,6 +229,13 @@ export interface RalpiConfig { commitTimeoutMs: number; /** Timeout for auto-review agent sessions in milliseconds */ reviewTimeoutMs: number; + /** Max review-fix re-execution attempts before giving up and committing + * anyway (0 = no retries; review runs once, reject = commit anyway). + * Only active when both autoCommit AND autoReview are enabled. */ + maxReviewRetries: number; + /** When true, a 'fail' review verdict after exhausting maxReviewRetries + * marks the task as failed instead of committing. */ + reviewBlockOnFail: boolean; /** Maximum total duration for the entire loop execution in milliseconds (0 = no limit). Checked between batches — in-progress tasks finish naturally. */ loopTimeoutMs: number; }; @@ -227,6 +269,8 @@ export const DEFAULT_CONFIG: RalpiConfig = { implModel: "", commitTimeoutMs: 0, // 0 = inherit Pi's own defaults (no ralpi-level timeout) reviewTimeoutMs: 0, // 0 = inherit Pi's own defaults (no ralpi-level timeout) + maxReviewRetries: 2, // 2 re-execution attempts on review rejection before giving up + reviewBlockOnFail: false, // false = commit anyway after retries exhausted loopTimeoutMs: 0, // 0 = no limit }, prompts: { diff --git a/src/utils.ts b/src/utils.ts index 00c8afb..ee97089 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -40,7 +40,8 @@ export function writeFileSafe(filePath: string, content: string): void { /** * State persisted to disk when a ralpi execution loop is active. - * Used to re-instantiate widgets after a session reload. + * Used to re-instantiate widgets after a session reload, and to resume + * the loop non-interactively when a reload interrupts in-progress tasks. */ export interface LoopActiveState { taskFile: string; @@ -48,6 +49,11 @@ export interface LoopActiveState { startedAt: string; taskIds: string[]; prdKey: string; + /** Loop option snapshot at loop start, so a reload can resume without + * re-prompting the user. */ + autoCommit?: boolean; + autoReview?: boolean; + saveReviews?: boolean; } /**