FN-8503: preserve unbounded Code Review retries

Keep Code Review remediation retry policies accurate across graph execution and recovery.

- Preserve unlimited retry presentation when Code Review has no configured cap
- Enforce finite Code Review caps during failed-step recovery
- Validate non-negative revision settings and document the active retry policy

Files changed:
 .../fn-8503-unbounded-code-review-retries.md       |   7 ++
 docs/workflow-steps.md                             |   2 +-
 .../core/src/__tests__/builtin-workflows.test.ts   |   8 +-
 packages/core/src/builtin-workflow-settings.ts     |   4 +
 .../workflow-graph-optional-step-fix.test.ts       | 135 +++++++++++++++++++++
 packages/engine/src/executor.ts                    |  51 ++++++--
 6 files changed, 193 insertions(+), 14 deletions(-)

Fusion-Task-Id: FN-8503

Fusion-Task-Lineage: 7bd555d1-23e5-42ea-b6f5-0b9fe4da7f94

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>

.changeset/fn-8503-unbounded-code-review-retries.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": minor
+---
+
+summary: Keep default Code Review remediation retries unlimited and show the active policy.
+category: feature
+dev: Code Review retry prompts now preserve resolved unlimited or finite workflow revision budgets.

docs/workflow-steps.md

@@ -751,7 +751,7 @@ Retryable graph failures at explicit remediation nodes (for example `code-review
 
 During a live graph run, an enabled **pre-merge** optional step that returns `REVISE` (including the built-in **Code Review** / `code-review` and **Browser Verification** / `browser-verification` groups) sends the task back to the executor for a fix pass before the graph continues to review or merge. The workflow graph restarts on the next executor pass, re-launches task execution, and reopens the terminal verification/delivery suffix plus the nearest preceding implementation step so the verdict-demanded fix can be made rather than merely replaying a trivial trailing step. The optional step re-runs only after the executor drives those reopened steps back to `done`; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the resolved revision budget is exhausted. Generic optional gates use the workflow/project `maxPostReviewFixes` value (built-in default: 3 fix passes). Built-in Plan Review and Code Review default to `"unbounded"` so they continue until approval unless `planReviewMaxRevisions`, `codeReviewMaxRevisions`, or the node's `config.maxRevisions` sets a numeric cap. The aggregate `postReviewFixCount` remains for dashboard visibility, but budget checks count attempts per workflow-step key so Plan Review, Code Review, and Browser Verification do not consume each other's caps.
 
-The same resolved per-step budget is used by self-healing when it revives an `in-review` task that is parked with a failed pre-merge workflow result. If the failed step's IR cannot be resolved, self-healing falls back to `maxPostReviewFixes` so existing behavior is preserved. `"unbounded"` relies on the optional step eventually approving; a step that always returns `REVISE` will continue cycling until a human intervenes or another guard (pause, worktree/lease, auto-merge policy, dependency blocker) stops recovery. When the budget is exhausted or disabled, behavior falls through to the prior semantics: advisory results remain non-blocking and gate failures remain failed/parked.
+The same resolved per-step budget is used by self-healing when it revives an `in-review` task that is parked with a failed pre-merge workflow result. If the failed step's IR cannot be resolved, self-healing falls back to `maxPostReviewFixes` so existing behavior is preserved. `"unbounded"` relies on the optional step eventually approving; a step that always returns `REVISE` will continue cycling until a human intervenes or another guard (pause, worktree/lease, auto-merge policy, dependency blocker) stops recovery. The remediation instructions show `attempt/unbounded` and unlimited remaining retries for this policy rather than a misleading legacy `3/3` label. When the budget is exhausted or disabled, behavior falls through to the prior semantics: advisory results remain non-blocking and gate failures remain failed/parked.
 
 Post-merge optional groups never trigger this send-back path because merge has already happened; their failures are recorded/logged as non-blocking post-merge results.
 

packages/core/src/__tests__/builtin-workflows.test.ts

@@ -73,9 +73,11 @@ describe("built-in workflows", () => {
       expect(ir.settings?.find((setting) => setting.id === "planReviewMaxRevisions"), workflow.id).not.toHaveProperty(
         "default",
       );
-      expect(ir.settings?.find((setting) => setting.id === "codeReviewMaxRevisions"), workflow.id).not.toHaveProperty(
+      const codeReviewCap = ir.settings?.find((setting) => setting.id === "codeReviewMaxRevisions");
-        "default",
+      expect(codeReviewCap, workflow.id).not.toHaveProperty("default");
-      );
+      // Empty is the canonical unlimited policy; stored numeric values must be
+      // non-negative whole numbers so `0` has the documented disable semantics.
+      expect(codeReviewCap, workflow.id).toMatchObject({ type: "number", minimum: 0, integer: true });
     }
   });
 

packages/core/src/builtin-workflow-settings.ts

@@ -475,6 +475,8 @@ export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [
     id: "planReviewMaxRevisions",
     name: "Plan Review revision cap",
     type: "number",
+    minimum: 0,
+    integer: true,
     /*
      * FNXC:WorkflowRevisionBudget 2026-06-30-19:45:
      * Built-in Plan Review/spec remediation is unbounded when this workflow value is unset. Operators can store a non-negative integer per workflow to cap automatic replans, and `0` disables automatic Plan Review revision entirely without duplicating a read-only built-in workflow.
@@ -486,6 +488,8 @@ export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [
     id: "codeReviewMaxRevisions",
     name: "Code Review revision cap",
     type: "number",
+    minimum: 0,
+    integer: true,
     /*
      * FNXC:WorkflowRevisionBudget 2026-06-30-19:45:
      * Built-in Code Review remediation is unbounded when this workflow value is unset. Operators can store a non-negative integer per workflow to cap automatic code-fix passes, and `0` disables automatic Code Review remediation for that workflow.

packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts

@@ -1,5 +1,8 @@
 import "./executor-test-helpers.js";
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import type { Task } from "@fusion/core";
 
 import { TaskExecutor } from "../executor.js";
@@ -219,6 +222,9 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
         testCase.feedback,
         testCase.stepName,
         expect.stringContaining("requested revision"),
+        true,
+        false,
+        { attempt: 1, max: 3 },
       );
     }
   });
@@ -274,6 +280,9 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
       "packages/engine/src/example.ts:1 needs a guard",
       "Code Review",
       expect.stringContaining("requested revision"),
+      true,
+      false,
+      { attempt: 1, max: 2 },
     );
     expect(store.updateTask.mock.invocationCallOrder[0]).toBeLessThan(sendBack.mock.invocationCallOrder[0]);
   });
@@ -640,6 +649,132 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
     expect(sendBackCalls).toEqual([0, 1, 2]);
   });
 
+  it("keeps graph-owned Code Review remediation unbounded past the legacy three-pass display cap", async () => {
+    for (const count of [0, 1, 2, 3, 4, 5, 6]) {
+      const store = createMockStore();
+      const liveTask = task({
+        postReviewFixCount: count,
+        log: Array.from({ length: count }, (_, index) => revisionLog("Code Review", "code-review", index + 1)),
+      });
+      store.getTask.mockResolvedValue(liveTask);
+      // The generic optional-gate fallback stays three; the graph-owned Code Review
+      // node must not inherit it when the workflow-specific value is unset.
+      store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
+      const executor = new TaskExecutor(store, "/tmp/test");
+      const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
+
+      await expect((executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
+        ...reviseInfo,
+        nodeId: "code-review",
+      })).resolves.toBe(true);
+
+      expect(store.logEntry).toHaveBeenCalledWith(
+        liveTask.id,
+        expect.stringContaining(`attempt ${count + 1}/unbounded`),
+        expect.stringContaining("Workflow revision key: code-review"),
+        undefined,
+      );
+      expect(sendBack).toHaveBeenCalledWith(
+        liveTask,
+        liveTask.worktree,
+        reviseInfo.feedback,
+        reviseInfo.stepName,
+        expect.any(String),
+        true,
+        false,
+        { attempt: count + 1, max: undefined },
+      );
+    }
+  });
+
+  it("keeps the retry presentation aligned with the next attempt during failed-step recovery", async () => {
+    const store = createMockStore();
+    const liveTask = task({
+      column: "in-review",
+      log: Array.from({ length: 3 }, (_, index) => revisionLog("Code Review", "code-review", index + 1)),
+      workflowStepResults: [{
+        workflowStepId: "code-review",
+        workflowStepName: "Code Review",
+        phase: "pre-merge",
+        status: "failed",
+        output: "Fix the review finding.",
+        completedAt: new Date().toISOString(),
+      }],
+    });
+    store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
+    const executor = new TaskExecutor(store, "/tmp/test");
+    const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
+
+    await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(true);
+
+    expect(sendBack).toHaveBeenCalledWith(
+      liveTask,
+      liveTask.worktree,
+      "Fix the review finding.",
+      "Code Review",
+      expect.any(String),
+      true,
+      false,
+      { attempt: 4, max: undefined },
+    );
+  });
+
+  /*
+   * FNXC:WorkflowRevisionBudget 2026-07-22-18:30:
+   * Self-healing calls the failed-step recovery seam directly. It must not
+   * bypass an operator's finite Code Review cap merely because the candidate
+   * filter was skipped or raced; unlimited remains eligible by default.
+   */
+  it.each([
+    { label: "zero automatic remediations", codeReviewMaxRevisions: 0, attempts: 0 },
+    { label: "an exhausted finite cap", codeReviewMaxRevisions: 2, attempts: 2 },
+  ])("does not recover Code Review after $label", async ({ codeReviewMaxRevisions, attempts }) => {
+    const store = createMockStore();
+    const liveTask = task({
+      column: "in-review",
+      log: Array.from({ length: attempts }, (_, index) => revisionLog("Code Review", "code-review", index + 1)),
+      workflowStepResults: [{
+        workflowStepId: "code-review",
+        workflowStepName: "Code Review",
+        phase: "pre-merge",
+        status: "failed",
+        output: "Fix the review finding.",
+        completedAt: new Date().toISOString(),
+      }],
+    });
+    store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3, codeReviewMaxRevisions });
+    const executor = new TaskExecutor(store, "/tmp/test");
+    const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
+
+    await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(false);
+
+    expect(sendBack).not.toHaveBeenCalled();
+  });
+
+  it("writes an unbounded retry label into Code Review remediation instructions", async () => {
+    const root = await mkdtemp(join(tmpdir(), "fn-8503-"));
+    const fusionDir = join(root, ".fusion");
+    const promptPath = join(fusionDir, "tasks", "FN-7066", "PROMPT.md");
+    try {
+      await mkdir(join(fusionDir, "tasks", "FN-7066"), { recursive: true });
+      await writeFile(promptPath, "# Task\n\n## Steps\n- Fix it\n");
+      const store = createMockStore();
+      store.getFusionDir.mockReturnValue(fusionDir);
+      const executor = new TaskExecutor(store, "/tmp/test");
+
+      await (executor as any).injectWorkflowStepFailureInstructions(
+        task(),
+        "Address the Code Review finding.",
+        "Code Review",
+        { attempt: 6, max: undefined },
+      );
+
+      await expect(readFile(promptPath, "utf8")).resolves.toContain("**Retry:** 6/unbounded (unlimited remaining)");
+    } finally {
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+
   it("lets per-step maxRevisions override the global budget", async () => {
     for (const count of [1, 2]) {
       const store = createMockStore();

packages/engine/src/executor.ts

@@ -4989,6 +4989,9 @@ export class TaskExecutor {
       info.feedback,
       info.stepName,
       `Pre-merge optional workflow step "${info.stepName}" requested revision`,
+      true,
+      false,
+      { attempt: nextCount, max: budget.unbounded ? undefined : budget.max },
     );
     return true;
   }
@@ -5087,9 +5090,9 @@ export class TaskExecutor {
    *
    * Picks the latest failed pre-merge workflow step result (there is usually only
    * one, but if several ran we want the most recent), injects its feedback into
-   * `PROMPT.md`, resets steps, and schedules todo → in-progress. The call site
+   * `PROMPT.md`, resets steps, and schedules todo → in-progress. The caller may
-   * is responsible for enforcing the `maxPostReviewFixes` budget before invoking
+   * account for a scheduled retry, but this method independently enforces the
-   * this method — this method itself does no accounting.
+   * effective finite-or-unlimited revision budget before it can reopen work.
    *
    * @returns true when the task was sent back, false when no eligible failed
    *          step exists (caller should skip).
@@ -5123,6 +5126,18 @@ export class TaskExecutor {
 
       const feedback = target.output?.trim() || "(no feedback captured)";
       const stepName = target.workflowStepName || target.workflowStepId || "Unknown";
+      const budget = await this.resolveFailedPreMergeWorkflowStepBudget(task, target);
+      /*
+       * FNXC:WorkflowRevisionBudget 2026-07-22-18:30:
+       * Failed-step recovery is also a remediation entry point, not merely a
+       * retry-label formatter. Enforce the same finite Code Review budget here
+       * as live and restart-local graph remediation: an unset policy remains
+       * unlimited, while zero or an exhausted explicit cap cannot silently send
+       * work back for another fix. Progress-loop termination stays owned by the
+       * graph executor's signature guard rather than this budget check.
+       */
+      if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false;
+      if (!budget.unbounded && budget.attempts >= budget.max) return false;
 
       await this.sendTaskBackForFix(
         task,
@@ -5130,6 +5145,9 @@ export class TaskExecutor {
         feedback,
         stepName,
         `Auto-revived from in-review: pre-merge workflow step "${stepName}" had failed`,
+        true,
+        false,
+        { attempt: budget.attempts + 1, max: budget.unbounded ? undefined : budget.max },
       );
       return true;
     } catch (err: unknown) {
@@ -16084,6 +16102,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
     reason: string,
     preserveResumeState: boolean = true,
     mergeVerificationFailure: boolean = false,
+    retryPresentation?: { attempt: number; max?: number },
   ): Promise<void> {
     const taskId = task.id;
     this.clearCompletedTaskWatchdog(taskId);
@@ -16103,9 +16122,20 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
       `${reason} — moved back to in-progress for remediation`,
     );
 
-    // 3. Inject failure feedback into PROMPT.md using the existing method
+    /*
-    // Pass MAX_WORKFLOW_STEP_RETRIES to indicate retries are exhausted (shows "3/3 (0 remaining)")
+     * FNXC:CodeReviewRetryBudget 2026-07-22-00:00:
-    await this.injectWorkflowStepFailureInstructions(task, failureFeedback, stepName, MAX_WORKFLOW_STEP_RETRIES);
+     * A graph-owned Code Review REVISE is not a workflow-step hard-failure retry.
+     * Preserve its resolved per-step budget in PROMPT.md: unset Code Review policy
+     * is unlimited, while an explicit finite value (including zero at the gate)
+     * remains operator-visible. The execute requeue progress-signature guard, not
+     * this display, remains the safety boundary for unchanged remediation loops.
+     */
+    await this.injectWorkflowStepFailureInstructions(
+      task,
+      failureFeedback,
+      stepName,
+      retryPresentation ?? { attempt: MAX_WORKFLOW_STEP_RETRIES, max: MAX_WORKFLOW_STEP_RETRIES },
+    );
 
     // 4. Re-open only the last step for a single in-place fix pass. Earlier
     // done steps stay done so the executor doesn't redo finished work.
@@ -16145,7 +16175,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
     task: Task,
     failureFeedback: string,
     stepName: string,
-    retryCount: number,
+    retry: { attempt: number; max?: number },
   ): Promise<void> {
     const promptPath = join(this.store.getFusionDir(), "tasks", task.id, "PROMPT.md");
 
@@ -16158,7 +16188,8 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
       return;
     }
 
-    const remainingRetries = MAX_WORKFLOW_STEP_RETRIES - retryCount;
+    const retryLabel = retry.max === undefined ? "unbounded" : String(retry.max);
+    const remainingRetries = retry.max === undefined ? "unlimited" : String(Math.max(0, retry.max - retry.attempt));
     const failureSectionHeader = "## Workflow Step Failure";
     const scopeGuard = this.buildWorkflowFailureScopeGuard(task, content);
     const failureSectionContent = `${failureSectionHeader}
@@ -16172,7 +16203,7 @@ ${failureFeedback}
 
 ${scopeGuard}
 
-**Retry:** ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES} (${remainingRetries} remaining)
+**Retry:** ${retry.attempt}/${retryLabel} (${remainingRetries} remaining)
 
 **Important:** This is a workflow step failure — fix the issues above by making the necessary code changes. The task has been sent back to in-progress for remediation. The executor will attempt to fix the issues on the next pass.
 
@@ -16215,7 +16246,7 @@ ${scopeGuard}
     // Write updated content
     try {
       await writeFile(promptPath, newContent);
-      executorLog.log(`${task.id}: injected workflow step failure instructions into PROMPT.md (retry ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES})`);
+      executorLog.log(`${task.id}: injected workflow step failure instructions into PROMPT.md (retry ${retry.attempt}/${retryLabel})`);
     } catch (err: unknown) {
       const errorMessage = err instanceof Error ? err.message : String(err);
       executorLog.error(`${task.id}: failed to inject workflow step failure instructions: ${errorMessage}`);