fix: skip agent spawn on zero-conflict merges; carry prior review context across passes
- resolveConflictsSession: guard against reattemptMerge returning clean=false with zero unmerged paths (branch already merged, dirty index, stale ref). Previously fell through to spawning a full agent session with no conflicts to resolve. Now tries completeMerge or aborts without spawning. - Review-gated loop: accumulate rejected reviews in a priorReviews array and inject them into buildReviewPrompt via a new ReviewPromptOptions.priorReviews field. The reviewer now sees prior findings and can verify they were addressed instead of re-reviewing from scratch each pass. Added renderPriorReviews() helper and wired it into both buildReviewPrompt and buildReviewPromptUncommitted.
This commit is contained in:
@@ -936,6 +936,10 @@ async function executeTask(
|
|||||||
// A FAILED range computation (broken/stale base ref, git error) is
|
// A FAILED range computation (broken/stale base ref, git error) is
|
||||||
// logged as a distinct warning and is never treated as a clean,
|
// logged as a distinct warning and is never treated as a clean,
|
||||||
// verified task — only a GENUINE "no changes" skips review.
|
// verified task — only a GENUINE "no changes" skips review.
|
||||||
|
// Accumulated rejected reviews from earlier passes — injected
|
||||||
|
// into the next review prompt so the reviewer sees prior
|
||||||
|
// findings and can verify they were addressed.
|
||||||
|
const priorReviews: ReviewResult[] = [];
|
||||||
while (true) {
|
while (true) {
|
||||||
if (!baseRef) {
|
if (!baseRef) {
|
||||||
sendChatMessage?.(
|
sendChatMessage?.(
|
||||||
@@ -973,16 +977,17 @@ async function executeTask(
|
|||||||
reviewInfo.hash,
|
reviewInfo.hash,
|
||||||
reviewInfo.subject,
|
reviewInfo.subject,
|
||||||
reviewInfo.diff,
|
reviewInfo.diff,
|
||||||
{
|
{
|
||||||
projectContext: config.prompts.projectContext,
|
projectContext: config.prompts.projectContext,
|
||||||
focus: config.prompts.reviewFocus,
|
focus: config.prompts.reviewFocus,
|
||||||
diffOptions: {
|
priorReviews,
|
||||||
extraPatterns: compileIgnorePatterns(
|
diffOptions: {
|
||||||
config.review.extraIgnorePatterns,
|
extraPatterns: compileIgnorePatterns(
|
||||||
),
|
config.review.extraIgnorePatterns,
|
||||||
ignorePaths: config.review.ignorePaths,
|
),
|
||||||
},
|
ignorePaths: config.review.ignorePaths,
|
||||||
},
|
},
|
||||||
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
const reviewModel = resolveFollowUpModel(
|
const reviewModel = resolveFollowUpModel(
|
||||||
@@ -1097,6 +1102,9 @@ async function executeTask(
|
|||||||
break; // changes already committed — merge proceeds
|
break; // changes already committed — merge proceeds
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Accumulate the rejected review so the next review pass
|
||||||
|
// sees prior findings and can verify they were addressed.
|
||||||
|
if (review) priorReviews.push(review);
|
||||||
attempt++;
|
attempt++;
|
||||||
reviewRetries++;
|
reviewRetries++;
|
||||||
sendChatMessage?.(
|
sendChatMessage?.(
|
||||||
@@ -1779,6 +1787,52 @@ async function resolveConflictsSession(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Merge failed but produced no unmerged paths — no real conflicts to
|
||||||
|
// resolve. This can happen when the branch tip was already merged by the
|
||||||
|
// first attempt (mergeWorktree) before it aborted, or when the merge
|
||||||
|
// fails for a non-conflict reason (dirty index, stale ref). Don't spawn
|
||||||
|
// an agent session for zero conflicts — abort or complete and finish.
|
||||||
|
if (attempt.conflicts.length === 0) {
|
||||||
|
// Try to complete whatever merge state exists; if there's nothing to
|
||||||
|
// commit, abort to leave the working tree clean.
|
||||||
|
if (completeMerge(projectDir)) {
|
||||||
|
sendChatMessage?.(
|
||||||
|
`✓ conflicts auto-resolved for ${task.id} · ${task.title}`,
|
||||||
|
);
|
||||||
|
removeWorktree(projectDir, worktree);
|
||||||
|
progress.markCompleted(
|
||||||
|
task.id,
|
||||||
|
0,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
[],
|
||||||
|
"",
|
||||||
|
undefined,
|
||||||
|
0,
|
||||||
|
);
|
||||||
|
try {
|
||||||
|
updateTaskInFile(project.sourcePath, task.id, "completed");
|
||||||
|
} catch {
|
||||||
|
// Best-effort
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
abortMerge(projectDir);
|
||||||
|
sendChatMessage?.(
|
||||||
|
`~ conflict resolution for ${task.id} · ${task.title} — merge produced no conflicts but could not be completed`,
|
||||||
|
);
|
||||||
|
progress.markFailed(
|
||||||
|
task.id,
|
||||||
|
`Merge of ${branch} produced no conflicts but could not be completed`,
|
||||||
|
);
|
||||||
|
try {
|
||||||
|
updateTaskInFile(project.sourcePath, task.id, "failed");
|
||||||
|
} catch {
|
||||||
|
// Best-effort
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
// Conflicts exist — spawn a resolution agent session.
|
// Conflicts exist — spawn a resolution agent session.
|
||||||
const prompt = buildConflictResolutionPrompt(
|
const prompt = buildConflictResolutionPrompt(
|
||||||
task,
|
task,
|
||||||
|
|||||||
@@ -29,6 +29,12 @@ export interface ReviewPromptOptions {
|
|||||||
focus?: string;
|
focus?: string;
|
||||||
/** Noise-filter overrides (config.review.*). */
|
/** Noise-filter overrides (config.review.*). */
|
||||||
diffOptions?: DiffOptions;
|
diffOptions?: DiffOptions;
|
||||||
|
/** Prior review results from earlier passes in a review-gated loop.
|
||||||
|
* Each rejected review is injected into the next review prompt so the
|
||||||
|
* reviewer can verify prior findings were addressed and catch new
|
||||||
|
* regressions introduced by the fix attempt — instead of re-reviewing
|
||||||
|
* from scratch. */
|
||||||
|
priorReviews?: ReviewResult[];
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── Task Prompt ─────────────────────────────────────────────────────────────
|
// ─── Task Prompt ─────────────────────────────────────────────────────────────
|
||||||
@@ -258,8 +264,12 @@ export function buildReviewPrompt(
|
|||||||
parts.push(renderDiffSection(summary, filtered, "### Diff"));
|
parts.push(renderDiffSection(summary, filtered, "### Diff"));
|
||||||
parts.push("");
|
parts.push("");
|
||||||
|
|
||||||
// ── Custom Review Focus ──
|
// ── Prior Review History (review-gated re-review) ──
|
||||||
|
|
||||||
|
parts.push(renderPriorReviews(opts.priorReviews ?? []));
|
||||||
|
if (opts.priorReviews && opts.priorReviews.length > 0) parts.push("");
|
||||||
|
|
||||||
|
// ── Custom Review Focus ──
|
||||||
if (opts.focus) {
|
if (opts.focus) {
|
||||||
parts.push("## Custom Review Focus");
|
parts.push("## Custom Review Focus");
|
||||||
parts.push(opts.focus);
|
parts.push(opts.focus);
|
||||||
@@ -360,7 +370,11 @@ export function buildReviewPromptUncommitted(
|
|||||||
parts.push(
|
parts.push(
|
||||||
renderDiffSection(summary, filtered, "### Current Tracked Diff (git diff)"),
|
renderDiffSection(summary, filtered, "### Current Tracked Diff (git diff)"),
|
||||||
);
|
);
|
||||||
parts.push("");
|
|
||||||
|
// ── Prior Review History (review-gated re-review) ──
|
||||||
|
|
||||||
|
parts.push(renderPriorReviews(opts.priorReviews ?? []));
|
||||||
|
if (opts.priorReviews && opts.priorReviews.length > 0) parts.push("");
|
||||||
|
|
||||||
// ── Custom Review Focus ──
|
// ── Custom Review Focus ──
|
||||||
|
|
||||||
@@ -469,6 +483,45 @@ function renderDiffSection(
|
|||||||
return lines.join("\n");
|
return lines.join("\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Render a "Prior Review History" section from earlier rejected reviews.
|
||||||
|
* Returns an empty string when there are no prior reviews so callers omit
|
||||||
|
* the section entirely.
|
||||||
|
*
|
||||||
|
* Each prior review's verdict, summary, and findings are listed so the
|
||||||
|
* reviewer can verify the developer addressed them and watch for new
|
||||||
|
* regressions — instead of re-reviewing from scratch on each pass.
|
||||||
|
*/
|
||||||
|
function renderPriorReviews(priorReviews: ReviewResult[]): string {
|
||||||
|
if (priorReviews.length === 0) return "";
|
||||||
|
const lines: string[] = [];
|
||||||
|
lines.push("## Prior Review History");
|
||||||
|
lines.push(
|
||||||
|
"Previous review pass(es) rejected this task. Verify each finding was",
|
||||||
|
"addressed in the current diff and watch for new regressions:",
|
||||||
|
);
|
||||||
|
lines.push("");
|
||||||
|
for (let i = 0; i < priorReviews.length; i++) {
|
||||||
|
const r = priorReviews[i];
|
||||||
|
if (!r) continue;
|
||||||
|
lines.push(`### Review ${i + 1} — ${r.verdict.toUpperCase()}`);
|
||||||
|
lines.push(`Summary: ${r.summary}`);
|
||||||
|
if (r.findings.length > 0) {
|
||||||
|
lines.push("Findings:");
|
||||||
|
for (const f of r.findings) {
|
||||||
|
const loc = f.file
|
||||||
|
? f.line
|
||||||
|
? ` (${f.file}:${f.line})`
|
||||||
|
: ` (${f.file})`
|
||||||
|
: "";
|
||||||
|
lines.push(`- [${f.severity}]${loc} ${f.message}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
function reviewInstructions(): string[] {
|
function reviewInstructions(): string[] {
|
||||||
return [
|
return [
|
||||||
"- **Correctness**: Does the implementation fulfill the task requirements?",
|
"- **Correctness**: Does the implementation fulfill the task requirements?",
|
||||||
|
|||||||
Reference in New Issue
Block a user