From 39d20be05f7f378c3960b3ced8a947ea51778201 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 27 Jun 2026 13:05:50 -0700 Subject: [PATCH] FN-7129: add per-step optional revision budgets Add optional workflow-step revision budgets that can cap or unbound remediation loops. - Add optional-group maxRevisions parsing, validation, exports, and docs. - Expose numeric and unbounded revision budget controls in the workflow node editor. - Apply per-step budgets in executor and self-healing fix loops with regression coverage. Files changed: .changeset/fn-7129-per-step-revision-budget.md | 7 ++ docs/dashboard-guide.md | 1 + docs/settings-reference.md | 2 +- docs/workflow-steps.md | 10 ++- .../__tests__/workflow-ir-optional-group.test.ts | 29 ++++++- packages/core/src/index.ts | 2 + packages/core/src/workflow-ir-types.ts | 37 +++++++++ packages/core/src/workflow-ir.ts | 15 ++++ .../app/components/WorkflowNodeEditor.tsx | 87 ++++++++++++++++---- .../__tests__/WorkflowNodeEditor.test.tsx | 95 ++++++++++++++++++++++ packages/engine/src/__tests__/self-healing.test.ts | 63 ++++++++++++++ .../workflow-graph-optional-group.test.ts | 30 ++++++- .../workflow-graph-optional-step-fix.test.ts | 53 ++++++++++++ packages/engine/src/executor.ts | 15 ++-- packages/engine/src/self-healing.ts | 64 +++++++++++---- packages/engine/src/workflow-graph-executor.ts | 6 +- 16 files changed, 469 insertions(+), 47 deletions(-) Fusion-Task-Id: FN-7129 Fusion-Task-Lineage: dbde94e5-3c99-4f67-aed9-458563e845ae Co-authored-by: Fusion (runfusion.ai) --- .../fn-7129-per-step-revision-budget.md | 7 ++ docs/dashboard-guide.md | 1 + docs/settings-reference.md | 2 +- docs/workflow-steps.md | 10 +- .../workflow-ir-optional-group.test.ts | 29 +++++- packages/core/src/index.ts | 2 + packages/core/src/workflow-ir-types.ts | 37 ++++++++ packages/core/src/workflow-ir.ts | 15 +++ .../app/components/WorkflowNodeEditor.tsx | 87 +++++++++++++---- .../__tests__/WorkflowNodeEditor.test.tsx | 95 +++++++++++++++++++ .../engine/src/__tests__/self-healing.test.ts | 63 ++++++++++++ .../workflow-graph-optional-group.test.ts | 30 +++++- .../workflow-graph-optional-step-fix.test.ts | 53 +++++++++++ packages/engine/src/executor.ts | 15 +-- packages/engine/src/self-healing.ts | 64 +++++++++---- .../engine/src/workflow-graph-executor.ts | 6 +- 16 files changed, 469 insertions(+), 47 deletions(-) create mode 100644 .changeset/fn-7129-per-step-revision-budget.md diff --git a/.changeset/fn-7129-per-step-revision-budget.md b/.changeset/fn-7129-per-step-revision-budget.md new file mode 100644 index 0000000000..5da46eb7b6 --- /dev/null +++ b/.changeset/fn-7129-per-step-revision-budget.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Workflow steps like Code Review and Browser Verification can set their own max fix revisions. +category: feature +dev: Adds optional `maxRevisions` (number | "unbounded") to optional-group workflow nodes, resolved by `resolveOptionalStepRevisionBudget` and threaded through `requestPreMergeOptionalStepFix` plus `recoverReviewTasksWithFailedPreMergeSteps`. Overrides global `maxPostReviewFixes`; absent preserves prior behavior. The Workflow Node Editor authors it with a number input and Unbounded toggle. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index b441ee682e..f8d00cd822 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -212,6 +212,7 @@ Behavior: - Opens a workflow node editor with a workflow list/sidebar, canvas, inspector, and settings/authoring panels - Built-in workflows are inspectable in the same canvas as custom workflows, including connected success, failure, and rework edges for their graph topology. Their graph structure stays read-only, but prompt/gate node Prompt fields can be edited per project and reset to the shipped default from the node inspector or expanded prompt editor. - Custom workflows can be created from blank, duplicated from built-ins/custom definitions, imported/exported, AI-designed, validated, and saved from the editor. +- Optional-group node inspectors include controls for `defaultOn` and per-step **Max revisions** (`maxRevisions`), including an **Unbounded** toggle for Code Review, Browser Verification, or custom pre-merge gates that should keep cycling until they approve. - The Settings panel is value-first for built-in workflows and groups workflow settings by Models, Review & Approval, Step Execution, and Advanced. Known workflow model values use the same model dropdown picker as **Settings → Project Models** so provider/model pairs are saved together; custom or non-model string values can still use typed inputs. Definitions remain available for custom workflow schema authoring. - The main Settings modal also exposes the default workflow's Plan/Triage, Executor, and Reviewer model lanes from **Project Models**; the modal's primary **Save** action writes those dropdown values as workflow setting values for the active default workflow. - On desktop, the editor uses a multi-panel canvas layout for editing the graph and adjacent workflow metadata. The **Show simple editor** toggle switches that same workflow into the graph-outline editor with dedicated **Graph**, **Add**, **Settings**, **Fields**, **Columns**, and **Actions** tabs. diff --git a/docs/settings-reference.md b/docs/settings-reference.md index b17ab74d2b..e53533d200 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -550,7 +550,7 @@ Default notes: | `maxReviewerContextRetries` | `number` | `2` | Max reviewer context-compaction retries (FN-4082) per task. | | `maxReviewerFallbackRetries` | `number` | `2` | Max reviewer fallback-model retries (FN-4092) per task. | | `maxTotalRetriesBeforeFail` | `number` | `25` | Master retry budget across all tracked retry counters; exceeding this fails the task with `RetryStormError`. | -| `maxPostReviewFixes` | `number` | `3` | Max automatic fix passes for review/pre-merge optional-step feedback, including self-healing auto-revival of in-review tasks failing pre-merge workflow steps. | +| `maxPostReviewFixes` | `number` | `3` | Default max automatic fix passes for review/pre-merge optional-step feedback, including self-healing auto-revival of in-review tasks failing pre-merge workflow steps. Individual `optional-group` workflow nodes can override this with `config.maxRevisions` (non-negative integer or `"unbounded"`). | | `maxSpawnedAgentsPerParent` | `number` | `5` | Max child agents per parent task. | | `maxSpawnedAgentsGlobal` | `number` | `20` | Max spawned agents across one executor instance. | | `maintenanceIntervalMs` | `number` | `300000` | Periodic maintenance interval in ms (5 min). | diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index 166d814d21..308688598e 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -267,9 +267,10 @@ FN-7039 retired the declaration-based optional-steps model (`WorkflowOptionalSte Optional quality gates are authored directly in the workflow graph as `optional-group` **nodes**. An `optional-group` node is a container (mirroring `foreach`/`loop`) whose `template` subgraph the executor runs **once** when the group is enabled for the task, and passes through (skips) when disabled. There is no iteration and no rework budget — a single pass — and rework edges inside the template are rejected by `validateOptionalGroup`. -Node config (`WorkflowOptionalGroupConfig`): `{ name?, defaultOn?, phase?: "pre-merge" | "post-merge", template: { nodes, edges } }`. +Node config (`WorkflowOptionalGroupConfig`): `{ name?, defaultOn?, maxRevisions?: number | "unbounded", phase?: "pre-merge" | "post-merge", template: { nodes, edges } }`. - `defaultOn` seeds the per-task enable set at task creation; operators can still toggle it. +- `maxRevisions` optionally overrides the workflow/project `maxPostReviewFixes` budget for this one optional group's pre-merge fix → re-review loop. Use a non-negative integer for a bounded number of automatic fix passes, `0` to disable automatic fixes for that step, or `"unbounded"` to keep cycling until the step returns `APPROVE` / `APPROVE_WITH_NOTES`. When omitted, the step keeps the global `maxPostReviewFixes` behavior. - `phase` defaults to `"pre-merge"` (the prior, only behavior). `"post-merge"` marks a group the executor runs after a successful merge (see [Execution Phases](#execution-phases)). - Enable state lives on the per-task `enabledWorkflowSteps` array, keyed by the **group node id** (for example `browser-verification`, `code-review`). The graph executor runs an optional-group node only when its id is present in `enabledWorkflowSteps`. @@ -573,9 +574,14 @@ If a task is found in `in-review` with failed pre-merge workflow results and no -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 bounded fix pass before the graph continues to review or merge. The workflow graph restarts on the next executor pass, so the optional step re-runs against the fixed diff; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the shared `postReviewFixCount` / `maxPostReviewFixes` budget is exhausted. The built-in default budget is 3 fix passes, matching other bounded rework defaults; FN-7129 tracks future per-step configurable or unbounded budgets. When the budget is exhausted or disabled (`maxPostReviewFixes <= 0`), behavior falls through to the prior semantics: advisory results remain non-blocking and gate failures remain failed/parked. +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, so the optional step re-runs against the fixed diff; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the resolved revision budget is exhausted. By default, each step uses the workflow/project `maxPostReviewFixes` value (built-in default: 3 fix passes). A workflow author can override that for a specific `optional-group` with `config.maxRevisions`: a non-negative integer sets that step's ceiling, `0` disables automatic fixes for that step, and `"unbounded"` removes the ceiling check. The counter remains the task's shared `postReviewFixCount`; per-step counters are not maintained. + +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. 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. diff --git a/packages/core/src/__tests__/workflow-ir-optional-group.test.ts b/packages/core/src/__tests__/workflow-ir-optional-group.test.ts index 88806e3383..518d8ae92d 100644 --- a/packages/core/src/__tests__/workflow-ir-optional-group.test.ts +++ b/packages/core/src/__tests__/workflow-ir-optional-group.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; -import { parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js"; +import { WorkflowIrError, parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js"; +import { resolveOptionalStepRevisionBudget } from "../workflow-ir-types.js"; import type { WorkflowIrEdge, WorkflowIrNode, WorkflowIrV2 } from "../workflow-ir-types.js"; /* @@ -66,6 +67,32 @@ describe("optional-group validation", () => { ); }); + it("accepts and round-trips per-step maxRevisions budgets", () => { + for (const maxRevisions of [2, "unbounded"] as const) { + const parsed = parseWorkflowIr(groupIr({ maxRevisions })) as WorkflowIrV2; + const group = parsed.nodes.find((n) => n.id === "browser-verification"); + expect(group?.config?.maxRevisions).toBe(maxRevisions); + expect(parseWorkflowIr(serializeWorkflowIr(parsed))).toEqual(parsed); + } + }); + + it("rejects invalid per-step maxRevisions budgets", () => { + for (const maxRevisions of [-1, 1.5, "sometimes"] as const) { + expect(() => parseWorkflowIr(groupIr({ maxRevisions }))).toThrow(WorkflowIrError); + expect(() => parseWorkflowIr(groupIr({ maxRevisions }))).toThrow( + /maxRevisions must be a non-negative integer or "unbounded"/, + ); + } + }); + + it("resolves optional-step revision budgets from numeric, unbounded, and fallback states", () => { + expect(resolveOptionalStepRevisionBudget(2, 3)).toEqual({ unbounded: false, max: 2 }); + expect(resolveOptionalStepRevisionBudget(0, 3)).toEqual({ unbounded: false, max: 0 }); + expect(resolveOptionalStepRevisionBudget("unbounded", 3)).toEqual({ unbounded: true, max: Number.POSITIVE_INFINITY }); + expect(resolveOptionalStepRevisionBudget(undefined, 3)).toEqual({ unbounded: false, max: 3 }); + expect(resolveOptionalStepRevisionBudget("sometimes", 3)).toEqual({ unbounded: false, max: 3 }); + }); + it("rejects an empty template", () => { expect(() => parseWorkflowIr(groupIr({ template: { nodes: [], edges: [] } }))).toThrow(/non-empty/); }); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9453685178..47539548ce 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -116,6 +116,7 @@ export type { WorkflowLoopConfig, WorkflowLoopExitCondition, WorkflowOptionalGroupConfig, + OptionalStepRevisionBudget, WorkflowIrArtifact, WorkflowFieldDefinition, WorkflowFieldType, @@ -134,6 +135,7 @@ export { DEFAULT_MAX_REWORK_CYCLES, MAX_REWORK_CYCLES_CAP, resolveMaxReworkCycles, + resolveOptionalStepRevisionBudget, } from "./workflow-ir-types.js"; export { instanceNodeId, diff --git a/packages/core/src/workflow-ir-types.ts b/packages/core/src/workflow-ir-types.ts index a8c0c64e06..3249c8b302 100644 --- a/packages/core/src/workflow-ir-types.ts +++ b/packages/core/src/workflow-ir-types.ts @@ -63,6 +63,33 @@ export function resolveMaxReworkCycles(raw: unknown): number { return Math.max(1, Math.min(MAX_REWORK_CYCLES_CAP, Math.floor(n))); } +export interface OptionalStepRevisionBudget { + unbounded: boolean; + max: number; +} + +/* +FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:15: +Optional-group steps can override the global `maxPostReviewFixes` cycle budget with a per-step non-negative integer or `"unbounded"`. The resolver keeps absent/invalid raw values byte-inert by falling back to the effective global budget, while `"unbounded"` explicitly removes the ceiling so Code Review, Browser Verification, or custom optional gates can cycle until they approve. +*/ +export function resolveOptionalStepRevisionBudget( + rawMaxRevisions: unknown, + fallback: number, +): OptionalStepRevisionBudget { + if (rawMaxRevisions === "unbounded") { + return { unbounded: true, max: Number.POSITIVE_INFINITY }; + } + if ( + typeof rawMaxRevisions === "number" && + Number.isFinite(rawMaxRevisions) && + Number.isInteger(rawMaxRevisions) && + rawMaxRevisions >= 0 + ) { + return { unbounded: false, max: rawMaxRevisions }; + } + return { unbounded: false, max: fallback }; +} + /** * Executor kinds selectable on a prompt/execute node's `config.executor` (CLI * Agent Executor, U7). The engine reads `config.executor` as an open string; this @@ -170,6 +197,9 @@ FNXC:WorkflowOptionalGroup 2026-06-21-11:00: An `optional-group` node is a container (mirroring `foreach`/`loop`) whose `template` subgraph the executor runs ONCE when the group is enabled for the task and passes through (skips) when disabled. Enable state reuses the per-task `enabledWorkflowSteps` facet keyed by the group node id, seeded from `defaultOn` at task creation — this replaces the execution-inert declaration-based optional-steps model (`WorkflowOptionalStep`/`optionalSteps`). Single pass only: no iteration, no rework budget. Rework edges are forbidden inside the template so the single-pass guarantee is unambiguous (validated in `validateOptionalGroup`). + +FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:15: +Optional-group remediation still runs the template once per graph pass, but workflow authors can now set a per-step `maxRevisions` override for the PRE-merge fix→re-review cycle. A non-negative integer caps that optional step against the shared task `postReviewFixCount`, `"unbounded"` removes the ceiling, and absence preserves the effective global `maxPostReviewFixes` behavior. */ /** Config for an `optional-group` container node. `defaultOn` seeds the per-task * enable set at creation; the `template` is the subgraph run once when enabled. @@ -179,6 +209,13 @@ export interface WorkflowOptionalGroupConfig { defaultOn?: boolean; /** Display name for the group (editor + per-task toggle surfaces). */ name?: string; + /** + * Per-step override for the global `maxPostReviewFixes` budget used by this + * optional step's PRE-merge fix→re-review cycle. A non-negative integer caps + * revisions for this step; `"unbounded"` keeps cycling until the step returns + * APPROVE/APPROVE_WITH_NOTES; absence falls back to effective settings. + */ + maxRevisions?: number | "unbounded"; /* FNXC:WorkflowPostMerge 2026-06-26-09:00: Execution phase of the optional-group step. Defaults to "pre-merge" (the prior, only diff --git a/packages/core/src/workflow-ir.ts b/packages/core/src/workflow-ir.ts index afba851d13..1e37af7b4b 100644 --- a/packages/core/src/workflow-ir.ts +++ b/packages/core/src/workflow-ir.ts @@ -668,6 +668,21 @@ function validateOptionalGroup( if (cfg.phase !== undefined && cfg.phase !== "pre-merge" && cfg.phase !== "post-merge") { throw new WorkflowIrError(`optional-group node '${node.id}' phase must be 'pre-merge' or 'post-merge'`); } + /* + * FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:22: + * Parse-time validation accepts only an explicit non-negative integer budget or `"unbounded"`; absent remains byte-inert and resolves through the global `maxPostReviewFixes` fallback at execution time. + */ + if (cfg.maxRevisions !== undefined) { + const maxRevisions = cfg.maxRevisions; + if ( + maxRevisions !== "unbounded" && + (typeof maxRevisions !== "number" || !Number.isInteger(maxRevisions) || maxRevisions < 0) + ) { + throw new WorkflowIrError( + `optional-group node '${node.id}' maxRevisions must be a non-negative integer or "unbounded"`, + ); + } + } const templateNodes = template.nodes; const templateIds = new Set(templateNodes.map((n) => n.id)); diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 7698241749..9a777af80c 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -4371,23 +4371,76 @@ function InnerEditor({ {/* FNXC:WorkflowOptionalGroup 2026-06-21-11:30: The optional-group inspector exposes the workflow-author `defaultOn` default (whether new tasks enable the group). The group name reuses the shared Name field above; the body is authored by dropping nodes inside, identical to foreach/loop. */} {selectedNode.data.kind === "optional-group" ? ( - <> - -

- {t( - "workflowNodes.optionalGroupNote", - "Runs the steps inside this group once when the task enables it (seeded from this default), and skips them when disabled. Drop the optional steps into the region.", - )} -

- + (() => { + const maxRevisions = selectedNode.data.config?.maxRevisions; + const isUnbounded = maxRevisions === "unbounded"; + return ( + <> + + + {/* FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:47: Optional-group authors need a per-step revision budget control that round-trips as `config.maxRevisions`; clearing deletes the key so existing workflows keep the global `maxPostReviewFixes` fallback, while Unbounded disables the numeric input and persists the explicit `"unbounded"` mode. */} + + + + +

+ {t( + "workflowNodes.optionalGroupNote", + "Runs the steps inside this group once when the task enables it (seeded from this default), and skips them when disabled. Drop the optional steps into the region.", + )} +

+ + ); + })() ) : null} {selectedNode.data.kind === "step-review" ? ( diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index 3350fa3301..435a10a11d 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -1903,6 +1903,101 @@ describe("WorkflowNodeEditor — U8 step-inversion authoring", () => { expect(template.nodes).toHaveLength(1); }); + it("edits optional-group maxRevisions and unbounded revision mode", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]); + vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...optionalGroupDef(), ...(updates as object) })); + vi.mocked(compileWorkflow).mockResolvedValue({ steps: [] }); + + render( {}} addToast={() => {}} />); + await screen.findByText("Save"); + fireEvent.click(await screen.findByTestId("wf-node-optional-group")); + + const maxInput = await screen.findByTestId("wf-optional-group-max-revisions") as HTMLInputElement; + const unbounded = await screen.findByTestId("wf-optional-group-max-revisions-unbounded") as HTMLInputElement; + expect(maxInput.disabled).toBe(false); + expect(unbounded.checked).toBe(false); + + fireEvent.change(maxInput, { target: { value: "2" } }); + fireEvent.click(screen.getByText("Save").closest("button")!); + await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1)); + let [, updates] = vi.mocked(updateWorkflow).mock.calls[0]; + let ir = (updates as { ir: { nodes: { kind: string; config?: Record }[] } }).ir; + let opt = ir.nodes.find((n) => n.kind === "optional-group"); + expect(opt!.config!.maxRevisions).toBe(2); + }); + + it("persists optional-group unbounded maxRevisions mode", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]); + vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...optionalGroupDef(), ...(updates as object) })); + vi.mocked(compileWorkflow).mockResolvedValue({ steps: [] }); + + render( {}} addToast={() => {}} />); + await screen.findByText("Save"); + fireEvent.click(await screen.findByTestId("wf-node-optional-group")); + + const unbounded = await screen.findByTestId("wf-optional-group-max-revisions-unbounded") as HTMLInputElement; + fireEvent.click(unbounded); + expect(unbounded.checked).toBe(true); + await waitFor(() => expect((screen.getByTestId("wf-optional-group-max-revisions") as HTMLInputElement).disabled).toBe(true)); + fireEvent.click(screen.getByText("Save").closest("button")!); + + await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1)); + const [, updates] = vi.mocked(updateWorkflow).mock.calls[0]; + const ir = (updates as { ir: { nodes: { kind: string; config?: Record }[] } }).ir; + const opt = ir.nodes.find((n) => n.kind === "optional-group"); + expect(opt!.config!.maxRevisions).toBe("unbounded"); + }); + + it("clears optional-group maxRevisions by deleting the config key", async () => { + const def = optionalGroupDef(); + const opt = def.ir.nodes.find((node) => node.kind === "optional-group")!; + opt.config = { ...opt.config, maxRevisions: 2 }; + vi.mocked(fetchWorkflows).mockResolvedValue([def]); + vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...def, ...(updates as object) })); + vi.mocked(compileWorkflow).mockResolvedValue({ steps: [] }); + + render( {}} addToast={() => {}} />); + await screen.findByText("Save"); + fireEvent.click(await screen.findByTestId("wf-node-optional-group")); + + const maxInput = await screen.findByTestId("wf-optional-group-max-revisions") as HTMLInputElement; + expect(maxInput.value).toBe("2"); + fireEvent.change(maxInput, { target: { value: "" } }); + fireEvent.click(screen.getByText("Save").closest("button")!); + + await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1)); + const [, updates] = vi.mocked(updateWorkflow).mock.calls[0]; + const ir = (updates as { ir: { nodes: { kind: string; config?: Record }[] } }).ir; + const saved = ir.nodes.find((node) => node.kind === "optional-group")!; + expect(saved.config!).not.toHaveProperty("maxRevisions"); + }); + + it("renders optional-group maxRevisions controls in the mobile inspector", async () => { + mockWorkflowEditorViewport("mobile"); + vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]); + render( {}} addToast={() => {}} />); + fireEvent.click(await screen.findByRole("button", { name: "Optional" })); + const optRow = await screen.findByTestId("mobile-wf-node-opt"); + fireEvent.click(within(optRow).getAllByRole("button")[0]); + + expect(await screen.findByTestId("wf-optional-group-max-revisions")).toBeInTheDocument(); + expect(await screen.findByTestId("wf-optional-group-max-revisions-unbounded")).toBeInTheDocument(); + }); + + it("does not render optional-group maxRevisions controls for non-optional nodes", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]); + render( {}} addToast={() => {}} />); + await screen.findByText("Save"); + const promptNode = await waitFor(() => { + const node = document.querySelector(`.react-flow__node[data-id="${foreachChildFlowId("opt", "verify")}"]`); + expect(node).toBeInTheDocument(); + return node as HTMLElement; + }); + fireEvent.click(promptNode); + expect(screen.queryByTestId("wf-optional-group-max-revisions")).not.toBeInTheDocument(); + expect(screen.queryByTestId("wf-optional-group-max-revisions-unbounded")).not.toBeInTheDocument(); + }); + it("toggles optional-group defaultOn, marks the editor dirty, and persists on save", async () => { vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]); vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...optionalGroupDef(), ...(updates as object) })); diff --git a/packages/engine/src/__tests__/self-healing.test.ts b/packages/engine/src/__tests__/self-healing.test.ts index b5f31b1443..8a0169e1ca 100644 --- a/packages/engine/src/__tests__/self-healing.test.ts +++ b/packages/engine/src/__tests__/self-healing.test.ts @@ -5325,6 +5325,69 @@ describe("SelfHealingManager", () => { managerWithRecovery.stop(); }); + it("honors per-step numeric and unbounded maxRevisions resolved from the task workflow IR", async () => { + const workflowIr = { + version: "v2" as const, + name: "review-budget-test", + columns: [{ id: "work", name: "Work", traits: [] }], + nodes: [ + { id: "start", kind: "start" as const }, + { + id: "WS-004", + kind: "optional-group" as const, + config: { + maxRevisions: 2, + template: { nodes: [{ id: "review", kind: "gate" as const, config: { prompt: "review" } }], edges: [] }, + }, + }, + { id: "end", kind: "end" as const }, + ], + edges: [{ from: "start", to: "WS-004" }, { from: "WS-004", to: "end" }], + }; + const recoverFn = vi.fn().mockResolvedValue(true); + const managerWithRecovery = new SelfHealingManager(store, { + rootDir: "/tmp/test-project", + recoverFailedPreMergeStep: recoverFn, + }); + (store.getSettings as ReturnType).mockResolvedValue({ maxPostReviewFixes: 9 }); + (store as unknown as { getTaskWorkflowSelection: ReturnType }).getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "WF-budget", stepIds: ["WS-004"] })); + (store as unknown as { getWorkflowDefinition: ReturnType }).getWorkflowDefinition = vi.fn().mockResolvedValue({ ir: workflowIr }); + (store.listTasks as ReturnType).mockResolvedValue([{ ...baseTask, postReviewFixCount: 2 }]); + + await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(0); + expect(recoverFn).not.toHaveBeenCalled(); + + workflowIr.nodes[1] = { + ...workflowIr.nodes[1], + config: { ...(workflowIr.nodes[1] as { config: Record }).config, maxRevisions: "unbounded" }, + }; + (store.listTasks as ReturnType).mockResolvedValue([{ ...baseTask, postReviewFixCount: 99 }]); + + await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1); + expect(store.logEntry).toHaveBeenLastCalledWith("FN-1572", expect.stringContaining("attempt 100/unbounded")); + expect(recoverFn).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-1572" })); + + managerWithRecovery.stop(); + }); + + it("falls back to maxPostReviewFixes when workflow IR resolution fails", async () => { + const recoverFn = vi.fn().mockResolvedValue(true); + const managerWithRecovery = new SelfHealingManager(store, { + rootDir: "/tmp/test-project", + recoverFailedPreMergeStep: recoverFn, + }); + (store.getSettings as ReturnType).mockResolvedValue({ maxPostReviewFixes: 1 }); + (store as unknown as { getTaskWorkflowSelection: ReturnType }).getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "WF-missing", stepIds: ["WS-004"] })); + (store as unknown as { getWorkflowDefinition: ReturnType }).getWorkflowDefinition = vi.fn().mockRejectedValue(new Error("boom")); + (store.listTasks as ReturnType).mockResolvedValue([{ ...baseTask, postReviewFixCount: 0 }]); + + await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1); + expect(store.logEntry).toHaveBeenCalledWith("FN-1572", expect.stringContaining("attempt 1/1")); + expect(recoverFn).toHaveBeenCalledOnce(); + + managerWithRecovery.stop(); + }); + it("skips tasks whose postReviewFixCount has reached maxPostReviewFixes", async () => { const recoverFn = vi.fn().mockResolvedValue(true); const managerWithRecovery = new SelfHealingManager(store, { 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 596b5954f7..dd6e822b2c 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts @@ -88,7 +88,7 @@ function taskWith(enabled: string[] | undefined): TaskDetail { return { id: "FN-OG", enabledWorkflowSteps: enabled } as TaskDetail; } -function reviseGroupIr(options: { phase?: "pre-merge" | "post-merge"; gateMode?: "advisory" | "gate" } = {}): WorkflowIr { +function reviseGroupIr(options: { phase?: "pre-merge" | "post-merge"; gateMode?: "advisory" | "gate"; maxRevisions?: number | "unbounded" } = {}): WorkflowIr { return { version: "v2", name: "optional-group-revise-test", @@ -102,6 +102,7 @@ function reviseGroupIr(options: { phase?: "pre-merge" | "post-merge"; gateMode?: name: options.phase === "post-merge" ? "Post-merge verification" : "Code Review", defaultOn: true, phase: options.phase, + maxRevisions: options.maxRevisions, template: { nodes: [{ id: "review", kind: options.gateMode === "gate" ? "gate" : "prompt", config: { prompt: "review" } }], edges: [], @@ -280,6 +281,8 @@ describe("WorkflowGraphExecutor optional-group", () => { phase: "pre-merge", status: "advisory_failure", verdict: "REVISE", + nodeId: "group", + maxRevisions: undefined, }); expect(calls).not.toContain("after"); expect(result.context["node:group:fixScheduled"]).toBe(true); @@ -288,6 +291,25 @@ describe("WorkflowGraphExecutor optional-group", () => { ])); }); + it("threads optional-group maxRevisions into the pre-merge fix seam", async () => { + const requestFix = vi.fn(async () => true); + const executor = new WorkflowGraphExecutor({ + handlers: { + prompt: async (node) => node.id === "review" + ? { outcome: "success", value: "REVISE", contextPatch: { output: "custom finding" } } + : { outcome: "success" }, + }, + requestPreMergeOptionalStepFix: requestFix, + }); + + await executor.run(taskWith(["group"]), settingsOn(), reviseGroupIr({ maxRevisions: "unbounded" })); + + expect(requestFix).toHaveBeenCalledWith("FN-OG", expect.objectContaining({ + nodeId: "group", + maxRevisions: "unbounded", + })); + }); + it("falls through unchanged when the pre-merge fix seam is absent or declines", async () => { for (const requestFix of [undefined, vi.fn(async () => false)] as const) { const calls: string[] = []; @@ -344,7 +366,7 @@ describe("WorkflowGraphExecutor optional-group", () => { }, requestPreMergeOptionalStepFix: requestFix, }); - await approveExecutor.run(taskWith(["group"]), settingsOn(), reviseGroupIr()); + await approveExecutor.run(taskWith(["group"]), settingsOn(), reviseGroupIr({ maxRevisions: "unbounded" })); expect(requestFix).not.toHaveBeenCalled(); const fastExecutor = new WorkflowGraphExecutor({ @@ -452,6 +474,8 @@ describe("WorkflowGraphExecutor optional-group", () => { expect(requestFix).toHaveBeenCalledWith(`FN-${groupId}`, expect.objectContaining({ stepName: groupId === "code-review" ? "Code Review" : "Browser Verification", feedback: `${groupId} finding`, + nodeId: groupId, + maxRevisions: undefined, })); expect(calls).not.toContain("review"); expect(result.context[`node:${groupId}:fixScheduled`]).toBe(true); @@ -494,6 +518,8 @@ describe("WorkflowGraphExecutor optional-group", () => { expect(stepwiseRequestFix).toHaveBeenCalledWith(`FN-stepwise-${groupId}`, expect.objectContaining({ stepName: groupId === "code-review" ? "Code Review" : "Browser Verification", feedback: `stepwise ${groupId} finding`, + nodeId: groupId, + maxRevisions: undefined, })); expect(stepwiseResult.context[`node:${groupId}:fixScheduled`]).toBe(true); } diff --git a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts index 40648d7483..72604a0a11 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-step-fix.test.ts @@ -101,6 +101,59 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => { expect(sendBackCalls).toEqual([0, 1, 2]); }); + it("lets per-step maxRevisions override the global budget", async () => { + for (const count of [1, 2]) { + const store = createMockStore(); + const liveTask = task({ postReviewFixCount: count }); + store.getTask.mockResolvedValue(liveTask); + store.getSettings.mockResolvedValue({ maxPostReviewFixes: 9 }); + const executor = new TaskExecutor(store, "/tmp/test"); + const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined); + + const scheduled = await (executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, { + ...reviseInfo, + maxRevisions: 2, + }); + + expect(scheduled).toBe(count < 2); + if (count < 2) { + expect(store.logEntry).toHaveBeenCalledWith("FN-7066", expect.stringContaining("attempt 2/2"), expect.any(String), undefined); + expect(sendBack).toHaveBeenCalledOnce(); + } else { + expect(sendBack).not.toHaveBeenCalled(); + } + } + }); + + it("honors unbounded and zero per-step maxRevisions states", async () => { + const unboundedStore = createMockStore(); + const exhaustedTask = task({ postReviewFixCount: 99 }); + unboundedStore.getTask.mockResolvedValue(exhaustedTask); + unboundedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 1 }); + const unboundedExecutor = new TaskExecutor(unboundedStore, "/tmp/test"); + const unboundedSendBack = vi.spyOn(unboundedExecutor as any, "sendTaskBackForFix").mockResolvedValue(undefined); + + await expect((unboundedExecutor as any).requestPreMergeOptionalStepFix(exhaustedTask.id, exhaustedTask, { + ...reviseInfo, + maxRevisions: "unbounded", + })).resolves.toBe(true); + expect(unboundedStore.logEntry).toHaveBeenCalledWith("FN-7066", expect.stringContaining("attempt 100/unbounded"), expect.any(String), undefined); + expect(unboundedSendBack).toHaveBeenCalledOnce(); + + const zeroStore = createMockStore(); + const liveTask = task({ postReviewFixCount: 0 }); + zeroStore.getTask.mockResolvedValue(liveTask); + zeroStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9 }); + const zeroExecutor = new TaskExecutor(zeroStore, "/tmp/test"); + const zeroSendBack = vi.spyOn(zeroExecutor as any, "sendTaskBackForFix").mockResolvedValue(undefined); + + await expect((zeroExecutor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, { + ...reviseInfo, + maxRevisions: 0, + })).resolves.toBe(false); + expect(zeroSendBack).not.toHaveBeenCalled(); + }); + it("declines without sending back when maxPostReviewFixes disables or exhausts the budget", async () => { for (const { settingsMax, count } of [ { settingsMax: 0, count: 0 }, diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 93d1297bbe..56b0d799cb 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -10,7 +10,7 @@ import { existsSync, lstatSync, realpathSync } from "node:fs"; import { readFile, rm, writeFile } from "node:fs/promises"; import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings, WorkflowStep, MissionStore, Slice, AgentState, AgentCapability, RunMutationContext, AgentHeartbeatConfig, Agent, AgentMemoryInclusionMode, ProjectSettings, MergeResult, WorkflowIrNode, WorkflowIrNodeKind, WorkflowStepResult as CoreWorkflowStepResult } from "@fusion/core"; import { getUnmetSchedulingDependencies } from "./scheduler.js"; -import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries } from "@fusion/core"; +import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget } from "@fusion/core"; import { mergeEffectiveSettings } from "./effective-settings.js"; import type { TaskStep, WorkflowIr, WorkflowFieldDefinition, WorkflowColumnAgent, EffectiveAgentInput, WorkflowWorkEngineDispatchResult } from "@fusion/core"; import { @@ -3710,7 +3710,7 @@ export class TaskExecutor { /* * FNXC:WorkflowOptionalStepFix 2026-06-26-16:35: - * Inline graph optional-step remediation consumes `postReviewFixCount` BEFORE calling `sendTaskBackForFix`, matching self-healing's budget-first ordering. This prevents a persistent Code Review / Browser Verification REVISE from ping-ponging forever: when `postReviewFixCount >= maxPostReviewFixes` (or max <= 0), the seam declines and graph execution falls through to the prior advisory/gate behavior. + * Inline graph optional-step remediation consumes `postReviewFixCount` BEFORE calling `sendTaskBackForFix`, matching self-healing's budget-first ordering. Persistent Code Review / Browser Verification REVISE loops are bounded by the optional-group `maxRevisions` override when present, otherwise by `maxPostReviewFixes`; `"unbounded"` intentionally skips the ceiling check so the step cycles until it returns APPROVE/APPROVE_WITH_NOTES or a human intervenes. */ private async requestPreMergeOptionalStepFix( taskId: string, @@ -3721,6 +3721,8 @@ export class TaskExecutor { phase: CoreWorkflowStepResult["phase"]; status: CoreWorkflowStepResult["status"]; verdict?: string; + nodeId?: string; + maxRevisions?: unknown; }, ): Promise { if (info.phase !== "pre-merge") return false; @@ -3729,17 +3731,18 @@ export class TaskExecutor { const liveTask = await this.store.getTask(taskId).catch(() => fallbackTask); const settings = await mergeEffectiveSettings(this.store, liveTask, await this.store.getSettings()); - const maxFixes = settings.maxPostReviewFixes ?? 3; - if (!Number.isFinite(maxFixes) || maxFixes <= 0) return false; + const budget = resolveOptionalStepRevisionBudget(info.maxRevisions, settings.maxPostReviewFixes ?? 3); + if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false; const currentCount = liveTask.postReviewFixCount ?? 0; - if (currentCount >= maxFixes) return false; + if (!budget.unbounded && currentCount >= budget.max) return false; const nextCount = currentCount + 1; + const budgetLabel = budget.unbounded ? "unbounded" : String(budget.max); await this.store.updateTask(taskId, { postReviewFixCount: nextCount }, this.getRunContextFor(taskId)); await this.store.logEntry( taskId, - `Pre-merge optional workflow step requested executor fixes (attempt ${nextCount}/${maxFixes})`, + `Pre-merge optional workflow step requested executor fixes (attempt ${nextCount}/${budgetLabel})`, `Step: ${info.stepName}\nStatus: ${info.status}\nFeedback:\n${info.feedback}`, this.getRunContextFor(taskId), ); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index d173660e06..36b73ec9ac 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { setImmediate as setImmediateCb } from "node:timers"; import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, resolveMaxAutoMergeRetries, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core"; +import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveWorkflowIrForTask, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core"; import type { MeshLeaseManager } from "./mesh-lease-manager.js"; import { createLogger, schedulerLog } from "./logger.js"; import { mergeEffectiveSettings } from "./effective-settings.js"; @@ -6079,15 +6079,48 @@ export class SelfHealingManager { const tasks = await this.store.listTasks({ column: "in-review", slim: true }); const executingIds = this.options.getExecutingTaskIds?.() ?? new Set(); - // Resolve the per-task effective `maxPostReviewFixes` (U3, KTD-3) — this is a - // cross-task recovery sweep, so the budget is resolved per task rather than - // from a single global read. Behavior-inert when nothing is customized. - const maxFixesByTask = new Map(); + const latestFailedPreMergeStep = (task: Pick): WorkflowStepResult | undefined => { + return (task.workflowStepResults ?? []) + .filter((r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed") + .sort((a, b) => { + const aTs = Date.parse(a.completedAt || a.startedAt || ""); + const bTs = Date.parse(b.completedAt || b.startedAt || ""); + return (Number.isFinite(bTs) ? bTs : 0) - (Number.isFinite(aTs) ? aTs : 0); + })[0]; + }; + + /* + * FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:34: + * Self-healing pre-computes the same optional-step budget the live graph seam uses before the synchronous candidate filter runs. The target step is the latest blocking pre-merge failure, matching `recoverFailedPreMergeWorkflowStep`; IR lookup failures fall back to the effective global `maxPostReviewFixes` so older tasks remain recoverable. + */ + const revisionBudgetByTask = new Map(); + const irCache = new Map>>(); for (const task of tasks) { const eff = await mergeEffectiveSettings(this.store, task, settings); - maxFixesByTask.set(task.id, eff.maxPostReviewFixes ?? 3); + const fallback = eff.maxPostReviewFixes ?? 3; + let rawMaxRevisions: unknown; + const target = latestFailedPreMergeStep(task); + if (target?.workflowStepId) { + try { + const ir = await resolveWorkflowIrForTask(this.store, task.id, irCache); + if (ir.version === "v2") { + const node = ir.nodes.find((candidate) => candidate.id === target.workflowStepId && candidate.kind === "optional-group"); + rawMaxRevisions = node?.config?.maxRevisions; + } + } catch { + rawMaxRevisions = undefined; + } + } + const budget = resolveOptionalStepRevisionBudget(rawMaxRevisions, fallback); + revisionBudgetByTask.set(task.id, { + ...budget, + label: budget.unbounded ? "unbounded" : String(budget.max), + }); } - const maxFixesFor = (taskId: string): number => maxFixesByTask.get(taskId) ?? 3; + const revisionBudgetFor = (taskId: string): { unbounded: boolean; max: number; label: string } => { + const budget = revisionBudgetByTask.get(taskId) ?? resolveOptionalStepRevisionBudget(undefined, 3); + return { ...budget, label: budget.unbounded ? "unbounded" : String(budget.max) }; + }; const candidates = tasks.filter((task) => { if (task.column !== "in-review") return false; @@ -6097,15 +6130,12 @@ export class SelfHealingManager { // merging, etc.). Only revive tasks that are otherwise idle. if (task.status) return false; if (executingIds.has(task.id)) return false; - const maxFixes = maxFixesFor(task.id); - if (!Number.isFinite(maxFixes) || maxFixes <= 0) return false; - if ((task.postReviewFixCount ?? 0) >= maxFixes) return false; + const budget = revisionBudgetFor(task.id); + if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false; + if (!budget.unbounded && (task.postReviewFixCount ?? 0) >= budget.max) return false; // Must have at least one failed pre-merge workflow step result. - const hasFailedPreMerge = (task.workflowStepResults ?? []).some( - (r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed", - ); - if (!hasFailedPreMerge) return false; + if (!latestFailedPreMergeStep(task)) return false; // Merge must be blocked *specifically* by the failed pre-merge step — // not by an unrelated condition (incomplete steps, etc.) that is @@ -6128,7 +6158,7 @@ export class SelfHealingManager { let recovered = 0; for (const task of candidates) { const nextCount = (task.postReviewFixCount ?? 0) + 1; - const maxFixes = maxFixesFor(task.id); + const budget = revisionBudgetFor(task.id); try { // Increment the counter BEFORE delegating so that even if the // executor path crashes or races, the budget is still consumed and @@ -6136,11 +6166,11 @@ export class SelfHealingManager { await this.store.updateTask(task.id, { postReviewFixCount: nextCount }); await this.store.logEntry( task.id, - `Auto-reviving in-review task with failed pre-merge workflow step (attempt ${nextCount}/${maxFixes})`, + `Auto-reviving in-review task with failed pre-merge workflow step (attempt ${nextCount}/${budget.label})`, ); const sentBack = await recoverFn(task); if (sentBack) { - log.log(`Revived ${task.id}: sent back for fix (${nextCount}/${maxFixes})`); + log.log(`Revived ${task.id}: sent back for fix (${nextCount}/${budget.label})`); recovered++; } else { log.warn(`Revival of ${task.id} was skipped by executor — budget already consumed`); diff --git a/packages/engine/src/workflow-graph-executor.ts b/packages/engine/src/workflow-graph-executor.ts index f6f8cfbf61..88062b0595 100644 --- a/packages/engine/src/workflow-graph-executor.ts +++ b/packages/engine/src/workflow-graph-executor.ts @@ -168,7 +168,7 @@ export interface WorkflowGraphExecutorDeps { recordWorkflowStepResult?: (taskId: string, result: WorkflowStepResult) => void | Promise; /* * FNXC:WorkflowOptionalStepFix 2026-06-26-16:20: - * Enabled PRE-merge optional workflow steps that return REVISE must offer the executor one bounded remediation path before normal advisory/gate fall-through. This seam returns true only when the caller already consumed the `maxPostReviewFixes` budget and scheduled `sendTaskBackForFix`; the graph must then stop before review/merge. Absent or false preserves prior byte-inert behavior for in-memory tests and exhausted budgets. + * Enabled PRE-merge optional workflow steps that return REVISE must offer the executor one remediation path before normal advisory/gate fall-through. The graph forwards the optional-group node id and per-step `maxRevisions` override so the executor can resolve the budget against `maxPostReviewFixes` or honor `"unbounded"`; absent or false preserves prior byte-inert behavior for in-memory tests and exhausted budgets. */ requestPreMergeOptionalStepFix?: (taskId: string, info: { stepName: string; @@ -176,6 +176,8 @@ export interface WorkflowGraphExecutorDeps { phase: WorkflowStepResult["phase"]; status: WorkflowStepResult["status"]; verdict?: string; + nodeId?: string; + maxRevisions?: unknown; }) => Promise | boolean; /** Project node-published task metadata onto the task row for dispatcher/UI. */ publishTaskProjection?: (taskId: string, patch: WorkflowTaskProjection, source: { nodeId: string; nodeKind: WorkflowIrNode["kind"] }) => void | Promise; @@ -672,6 +674,8 @@ export class WorkflowGraphExecutor { phase: stepPhase, status: stepStatus, verdict, + nodeId: node.id, + maxRevisions: node.config?.maxRevisions, }); if (fixScheduled) { context[`node:${node.id}:fixScheduled`] = true;