diff --git a/.changeset/bound-ce-review-remediation.md b/.changeset/bound-ce-review-remediation.md new file mode 100644 index 0000000000..f18cf56018 --- /dev/null +++ b/.changeset/bound-ce-review-remediation.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Compound Engineering Code Review now parks after two unsuccessful remediation attempts instead of retrying forever. +category: fix +dev: The built-in CE `code-review` optional group now declares `maxRevisions: 2`; custom workflow authors can still choose a different numeric cap or explicit unbounded behavior. diff --git a/.changeset/review-restart-recovery-latency.md b/.changeset/review-restart-recovery-latency.md index 7c2bd1514e..48e039db9d 100644 --- a/.changeset/review-restart-recovery-latency.md +++ b/.changeset/review-restart-recovery-latency.md @@ -4,4 +4,4 @@ summary: Reviews stalled by an engine restart now recover in one self-healing cycle instead of ~36 minutes. category: fix -dev: Moves `reconcile-orphaned-pending-step-results` ahead of `recover-failed-pre-merge-steps` in the periodic maintenance list (it produces the `failed` results that step consumes; it previously ran ~15 entries later, so an orphan found in cycle N was not re-dispatched until cycle N+1) and removes the now-duplicated later entry. Raises the `maxPostReviewFixes` default 3 -> 10 and routes the five inline `?? 3` fallbacks in executor.ts/self-healing.ts through the new exported `DEFAULT_MAX_POST_REVIEW_FIXES` so the declaration default and the unset-settings paths cannot drift again. Plan Review and Code Review are unaffected — they already resolve to "unbounded" when unset. +dev: Moves `reconcile-orphaned-pending-step-results` ahead of `recover-failed-pre-merge-steps` in the periodic maintenance list (it produces the `failed` results that step consumes; it previously ran ~15 entries later, so an orphan found in cycle N was not re-dispatched until cycle N+1) and removes the now-duplicated later entry. Raises the `maxPostReviewFixes` default 3 -> 10 and routes the five inline `?? 3` fallbacks in executor.ts/self-healing.ts through the new exported `DEFAULT_MAX_POST_REVIEW_FIXES` so the declaration default and the unset-settings paths cannot drift again. Plan Review remains unbounded when unset; Code Review uses the workflow-authored node cap (Compound Engineering: 2, most other built-ins: unbounded). diff --git a/docs/settings-reference.md b/docs/settings-reference.md index 28e006c33d..32f3d8a1b8 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -321,9 +321,11 @@ Actions. It has two tabs: updates both keys together. Declared primary and fallback lane thinking companions render inline and clear with the lane reset instead of as separate enum fields. Advanced/custom non-model settings still use typed controls. Built-in Plan Review/spec and Code Review revision caps also live here: - leave `planReviewMaxRevisions` or `codeReviewMaxRevisions` empty for unbounded - automatic revisions, enter a non-negative integer to cap attempts, or enter `0` - to disable automatic revision for that path. The separate triage Plan Review + leave `planReviewMaxRevisions` or `codeReviewMaxRevisions` empty to use the + workflow's authored default, enter a non-negative integer to cap attempts, or + enter `0` to disable automatic revision for that path. Compound Engineering's + authored Code Review default is two passes; most other built-in review loops + remain unbounded. The separate triage Plan Review replan ceiling, `planReviewReplanCap`, controls consecutive pre-execution `REVISE` → replan cycles before Fusion requests manual approval; leave it empty to use the built-in engine default, or set a non-negative integer (including @@ -409,7 +411,7 @@ The built-in workflows also declare triage/spec policy settings that were **not* | `leanPlanning` | `false` | Workflow-native fast-mode policy: select the lean `planning-fast` prompt variant instead of the full triage spec prompt. | | `autoApproveSpec` | `false` | Legacy compatibility setting. Workflow Plan Review now owns optional pre-execution AI plan approval. | | `planReviewMaxRevisions` | unset | Workflow-native Plan Review/spec revision cap. Unset/empty means unbounded automatic replans; a non-negative integer caps attempts; `0` disables automatic Plan Review revision. | -| `codeReviewMaxRevisions` | unset | Workflow-native Code Review remediation cap. Unset/empty means unbounded automatic code-fix passes; a non-negative integer caps attempts; `0` disables automatic Code Review remediation. | +| `codeReviewMaxRevisions` | unset | Workflow-native Code Review remediation cap. Unset/empty uses the workflow's authored default (Compound Engineering: 2; most other built-ins: unbounded); a non-negative integer overrides the cap; `0` disables automatic Code Review remediation. | | `planReviewReplanCap` | unset | Workflow-native triage Plan Review replan ceiling. It bounds consecutive pre-execution Plan Review `REVISE` → replan cycles before manual approval; unset/empty uses the built-in engine default, and a non-negative integer (including `0`) overrides it. | | `plannerOversightLevel` | `autonomous` | Workflow-native planner oversight mode. `off` disables oversight; `observe` watches only; `steer` injects guidance or suggests revisions; `autonomous` enables bounded retry and targeted-fix recovery — but merge/PR progression and any destructive or external-service side effect ALWAYS require an explicit, recorded human confirmation before they run, even at `autonomous` (FN-7513's confirmation gate; see `docs/architecture.md` → "Planner overseer confirmation gate"). Tasks may set a nullable `Task.plannerOversightLevel` override (same four values) that wins over this workflow value when present; `null`/unset means "inherit the workflow value". `resolveEffectivePlannerOversightLevel` in `@fusion/core` computes the effective level (task override → workflow effective → `autonomous`). The per-task override is exposed in the dashboard as a "Planner oversight" selector (Inherit from workflow / Off / Observe / Steer / Autonomous recovery) in both the New Task dialog and Task Detail edit form, threaded through `createTask`/`updateTask` (FN-7515); the project/global default is set via the **Workflow Editor → Values** tab on the default workflow's `plannerOversightLevel` value, not in Project Settings. FN-7517 additionally exposes a quick inline oversight-level select in the Task Detail modal's meta-controls cluster (same `updateTask` override plumbing, no parallel path) plus manual nudge/stop-oversight/explain-current-action controls that call the overseer runtime directly — see `docs/dashboard-guide.md`. Engine read-site behavior beyond the FN-7513 confirmation gate remains follow-up work (FN-7510+). | | `plannerOversightNotificationLevel` | `important` | Workflow-native planner-overseer notification verbosity (FN-7518). `silent` suppresses overseer notifications; `errors` notifies only on failures/escalations; `important` (the default) notifies on interventions/recovery actions and errors; `all` notifies on every observation. Resolves through the generic `resolveEffectiveSettings` default path with no special-casing, alongside `plannerOversightLevel`. This is a declaration-only setting: the notification-emission gating that reads it lands downstream in FN-7519 (intervention timeline) and FN-7520 (run-audit/activity events). | @@ -661,7 +663,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` | Default max automatic fix passes for generic 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"`). Built-in Plan Review/spec and Code Review use workflow values `planReviewMaxRevisions` / `codeReviewMaxRevisions` first, and are unbounded when those values are unset. | +| `maxPostReviewFixes` | `number` | `10` | Default max automatic fix passes for generic 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"`). Built-in Plan Review/spec and Code Review use workflow values `planReviewMaxRevisions` / `codeReviewMaxRevisions` first, then the authored node cap; Compound Engineering authors a two-pass Code Review cap while most other built-in review loops remain 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-editor.md b/docs/workflow-editor.md index 2c108faad2..b49c051cde 100644 --- a/docs/workflow-editor.md +++ b/docs/workflow-editor.md @@ -132,7 +132,7 @@ Workflow settings are typed settings declared by a workflow in its IR. The edito The **Settings** panel has two tabs: - **Definitions:** edit the workflow's setting schema — id, name, type, default, enum options, description, and widget. This tab is read-only for built-in workflows and editable for custom workflows. Declarations save with the workflow IR through the editor's normal **Save** action. -- **Values:** edit per-project values for the currently open workflow. Values are writable even for built-in workflows. This is where operators cap built-in Plan Review/spec and Code Review revision loops (`planReviewMaxRevisions`, `codeReviewMaxRevisions`) without duplicating the read-only workflow; empty values mean unbounded, non-negative integers cap attempts, and `0` disables automatic revision for that path. Edits batch locally and commit through the tab's dedicated **Save values** action, separate from the workflow IR save. +- **Values:** edit per-project values for the currently open workflow. Values are writable even for built-in workflows. This is where operators override built-in Plan Review/spec and Code Review revision loops (`planReviewMaxRevisions`, `codeReviewMaxRevisions`) without duplicating the read-only workflow; empty values use the workflow's authored default, non-negative integers cap attempts, and `0` disables automatic revision for that path. Compound Engineering authors a two-pass Code Review default while most other built-in review loops remain unbounded. Edits batch locally and commit through the tab's dedicated **Save values** action, separate from the workflow IR save. Resolution is `stored value ?? declaration default`. Stored values that no longer validate against the current declaration are treated as orphaned and dropped from the effective settings the engine reads. The Values tab exposes provider/model lane pairs with the same model dropdown used elsewhere in Settings, while custom settings use controls based on their declared type. See [Settings Reference → Workflow Settings](./settings-reference.md#workflow-settings) for moved settings, model lane hierarchy, export behavior, and sync posture. @@ -179,7 +179,7 @@ Fusion ships built-in workflows as read-only references: - `builtin:design` — a UI-heavy work path with a gated design/UX review before standard review and merge. - `builtin:lead-generation` — a lead workflow for sourcing, qualifying, enriching, and contacting prospects. -Built-ins can be viewed, exported, and used as templates, but their graph, columns, field declarations, and setting declarations are not editable. Their per-project setting **values** are editable from the Settings panel's Values tab, including the Plan Review/spec and Code Review revision-cap values that default to unbounded when left empty. Selectable built-ins all use a capacity-released queue column (`todo` or a workflow-specific backlog) that dispatches to the active WIP column through the standard hold/release sweep. +Built-ins can be viewed, exported, and used as templates, but their graph, columns, field declarations, and setting declarations are not editable. Their per-project setting **values** are editable from the Settings panel's Values tab, including the Plan Review/spec and Code Review revision-cap values that use the workflow's authored default when left empty. Compound Engineering authors a two-pass Code Review cap while most other built-in review loops remain unbounded. Selectable built-ins all use a capacity-released queue column (`todo` or a workflow-specific backlog) that dispatches to the active WIP column through the standard hold/release sweep. -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`. +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 per enabled graph attempt**, and passes through (skips) when disabled. The template contains no internal iteration or rework edges (`validateOptionalGroup` rejects them), but the outer graph may re-enter the group for pre-merge fix/re-review remediation governed by `maxRevisions`. Node config (`WorkflowOptionalGroupConfig`): `{ name?, defaultOn?, maxRevisions?: number | "unbounded", phase?: "pre-merge" | "post-merge", template: { nodes, edges } }`. - `defaultOn` contributes to the runtime/display effective enable set only when the task has no persisted `enabledWorkflowSteps` array; operators can still toggle persisted selections when creating or editing tasks. -- `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, generic optional gates keep the global `maxPostReviewFixes` behavior; built-in `plan-review` and `code-review` instead default to unbounded remediation unless a workflow setting value caps them. +- `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, generic optional gates keep the global `maxPostReviewFixes` behavior; built-in `plan-review` and most `code-review` groups default to unbounded remediation unless a workflow setting caps them. Compound Engineering authors a two-pass Code Review cap. - `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)). - Persisted enable state lives on the per-task `enabledWorkflowSteps` array, keyed by the **group node id** (for example `browser-verification`, `code-review`). For execution, Fusion treats a group as enabled when `enabledWorkflowSteps` is present and includes the group id; if the field is omitted, Fusion falls back to the workflow node's `defaultOn: true`. An explicit empty array disables every optional group and prevents default-on gates from reappearing. @@ -423,7 +423,7 @@ Built-in optional gates ship as inlined IR builders, not as a template catalog: - `builtin:coding` carries the `browser-verification` optional-group node (`builtin-browser-verification-group.ts`), default-off, so browser verification runs only for tasks whose `enabledWorkflowSteps` includes `browser-verification`. - `builtin:coding` and `builtin:stepwise-coding` carry the `plan-review` optional-group node (`builtin-plan-review-group.ts`), default-on, before `parse-steps` so the plan can be reviewed before execution begins even when a task has not persisted explicit optional-step ids. - The `code-review` optional-group node (`builtin-code-review-group.ts`) is the inlined default-on code-review gate. On default `builtin:coding`, this is the only final review surface before merge; it is effective by default even when no explicit optional-step ids are stored. On `builtin:stepwise-coding`, it remains a post-foreach optional final review gate before the workflow's final review seam. -- Plan Review/spec revision and Code Review remediation loops are unbounded by default for built-in workflows, including Compound Engineering's CE Code Review path. Operators can cap them per workflow from the Workflow Editor **Settings → Values** tab with `planReviewMaxRevisions` and `codeReviewMaxRevisions`; leaving the value empty means unbounded, a non-negative integer caps automatic revisions, and `0` disables automatic revision for that review path. Separately, `planReviewReplanCap` bounds consecutive **triage pre-execution** Plan Review `REVISE` → replan cycles before Fusion parks the task for an explicit human Approve or Reject decision. Leave that setting empty to use the built-in engine default; a non-negative integer (including `0`) overrides it. These values are editable for read-only built-ins without duplicating the workflow. +- Plan Review/spec revision and most Code Review remediation loops are unbounded by default for built-in workflows. Compound Engineering instead defaults CE Code Review to two remediation passes so persistent findings park for operator action. Operators can override these authored defaults per workflow from the Workflow Editor **Settings → Values** tab with `planReviewMaxRevisions` and `codeReviewMaxRevisions`; leaving the value empty uses the workflow's authored default, a non-negative integer caps automatic revisions, and `0` disables automatic revision for that review path. Separately, `planReviewReplanCap` bounds consecutive **triage pre-execution** Plan Review `REVISE` → replan cycles before Fusion parks the task for an explicit human Approve or Reject decision. Leave that setting empty to use the built-in engine default; a non-negative integer (including `0`) overrides it. These values are editable for read-only built-ins without duplicating the workflow. - A workflow (for example compound-engineering) can add a **post-merge** optional-group node via the generic `postMergeOptionalGroupNode(...)` builder (`builtin-post-merge-group.ts`) — e.g. a `document` step that runs after merge. Create-time optional-step controls appear in the quick-add action row and the **New Task** dialog inline quick buttons for the active workflow. They resolve the workflow's optional-group nodes (plus plugin-contributed palette templates, see [Plugin-Contributed Steps](#plugin-contributed-steps)) into toggleable rows. Selecting **Fast** clears currently enabled optional steps and submits `enabledWorkflowSteps: []` even if optional-step metadata is still loading, but the dropdown stays available once loaded; any manual reselection before create is submitted as explicit ids and executes even on the Fast task. Workflows with no optional groups render no trigger and omit `enabledWorkflowSteps` unless the operator selects Fast, where the explicit empty array preserves the speed-first opt-out. Unknown or removed ids are skipped during resolution so stale selections never render blank controls or break workflow loading. @@ -735,7 +735,7 @@ Authoritative cutover now depends on existing/current parity summary evidence, n #### Self-healing recovery for parked review tasks -If a task is found in `in-review` with failed pre-merge workflow results and no active executor, self-healing can auto-revive it by replaying the same remediation send-back flow. Generic optional gates use the resolved workflow/project budget; built-in Plan Review and Code Review are unbounded unless the workflow settings or node config explicitly set a numeric cap. +If a task is found in `in-review` with failed pre-merge workflow results and no active executor, self-healing can auto-revive it by replaying the same remediation send-back flow. Generic optional gates use the resolved workflow/project budget; built-in Plan Review and most Code Review groups are unbounded unless workflow settings or node config set a numeric cap. Compound Engineering's Code Review node supplies a two-pass cap. -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. +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: 10 fix passes). Built-in Plan Review and most Code Review groups default to `"unbounded"` so they continue until approval unless `planReviewMaxRevisions`, `codeReviewMaxRevisions`, or the node's `config.maxRevisions` sets a numeric cap. Compound Engineering's Code Review node authors a two-pass 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. 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. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index cd1378e1bc..162cacf33f 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -19,6 +19,7 @@ import { builtinPromptConfig, BUILTIN_SEAM_PROMPTS } from "../builtin-workflow-p import { BUILTIN_WORKFLOW_SETTINGS } from "../builtin-workflow-settings.js"; import { resolveColumnFlags } from "../trait-registry.js"; import { DEFAULT_WORKFLOW_COLUMN_IDS, parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js"; +import { resolveOptionalReviewRevisionBudget } from "../workflow-settings-resolver.js"; import { pgDescribe, createSharedPgTaskStoreTestHarness } from "../__test-utils__/pg-test-harness.js"; import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js"; @@ -133,6 +134,28 @@ describe("built-in workflows", () => { } }); + it("bounds Compound Engineering Code Review remediation", () => { + const workflow = getBuiltinWorkflow("builtin:compound-engineering")!; + const codeReview = workflow.ir.nodes.find((node) => node.id === "code-review"); + + expect(codeReview?.config?.maxRevisions).toBe(2); + expect(resolveOptionalReviewRevisionBudget({ + optionalGroupId: "code-review", + workflowSettings: {}, + nodeMaxRevisions: codeReview?.config?.maxRevisions, + })).toBe(2); + expect(resolveOptionalReviewRevisionBudget({ + optionalGroupId: "code-review", + workflowSettings: { codeReviewMaxRevisions: 4 }, + nodeMaxRevisions: codeReview?.config?.maxRevisions, + })).toBe(4); + expect(resolveOptionalReviewRevisionBudget({ + optionalGroupId: "code-review", + workflowSettings: { codeReviewMaxRevisions: 0 }, + nodeMaxRevisions: codeReview?.config?.maxRevisions, + })).toBe(0); + }); + it("engineering built-in review failures loop through graph-owned remediation", () => { const expectedLoops = [ { gate: "plan-review", remediation: "plan-replan" }, @@ -159,7 +182,12 @@ describe("built-in workflows", () => { expect(workflow.ir.nodes.find((node) => node.id === gate)?.config, `${workflow.id}:${gate}:reworkRegion`).toMatchObject({ reworkRegion: true, maxReworkCycles: 3, - maxRevisions: gate === "browser-verification" ? 3 : "unbounded", + maxRevisions: + gate === "browser-verification" + ? 3 + : workflow.id === "builtin:compound-engineering" && gate === "code-review" + ? 2 + : "unbounded", }); } } @@ -645,7 +673,7 @@ describe("built-in workflows", () => { }); expect(ir.settings?.find((setting) => setting.id === "codeReviewMaxRevisions")).toMatchObject({ type: "number", - description: expect.stringMatching(/unbounded/i), + description: expect.stringMatching(/workflow's authored default/i), }); }); diff --git a/packages/core/src/builtin-workflow-settings.ts b/packages/core/src/builtin-workflow-settings.ts index 1161cfcc72..767f32ff7e 100644 --- a/packages/core/src/builtin-workflow-settings.ts +++ b/packages/core/src/builtin-workflow-settings.ts @@ -139,12 +139,13 @@ export const BUILTIN_MOVED_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [ type: "number", /* * 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. + * This global budget remains the fallback for custom optional gates that define neither a workflow setting nor node `maxRevisions`. Most built-in Code Review groups set `maxRevisions: "unbounded"`; Compound Engineering authors a two-pass cap so persistent CE reviewer findings park instead of rebounding forever. * * FNXC:WorkflowOptionalStepCycle 2026-07-26-19:35: * Raised 3 -> 10 (operator request). Three passes is below the observed convergence length for * Browser Verification and custom gates — the gates this fallback actually governs, since - * Plan Review and Code Review resolve to "unbounded" when unset. Exhausting the budget parks + * Plan Review and most Code Review groups resolve to "unbounded" when unset. Compound + * Engineering instead resolves its authored two-pass Code Review cap. Exhausting a budget parks * the card for a human, so a too-low cap converts "needs another pass" into operator toil. * This is a fallback, not a ceiling: an explicit workflow value or node `maxRevisions` still wins. */ @@ -499,10 +500,10 @@ export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [ 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. + * An unset workflow value defers to the authored Code Review node: Compound Engineering defaults to two remediation passes while other built-ins may remain unbounded. Operators can store a non-negative integer per workflow to override the authored cap, and `0` disables automatic Code Review remediation for that workflow. */ description: - "Maximum automatic Code Review remediation attempts for this workflow. Leave unset for unbounded; set 0 to disable automatic revision.", + "Maximum automatic Code Review remediation attempts for this workflow. Leave unset to use the workflow's authored default; set 0 to disable automatic revision.", }, { id: "planReviewReplanCap", diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index d52c5e0107..7c6585b0dc 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -79,9 +79,9 @@ function ceCodeReviewOptionalGroupNode(column: string): WorkflowIrNode { maxReworkCycles: 3, /* * 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. + * The CE Code Review group is custom because it invokes the CE skill, but Code Review REVISE is still ordinary repair feedback. Bound the built-in to two remediation attempts: a reviewer that keeps returning blocking findings must park the task instead of creating an unbounded Execute/Review loop. Workflow authors can still opt into a different numeric cap or unbounded behavior explicitly. */ - maxRevisions: "unbounded", + maxRevisions: 2, template: { nodes: [ { diff --git a/packages/core/src/workflow-ir-types.ts b/packages/core/src/workflow-ir-types.ts index 5a066af039..8795608a41 100644 --- a/packages/core/src/workflow-ir-types.ts +++ b/packages/core/src/workflow-ir-types.ts @@ -202,19 +202,20 @@ export interface WorkflowLoopConfig { /* 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. +An `optional-group` node is a container (mirroring `foreach`/`loop`) whose `template` subgraph the executor runs once per enabled graph attempt 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`). +The template itself contains no iteration or internal rework edges (validated in `validateOptionalGroup`). The outer graph may re-enter the group for pre-merge fix/re-review remediation governed by `maxRevisions`. FNXC:WorkflowOptionalStepRevisionBudget 2026-06-27-12:15: Optional-group remediation still runs the template once per graph pass, but workflow authors can set a per-step `maxRevisions` override for the PRE-merge fix→re-review cycle. A non-negative integer caps that optional step against its own review-attempt partition, `"unbounded"` removes the ceiling, and absence preserves the effective global `maxPostReviewFixes` behavior for generic optional gates. FNXC:WorkflowRevisionBudget 2026-06-30-20:34: -Built-in Plan Review/spec and Code Review groups have workflow-value overrides (`planReviewMaxRevisions`, `codeReviewMaxRevisions`) that resolve before this node config; when those workflow values are unset, those two built-in review paths default to unbounded remediation. +Built-in Plan Review/spec and Code Review groups have workflow-value overrides (`planReviewMaxRevisions`, `codeReviewMaxRevisions`) that resolve before this node config; when those workflow values are unset, the authored node config applies. Plan Review and most Code Review groups remain unbounded, while Compound Engineering authors a two-pass Code Review cap. */ /** 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. - * Unlike `foreach`/`loop`, there is no iteration or rework — a single pass. */ + * enable set at creation; the `template` is the subgraph run once per enabled + * graph attempt. Unlike `foreach`/`loop`, the template has no internal iteration; + * an outer remediation edge may re-enter it subject to `maxRevisions`. */ export interface WorkflowOptionalGroupConfig { /** Workflow-author default for whether new tasks enable this group. */ defaultOn?: boolean; diff --git a/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx index 5d9442dc86..270d27d51d 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowSettingsPanel.test.tsx @@ -214,7 +214,7 @@ describe("WorkflowSettingsPanel — Values tab", () => { expect(screen.getByLabelText("Plan Review revision cap")).toBeInTheDocument(); expect(screen.getByText(/Leave empty for unbounded automatic Plan Review\/spec revision/i)).toBeInTheDocument(); expect(screen.getByLabelText("Code Review revision cap")).toBeInTheDocument(); - expect(screen.getByText(/Leave empty for unbounded automatic Code Review remediation/i)).toBeInTheDocument(); + expect(screen.getByText(/Leave empty to use this workflow's authored Code Review default/i)).toBeInTheDocument(); }); // FNXC:PlannerOversight 2026-07-04-00:00: the Values tab is the project/global default @@ -537,7 +537,7 @@ describe("WorkflowSettingsPanel — Values tab", () => { await waitFor(() => expect(mockFetchValues).toHaveBeenCalledWith("wf-1", "proj-1")); expect(screen.getByTestId("wf-settings-tab-values")).toHaveAttribute("aria-selected", "true"); expect(screen.getByText(/Leave empty for unbounded automatic Plan Review\/spec revision/i)).toBeInTheDocument(); - expect(screen.getByText(/Leave empty for unbounded automatic Code Review remediation/i)).toBeInTheDocument(); + expect(screen.getByText(/Leave empty to use this workflow's authored Code Review default/i)).toBeInTheDocument(); fireEvent.change(screen.getByLabelText("Plan Review revision cap"), { target: { value: "2" } }); fireEvent.change(screen.getByLabelText("Code Review revision cap"), { target: { value: "0" } }); diff --git a/packages/dashboard/app/components/workflow-setting-display.ts b/packages/dashboard/app/components/workflow-setting-display.ts index 55a5c882cc..ae37892ecd 100644 --- a/packages/dashboard/app/components/workflow-setting-display.ts +++ b/packages/dashboard/app/components/workflow-setting-display.ts @@ -109,9 +109,9 @@ const DISPLAY: Record = { label: "Code Review revision cap", /* * FNXC:WorkflowRevisionBudget 2026-06-30-19:47: - * Built-in workflow values are editable even when definitions are read-only, so this label is the operator-facing cap for Code Review remediation without requiring a workflow duplicate. + * Built-in workflow values are editable even when definitions are read-only, so this label is the operator-facing override for Code Review remediation without requiring a workflow duplicate. Empty values preserve each workflow's authored node cap. */ - description: "Leave empty for unbounded automatic Code Review remediation; set 0 to disable automatic revision.", + description: "Leave empty to use this workflow's authored Code Review default; set 0 to disable automatic revision.", }, planReviewReplanCap: { group: "review",