From 9fd286b1d3e47a866b407991c1958398ed3e6f48 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 29 Jun 2026 18:41:43 -0700 Subject: [PATCH] fix(FN-7242): keep code review remediation recovering --- .changeset/fn-7242-code-review-recovery.md | 7 +++++++ .../src/__tests__/builtin-code-review-group.test.ts | 6 +++++- .../core/src/__tests__/builtin-workflows.test.ts | 2 +- packages/core/src/builtin-code-review-group.ts | 12 +++++++++--- packages/core/src/builtin-workflow-settings.ts | 5 ++--- packages/core/src/builtin-workflows.ts | 6 +++--- packages/core/src/types.ts | 11 ++++++----- .../__tests__/workflow-graph-optional-group.test.ts | 4 ++-- 8 files changed, 35 insertions(+), 18 deletions(-) create mode 100644 .changeset/fn-7242-code-review-recovery.md diff --git a/.changeset/fn-7242-code-review-recovery.md b/.changeset/fn-7242-code-review-recovery.md new file mode 100644 index 0000000000..c0bbdd35e9 --- /dev/null +++ b/.changeset/fn-7242-code-review-recovery.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep built-in Code Review remediation recovering until review passes. +category: fix +dev: Built-in Code Review now defaults maxRevisions to unbounded while preserving workflow-authored numeric caps. diff --git a/packages/core/src/__tests__/builtin-code-review-group.test.ts b/packages/core/src/__tests__/builtin-code-review-group.test.ts index 5f1fbf7cad..2b6ed14bc2 100644 --- a/packages/core/src/__tests__/builtin-code-review-group.test.ts +++ b/packages/core/src/__tests__/builtin-code-review-group.test.ts @@ -24,6 +24,10 @@ U6 deleted the `WORKFLOW_STEP_TEMPLATES` catalog. The former "code-review catalo fields" assertions are gone; the inlined literal values (name/toolMode/gateMode/prompt verdict convention) are now asserted directly on the built group node below, which is the parity oracle. + +FNXC:CodeReviewStep 2026-06-29-17:55: +Built-in Code Review defaults to unbounded remediation so repeated REVISE feedback +keeps cycling through implementation fixes instead of terminal-failing the task. */ describe("codeReviewOptionalGroupNode", () => { @@ -52,7 +56,7 @@ describe("codeReviewOptionalGroupNode", () => { expect(node.config?.name).toBe("Code Review"); // Default-ON (runs by default), but still an optional-group → toggleable per task. expect(node.config?.defaultOn).toBe(true); - expect(node.config?.maxRevisions).toBe(3); + expect(node.config?.maxRevisions).toBe("unbounded"); const template = node.config?.template as { nodes: { id: string; kind: string; config?: Record }[] }; expect(template.nodes).toHaveLength(1); diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 179b6bc64d..066adf2215 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -139,7 +139,7 @@ describe("built-in workflows", () => { expect(workflow.ir.nodes.find((node) => node.id === gate)?.config, `${workflow.id}:${gate}:reworkRegion`).toMatchObject({ reworkRegion: true, maxReworkCycles: 3, - maxRevisions: 3, + maxRevisions: gate === "code-review" ? "unbounded" : 3, }); } } diff --git a/packages/core/src/builtin-code-review-group.ts b/packages/core/src/builtin-code-review-group.ts index d5f756cf6f..ed374af928 100644 --- a/packages/core/src/builtin-code-review-group.ts +++ b/packages/core/src/builtin-code-review-group.ts @@ -29,6 +29,12 @@ because the generic built-in was authored as advisory and the graph continued af remediation budget was exhausted. Keep browser verification advisory, but make Code Review a gate so REVISE records a blocking failed workflow step and cannot advance to review or merge. + +FNXC:CodeReviewStep 2026-06-29-17:55: +Code Review REVISE is ordinary repair feedback, not a terminal task failure. FN-7242 +exhausted the old built-in three-pass default and parked failed at +`code-review-remediation`; default built-in Code Review must keep recovering until it +passes unless a workflow author explicitly sets a numeric maxRevisions cap. */ /** Stable per-task enable key + group node id. */ @@ -94,10 +100,10 @@ export function codeReviewOptionalGroupNode( reworkRegion: true, maxReworkCycles: 3, /* - * FNXC:WorkflowRemediationBudget 2026-06-29-13:56: - * Built-in workflows own their optional-step remediation policy. Default Code Review to three fix→review attempts while preserving workflow-authored overrides through `config.maxRevisions`. + * FNXC:WorkflowRemediationBudget 2026-06-29-17:55: + * Built-in Code Review should never terminal-fail merely because repair feedback repeated. Default to unbounded fix→review recovery while preserving workflow-authored numeric caps through `config.maxRevisions`. */ - maxRevisions: options.maxRevisions ?? 3, + maxRevisions: options.maxRevisions ?? "unbounded", template: { nodes: [ { diff --git a/packages/core/src/builtin-workflow-settings.ts b/packages/core/src/builtin-workflow-settings.ts index 32f431e47f..202b5802bb 100644 --- a/packages/core/src/builtin-workflow-settings.ts +++ b/packages/core/src/builtin-workflow-settings.ts @@ -126,8 +126,8 @@ export const BUILTIN_MOVED_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [ name: "Max post-review fixes", type: "number", /* - * FNXC:WorkflowOptionalStepCycle 2026-06-27-11:11: - * Built-in Code Review and Browser Verification must cycle through executor fixes and re-review until they pass, bounded by this default budget of three passes. FN-7129 owns future per-step configurable or unbounded budgets. + * FNXC:WorkflowOptionalStepCycle 2026-06-29-17:55: + * This global budget remains the fallback for custom optional gates and explicitly capped built-in gates. Built-in Code Review now sets `maxRevisions: "unbounded"` so ordinary reviewer feedback keeps recovering instead of terminal-failing after three passes. */ default: 3, description: "Maximum automatic fix passes after review/optional-step feedback; the step re-runs each pass until it passes or this budget is exhausted.", @@ -401,4 +401,3 @@ export function renderTriagePolicyPlaceholders(prompt: string, settings: Partial } return rendered; } - diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 6b696fa7d2..39629e03d2 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -60,10 +60,10 @@ function ceCodeReviewOptionalGroupNode(column: string): WorkflowIrNode { reworkRegion: true, maxReworkCycles: 3, /* - * FNXC:WorkflowRemediationBudget 2026-06-29-13:56: - * The CE Code Review group is custom because it invokes the CE skill, but its workflow-owned remediation budget must match the other built-in optional gates by defaulting to three attempts while remaining editable in workflow config. + * FNXC:WorkflowRemediationBudget 2026-06-29-17:55: + * The CE Code Review group is custom because it invokes the CE skill, but Code Review REVISE is still ordinary repair feedback. Default CE review remediation to unbounded so CE tasks do not terminal-fail after repeated reviewer feedback unless a workflow author sets a numeric cap. */ - maxRevisions: 3, + maxRevisions: "unbounded", template: { nodes: [ { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index bab2be5f48..646fddf535 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -2313,11 +2313,12 @@ export interface Task { /** Compact execution-progress snapshot captured at the last reclaim/unpause * attempt (current step + step statuses) for resume-limbo detection. */ resumeLimboStepSignature?: string; - /** Number of times the self-healing manager has auto-revived this task from - * `in-review` after a failed pre-merge workflow step. Incremented each time the - * `recoverReviewTasksWithFailedPreMergeSteps` scan sends the task back with the - * failure feedback injected. Capped by `maxPostReviewFixes`; when exhausted the - * task remains parked in `in-review` for human intervention. */ + /** Number of times workflow remediation has auto-revived this task after + * failed pre-merge review feedback. Incremented each time the engine sends the + * task back with failure feedback injected. Capped only when the workflow step + * resolves to a numeric maxRevisions/maxPostReviewFixes budget; built-in Code + * Review defaults to unbounded recovery so ordinary REVISE feedback does not + * terminal-fail the task. */ postReviewFixCount?: number; /** Number of bounded recovery retry attempts for transient executor/triage failures. * Distinct from `mergeRetries` (merge-conflict-specific). Incremented by the diff --git a/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts index 15fe612ab0..912d40b0c2 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts @@ -816,7 +816,7 @@ describe("WorkflowGraphExecutor optional-group", () => { stepName: groupId === "code-review" ? "Code Review" : "Browser Verification", feedback: `${groupId} finding`, nodeId: groupId, - maxRevisions: undefined, + maxRevisions: groupId === "code-review" ? "unbounded" : 3, })); expect(calls).not.toContain("review"); expect(result.context[`node:${groupId}:fixScheduled`]).toBe(true); @@ -868,7 +868,7 @@ describe("WorkflowGraphExecutor optional-group", () => { stepName: groupId === "code-review" ? "Code Review" : "Browser Verification", feedback: `stepwise ${groupId} finding`, nodeId: groupId, - maxRevisions: undefined, + maxRevisions: groupId === "code-review" ? "unbounded" : 3, })); expect(stepwiseResult.context[`node:${groupId}:fixScheduled`]).toBe(true); }