fix(engine): break Plan Review REVISE replan loop (feedback + bounded cap) (#2078)
## Problem A task whose Plan Review step returns verdict `REVISE` can loop forever: plan → plan-review REVISE → `needs-replan` → re-plan → near-identical plan → REVISE → repeat. The triage **pre-execution** Plan Review gate (`runPlanReviewBeforeExecution`) sets `status: "needs-replan"` on REVISE with **no cap and no escape to `awaiting-approval`** — unlike the executor graph path, which already has `PLAN_REVIEW_REPLAN_HARD_CAP`. Under `planApprovalMode: require-all` there is also no human exit, because the task never reaches `awaiting-approval`. Separately, replan feedback (`triage.ts`) was derived only from `task.log` comment actions + the latest user comment; it never consulted the plan-review verdict stored in `task.workflowStepResults`. ## Fix 1. **Thread plan-review feedback into replan** — when re-planning with no comment-derived feedback, seed `buildSpecificationPrompt` from the most recent `plan-review` REVISE `output` in `workflowStepResults` (existing user/AI-comment precedence preserved). 2. **Bounded cap** — new `planReviewReplanCount` counter (`types.ts`, `store.ts` column + updateTask, `db.ts` migration 146, `manual-retry-reset.ts`). After `PLAN_REVIEW_GATE_REPLAN_CAP = 3` consecutive REVISE replans the task escalates to `awaiting-approval` (`awaitingApprovalReason: "plan-review-replan-cap"`) instead of replanning. Counter resets on APPROVE. ## Tests Adds `triage-replan-feedback-from-plan-review.test.ts` and `triage-plan-review-replan-cap.test.ts`. Merge gate green locally (`verify:fast`, `test:gate` 337+63, `lint`); changeset included. Made with Claude (see `Co-Authored-By` trailer). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented Plan Review “REVISE” from looping indefinitely by enforcing a bounded replan cap. * After repeated Plan Review replans, tasks now escalate to an approval-hold state with a dedicated reason. * Improved replan feedback by seeding from the latest Plan Review output when no explicit feedback is available; the counter clears when Plan Review approves. * Manual retries now reset the Plan Review replan cap counter. * **Documentation** * Added release notes describing the Plan Review replan safeguards. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This commit is contained in:
Victor Canô
@@ -0,0 +1,186 @@
|
||||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import type { Settings, Task, TaskStore } from "@fusion/core";
|
||||
import { join } from "node:path";
|
||||
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { TriageProcessor } from "../triage.js";
|
||||
|
||||
/*
|
||||
* Bug A (part 2): the triage pre-execution Plan Review gate must bound consecutive
|
||||
* REVISE replans so a persistent planner/reviewer disagreement escalates to
|
||||
* awaiting-approval instead of looping plan -> Plan Review REVISE -> replan forever.
|
||||
*/
|
||||
|
||||
const { mockReviewStep, mockCreateFnAgent } = vi.hoisted(() => ({
|
||||
mockReviewStep: vi.fn(),
|
||||
mockCreateFnAgent: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../reviewer.js", () => ({
|
||||
reviewStep: mockReviewStep,
|
||||
}));
|
||||
|
||||
vi.mock("../pi.js", () => ({
|
||||
createFnAgent: mockCreateFnAgent,
|
||||
describeModel: vi.fn().mockReturnValue("mock-model"),
|
||||
promptWithFallback: vi.fn().mockReturnValue("mock-prompt"),
|
||||
}));
|
||||
|
||||
vi.mock("@fusion/core", async (importOriginal) => {
|
||||
const { createEngineCoreMock } = await import("../test/mockCore.js");
|
||||
const original = await importOriginal<typeof import("@fusion/core")>();
|
||||
return createEngineCoreMock(() => Promise.resolve(original));
|
||||
});
|
||||
|
||||
async function createFixtureRoot(): Promise<string> {
|
||||
return mkdtemp(join(tmpdir(), "fusion-triage-plan-review-replan-cap-"));
|
||||
}
|
||||
|
||||
async function cleanupFixtureRoot(rootDir: string): Promise<void> {
|
||||
await rm(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
function createRetryTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-REPLAN-CAP",
|
||||
description: "Bounded Plan Review replan",
|
||||
title: "Bounded Plan Review replan",
|
||||
column: "triage",
|
||||
status: "plan-review-unavailable",
|
||||
nextRecoveryAt: "2026-01-01T00:00:00.000Z",
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function createStore(task: Task, settingsOverrides: Partial<Settings> = {}): TaskStore {
|
||||
return {
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
listTasks: vi.fn().mockResolvedValue([task]),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10_000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
requirePlanApproval: false,
|
||||
...settingsOverrides,
|
||||
} as Settings),
|
||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
appendAgentLog: vi.fn().mockResolvedValue(undefined),
|
||||
getAgentLogs: vi.fn().mockResolvedValue([]),
|
||||
parseDependenciesFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseStepsFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
createTask: vi.fn(),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
updateSettings: vi.fn(),
|
||||
addSteeringComment: vi.fn(),
|
||||
getTaskWorkflowSelection: vi.fn().mockReturnValue({ workflowId: "builtin:coding", stepIds: [] }),
|
||||
getWorkflowDefinition: vi.fn().mockResolvedValue(undefined),
|
||||
getWorkflowSettingValues: vi.fn().mockReturnValue({}),
|
||||
getWorkflowSettingsProjectId: vi.fn().mockReturnValue("project-plan-review-replan-cap"),
|
||||
on: vi.fn(),
|
||||
emit: vi.fn(),
|
||||
} as unknown as TaskStore;
|
||||
}
|
||||
|
||||
async function writePrompt(rootDir: string, taskId: string, prompt: string): Promise<string> {
|
||||
const taskDir = join(rootDir, ".fusion", "tasks", taskId);
|
||||
await mkdir(taskDir, { recursive: true });
|
||||
const promptPath = join(taskDir, "PROMPT.md");
|
||||
await writeFile(promptPath, prompt, "utf-8");
|
||||
return promptPath;
|
||||
}
|
||||
|
||||
async function runGate(rootDir: string, task: Task, store = createStore(task)): Promise<TaskStore> {
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
await processor.specifyTask(task);
|
||||
return store;
|
||||
}
|
||||
|
||||
describe("Plan Review replan cap", () => {
|
||||
let roots: string[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
mockReviewStep.mockReset();
|
||||
mockCreateFnAgent.mockReset();
|
||||
await Promise.all(roots.map(cleanupFixtureRoot));
|
||||
roots = [];
|
||||
});
|
||||
|
||||
it("increments the replan counter and stays in needs-replan below the cap", async () => {
|
||||
const rootDir = await createFixtureRoot();
|
||||
roots.push(rootDir);
|
||||
const task = createRetryTask({ id: "FN-REPLAN-CAP-BELOW", planReviewReplanCount: 1 });
|
||||
const prompt = `# Task: ${task.id} - Existing draft\n\n## Mission\n\nOnly rewrite after reviewer feedback.\n`;
|
||||
await writePrompt(rootDir, task.id, prompt);
|
||||
const store = createStore(task);
|
||||
mockReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Please tighten the file scope.", summary: "Needs revision." });
|
||||
|
||||
await runGate(rootDir, task, store);
|
||||
|
||||
// Still replans, but bumps the consecutive-REVISE counter toward the cap.
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
status: "needs-replan",
|
||||
planReviewReplanCount: 2,
|
||||
}));
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
status: "awaiting-approval",
|
||||
}));
|
||||
});
|
||||
|
||||
it("escalates to awaiting-approval instead of replanning once the cap is reached", async () => {
|
||||
const rootDir = await createFixtureRoot();
|
||||
roots.push(rootDir);
|
||||
// Cap is 3: a task that has already consumed 3 consecutive REVISE replans must
|
||||
// escalate on the next REVISE rather than replanning a 4th time.
|
||||
const task = createRetryTask({ id: "FN-REPLAN-CAP-HIT", planReviewReplanCount: 3 });
|
||||
const prompt = `# Task: ${task.id} - Existing draft\n\n## Mission\n\nOnly rewrite after reviewer feedback.\n`;
|
||||
await writePrompt(rootDir, task.id, prompt);
|
||||
const store = createStore(task);
|
||||
const feedback = "Reviewer keeps rejecting the same plan.";
|
||||
mockReviewStep.mockResolvedValue({ verdict: "REVISE", review: feedback, summary: "Needs revision." });
|
||||
|
||||
await runGate(rootDir, task, store);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
}));
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
status: "needs-replan",
|
||||
}));
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
"Plan Review replan cap reached — escalating to manual approval",
|
||||
expect.stringContaining(feedback),
|
||||
);
|
||||
});
|
||||
|
||||
it("resets the replan counter when Plan Review passes", async () => {
|
||||
const rootDir = await createFixtureRoot();
|
||||
roots.push(rootDir);
|
||||
const task = createRetryTask({ id: "FN-REPLAN-CAP-RESET", planReviewReplanCount: 2 });
|
||||
const prompt = `# Task: ${task.id} - Existing draft\n\n## Mission\n\nKeep this exact text.\n`;
|
||||
await writePrompt(rootDir, task.id, prompt);
|
||||
const store = createStore(task);
|
||||
mockReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "Approved.", summary: "Ready." });
|
||||
|
||||
await runGate(rootDir, task, store);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
planReviewReplanCount: null,
|
||||
}));
|
||||
expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,232 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Settings, Task, TaskDetail, TaskStore } from "@fusion/core";
|
||||
import { mkdtemp, mkdir, rm } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { TriageProcessor } from "../triage.js";
|
||||
|
||||
/*
|
||||
* Bug A (part 1): when re-planning and no explicit user/AI-comment feedback exists,
|
||||
* the planner prompt must be seeded from the most recent Plan Review REVISE output
|
||||
* stored in workflowStepResults — otherwise the planner re-plans with
|
||||
* `feedback: undefined` and regenerates the same rejected plan.
|
||||
*/
|
||||
|
||||
const { mockCreateResolvedAgentSession, mockPromptWithFallback } = vi.hoisted(() => ({
|
||||
mockCreateResolvedAgentSession: vi.fn(),
|
||||
mockPromptWithFallback: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../agent-session-helpers.js", () => ({
|
||||
createResolvedAgentSession: mockCreateResolvedAgentSession,
|
||||
extractRuntimeHint: vi.fn(),
|
||||
resolvePlanningSessionModel: vi.fn().mockReturnValue({ provider: "mock", modelId: "mock-model" }),
|
||||
resolveExecutorThinkingLevel: vi.fn(() => undefined),
|
||||
resolveExecutorFallbackThinkingLevel: vi.fn(() => undefined),
|
||||
resolvePlanningThinkingLevel: vi.fn(() => undefined),
|
||||
resolvePlanningFallbackThinkingLevel: vi.fn(() => undefined),
|
||||
resolveValidatorThinkingLevel: vi.fn(() => undefined),
|
||||
resolveValidatorFallbackThinkingLevel: vi.fn(() => undefined),
|
||||
resolveMergerThinkingLevel: vi.fn(() => undefined),
|
||||
resolveMergerFallbackThinkingLevel: vi.fn(() => undefined),
|
||||
resolveImplicitPlanningFallbackModel: vi.fn(() => ({ provider: undefined, modelId: undefined })),
|
||||
}));
|
||||
|
||||
vi.mock("../pi.js", () => {
|
||||
class ModelFallbackExhaustedError extends Error {}
|
||||
return {
|
||||
describeModel: vi.fn().mockReturnValue("mock-model"),
|
||||
promptWithFallback: mockPromptWithFallback,
|
||||
formatModelMarkerDetails: vi.fn((model: string) => model),
|
||||
ModelFallbackExhaustedError,
|
||||
};
|
||||
});
|
||||
|
||||
function createTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-REPLAN-FEEDBACK",
|
||||
title: "Replan feedback source",
|
||||
description: "Re-plan a task that only has Plan Review REVISE feedback",
|
||||
column: "triage",
|
||||
status: "needs-replan",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-07-13T00:00:00.000Z",
|
||||
updatedAt: "2026-07-13T00:00:00.000Z",
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function toDetail(task: Task): TaskDetail {
|
||||
return {
|
||||
...task,
|
||||
attachments: [],
|
||||
comments: [],
|
||||
log: task.log ?? [],
|
||||
} as TaskDetail;
|
||||
}
|
||||
|
||||
function createMutableStore(initialTask: Task, settings: Partial<Settings> = {}) {
|
||||
let currentTask: Task = { ...initialTask, log: [...(initialTask.log ?? [])] };
|
||||
const store = {
|
||||
getTask: vi.fn(async () => toDetail(currentTask)),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
pollIntervalMs: 60_000,
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 1,
|
||||
autoMerge: true,
|
||||
groupOverlappingFiles: false,
|
||||
maxStuckKills: 6,
|
||||
requirePlanApproval: false,
|
||||
...settings,
|
||||
} as Settings),
|
||||
getTaskDocument: vi.fn(async () => null),
|
||||
updateTask: vi.fn(async (_id: string, updates: Partial<Task>) => {
|
||||
currentTask = { ...currentTask, ...updates, updatedAt: "2026-07-13T00:01:00.000Z" } as Task;
|
||||
return currentTask;
|
||||
}),
|
||||
moveTask: vi.fn(async (_id: string, column: Task["column"]) => {
|
||||
currentTask = { ...currentTask, column, status: null } as Task;
|
||||
return currentTask;
|
||||
}),
|
||||
logEntry: vi.fn(async (_id: string, action: string, outcome?: string) => {
|
||||
currentTask = {
|
||||
...currentTask,
|
||||
log: [...(currentTask.log ?? []), { timestamp: new Date().toISOString(), action, outcome }],
|
||||
} as Task;
|
||||
}),
|
||||
appendAgentLog: vi.fn().mockResolvedValue(undefined),
|
||||
parseDependenciesFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseStepsFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue([]),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
return {
|
||||
store,
|
||||
get currentTask() {
|
||||
return currentTask;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function createRoot(taskId: string): Promise<string> {
|
||||
const rootDir = await mkdtemp(join(tmpdir(), "fusion-triage-replan-feedback-"));
|
||||
const taskDir = join(rootDir, ".fusion", "tasks", taskId);
|
||||
await mkdir(taskDir, { recursive: true });
|
||||
return rootDir;
|
||||
}
|
||||
|
||||
function mockSession() {
|
||||
mockCreateResolvedAgentSession.mockResolvedValue({
|
||||
session: {
|
||||
state: {},
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue(null) },
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async function cleanup(rootDir: string | undefined) {
|
||||
if (rootDir) {
|
||||
await rm(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("triage replan feedback falls back to Plan Review REVISE output", () => {
|
||||
let rootDir: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockSession();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanup(rootDir);
|
||||
rootDir = undefined;
|
||||
});
|
||||
|
||||
it("seeds the planner prompt from the latest plan-review REVISE output when no comment feedback exists", async () => {
|
||||
const reviseOutput = "PLAN-REVIEW-REVISE-MARKER: the plan omits the required migration step and must add it.";
|
||||
const task = createTask({
|
||||
id: "FN-REPLAN-FEEDBACK-WSR",
|
||||
// No user comments and no "AI spec revision requested" log entry — the only
|
||||
// available feedback is the Plan Review REVISE result in workflowStepResults.
|
||||
log: [],
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
output: reviseOutput,
|
||||
notes: "Needs a migration step.",
|
||||
},
|
||||
],
|
||||
});
|
||||
rootDir = await createRoot(task.id);
|
||||
const harness = createMutableStore(task);
|
||||
const processor = new TriageProcessor(harness.store, rootDir);
|
||||
|
||||
let capturedPrompt: string | undefined;
|
||||
mockPromptWithFallback.mockImplementationOnce(async (_session: unknown, agentPrompt: string) => {
|
||||
capturedPrompt = agentPrompt;
|
||||
// Short-circuit the rest of planning; we only assert the prompt was seeded.
|
||||
processor.markStuckAborted(task.id);
|
||||
});
|
||||
|
||||
await processor.specifyTask(harness.currentTask);
|
||||
|
||||
expect(mockPromptWithFallback).toHaveBeenCalled();
|
||||
expect(capturedPrompt).toBeDefined();
|
||||
expect(capturedPrompt).toContain(reviseOutput);
|
||||
});
|
||||
|
||||
it("prefers an explicit AI spec revision comment over the workflowStepResults fallback", async () => {
|
||||
const reviseOutput = "PLAN-REVIEW-REVISE-MARKER: stale fallback that must not win.";
|
||||
const explicitFeedback = "EXPLICIT-COMMENT-FEEDBACK: address the auth edge case first.";
|
||||
const task = createTask({
|
||||
id: "FN-REPLAN-FEEDBACK-PRECEDENCE",
|
||||
log: [
|
||||
{
|
||||
timestamp: "2026-07-13T00:00:30.000Z",
|
||||
action: "AI spec revision requested",
|
||||
outcome: explicitFeedback,
|
||||
},
|
||||
],
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
output: reviseOutput,
|
||||
},
|
||||
],
|
||||
});
|
||||
rootDir = await createRoot(task.id);
|
||||
const harness = createMutableStore(task);
|
||||
const processor = new TriageProcessor(harness.store, rootDir);
|
||||
|
||||
let capturedPrompt: string | undefined;
|
||||
mockPromptWithFallback.mockImplementationOnce(async (_session: unknown, agentPrompt: string) => {
|
||||
capturedPrompt = agentPrompt;
|
||||
processor.markStuckAborted(task.id);
|
||||
});
|
||||
|
||||
await processor.specifyTask(harness.currentTask);
|
||||
|
||||
expect(capturedPrompt).toBeDefined();
|
||||
expect(capturedPrompt).toContain(explicitFeedback);
|
||||
expect(capturedPrompt).not.toContain(reviseOutput);
|
||||
});
|
||||
});
|
||||
@@ -51,6 +51,20 @@ type TaskListFormatter = (
|
||||
const TRIAGE_STUCK_RESUME_LOG_ACTION = "Triage stuck re-queue will resume existing planning draft";
|
||||
const TRIAGE_STUCK_RESUME_FEEDBACK = "The previous triage session was killed by the stuck-task detector after writing a non-empty planning draft. Resume from the existing draft below: preserve useful structure and decisions, fill gaps, and continue toward review instead of restarting planning from scratch.";
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-13-00:00:
|
||||
The triage pre-execution Plan Review gate (runPlanReviewBeforeExecution) routes a REVISE
|
||||
verdict back to `needs-replan`, which re-plans and re-reviews. Without a ceiling, a planner
|
||||
and reviewer that persistently disagree loop plan → Plan Review REVISE → replan forever
|
||||
(observed on TC-002), and in `planApprovalMode: require-all` there is no human escape because
|
||||
the task never reaches `awaiting-approval`. Bound the consecutive REVISE replans with a small
|
||||
cap (mirroring the executor graph's PLAN_REVIEW_REPLAN_HARD_CAP backstop): after this many
|
||||
replans the gate escalates the task to `awaiting-approval` for a human decision instead of
|
||||
replanning again. The counter (Task.planReviewReplanCount) resets when the gate passes.
|
||||
*/
|
||||
const PLAN_REVIEW_GATE_REPLAN_CAP = 3;
|
||||
const PLAN_REVIEW_REPLAN_CAP_LOG_ACTION = "Plan Review replan cap reached — escalating to manual approval";
|
||||
|
||||
export function inlineTaskListFallback(
|
||||
lines: string[],
|
||||
opts: { maxChars?: number } = {},
|
||||
@@ -1258,6 +1272,28 @@ export class TriageProcessor {
|
||||
feedback = latestUserComment?.text;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-13-00:00:
|
||||
When re-planning and neither an explicit user/AI re-specification comment nor a
|
||||
user comment supplied feedback, fall back to the most recent Plan Review REVISE
|
||||
verdict recorded in `workflowStepResults`. The pre-execution Plan Review gate
|
||||
(runPlanReviewBeforeExecution) stores its rejection reasoning there authoritatively
|
||||
(it is upserted every cycle and never evicted by the activity-log cap), so this
|
||||
keeps the planner regenerating against the reviewer's actual objections instead of
|
||||
reproducing the same rejected plan with `feedback: undefined` and looping. Explicit
|
||||
comment-derived feedback still wins because this only runs when none was found.
|
||||
*/
|
||||
if (!feedback) {
|
||||
const latestPlanReviewRevise = [...(currentTask.workflowStepResults || [])]
|
||||
.reverse()
|
||||
.find((result) =>
|
||||
result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
&& result.verdict === "REVISE"
|
||||
&& Boolean((result.output ?? result.notes)?.trim()),
|
||||
);
|
||||
feedback = latestPlanReviewRevise?.output ?? latestPlanReviewRevise?.notes ?? feedback;
|
||||
}
|
||||
|
||||
planLog.log(
|
||||
`${task.id} re-planning with feedback: ${feedback?.slice(0, 100)}...`,
|
||||
);
|
||||
@@ -2004,6 +2040,55 @@ export class TriageProcessor {
|
||||
await this.store.updateTask(task.id, { workflowStepResults: existing });
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-13-00:00:
|
||||
Shared terminal step for a triage Plan Review gate REVISE. Increments the consecutive-replan
|
||||
counter and routes the task back to `needs-replan` for another planning pass — until the count
|
||||
reaches PLAN_REVIEW_GATE_REPLAN_CAP, after which it escalates to `awaiting-approval` (with a
|
||||
clear log entry and a distinct awaitingApprovalReason) so a persistent planner/reviewer
|
||||
disagreement surfaces to a human instead of looping forever. Callers still record the workflow
|
||||
step result and the "AI spec revision requested" feedback log before invoking this.
|
||||
*/
|
||||
private async blockAfterPlanReviewRevise(task: Task, latestFeedback: string): Promise<void> {
|
||||
const priorCount = task.planReviewReplanCount ?? 0;
|
||||
if (priorCount >= PLAN_REVIEW_GATE_REPLAN_CAP) {
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
PLAN_REVIEW_REPLAN_CAP_LOG_ACTION,
|
||||
`The triage Plan Review gate requested a planning revision ${priorCount} consecutive times without converging (cap ${PLAN_REVIEW_GATE_REPLAN_CAP}). To avoid an endless plan → Plan Review REVISE → replan loop, the task is being routed to awaiting-approval for a human decision instead of replanning again. Latest Plan Review feedback:\n${latestFeedback}`,
|
||||
);
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-13-00:00:
|
||||
`awaitingApprovalReason` is not a persisted `updateTask` column in the PostgreSQL
|
||||
store (it survives only as a Task type field after the release-authorization gate
|
||||
was removed), so the distinct reason is written through a Record<string, unknown>
|
||||
the same way the manual plan-approval hold clears it below. The escalated task
|
||||
renders as an ordinary manual plan-approval hold (only the legacy
|
||||
"release-authorization" value is special-cased in the dashboard), which is exactly
|
||||
the intended human Approve/Reject decision point.
|
||||
*/
|
||||
const escalationUpdates: Record<string, unknown> = {
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
error: null,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
};
|
||||
await this.store.updateTask(task.id, escalationUpdates);
|
||||
planLog.warn(
|
||||
`${task.id} Plan Review replan cap (${PLAN_REVIEW_GATE_REPLAN_CAP}) reached after ${priorCount} REVISE replans — escalating to awaiting-approval instead of replanning`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "needs-replan",
|
||||
planReviewReplanCount: priorCount + 1,
|
||||
error: null,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
});
|
||||
}
|
||||
|
||||
private async runPlanReviewBeforeExecution(task: Task, promptContent: string, settings: Settings): Promise<"approved" | "blocked"> {
|
||||
if (!this.isPlanReviewEnabled(task)) {
|
||||
return "approved";
|
||||
@@ -2048,12 +2133,7 @@ export class TriageProcessor {
|
||||
"AI spec revision requested",
|
||||
`Plan Review deterministic external-integration evidence check requested a planning revision before execution.\n\nFeedback:\n${diagnostic}`,
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "needs-replan",
|
||||
error: null,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
});
|
||||
await this.blockAfterPlanReviewRevise(task, diagnostic);
|
||||
return "blocked";
|
||||
}
|
||||
}
|
||||
@@ -2117,6 +2197,11 @@ export class TriageProcessor {
|
||||
startedAt,
|
||||
completedAt,
|
||||
});
|
||||
// FNXC:PlanReviewReplan 2026-07-13-00:00: a passing gate clears the consecutive-REVISE
|
||||
// replan counter so a later, unrelated revision cycle starts from a fresh budget.
|
||||
if ((task.planReviewReplanCount ?? 0) > 0) {
|
||||
await this.store.updateTask(task.id, { planReviewReplanCount: null });
|
||||
}
|
||||
await this.store.logEntry(task.id, "[pre-merge] Workflow step completed: Plan Review", review.summary);
|
||||
return "approved";
|
||||
}
|
||||
@@ -2134,17 +2219,13 @@ export class TriageProcessor {
|
||||
completedAt,
|
||||
});
|
||||
await this.store.logEntry(task.id, "[pre-merge] Workflow step failed: Plan Review", review.review);
|
||||
const reviseFeedback = review.review || review.summary || "(no feedback captured)";
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"AI spec revision requested",
|
||||
`Plan Review requested a planning revision before execution.\n\nStatus: ${review.verdict}\nFeedback:\n${review.review || review.summary || "(no feedback captured)"}`,
|
||||
`Plan Review requested a planning revision before execution.\n\nStatus: ${review.verdict}\nFeedback:\n${reviseFeedback}`,
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "needs-replan",
|
||||
error: null,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
});
|
||||
await this.blockAfterPlanReviewRevise(task, reviseFeedback);
|
||||
return "blocked";
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user