diff --git a/README.md b/README.md index 5e94ec2..4cf4a13 100644 --- a/README.md +++ b/README.md @@ -120,8 +120,8 @@ execution: models: # round-robin in / format - google/gemini-3.5-flash # 1st and 3rd task in parallel - openai/gpt-5.5 # 2nd task in parallel - autoCommit: true # spawn a commit agent after each task completes - autoReview: false # spawn a review agent to review each commit + autoCommit: true # commit changes after each task passes review (or when autoReview is off) + autoReview: false # review changes BEFORE commit; loop on fail, commit on pass implModel: "" # model for task impl (sequential mode, empty = inherit parent) commitModel: "" # model for commit sessions (empty = inherit task model) reviewModel: "" # model for review sessions (empty = inherit task model) @@ -143,13 +143,22 @@ prompts: > **NOTE**: this is only used in parallel execution, in sequential mode the > parent pi session's model is used -#### Auto-commit and Auto-review +#### Auto-review and Auto-commit -When `autoCommit` is enabled (default), a follow-up agent session is spawned -after each task to stage and commit uncommitted changes. When `autoReview` is -enabled, a second follow-up session reviews the latest commit against the task -description. Both options can be overridden at loop startup via a selection -prompt. +At loop startup the review question is asked FIRST, then the commit +question. When `autoReview` is enabled, a review gate runs BEFORE the +commit: each task's uncommitted changes are reviewed against the task +description, and on a `fail` verdict the task is re-executed with the +review feedback injected into the prompt (looping until the review +passes or `maxReviewRetries` is exhausted). When `autoCommit` is also +enabled, a follow-up agent stages and commits the changes once the +review passes (or after retries exhaust); otherwise the changes are +left uncommitted for manual inspection. + +When `autoReview` is disabled, `autoCommit` runs a follow-up commit +agent after each task with no review. Both options can be overridden at +loop startup via a selection prompt (config YAML values are honored +without prompting when set explicitly). `commitModel` and `reviewModel` accept `/` strings (e.g. `anthropic/claude-sonnet-4`) resolved via the model registry. When empty, the diff --git a/index.ts b/index.ts index d306704..c8e2e7d 100644 --- a/index.ts +++ b/index.ts @@ -122,9 +122,11 @@ function buildPlanByMode( } /** - * Prompt the user to select auto-commit and auto-review options for this loop. - * Defaults are taken from config; the user can override at loop startup. - * Fields explicitly set in the config YAML are skipped (no prompt). + * Prompt the user to select auto-review and auto-commit options for this loop. + * Reviews are asked about FIRST, then commits — so the user can opt into a + * review gate (review changes, loop on fail) and decide whether a passing + * review should commit. Fields explicitly set in the config YAML are + * skipped (no prompt). * Returns the selected options (or config defaults if cancelled). */ async function selectLoopOptions( @@ -133,10 +135,62 @@ async function selectLoopOptions( ): Promise<{ autoCommit: boolean; autoReview: boolean; saveReviews: boolean }> { const explicit = config.execution.explicitKeys; - // Skip the commit prompt when the YAML explicitly sets it. + // ── 1. Auto-review (asked FIRST) ── + // When enabled, a review gate runs BEFORE the commit: uncommitted changes + // are reviewed, and on a "fail" verdict the task is re-executed with the + // review feedback injected (looping until pass or maxReviewRetries is + // exhausted). Only then does the commit session run (when autoCommit is + // also enabled). + let autoReview: boolean; + if (explicit?.has("autoReview")) { + autoReview = config.execution.autoReview; + } else { + const reviewChoice = await ctx.ui.select("Auto-review after each task?", [ + "Yes — review changes and loop on failures (re-execute until pass)", + "No — skip review", + ]); + autoReview = reviewChoice + ? reviewChoice.startsWith("Yes") + : config.execution.autoReview; + } + + // ── 2. Save full review output to disk (only when review is enabled) ── + let saveReviews = false; + if (autoReview) { + if (explicit?.has("saveReviews")) { + saveReviews = config.execution.saveReviews; + } else { + const saveChoice = await ctx.ui.select( + "Save full review output to disk?", + [ + "Yes — write each review to .ralpi/reviews//.md", + "No — keep reviews in-chat only", + ], + ); + saveReviews = saveChoice + ? saveChoice.startsWith("Yes") + : config.execution.saveReviews; + } + } + + // ── 3. Auto-commit (asked AFTER review) ── + // When review is enabled, this gates whether the changes are committed + // after a passing review (or after retries exhaust). When review is + // disabled, this is a stand-alone "commit per task" toggle. let autoCommit: boolean; if (explicit?.has("autoCommit")) { autoCommit = config.execution.autoCommit; + } else if (autoReview) { + const commitChoice = await ctx.ui.select( + "Auto-commit after a passing review?", + [ + "Yes — commit changes once the review passes", + "No — leave changes uncommitted after review", + ], + ); + autoCommit = commitChoice + ? commitChoice.startsWith("Yes") + : config.execution.autoCommit; } else { const commitChoice = await ctx.ui.select("Auto-commit after each task?", [ "Yes — stage and commit changes automatically", @@ -147,41 +201,6 @@ async function selectLoopOptions( : config.execution.autoCommit; } - let autoReview = false; - let saveReviews = false; - if (autoCommit) { - // Skip the review prompt when the YAML explicitly sets it. - if (explicit?.has("autoReview")) { - autoReview = config.execution.autoReview; - } else { - const reviewChoice = await ctx.ui.select( - "Auto-review each commit against the task?", - ["Yes — spawn a review agent after each commit", "No — skip review"], - ); - autoReview = reviewChoice - ? reviewChoice.startsWith("Yes") - : config.execution.autoReview; - } - - // Only ask to persist reviews when reviews are actually enabled. - if (autoReview) { - if (explicit?.has("saveReviews")) { - saveReviews = config.execution.saveReviews; - } else { - const saveChoice = await ctx.ui.select( - "Save full review output to disk?", - [ - "Yes — write each review to .ralpi/reviews//.md", - "No — keep reviews in-chat only", - ], - ); - saveReviews = saveChoice - ? saveChoice.startsWith("Yes") - : config.execution.saveReviews; - } - } - } - return { autoCommit, autoReview, saveReviews }; } diff --git a/src/executor.ts b/src/executor.ts index a759a8d..6626417 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -15,7 +15,6 @@ import type { } from "@earendil-works/pi-coding-agent"; import { buildTaskPrompt, - buildReviewPrompt, buildReviewPromptUncommitted, buildConflictResolutionPrompt, MAX_DIFF_BYTES, @@ -46,7 +45,6 @@ import { hasUncommittedChanges, getGitStatusPorcelain, getGitDiff, - getLatestCommitDiff, resolveModelSpec, formatDuration, } from "./utils"; @@ -827,12 +825,13 @@ async function executeTask( 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. + if (config.execution.autoReview) { + // ── Review-gated loop: review FIRST, re-execute on reject, commit on pass ── + // The review examines uncommitted changes. If the verdict is "fail", + // the task is re-executed with the review feedback injected into the + // prompt (up to maxReviewRetries). On pass (or after retries exhaust) + // the commit session runs when autoCommit is also enabled — otherwise + // the changes are left uncommitted for manual inspection. const maxRetries = config.execution.maxReviewRetries; let attempt = 0; @@ -876,7 +875,7 @@ async function executeTask( `~ review for ${task.id} · ${task.title} — review session failed: ${reviewResult.error}`, { toolCalls: reviewToolCalls }, ); - break; // commit what we have + break; // commit on autoCommit, otherwise leave uncommitted } const reviewText = reviewResult.text.trim(); @@ -953,9 +952,11 @@ async function executeTask( return; } sendChatMessage?.( - `~ review for ${task.id} · ${task.title} — max retries (${maxRetries}) exhausted, committing anyway`, + config.execution.autoCommit + ? `~ review for ${task.id} · ${task.title} — max retries (${maxRetries}) exhausted, committing anyway` + : `~ review for ${task.id} · ${task.title} — max retries (${maxRetries}) exhausted, leaving changes as-is`, ); - break; // commit what we have + break; // commit on autoCommit, otherwise leave uncommitted } attempt++; @@ -998,7 +999,12 @@ async function executeTask( } // ── Commit (after review passes or retries exhausted) ── - if (hasUncommittedChanges(worktreeDir)) { + // Only commit when autoCommit is enabled; review-only mode leaves + // changes uncommitted so the user can inspect them manually. + if ( + config.execution.autoCommit && + hasUncommittedChanges(worktreeDir) + ) { const commitResult = await runCommitSession( ctx, config, @@ -1026,7 +1032,7 @@ async function executeTask( ); } } else if (config.execution.autoCommit) { - // ── Commit only (no review) — legacy path ── + // ── Commit only (no review) ── try { if (hasUncommittedChanges(worktreeDir)) { const commitResult = await runCommitSession( @@ -1055,97 +1061,6 @@ async function executeTask( }`, ); } - } else if (config.execution.autoReview) { - // ── Review only (no commit) — reviews latest commit — legacy path ── - try { - const commitInfo = getLatestCommitDiff(worktreeDir); - if (commitInfo && commitInfo.diff) { - const reviewPrompt = buildReviewPrompt( - task, - project, - commitInfo.hash, - commitInfo.subject, - commitInfo.diff, - config.prompts.projectContext, - ); - - const reviewModel = resolveFollowUpModel( - ctx, - config.execution.reviewModel, - currentModel, - ); - const reviewModels = buildFailoverModels(reviewModel, roundRobin); - - const { result: reviewResult, toolCalls: reviewToolCalls } = - await runFollowUpSession( - ctx, - config, - reviewPrompt, - worktreeDir, - `review for ${task.id} · ${task.title}`, - `review-${task.id}`, - config.execution.reviewTimeoutMs, - reviewModels, - ); - - if (reviewResult.success) { - const reviewText = reviewResult.text.trim(); - const review = extractReview( - reviewText, - task.id, - commitInfo.hash, - ); - finalReview = review ?? undefined; - - let reviewPath: string | undefined; - if (review && config.execution.saveReviews) { - reviewPath = saveReviewJson( - projectDir, - config.paths.reviewsDir, - review, - progress.getKey(), - ); - } - - 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}`, - { toolCalls: reviewToolCalls }, - ); - } - } - } catch (error) { - sendChatMessage?.( - `~ review for ${task.id} · ${task.title} — auto-review error: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } } // Save reflection diff --git a/src/types.ts b/src/types.ts index 2cfd65e..c7b5d93 100644 --- a/src/types.ts +++ b/src/types.ts @@ -211,7 +211,12 @@ export interface RalpiConfig { models: string[]; /** Spawn a follow-up agent to commit changes after each task completes */ autoCommit: boolean; - /** Spawn a review agent to review the commit against the task description */ + /** Spawn a review agent to review uncommitted changes against the + * task description BEFORE committing. On a 'fail' verdict the task is + * re-executed with the review feedback injected (loops until pass or + * maxReviewRetries is exhausted). When autoCommit is also enabled, the + * commit runs after the review passes (or retries exhaust); otherwise + * changes are left uncommitted. */ autoReview: boolean; /** Persist the full review output to `.ralpi/reviews/.md`. * Only active when autoReview is true and the user opts in at loop start. */ @@ -229,12 +234,15 @@ 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. */ + /** Max review-fix re-execution attempts before giving up (0 = no retries; + * review runs once, reject = stop). Active whenever autoReview is + * enabled. On exhaustion: if autoCommit is enabled, the changes are + * committed anyway; if reviewBlockOnFail is set, the task is marked + * failed instead of committing. */ maxReviewRetries: number; /** When true, a 'fail' review verdict after exhausting maxReviewRetries - * marks the task as failed instead of committing. */ + * marks the task as failed instead of leaving changes (committing when + * autoCommit, or leaving uncommitted otherwise). */ 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;