feat: review-gated loop triggers on autoReview alone, ask review before commit at startup
- Review now runs whenever autoReview is enabled (not only when both autoCommit && autoReview). On fail, the task re-executes with review feedback injected and loops until pass or maxReviewRetries exhausted. - Commit after a passing review is gated by autoCommit; review-only mode leaves changes uncommitted for manual inspection. - Remove legacy post-commit review path and unused imports (buildReviewPrompt, getLatestCommitDiff). - Reorder selectLoopOptions: ask autoReview first, then saveReviews, then autoCommit (reworded to 'commit after a passing review' when review is enabled). - Update types.ts doc comments and README to reflect new semantics.
This commit is contained in:
25
README.md
25
README.md
@@ -120,8 +120,8 @@ execution:
|
|||||||
models: # round-robin in <provider>/<model> format
|
models: # round-robin in <provider>/<model> format
|
||||||
- google/gemini-3.5-flash # 1st and 3rd task in parallel
|
- google/gemini-3.5-flash # 1st and 3rd task in parallel
|
||||||
- openai/gpt-5.5 # 2nd task in parallel
|
- openai/gpt-5.5 # 2nd task in parallel
|
||||||
autoCommit: true # spawn a commit agent after each task completes
|
autoCommit: true # commit changes after each task passes review (or when autoReview is off)
|
||||||
autoReview: false # spawn a review agent to review each commit
|
autoReview: false # review changes BEFORE commit; loop on fail, commit on pass
|
||||||
implModel: "" # model for task impl (sequential mode, empty = inherit parent)
|
implModel: "" # model for task impl (sequential mode, empty = inherit parent)
|
||||||
commitModel: "" # model for commit sessions (empty = inherit task model)
|
commitModel: "" # model for commit sessions (empty = inherit task model)
|
||||||
reviewModel: "" # model for review 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
|
> **NOTE**: this is only used in parallel execution, in sequential mode the
|
||||||
> parent pi session's model is used
|
> 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
|
At loop startup the review question is asked FIRST, then the commit
|
||||||
after each task to stage and commit uncommitted changes. When `autoReview` is
|
question. When `autoReview` is enabled, a review gate runs BEFORE the
|
||||||
enabled, a second follow-up session reviews the latest commit against the task
|
commit: each task's uncommitted changes are reviewed against the task
|
||||||
description. Both options can be overridden at loop startup via a selection
|
description, and on a `fail` verdict the task is re-executed with the
|
||||||
prompt.
|
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 `<provider>/<model>` strings (e.g.
|
`commitModel` and `reviewModel` accept `<provider>/<model>` strings (e.g.
|
||||||
`anthropic/claude-sonnet-4`) resolved via the model registry. When empty, the
|
`anthropic/claude-sonnet-4`) resolved via the model registry. When empty, the
|
||||||
|
|||||||
71
index.ts
71
index.ts
@@ -122,9 +122,11 @@ function buildPlanByMode(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Prompt the user to select auto-commit and auto-review options for this loop.
|
* Prompt the user to select auto-review and auto-commit options for this loop.
|
||||||
* Defaults are taken from config; the user can override at loop startup.
|
* Reviews are asked about FIRST, then commits — so the user can opt into a
|
||||||
* Fields explicitly set in the config YAML are skipped (no prompt).
|
* 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).
|
* Returns the selected options (or config defaults if cancelled).
|
||||||
*/
|
*/
|
||||||
async function selectLoopOptions(
|
async function selectLoopOptions(
|
||||||
@@ -133,37 +135,27 @@ async function selectLoopOptions(
|
|||||||
): Promise<{ autoCommit: boolean; autoReview: boolean; saveReviews: boolean }> {
|
): Promise<{ autoCommit: boolean; autoReview: boolean; saveReviews: boolean }> {
|
||||||
const explicit = config.execution.explicitKeys;
|
const explicit = config.execution.explicitKeys;
|
||||||
|
|
||||||
// Skip the commit prompt when the YAML explicitly sets it.
|
// ── 1. Auto-review (asked FIRST) ──
|
||||||
let autoCommit: boolean;
|
// When enabled, a review gate runs BEFORE the commit: uncommitted changes
|
||||||
if (explicit?.has("autoCommit")) {
|
// are reviewed, and on a "fail" verdict the task is re-executed with the
|
||||||
autoCommit = config.execution.autoCommit;
|
// review feedback injected (looping until pass or maxReviewRetries is
|
||||||
} else {
|
// exhausted). Only then does the commit session run (when autoCommit is
|
||||||
const commitChoice = await ctx.ui.select("Auto-commit after each task?", [
|
// also enabled).
|
||||||
"Yes — stage and commit changes automatically",
|
let autoReview: boolean;
|
||||||
"No — skip auto-commit",
|
|
||||||
]);
|
|
||||||
autoCommit = commitChoice
|
|
||||||
? commitChoice.startsWith("Yes")
|
|
||||||
: 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")) {
|
if (explicit?.has("autoReview")) {
|
||||||
autoReview = config.execution.autoReview;
|
autoReview = config.execution.autoReview;
|
||||||
} else {
|
} else {
|
||||||
const reviewChoice = await ctx.ui.select(
|
const reviewChoice = await ctx.ui.select("Auto-review after each task?", [
|
||||||
"Auto-review each commit against the task?",
|
"Yes — review changes and loop on failures (re-execute until pass)",
|
||||||
["Yes — spawn a review agent after each commit", "No — skip review"],
|
"No — skip review",
|
||||||
);
|
]);
|
||||||
autoReview = reviewChoice
|
autoReview = reviewChoice
|
||||||
? reviewChoice.startsWith("Yes")
|
? reviewChoice.startsWith("Yes")
|
||||||
: config.execution.autoReview;
|
: config.execution.autoReview;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Only ask to persist reviews when reviews are actually enabled.
|
// ── 2. Save full review output to disk (only when review is enabled) ──
|
||||||
|
let saveReviews = false;
|
||||||
if (autoReview) {
|
if (autoReview) {
|
||||||
if (explicit?.has("saveReviews")) {
|
if (explicit?.has("saveReviews")) {
|
||||||
saveReviews = config.execution.saveReviews;
|
saveReviews = config.execution.saveReviews;
|
||||||
@@ -180,6 +172,33 @@ async function selectLoopOptions(
|
|||||||
: config.execution.saveReviews;
|
: 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",
|
||||||
|
"No — skip auto-commit",
|
||||||
|
]);
|
||||||
|
autoCommit = commitChoice
|
||||||
|
? commitChoice.startsWith("Yes")
|
||||||
|
: config.execution.autoCommit;
|
||||||
}
|
}
|
||||||
|
|
||||||
return { autoCommit, autoReview, saveReviews };
|
return { autoCommit, autoReview, saveReviews };
|
||||||
|
|||||||
123
src/executor.ts
123
src/executor.ts
@@ -15,7 +15,6 @@ import type {
|
|||||||
} from "@earendil-works/pi-coding-agent";
|
} from "@earendil-works/pi-coding-agent";
|
||||||
import {
|
import {
|
||||||
buildTaskPrompt,
|
buildTaskPrompt,
|
||||||
buildReviewPrompt,
|
|
||||||
buildReviewPromptUncommitted,
|
buildReviewPromptUncommitted,
|
||||||
buildConflictResolutionPrompt,
|
buildConflictResolutionPrompt,
|
||||||
MAX_DIFF_BYTES,
|
MAX_DIFF_BYTES,
|
||||||
@@ -46,7 +45,6 @@ import {
|
|||||||
hasUncommittedChanges,
|
hasUncommittedChanges,
|
||||||
getGitStatusPorcelain,
|
getGitStatusPorcelain,
|
||||||
getGitDiff,
|
getGitDiff,
|
||||||
getLatestCommitDiff,
|
|
||||||
resolveModelSpec,
|
resolveModelSpec,
|
||||||
formatDuration,
|
formatDuration,
|
||||||
} from "./utils";
|
} from "./utils";
|
||||||
@@ -827,12 +825,13 @@ async function executeTask(
|
|||||||
let finalReview: ReviewResult | undefined;
|
let finalReview: ReviewResult | undefined;
|
||||||
let reviewRetries = 0;
|
let reviewRetries = 0;
|
||||||
|
|
||||||
if (config.execution.autoCommit && config.execution.autoReview) {
|
if (config.execution.autoReview) {
|
||||||
// ── Review-gated commit: review FIRST, loop on reject, commit on pass ──
|
// ── Review-gated loop: review FIRST, re-execute on reject, commit on pass ──
|
||||||
// The review examines uncommitted changes before the commit. If the
|
// The review examines uncommitted changes. If the verdict is "fail",
|
||||||
// verdict is "fail", the task is re-executed with the review feedback
|
// the task is re-executed with the review feedback injected into the
|
||||||
// injected into the prompt (up to maxReviewRetries). Only when the
|
// prompt (up to maxReviewRetries). On pass (or after retries exhaust)
|
||||||
// review passes (or retries exhaust) does the commit session run.
|
// the commit session runs when autoCommit is also enabled — otherwise
|
||||||
|
// the changes are left uncommitted for manual inspection.
|
||||||
const maxRetries = config.execution.maxReviewRetries;
|
const maxRetries = config.execution.maxReviewRetries;
|
||||||
let attempt = 0;
|
let attempt = 0;
|
||||||
|
|
||||||
@@ -876,7 +875,7 @@ async function executeTask(
|
|||||||
`~ review for ${task.id} · ${task.title} — review session failed: ${reviewResult.error}`,
|
`~ review for ${task.id} · ${task.title} — review session failed: ${reviewResult.error}`,
|
||||||
{ toolCalls: reviewToolCalls },
|
{ toolCalls: reviewToolCalls },
|
||||||
);
|
);
|
||||||
break; // commit what we have
|
break; // commit on autoCommit, otherwise leave uncommitted
|
||||||
}
|
}
|
||||||
|
|
||||||
const reviewText = reviewResult.text.trim();
|
const reviewText = reviewResult.text.trim();
|
||||||
@@ -953,9 +952,11 @@ async function executeTask(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
sendChatMessage?.(
|
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++;
|
attempt++;
|
||||||
@@ -998,7 +999,12 @@ async function executeTask(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ── Commit (after review passes or retries exhausted) ──
|
// ── 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(
|
const commitResult = await runCommitSession(
|
||||||
ctx,
|
ctx,
|
||||||
config,
|
config,
|
||||||
@@ -1026,7 +1032,7 @@ async function executeTask(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
} else if (config.execution.autoCommit) {
|
} else if (config.execution.autoCommit) {
|
||||||
// ── Commit only (no review) — legacy path ──
|
// ── Commit only (no review) ──
|
||||||
try {
|
try {
|
||||||
if (hasUncommittedChanges(worktreeDir)) {
|
if (hasUncommittedChanges(worktreeDir)) {
|
||||||
const commitResult = await runCommitSession(
|
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
|
// Save reflection
|
||||||
|
|||||||
18
src/types.ts
18
src/types.ts
@@ -211,7 +211,12 @@ export interface RalpiConfig {
|
|||||||
models: string[];
|
models: string[];
|
||||||
/** Spawn a follow-up agent to commit changes after each task completes */
|
/** Spawn a follow-up agent to commit changes after each task completes */
|
||||||
autoCommit: boolean;
|
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;
|
autoReview: boolean;
|
||||||
/** Persist the full review output to `.ralpi/reviews/<task-id>.md`.
|
/** Persist the full review output to `.ralpi/reviews/<task-id>.md`.
|
||||||
* Only active when autoReview is true and the user opts in at loop start. */
|
* Only active when autoReview is true and the user opts in at loop start. */
|
||||||
@@ -229,12 +234,15 @@ export interface RalpiConfig {
|
|||||||
commitTimeoutMs: number;
|
commitTimeoutMs: number;
|
||||||
/** Timeout for auto-review agent sessions in milliseconds */
|
/** Timeout for auto-review agent sessions in milliseconds */
|
||||||
reviewTimeoutMs: number;
|
reviewTimeoutMs: number;
|
||||||
/** Max review-fix re-execution attempts before giving up and committing
|
/** Max review-fix re-execution attempts before giving up (0 = no retries;
|
||||||
* anyway (0 = no retries; review runs once, reject = commit anyway).
|
* review runs once, reject = stop). Active whenever autoReview is
|
||||||
* Only active when both autoCommit AND autoReview are enabled. */
|
* 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;
|
maxReviewRetries: number;
|
||||||
/** When true, a 'fail' review verdict after exhausting maxReviewRetries
|
/** 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;
|
reviewBlockOnFail: boolean;
|
||||||
/** Maximum total duration for the entire loop execution in milliseconds (0 = no limit). Checked between batches — in-progress tasks finish naturally. */
|
/** Maximum total duration for the entire loop execution in milliseconds (0 = no limit). Checked between batches — in-progress tasks finish naturally. */
|
||||||
loopTimeoutMs: number;
|
loopTimeoutMs: number;
|
||||||
|
|||||||
Reference in New Issue
Block a user