FN-7293: honor workflow review revision overrides
Allow workflow review steps to opt into unbounded or capped revision retries while keeping recovery caps accurate. - Add workflow settings for Plan Review and Code Review max revisions with display, docs, and changeset coverage. - Resolve optional-step revision budgets through workflow settings, node overrides, and existing fallback limits. - Track revision attempts per workflow step and hydrate slim self-healing rows before enforcing retry caps. - Cover unbounded, capped, per-step, and slim-row recovery behavior with focused tests. Files changed: .changeset/fn-7293-unbounded-review-revisions.md | 7 + docs/settings-reference.md | 12 +- docs/workflow-editor.md | 4 +- docs/workflow-steps.md | 3 +- .../builtin-workflow-settings-triage.test.ts | 28 +++- .../core/src/__tests__/builtin-workflows.test.ts | 29 ++++- .../__tests__/workflow-settings-resolver.test.ts | 73 +++++++++++ packages/core/src/builtin-plan-review-group.ts | 6 +- packages/core/src/builtin-workflow-settings.ts | 30 +++++ packages/core/src/index.ts | 5 + packages/core/src/workflow-ir-types.ts | 15 ++- packages/core/src/workflow-ir.ts | 5 +- packages/core/src/workflow-settings-resolver.ts | 49 ++++++- .../__tests__/WorkflowSettingsPanel.test.tsx | 52 ++++++++ .../app/components/workflow-setting-display.ts | 18 +++ packages/engine/src/__tests__/self-healing.test.ts | 143 ++++++++++++++++++++- .../workflow-graph-optional-step-fix.test.ts | 130 ++++++++++++++++++- packages/engine/src/executor.ts | 77 +++++++++-- packages/engine/src/self-healing.ts | 82 ++++++++++-- packages/engine/src/workflow-graph-executor.ts | 5 +- 20 files changed, 725 insertions(+), 48 deletions(-) Fusion-Task-Id: FN-7293 Fusion-Task-Lineage: 19ee533f-ab8d-4e20-9a27-9cf0cf6dfec9 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7293-unbounded-review-revisions.md
Normal file
7
.changeset/fn-7293-unbounded-review-revisions.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Make built-in Plan Review and Code Review revisions unbounded unless workflows set a cap.
|
||||
category: fix
|
||||
dev: Adds workflow values planReviewMaxRevisions and codeReviewMaxRevisions for per-workflow caps, including read-only built-ins.
|
||||
@@ -269,7 +269,11 @@ Actions. It has two tabs:
|
||||
(Plan/Triage, Executor, Reviewer, and fallbacks declared by the workflow) use the
|
||||
same model dropdown picker as Project Models so clearing or selecting a model
|
||||
updates both keys together. Advanced/custom non-model settings still use typed
|
||||
controls. Edits batch and commit through a single **Save** in the Values tab.
|
||||
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. Edits batch and commit through a
|
||||
single **Save** in the Values tab.
|
||||
|
||||
**How values resolve.** The engine resolves *effective settings* per task as
|
||||
`stored value ?? declaration default`. The task-detail Workflow, Chat, and Agent
|
||||
@@ -307,7 +311,7 @@ These groups moved out of project settings and into workflow settings (built-in
|
||||
| Group | Keys (examples) |
|
||||
|---|---|
|
||||
| **Step execution** | `workflowStepTimeoutMs`, `runStepsInNewSessions`, `maxParallelSteps`, `workflowStepScopeEnforcement`, `strictScopeEnforcement`, `verificationFixRetries`, `maxPostReviewFixes`, `buildRetryCount` |
|
||||
| **Review / approval** | Workflow values: `requirePrApproval`, `requirePlanApproval`, `reviewHandoffPolicy`, `maxReviewerContextRetries`, `maxReviewerFallbackRetries`; project override: `planApprovalMode` |
|
||||
| **Review / approval** | Workflow values: `requirePrApproval`, `requirePlanApproval`, `reviewHandoffPolicy`, `maxReviewerContextRetries`, `maxReviewerFallbackRetries`, `planReviewMaxRevisions`, `codeReviewMaxRevisions`; project override: `planApprovalMode` |
|
||||
| **Per-phase model lanes** | `executionProvider`/`executionModelId`, `planningProvider`/`planningModelId` (+ fallbacks), `validatorProvider`/`validatorModelId` (+ fallbacks) |
|
||||
|
||||
### Workflow-native triage policy settings
|
||||
@@ -335,6 +339,8 @@ The built-in workflows also declare triage/spec policy settings that were **not*
|
||||
| `triageDefaultWorkflowId` | `builtin:coding` | Default workflow for standard coding tasks and for existing tasks without an explicit user-requested or creator-owned workflow selection. |
|
||||
| `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. |
|
||||
|
||||
In the dashboard Settings modal, Project Models exposes Plan/Triage, Executor,
|
||||
Reviewer, and declared fallback dropdown controls for the default workflow. The
|
||||
@@ -562,7 +568,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 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"`). |
|
||||
| `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. |
|
||||
| `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). |
|
||||
|
||||
@@ -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. 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 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.
|
||||
|
||||
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. 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 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.
|
||||
|
||||
<!--
|
||||
FNXC:WorkflowEditorDocs 2026-06-30-09:05:
|
||||
|
||||
@@ -372,7 +372,7 @@ Optional quality gates are authored directly in the workflow graph as `optional-
|
||||
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, the step keeps the global `maxPostReviewFixes` behavior.
|
||||
- `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.
|
||||
- `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.
|
||||
|
||||
@@ -381,6 +381,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. 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.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
BUILTIN_MOVED_WORKFLOW_SETTINGS,
|
||||
BUILTIN_REVIEW_REVISION_SETTINGS,
|
||||
BUILTIN_TRIAGE_POLICY_SETTINGS,
|
||||
BUILTIN_WORKFLOW_SETTINGS,
|
||||
renderTriagePolicyPlaceholders,
|
||||
@@ -27,8 +28,8 @@ const expectedDefaults: Record<string, { type: string; default: unknown }> = {
|
||||
autoApproveSpec: { type: "boolean", default: false },
|
||||
};
|
||||
|
||||
describe("workflow-native triage policy settings", () => {
|
||||
it("declares behavior-equivalent typed defaults outside the moved-key catalog", () => {
|
||||
describe("workflow-native built-in workflow settings", () => {
|
||||
it("declares behavior-equivalent triage defaults outside the moved-key catalog", () => {
|
||||
const triageById = new Map(BUILTIN_TRIAGE_POLICY_SETTINGS.map((setting) => [setting.id, setting]));
|
||||
const fullIds = new Set(BUILTIN_WORKFLOW_SETTINGS.map((setting) => setting.id));
|
||||
const movedIds = new Set(BUILTIN_MOVED_WORKFLOW_SETTINGS.map((setting) => setting.id));
|
||||
@@ -46,6 +47,29 @@ describe("workflow-native triage policy settings", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("declares review revision caps as unset workflow values outside moved/project settings", () => {
|
||||
const revisionById = new Map(BUILTIN_REVIEW_REVISION_SETTINGS.map((setting) => [setting.id, setting]));
|
||||
const fullIds = new Set(BUILTIN_WORKFLOW_SETTINGS.map((setting) => setting.id));
|
||||
const movedIds = new Set(BUILTIN_MOVED_WORKFLOW_SETTINGS.map((setting) => setting.id));
|
||||
const movedKeyIds = new Set(MOVED_SETTINGS_KEYS);
|
||||
|
||||
expect(BUILTIN_REVIEW_REVISION_SETTINGS.map((setting) => setting.id)).toEqual([
|
||||
"planReviewMaxRevisions",
|
||||
"codeReviewMaxRevisions",
|
||||
]);
|
||||
for (const id of ["planReviewMaxRevisions", "codeReviewMaxRevisions"]) {
|
||||
const setting = revisionById.get(id);
|
||||
expect(setting, `${id} should be declared`).toBeDefined();
|
||||
expect(setting?.type).toBe("number");
|
||||
expect(setting).not.toHaveProperty("default");
|
||||
expect(setting?.description).toMatch(/Leave unset for unbounded|Leave unset|unbounded/i);
|
||||
expect(setting?.description).toContain("0");
|
||||
expect(fullIds.has(id), `${id} should be in the full built-in catalog`).toBe(true);
|
||||
expect(movedIds.has(id), `${id} should not be in the moved-key catalog`).toBe(false);
|
||||
expect(movedKeyIds.has(id), `${id} should not be in MOVED_SETTINGS_KEYS`).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("renders placeholders from resolved settings and rejects dangling tokens", () => {
|
||||
const prompt = [
|
||||
"Size S (<{{triageSizeSmallMaxHours}}h)",
|
||||
|
||||
@@ -74,6 +74,24 @@ describe("built-in workflows", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("all built-ins expose workflow-native review revision cap settings", () => {
|
||||
for (const workflow of BUILTIN_WORKFLOWS) {
|
||||
if (workflow.kind === "fragment") continue;
|
||||
const ir = parseWorkflowIr(workflow.ir);
|
||||
expect(ir.version, workflow.id).toBe("v2");
|
||||
if (ir.version !== "v2") throw new Error(`expected ${workflow.id} v2`);
|
||||
expect(ir.settings?.map((setting) => setting.id), workflow.id).toEqual(
|
||||
expect.arrayContaining(["planReviewMaxRevisions", "codeReviewMaxRevisions"]),
|
||||
);
|
||||
expect(ir.settings?.find((setting) => setting.id === "planReviewMaxRevisions"), workflow.id).not.toHaveProperty(
|
||||
"default",
|
||||
);
|
||||
expect(ir.settings?.find((setting) => setting.id === "codeReviewMaxRevisions"), workflow.id).not.toHaveProperty(
|
||||
"default",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("engineering built-ins expose plan, code, and browser optional groups with expected defaults", () => {
|
||||
const expectedDefaults: Record<string, Record<string, boolean>> = {
|
||||
"builtin:coding": { "plan-review": true, "code-review": true, "browser-verification": false },
|
||||
@@ -152,7 +170,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: gate === "code-review" ? "unbounded" : 3,
|
||||
maxRevisions: gate === "browser-verification" ? 3 : "unbounded",
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -511,6 +529,7 @@ describe("built-in workflows", () => {
|
||||
expect(byId.get("plan")?.column).toBe("in-progress");
|
||||
expect(byId.get("plan-review")?.kind).toBe("optional-group");
|
||||
expect(byId.get("plan-review")?.column).toBe("in-progress");
|
||||
expect(byId.get("plan-review")?.config?.maxRevisions).toBe("unbounded");
|
||||
expect(planReviewInnerConfig(ir)).toMatchObject({
|
||||
toolMode: "readonly",
|
||||
gateMode: "gate",
|
||||
@@ -538,6 +557,14 @@ describe("built-in workflows", () => {
|
||||
expect(byId.get("merge-attempt")?.column).toBe("in-review");
|
||||
expect(byId.get("recovery-router")?.column).toBe("in-review");
|
||||
expect(ir.settings).toEqual(BUILTIN_WORKFLOW_SETTINGS);
|
||||
expect(ir.settings?.find((setting) => setting.id === "planReviewMaxRevisions")).toMatchObject({
|
||||
type: "number",
|
||||
description: expect.stringMatching(/unbounded/i),
|
||||
});
|
||||
expect(ir.settings?.find((setting) => setting.id === "codeReviewMaxRevisions")).toMatchObject({
|
||||
type: "number",
|
||||
description: expect.stringMatching(/unbounded/i),
|
||||
});
|
||||
});
|
||||
|
||||
it("includes the marketing built-in with custom columns, prompts, and lifecycle traits", () => {
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { WorkflowIr } from "../workflow-ir-types.js";
|
||||
import {
|
||||
resolveEffectiveSettings,
|
||||
resolveEffectiveSettingsById,
|
||||
resolveOptionalReviewRevisionBudget,
|
||||
type WorkflowSettingsResolverStore,
|
||||
} from "../workflow-settings-resolver.js";
|
||||
|
||||
@@ -57,6 +58,78 @@ function makeStore(opts: {
|
||||
};
|
||||
}
|
||||
|
||||
describe("resolveOptionalReviewRevisionBudget", () => {
|
||||
it("treats unset built-in Plan Review and Code Review settings as unbounded", () => {
|
||||
expect(resolveOptionalReviewRevisionBudget({ optionalGroupId: "plan-review", workflowSettings: {} })).toBe("unbounded");
|
||||
expect(resolveOptionalReviewRevisionBudget({ optionalGroupId: "code-review", workflowSettings: {} })).toBe("unbounded");
|
||||
});
|
||||
|
||||
it("uses explicit workflow values before node config, including zero", () => {
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "plan-review",
|
||||
workflowSettings: { planReviewMaxRevisions: 2 },
|
||||
nodeMaxRevisions: "unbounded",
|
||||
}),
|
||||
).toBe(2);
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "code-review",
|
||||
workflowSettings: { codeReviewMaxRevisions: 0 },
|
||||
nodeMaxRevisions: "unbounded",
|
||||
}),
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it("preserves authored node maxRevisions for custom or duplicated workflows", () => {
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "plan-review",
|
||||
workflowSettings: {},
|
||||
nodeMaxRevisions: 4,
|
||||
}),
|
||||
).toBe(4);
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "custom-review",
|
||||
workflowSettings: { planReviewMaxRevisions: 1 },
|
||||
nodeMaxRevisions: "unbounded",
|
||||
fallbackMaxRevisions: 3,
|
||||
}),
|
||||
).toBe("unbounded");
|
||||
});
|
||||
|
||||
it("ignores invalid workflow and node budgets safely", () => {
|
||||
for (const invalid of [-1, 1.5, Number.POSITIVE_INFINITY, Number.NaN, "3"]) {
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "plan-review",
|
||||
workflowSettings: { planReviewMaxRevisions: invalid },
|
||||
nodeMaxRevisions: 5,
|
||||
}),
|
||||
).toBe(5);
|
||||
}
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "custom-review",
|
||||
nodeMaxRevisions: -1 as never,
|
||||
fallbackMaxRevisions: 3,
|
||||
}),
|
||||
).toBe(3);
|
||||
});
|
||||
|
||||
it("leaves Browser Verification and other optional gates on the existing fallback unless configured", () => {
|
||||
expect(
|
||||
resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: "browser-verification",
|
||||
workflowSettings: {},
|
||||
fallbackMaxRevisions: 3,
|
||||
}),
|
||||
).toBe(3);
|
||||
expect(resolveOptionalReviewRevisionBudget({ optionalGroupId: "browser-verification", workflowSettings: {} })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveEffectiveSettings (per-task)", () => {
|
||||
it("parity anchor: builtin:coding with no stored values → effective equals declaration defaults", async () => {
|
||||
const store = makeStore({
|
||||
|
||||
@@ -72,10 +72,10 @@ export function planReviewOptionalGroupNode(
|
||||
reworkRegion: true,
|
||||
maxReworkCycles: 3,
|
||||
/*
|
||||
* FNXC:WorkflowRemediationBudget 2026-06-29-13:56:
|
||||
* Built-in Plan Review owns the pre-execution replan budget. Default to three graph-mediated revise→replan→review attempts while allowing workflow authors to override `maxRevisions`.
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-19:49:
|
||||
* Built-in Plan Review/spec remediation is unbounded by default; workflow setting value `planReviewMaxRevisions` is the operator cap for read-only built-ins, while authored `maxRevisions` still lets custom/duplicated workflows encode their own budget.
|
||||
*/
|
||||
maxRevisions: options.maxRevisions ?? 3,
|
||||
maxRevisions: options.maxRevisions ?? "unbounded",
|
||||
template: {
|
||||
nodes: [
|
||||
{
|
||||
|
||||
@@ -16,6 +16,10 @@ import type { WorkflowSettingDefinition } from "./workflow-ir-types.js";
|
||||
* older engine copy) and packages/modules threshold `3`. Fast-mode policy is
|
||||
* workflow-native here too: `leanPlanning` selects the lean planning variant,
|
||||
* and `autoApproveSpec` skips the independent spec reviewer.
|
||||
*
|
||||
* `BUILTIN_REVIEW_REVISION_SETTINGS` is workflow-native review-loop policy.
|
||||
* These keys also never lived in project/global settings and intentionally omit
|
||||
* declaration defaults: an unset workflow value means unbounded remediation.
|
||||
*/
|
||||
|
||||
/**
|
||||
@@ -371,9 +375,35 @@ export const BUILTIN_TRIAGE_POLICY_SETTINGS: WorkflowSettingDefinition[] = [
|
||||
},
|
||||
];
|
||||
|
||||
export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [
|
||||
{
|
||||
id: "planReviewMaxRevisions",
|
||||
name: "Plan Review revision cap",
|
||||
type: "number",
|
||||
/*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-19:45:
|
||||
* Built-in Plan Review/spec remediation is unbounded when this workflow value is unset. Operators can store a non-negative integer per workflow to cap automatic replans, and `0` disables automatic Plan Review revision entirely without duplicating a read-only built-in workflow.
|
||||
*/
|
||||
description:
|
||||
"Maximum automatic Plan Review/spec revision attempts for this workflow. Leave unset for unbounded; set 0 to disable automatic revision.",
|
||||
},
|
||||
{
|
||||
id: "codeReviewMaxRevisions",
|
||||
name: "Code Review revision cap",
|
||||
type: "number",
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
description:
|
||||
"Maximum automatic Code Review remediation attempts for this workflow. Leave unset for unbounded; set 0 to disable automatic revision.",
|
||||
},
|
||||
];
|
||||
|
||||
export const BUILTIN_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [
|
||||
...BUILTIN_MOVED_WORKFLOW_SETTINGS,
|
||||
...BUILTIN_TRIAGE_POLICY_SETTINGS,
|
||||
...BUILTIN_REVIEW_REVISION_SETTINGS,
|
||||
];
|
||||
|
||||
const TRIAGE_POLICY_DEFAULTS = new Map(
|
||||
|
||||
@@ -407,9 +407,14 @@ export {
|
||||
resolveEffectiveSettings,
|
||||
resolveEffectiveSettingsDetailed,
|
||||
resolveEffectiveSettingsById,
|
||||
resolveOptionalReviewRevisionBudget,
|
||||
PLAN_REVIEW_MAX_REVISIONS_SETTING_ID,
|
||||
CODE_REVIEW_MAX_REVISIONS_SETTING_ID,
|
||||
type WorkflowSettingsResolverStore,
|
||||
type EffectiveSettingsResult,
|
||||
type EffectiveSettingsTaskRef,
|
||||
type OptionalReviewRevisionBudget,
|
||||
type ResolveOptionalReviewRevisionBudgetInput,
|
||||
} from "./workflow-settings-resolver.js";
|
||||
export {
|
||||
applyWorkflowSettingsOverlay,
|
||||
|
||||
@@ -199,7 +199,10 @@ Enable state reuses the per-task `enabledWorkflowSteps` facet keyed by the group
|
||||
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.
|
||||
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.
|
||||
*/
|
||||
/** 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.
|
||||
@@ -210,10 +213,12 @@ export interface WorkflowOptionalGroupConfig {
|
||||
/** 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.
|
||||
* Per-step override for the 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. Plan Review and Code Review
|
||||
* workflow-setting values override this field; absence falls back to the
|
||||
* gate-specific runtime default (unbounded for those built-in reviews, global
|
||||
* fallback for generic optional gates).
|
||||
*/
|
||||
maxRevisions?: number | "unbounded";
|
||||
/*
|
||||
|
||||
@@ -670,7 +670,10 @@ function validateOptionalGroup(
|
||||
}
|
||||
/*
|
||||
* 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.
|
||||
* Parse-time validation accepts only an explicit non-negative integer budget or `"unbounded"`; absent remains byte-inert and resolves through the gate-specific runtime fallback at execution time.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-20:36:
|
||||
* The Plan Review/Code Review workflow setting values are validated separately from IR authoring. Invalid runtime values are ignored by the shared budget resolver, while invalid authored node budgets remain a parse error so custom workflow definitions cannot persist ambiguous caps.
|
||||
*/
|
||||
if (cfg.maxRevisions !== undefined) {
|
||||
const maxRevisions = cfg.maxRevisions;
|
||||
|
||||
@@ -34,7 +34,54 @@ import {
|
||||
} from "./workflow-ir-resolver.js";
|
||||
import { resolveEffectiveSettingValues, findOrphanedSettingValues } from "./workflow-settings.js";
|
||||
import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js";
|
||||
import type { WorkflowSettingDefinition, WorkflowIr } from "./workflow-ir-types.js";
|
||||
import type { WorkflowSettingDefinition, WorkflowIr, WorkflowOptionalGroupConfig } from "./workflow-ir-types.js";
|
||||
|
||||
export const PLAN_REVIEW_MAX_REVISIONS_SETTING_ID = "planReviewMaxRevisions";
|
||||
export const CODE_REVIEW_MAX_REVISIONS_SETTING_ID = "codeReviewMaxRevisions";
|
||||
export type OptionalReviewRevisionBudget = NonNullable<WorkflowOptionalGroupConfig["maxRevisions"]>;
|
||||
|
||||
const REVIEW_REVISION_SETTING_BY_GROUP_ID: Record<string, string | undefined> = {
|
||||
"plan-review": PLAN_REVIEW_MAX_REVISIONS_SETTING_ID,
|
||||
"code-review": CODE_REVIEW_MAX_REVISIONS_SETTING_ID,
|
||||
};
|
||||
|
||||
function asRevisionBudget(value: unknown): OptionalReviewRevisionBudget | undefined {
|
||||
if (value === "unbounded") return value;
|
||||
if (typeof value !== "number" || !Number.isFinite(value) || !Number.isInteger(value) || value < 0) return undefined;
|
||||
return value;
|
||||
}
|
||||
|
||||
export interface ResolveOptionalReviewRevisionBudgetInput {
|
||||
optionalGroupId: string;
|
||||
workflowSettings?: Record<string, unknown>;
|
||||
nodeMaxRevisions?: unknown;
|
||||
fallbackMaxRevisions?: OptionalReviewRevisionBudget;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the automatic remediation budget for graph-native optional review gates.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-20:31:
|
||||
* Built-in Plan Review/spec and Code Review remediation are unbounded when their workflow value is unset. A stored non-negative integer workflow value wins first (including `0` to disable automatic remediation), then an authored node `maxRevisions` keeps custom workflow semantics, and only matching built-in review groups fall back to unbounded; Browser Verification and custom optional gates keep their caller fallback.
|
||||
*/
|
||||
export function resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId,
|
||||
workflowSettings,
|
||||
nodeMaxRevisions,
|
||||
fallbackMaxRevisions,
|
||||
}: ResolveOptionalReviewRevisionBudgetInput): OptionalReviewRevisionBudget | undefined {
|
||||
const settingId = REVIEW_REVISION_SETTING_BY_GROUP_ID[optionalGroupId];
|
||||
if (settingId) {
|
||||
const workflowBudget = asRevisionBudget(workflowSettings?.[settingId]);
|
||||
if (workflowBudget !== undefined) return workflowBudget;
|
||||
}
|
||||
|
||||
const nodeBudget = asRevisionBudget(nodeMaxRevisions);
|
||||
if (nodeBudget !== undefined) return nodeBudget;
|
||||
|
||||
if (settingId) return "unbounded";
|
||||
return fallbackMaxRevisions;
|
||||
}
|
||||
|
||||
/**
|
||||
* The effective map PLUS the subset of keys whose value came from an EXPLICIT
|
||||
|
||||
@@ -193,6 +193,8 @@ describe("WorkflowSettingsPanel — Values tab", () => {
|
||||
{ id: "planningModelId", name: "Planning model", type: "string" },
|
||||
{ id: "validatorProvider", name: "Validator provider", type: "string" },
|
||||
{ id: "requirePlanApproval", name: "Require plan approval", type: "boolean" },
|
||||
{ id: "planReviewMaxRevisions", name: "Plan Review revision cap", type: "number" },
|
||||
{ id: "codeReviewMaxRevisions", name: "Code Review revision cap", type: "number" },
|
||||
{ id: "workflowStepTimeoutMs", name: "Step timeout", type: "number" },
|
||||
{ id: "customThing", name: "Custom thing", type: "string" },
|
||||
]}
|
||||
@@ -206,6 +208,10 @@ describe("WorkflowSettingsPanel — Values tab", () => {
|
||||
expect(within(screen.getByTestId("wf-settings-group-advanced")).getByText("Advanced")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("Plan/Triage Model")).toBeInTheDocument();
|
||||
expect(screen.getByLabelText("Reviewer provider")).toBeInTheDocument();
|
||||
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();
|
||||
});
|
||||
|
||||
it("batches three field edits into exactly ONE patch on Save values", async () => {
|
||||
@@ -350,6 +356,52 @@ describe("WorkflowSettingsPanel — Values tab", () => {
|
||||
await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledWith("wf-1", { "old-key": null }, "proj-1"));
|
||||
});
|
||||
|
||||
it("edits, saves, clears, and refetches built-in review revision caps without duplicating definitions", async () => {
|
||||
const builtinReviewCaps: WorkflowSettingDefinition[] = [
|
||||
{ id: "planReviewMaxRevisions", name: "Plan Review revision cap", type: "number" },
|
||||
{ id: "codeReviewMaxRevisions", name: "Code Review revision cap", type: "number" },
|
||||
];
|
||||
mockFetchValues
|
||||
.mockResolvedValueOnce(payload({ effective: {} }))
|
||||
.mockResolvedValueOnce(payload({ stored: { planReviewMaxRevisions: 2, codeReviewMaxRevisions: 0 }, effective: { planReviewMaxRevisions: 2, codeReviewMaxRevisions: 0 } }));
|
||||
mockUpdateValues
|
||||
.mockResolvedValueOnce(payload({ stored: { planReviewMaxRevisions: 2, codeReviewMaxRevisions: 0 }, effective: { planReviewMaxRevisions: 2, codeReviewMaxRevisions: 0 } }))
|
||||
.mockResolvedValueOnce(payload({ stored: { codeReviewMaxRevisions: 0 }, effective: { codeReviewMaxRevisions: 0 } }));
|
||||
|
||||
const firstRender = render(<Host initial={builtinReviewCaps} readOnly />);
|
||||
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();
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Plan Review revision cap"), { target: { value: "2" } });
|
||||
fireEvent.change(screen.getByLabelText("Code Review revision cap"), { target: { value: "0" } });
|
||||
fireEvent.click(screen.getByTestId("wf-settings-save-values"));
|
||||
|
||||
await waitFor(() => expect(mockUpdateValues).toHaveBeenCalledWith(
|
||||
"wf-1",
|
||||
{ planReviewMaxRevisions: 2, codeReviewMaxRevisions: 0 },
|
||||
"proj-1",
|
||||
));
|
||||
expect(screen.getByTestId("wf-settings-customized-planReviewMaxRevisions")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("wf-settings-customized-codeReviewMaxRevisions")).toBeInTheDocument();
|
||||
|
||||
firstRender.unmount();
|
||||
render(<Host initial={builtinReviewCaps} readOnly />);
|
||||
await waitFor(() => expect(mockFetchValues).toHaveBeenCalledTimes(2));
|
||||
expect(screen.getByLabelText("Plan Review revision cap")).toHaveValue(2);
|
||||
expect(screen.getByLabelText("Code Review revision cap")).toHaveValue(0);
|
||||
|
||||
const planRow = screen.getByTestId("wf-settings-value-planReviewMaxRevisions");
|
||||
fireEvent.click(within(planRow).getByRole("button"));
|
||||
fireEvent.click(screen.getByTestId("wf-settings-save-values"));
|
||||
await waitFor(() => expect(mockUpdateValues).toHaveBeenLastCalledWith(
|
||||
"wf-1",
|
||||
{ planReviewMaxRevisions: null },
|
||||
"proj-1",
|
||||
));
|
||||
});
|
||||
|
||||
it("clear-to-default emits a null patch for a customized value", async () => {
|
||||
mockFetchValues.mockResolvedValue(payload({ stored: { "timeout-ms": 5000 }, effective: { "timeout-ms": 5000 } }));
|
||||
render(<Host initial={decls} />);
|
||||
|
||||
@@ -83,6 +83,24 @@ const DISPLAY: Record<string, WorkflowSettingDisplay> = {
|
||||
group: "review",
|
||||
label: "Reflection enabled",
|
||||
},
|
||||
planReviewMaxRevisions: {
|
||||
group: "review",
|
||||
label: "Plan Review revision cap",
|
||||
/*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-19:47:
|
||||
* The Values tab must tell operators that an empty built-in workflow value is intentionally unbounded, not the old three-pass Plan Review default. Numeric values cap retries and `0` turns off automatic revision.
|
||||
*/
|
||||
description: "Leave empty for unbounded automatic Plan Review/spec revision; set 0 to disable automatic revision.",
|
||||
},
|
||||
codeReviewMaxRevisions: {
|
||||
group: "review",
|
||||
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.
|
||||
*/
|
||||
description: "Leave empty for unbounded automatic Code Review remediation; set 0 to disable automatic revision.",
|
||||
},
|
||||
workflowStepTimeoutMs: {
|
||||
group: "steps",
|
||||
label: "Step timeout",
|
||||
|
||||
@@ -5317,6 +5317,12 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
|
||||
describe("recoverReviewTasksWithFailedPreMergeSteps", () => {
|
||||
const revisionLog = (stepName: string, key: string, attempt: number) => ({
|
||||
timestamp: new Date().toISOString(),
|
||||
action: `Auto-reviving in-review task with failed pre-merge workflow step (attempt ${attempt}/2)`,
|
||||
outcome: `Step: ${stepName}\nWorkflow revision key: ${key}`,
|
||||
});
|
||||
|
||||
const baseTask = {
|
||||
id: "FN-1572",
|
||||
column: "in-review" as const,
|
||||
@@ -5361,6 +5367,7 @@ describe("SelfHealingManager", () => {
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-1572",
|
||||
expect.stringContaining("Auto-reviving in-review task"),
|
||||
expect.stringContaining("Workflow revision key: ws-004"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
@@ -5393,7 +5400,11 @@ describe("SelfHealingManager", () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ maxPostReviewFixes: 9 });
|
||||
(store as unknown as { getTaskWorkflowSelection: ReturnType<typeof vi.fn> }).getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "WF-budget", stepIds: ["WS-004"] }));
|
||||
(store as unknown as { getWorkflowDefinition: ReturnType<typeof vi.fn> }).getWorkflowDefinition = vi.fn().mockResolvedValue({ ir: workflowIr });
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, postReviewFixCount: 2 }]);
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{
|
||||
...baseTask,
|
||||
postReviewFixCount: 2,
|
||||
log: [revisionLog("Browser Verification", "WS-004", 1), revisionLog("Browser Verification", "WS-004", 2)],
|
||||
}]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
@@ -5402,10 +5413,101 @@ describe("SelfHealingManager", () => {
|
||||
...workflowIr.nodes[1],
|
||||
config: { ...(workflowIr.nodes[1] as { config: Record<string, unknown> }).config, maxRevisions: "unbounded" },
|
||||
};
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, postReviewFixCount: 99 }]);
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{
|
||||
...baseTask,
|
||||
postReviewFixCount: 99,
|
||||
log: Array.from({ length: 99 }, (_, index) => revisionLog("Browser Verification", "WS-004", index + 1)),
|
||||
}]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1);
|
||||
expect(store.logEntry).toHaveBeenLastCalledWith("FN-1572", expect.stringContaining("attempt 100/unbounded"));
|
||||
expect(store.logEntry).toHaveBeenLastCalledWith("FN-1572", expect.stringContaining("attempt 100/unbounded"), expect.stringContaining("Workflow revision key: ws-004"));
|
||||
expect(recoverFn).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-1572" }));
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("honors workflow-setting caps for stale Code Review recovery", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 9,
|
||||
codeReviewMaxRevisions: 0,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
...baseTask,
|
||||
workflowStepResults: [
|
||||
{
|
||||
...baseTask.workflowStepResults[0],
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
codeReviewMaxRevisions: 2,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
...baseTask,
|
||||
postReviewFixCount: 1,
|
||||
log: [revisionLog("Code Review", "code-review", 1)],
|
||||
workflowStepResults: [
|
||||
{
|
||||
...baseTask.workflowStepResults[0],
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1);
|
||||
expect(store.logEntry).toHaveBeenLastCalledWith("FN-1572", expect.stringContaining("attempt 2/2"), expect.stringContaining("Workflow revision key: code-review"));
|
||||
expect(recoverFn).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-1572" }));
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("keeps Plan Review and Code Review workflow caps independent during recovery", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 9,
|
||||
planReviewMaxRevisions: 1,
|
||||
codeReviewMaxRevisions: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
...baseTask,
|
||||
postReviewFixCount: 1,
|
||||
log: [revisionLog("Plan Review", "plan-review", 1)],
|
||||
workflowStepResults: [
|
||||
{
|
||||
...baseTask.workflowStepResults[0],
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1572", { postReviewFixCount: 2 });
|
||||
expect(store.logEntry).toHaveBeenLastCalledWith("FN-1572", expect.stringContaining("attempt 1/1"), expect.stringContaining("Workflow revision key: code-review"));
|
||||
expect(recoverFn).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-1572" }));
|
||||
|
||||
managerWithRecovery.stop();
|
||||
@@ -5423,13 +5525,13 @@ describe("SelfHealingManager", () => {
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([{ ...baseTask, postReviewFixCount: 0 }]);
|
||||
|
||||
await expect(managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps()).resolves.toBe(1);
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-1572", expect.stringContaining("attempt 1/1"));
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-1572", expect.stringContaining("attempt 1/1"), expect.stringContaining("Workflow revision key: ws-004"));
|
||||
expect(recoverFn).toHaveBeenCalledOnce();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips tasks whose postReviewFixCount has reached maxPostReviewFixes", async () => {
|
||||
it("skips tasks whose per-step attempts have reached maxPostReviewFixes", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
@@ -5439,7 +5541,7 @@ describe("SelfHealingManager", () => {
|
||||
maxPostReviewFixes: 2,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{ ...baseTask, postReviewFixCount: 2 },
|
||||
{ ...baseTask, postReviewFixCount: 2, log: [revisionLog("Browser Verification", "WS-004", 1), revisionLog("Browser Verification", "WS-004", 2)] },
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
@@ -5451,6 +5553,35 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("hydrates slim in-review rows before enforcing per-step revision caps", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 2,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{ ...baseTask, postReviewFixCount: 2, log: [] },
|
||||
]);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...baseTask,
|
||||
postReviewFixCount: 2,
|
||||
log: [revisionLog("Browser Verification", "WS-004", 1), revisionLog("Browser Verification", "WS-004", 2)],
|
||||
});
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.listTasks).toHaveBeenCalledWith({ column: "in-review", slim: true });
|
||||
expect(store.getTask).toHaveBeenCalledWith("FN-1572");
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("no-ops when recoverFailedPreMergeStep callback is not supplied", async () => {
|
||||
const managerWithoutCallback = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
|
||||
@@ -33,6 +33,14 @@ const reviseInfo = {
|
||||
verdict: "REVISE",
|
||||
};
|
||||
|
||||
function revisionLog(stepName: string, key: string, attempt: number) {
|
||||
return {
|
||||
timestamp: new Date().toISOString(),
|
||||
action: `Pre-merge optional workflow step requested executor fixes (attempt ${attempt}/2)`,
|
||||
outcome: `Step: ${stepName}\nWorkflow revision key: ${key}`,
|
||||
};
|
||||
}
|
||||
|
||||
describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
@@ -152,7 +160,14 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
expect.stringContaining("PROMPT.md is missing the new workflow-order requirement"),
|
||||
undefined,
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-7066",
|
||||
"Plan Review failed — moved to triage for automatic replan (attempt 1/unbounded)",
|
||||
expect.stringContaining("PROMPT.md is missing the new workflow-order requirement"),
|
||||
undefined,
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-7066", "triage");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7066", {
|
||||
status: "needs-replan",
|
||||
error: null,
|
||||
@@ -160,10 +175,50 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
nextRecoveryAt: null,
|
||||
graphResumeRetryCount: 0,
|
||||
}, undefined);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
||||
expect((executor as any).pausedAborted.has("FN-7066")).toBe(false);
|
||||
});
|
||||
|
||||
it("honors Plan Review workflow-setting caps before automatic replan", async () => {
|
||||
const zeroStore = createMockStore();
|
||||
const zeroTask = task({ postReviewFixCount: 0, column: "in-progress" });
|
||||
zeroStore.getTask.mockResolvedValue(zeroTask);
|
||||
zeroStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, planReviewMaxRevisions: 0 });
|
||||
const zeroExecutor = new TaskExecutor(zeroStore, "/tmp/test");
|
||||
|
||||
await expect((zeroExecutor as any).requestPreMergeOptionalStepFix(zeroTask.id, zeroTask, {
|
||||
stepName: "Plan Review",
|
||||
feedback: "needs spec edits",
|
||||
phase: "pre-merge" as const,
|
||||
status: "failed" as const,
|
||||
verdict: "REVISE",
|
||||
nodeId: "plan-review",
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(false);
|
||||
expect(zeroStore.moveTask).not.toHaveBeenCalled();
|
||||
expect(zeroStore.updateTask).not.toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
||||
|
||||
const cappedStore = createMockStore();
|
||||
const exhaustedTask = task({
|
||||
postReviewFixCount: 2,
|
||||
column: "in-progress",
|
||||
log: [revisionLog("Plan Review", "plan-review", 1), revisionLog("Plan Review", "plan-review", 2)],
|
||||
});
|
||||
cappedStore.getTask.mockResolvedValue(exhaustedTask);
|
||||
cappedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, planReviewMaxRevisions: 2 });
|
||||
const cappedExecutor = new TaskExecutor(cappedStore, "/tmp/test");
|
||||
|
||||
await expect((cappedExecutor as any).requestPreMergeOptionalStepFix(exhaustedTask.id, exhaustedTask, {
|
||||
stepName: "Plan Review",
|
||||
feedback: "needs spec edits",
|
||||
phase: "pre-merge" as const,
|
||||
status: "failed" as const,
|
||||
verdict: "REVISE",
|
||||
nodeId: "plan-review",
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(false);
|
||||
expect(cappedStore.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears stale pause-abort provenance silently before a fresh unpaused execution dispatch", async () => {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({ column: "todo", paused: false, userPaused: false });
|
||||
@@ -216,7 +271,10 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
|
||||
for (const count of [0, 1, 2, 3]) {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({ postReviewFixCount: count });
|
||||
const liveTask = task({
|
||||
postReviewFixCount: count,
|
||||
log: Array.from({ length: count }, (_, index) => revisionLog("Code Review", "code review", index + 1)),
|
||||
});
|
||||
store.getTask.mockResolvedValue(liveTask);
|
||||
store.getSettings.mockResolvedValue({});
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
@@ -249,7 +307,10 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
it("lets per-step maxRevisions override the global budget", async () => {
|
||||
for (const count of [1, 2]) {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({ postReviewFixCount: count });
|
||||
const liveTask = task({
|
||||
postReviewFixCount: count,
|
||||
log: Array.from({ length: count }, (_, index) => revisionLog("Code Review", "code review", index + 1)),
|
||||
});
|
||||
store.getTask.mockResolvedValue(liveTask);
|
||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 9 });
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
@@ -292,9 +353,65 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
expect(guard).toContain("split them into a separate task");
|
||||
});
|
||||
|
||||
it("honors workflow-setting revision caps before node and global budgets for Code Review", async () => {
|
||||
const cappedStore = createMockStore();
|
||||
const cappedTask = task({ postReviewFixCount: 1, log: [revisionLog("Code Review", "code-review", 1)] });
|
||||
cappedStore.getTask.mockResolvedValue(cappedTask);
|
||||
cappedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, codeReviewMaxRevisions: 2 });
|
||||
const cappedExecutor = new TaskExecutor(cappedStore, "/tmp/test");
|
||||
const cappedSendBack = vi.spyOn(cappedExecutor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
|
||||
|
||||
await expect((cappedExecutor as any).requestPreMergeOptionalStepFix(cappedTask.id, cappedTask, {
|
||||
...reviseInfo,
|
||||
nodeId: "code-review",
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(true);
|
||||
expect(cappedStore.logEntry).toHaveBeenCalledWith("FN-7066", expect.stringContaining("attempt 2/2"), expect.any(String), undefined);
|
||||
expect(cappedSendBack).toHaveBeenCalledOnce();
|
||||
|
||||
const zeroStore = createMockStore();
|
||||
const zeroTask = task({ postReviewFixCount: 0 });
|
||||
zeroStore.getTask.mockResolvedValue(zeroTask);
|
||||
zeroStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, codeReviewMaxRevisions: 0 });
|
||||
const zeroExecutor = new TaskExecutor(zeroStore, "/tmp/test");
|
||||
const zeroSendBack = vi.spyOn(zeroExecutor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
|
||||
|
||||
await expect((zeroExecutor as any).requestPreMergeOptionalStepFix(zeroTask.id, zeroTask, {
|
||||
...reviseInfo,
|
||||
nodeId: "code-review",
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(false);
|
||||
expect(zeroSendBack).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps Plan Review and Code Review workflow caps independent", async () => {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({
|
||||
postReviewFixCount: 1,
|
||||
log: [revisionLog("Plan Review", "plan-review", 1)],
|
||||
});
|
||||
store.getTask.mockResolvedValue(liveTask);
|
||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, planReviewMaxRevisions: 1, codeReviewMaxRevisions: 1 });
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
|
||||
|
||||
await expect((executor as any).requestPreMergeOptionalStepFix(liveTask.id, liveTask, {
|
||||
...reviseInfo,
|
||||
nodeId: "code-review",
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(true);
|
||||
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-7066", expect.stringContaining("attempt 1/1"), expect.stringContaining("Workflow revision key: code-review"), undefined);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 2 }, undefined);
|
||||
expect(sendBack).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("honors unbounded and zero per-step maxRevisions states", async () => {
|
||||
const unboundedStore = createMockStore();
|
||||
const exhaustedTask = task({ postReviewFixCount: 99 });
|
||||
const exhaustedTask = task({
|
||||
postReviewFixCount: 99,
|
||||
log: Array.from({ length: 99 }, (_, index) => revisionLog("Code Review", "code review", index + 1)),
|
||||
});
|
||||
unboundedStore.getTask.mockResolvedValue(exhaustedTask);
|
||||
unboundedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 1 });
|
||||
const unboundedExecutor = new TaskExecutor(unboundedStore, "/tmp/test");
|
||||
@@ -327,7 +444,10 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
{ settingsMax: 1, count: 1 },
|
||||
]) {
|
||||
const store = createMockStore();
|
||||
const liveTask = task({ postReviewFixCount: count });
|
||||
const liveTask = task({
|
||||
postReviewFixCount: count,
|
||||
log: Array.from({ length: count }, (_, index) => revisionLog("Code Review", "code review", index + 1)),
|
||||
});
|
||||
store.getTask.mockResolvedValue(liveTask);
|
||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: settingsMax });
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
|
||||
@@ -11,7 +11,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, resolveOptionalStepRevisionBudget } from "@fusion/core";
|
||||
import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget } from "@fusion/core";
|
||||
import { finalizeProvenAutoMergeTask } from "./auto-merge-finalization.js";
|
||||
import { mergeEffectiveSettings } from "./effective-settings.js";
|
||||
import type { TaskStep, WorkflowIr, WorkflowFieldDefinition, WorkflowColumnAgent, EffectiveAgentInput, WorkflowWorkEngineDispatchResult } from "@fusion/core";
|
||||
@@ -428,6 +428,37 @@ function getResumeOrphanDelayMs(): number {
|
||||
|
||||
const tokenCacheMetricsLog = createLogger("token-cache-metrics");
|
||||
|
||||
const OPTIONAL_STEP_REVISION_KEY_MARKER = "Workflow revision key:";
|
||||
|
||||
function normalizeOptionalStepRevisionKey(value: string | undefined): string {
|
||||
return (value ?? "").trim().toLowerCase();
|
||||
}
|
||||
|
||||
function optionalStepRevisionKey(nodeId: string | undefined, stepName: string | undefined): string {
|
||||
return normalizeOptionalStepRevisionKey(nodeId) || normalizeOptionalStepRevisionKey(stepName) || "pre-merge-optional-step";
|
||||
}
|
||||
|
||||
function countOptionalStepRevisionAttempts(task: Pick<Task, "log">, key: string, stepName: string | undefined): number {
|
||||
const normalizedKey = normalizeOptionalStepRevisionKey(key);
|
||||
const normalizedStepName = normalizeOptionalStepRevisionKey(stepName);
|
||||
return (task.log ?? []).filter((entry) => {
|
||||
const action = entry.action ?? "";
|
||||
const outcome = entry.outcome ?? "";
|
||||
if (!/attempt \d+\//.test(action)) return false;
|
||||
const markerIndex = outcome.indexOf(OPTIONAL_STEP_REVISION_KEY_MARKER);
|
||||
if (markerIndex >= 0) {
|
||||
const markerValue = outcome.slice(markerIndex + OPTIONAL_STEP_REVISION_KEY_MARKER.length).split(/\r?\n/, 1)[0]?.trim();
|
||||
return normalizeOptionalStepRevisionKey(markerValue) === normalizedKey;
|
||||
}
|
||||
if (!normalizedStepName) return false;
|
||||
return normalizeOptionalStepRevisionKey(outcome).includes(`step: ${normalizedStepName}`);
|
||||
}).length;
|
||||
}
|
||||
|
||||
function optionalStepRevisionLogOutcome(details: string, key: string): string {
|
||||
return `${details}\n${OPTIONAL_STEP_REVISION_KEY_MARKER} ${key}`;
|
||||
}
|
||||
|
||||
const STEP_STATUSES: StepStatus[] = ["pending", "in-progress", "done", "skipped"];
|
||||
|
||||
function canonicalizePath(path: string): string {
|
||||
@@ -3968,7 +3999,13 @@ 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. 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.
|
||||
* Inline graph optional-step remediation consumes `postReviewFixCount` BEFORE calling `sendTaskBackForFix`, matching self-healing's budget-first ordering. Persistent optional-step REVISE loops are bounded by the resolved optional-group budget; `"unbounded"` intentionally skips the ceiling check so the step cycles until it returns APPROVE/APPROVE_WITH_NOTES or a human intervenes.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-20:48:
|
||||
* Live Plan Review/spec and Code Review remediation must honor explicit workflow setting values before node `maxRevisions`, and must treat unset values as unbounded for those two built-in review paths. Browser Verification keeps the existing `maxPostReviewFixes` fallback unless its node config explicitly changes it.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-22:04:
|
||||
* Plan Review and Code Review caps are independent policy budgets, so attempts are counted by workflow step key instead of the legacy aggregate `postReviewFixCount`. The aggregate still increments for existing dashboard summaries, but it must not let a Plan Review replan consume a Code Review remediation slot.
|
||||
*/
|
||||
private async requestPreMergeOptionalStepFix(
|
||||
taskId: string,
|
||||
@@ -3999,6 +4036,22 @@ export class TaskExecutor {
|
||||
*/
|
||||
const feedback = info.feedback?.trim()
|
||||
|| "Plan Review failed before execution. Revise the task plan, then continue execution.";
|
||||
const settings = await mergeEffectiveSettings(this.store, liveTask, await this.store.getSettings());
|
||||
const maxRevisions = resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: info.nodeId ?? "plan-review",
|
||||
workflowSettings: settings as Record<string, unknown>,
|
||||
nodeMaxRevisions: info.maxRevisions,
|
||||
fallbackMaxRevisions: settings.maxPostReviewFixes ?? 3,
|
||||
});
|
||||
const budget = resolveOptionalStepRevisionBudget(maxRevisions, settings.maxPostReviewFixes ?? 3);
|
||||
if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false;
|
||||
const revisionKey = optionalStepRevisionKey(info.nodeId ?? "plan-review", info.stepName);
|
||||
const currentCount = countOptionalStepRevisionAttempts(liveTask, revisionKey, info.stepName);
|
||||
if (!budget.unbounded && currentCount >= budget.max) return false;
|
||||
const nextCount = currentCount + 1;
|
||||
const totalFixCount = (liveTask.postReviewFixCount ?? 0) + 1;
|
||||
const budgetLabel = budget.unbounded ? "unbounded" : String(budget.max);
|
||||
await this.store.updateTask(taskId, { postReviewFixCount: totalFixCount }, this.getRunContextFor(taskId));
|
||||
this.clearPausedAborted(taskId);
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
@@ -4008,8 +4061,8 @@ export class TaskExecutor {
|
||||
);
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
"Plan Review failed — moved to triage for automatic replan",
|
||||
feedback,
|
||||
`Plan Review failed — moved to triage for automatic replan (attempt ${nextCount}/${budgetLabel})`,
|
||||
optionalStepRevisionLogOutcome(feedback, revisionKey),
|
||||
this.getRunContextFor(taskId),
|
||||
);
|
||||
if (liveTask.column !== "triage") {
|
||||
@@ -4027,19 +4080,27 @@ export class TaskExecutor {
|
||||
|
||||
if (info.verdict !== "REVISE") return false;
|
||||
const settings = await mergeEffectiveSettings(this.store, liveTask, await this.store.getSettings());
|
||||
const budget = resolveOptionalStepRevisionBudget(info.maxRevisions, settings.maxPostReviewFixes ?? 3);
|
||||
const maxRevisions = resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: info.nodeId ?? "",
|
||||
workflowSettings: settings as Record<string, unknown>,
|
||||
nodeMaxRevisions: info.maxRevisions,
|
||||
fallbackMaxRevisions: settings.maxPostReviewFixes ?? 3,
|
||||
});
|
||||
const budget = resolveOptionalStepRevisionBudget(maxRevisions, settings.maxPostReviewFixes ?? 3);
|
||||
if (!budget.unbounded && (!Number.isFinite(budget.max) || budget.max <= 0)) return false;
|
||||
|
||||
const currentCount = liveTask.postReviewFixCount ?? 0;
|
||||
const revisionKey = optionalStepRevisionKey(info.nodeId, info.stepName);
|
||||
const currentCount = countOptionalStepRevisionAttempts(liveTask, revisionKey, info.stepName);
|
||||
if (!budget.unbounded && currentCount >= budget.max) return false;
|
||||
|
||||
const nextCount = currentCount + 1;
|
||||
const totalFixCount = (liveTask.postReviewFixCount ?? 0) + 1;
|
||||
const budgetLabel = budget.unbounded ? "unbounded" : String(budget.max);
|
||||
await this.store.updateTask(taskId, { postReviewFixCount: nextCount }, this.getRunContextFor(taskId));
|
||||
await this.store.updateTask(taskId, { postReviewFixCount: totalFixCount }, this.getRunContextFor(taskId));
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Pre-merge optional workflow step requested executor fixes (attempt ${nextCount}/${budgetLabel})`,
|
||||
`Step: ${info.stepName}\nStatus: ${info.status}\nFeedback:\n${info.feedback}`,
|
||||
optionalStepRevisionLogOutcome(`Step: ${info.stepName}\nStatus: ${info.status}\nFeedback:\n${info.feedback}`, revisionKey),
|
||||
this.getRunContextFor(taskId),
|
||||
);
|
||||
await this.sendTaskBackForFix(
|
||||
|
||||
@@ -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, 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 { 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, resolveOptionalReviewRevisionBudget, 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";
|
||||
@@ -78,6 +78,37 @@ import { filterPathsByIgnoreList, getUnmetSchedulingDependencies, isCoordination
|
||||
import { evaluateParkedAgentTaskLink, PARKED_AGENT_LINK_FRESH_RUN_MS } from "./task-agent-sync.js";
|
||||
|
||||
const log = createLogger("self-healing");
|
||||
const OPTIONAL_STEP_REVISION_KEY_MARKER = "Workflow revision key:";
|
||||
|
||||
function normalizeOptionalStepRevisionKey(value: string | undefined): string {
|
||||
return (value ?? "").trim().toLowerCase();
|
||||
}
|
||||
|
||||
function optionalStepRevisionKey(nodeId: string | undefined, stepName: string | undefined): string {
|
||||
return normalizeOptionalStepRevisionKey(nodeId) || normalizeOptionalStepRevisionKey(stepName) || "pre-merge-optional-step";
|
||||
}
|
||||
|
||||
function countOptionalStepRevisionAttempts(task: Pick<Task, "log">, key: string, stepName: string | undefined): number {
|
||||
const normalizedKey = normalizeOptionalStepRevisionKey(key);
|
||||
const normalizedStepName = normalizeOptionalStepRevisionKey(stepName);
|
||||
return (task.log ?? []).filter((entry) => {
|
||||
const action = entry.action ?? "";
|
||||
const outcome = entry.outcome ?? "";
|
||||
if (!/attempt \d+\//.test(action)) return false;
|
||||
const markerIndex = outcome.indexOf(OPTIONAL_STEP_REVISION_KEY_MARKER);
|
||||
if (markerIndex >= 0) {
|
||||
const markerValue = outcome.slice(markerIndex + OPTIONAL_STEP_REVISION_KEY_MARKER.length).split(/\r?\n/, 1)[0]?.trim();
|
||||
return normalizeOptionalStepRevisionKey(markerValue) === normalizedKey;
|
||||
}
|
||||
if (!normalizedStepName) return false;
|
||||
return normalizeOptionalStepRevisionKey(outcome).includes(`step: ${normalizedStepName}`);
|
||||
}).length;
|
||||
}
|
||||
|
||||
function optionalStepRevisionLogOutcome(details: string, key: string): string {
|
||||
return `${details}\n${OPTIONAL_STEP_REVISION_KEY_MARKER} ${key}`;
|
||||
}
|
||||
|
||||
const worktreeMetadataReconcileLog = createLogger("worktree-metadata-reconcile");
|
||||
const execAsync = promisify(exec);
|
||||
const yieldEventLoop = (): Promise<void> => new Promise((resolve) => setImmediateCb(resolve));
|
||||
@@ -6257,9 +6288,27 @@ export class SelfHealingManager {
|
||||
/*
|
||||
* 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.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-20:50:
|
||||
* Offline recovery must share live execution's workflow-value precedence: explicit `planReviewMaxRevisions`/`codeReviewMaxRevisions` caps win, unset Plan Review/spec and Code Review values are unbounded, and Browser Verification keeps the existing fallback budget.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-22:06:
|
||||
* Self-healing uses the same per-step attempt partition as live execution. `postReviewFixCount` remains an aggregate observability counter, but cap exhaustion is computed from prior log markers for the failed workflow step so Plan Review and Code Review budgets do not consume each other.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-23:03:
|
||||
* The in-review sweep stays slim for board-scale filtering, but slim TaskStore rows intentionally omit `log`. Hydrate the full task before counting revision markers so offline recovery enforces Code Review and Plan Review caps against production data instead of treating every task as attempt zero.
|
||||
*/
|
||||
const revisionBudgetByTask = new Map<string, { unbounded: boolean; max: number; label: string }>();
|
||||
const revisionBudgetByTask = new Map<string, { unbounded: boolean; max: number; label: string; key: string; stepName?: string; attempts: number }>();
|
||||
const irCache = new Map<string, Awaited<ReturnType<typeof resolveWorkflowIrForTask>>>();
|
||||
const loadRevisionAttemptSource = async (task: Task): Promise<Pick<Task, "log">> => {
|
||||
try {
|
||||
const fullTask = await this.store.getTask(task.id);
|
||||
if (fullTask?.id === task.id && Array.isArray(fullTask.log)) return fullTask;
|
||||
} catch {
|
||||
// Keep recovery fail-soft; older stores/tests can still provide log entries on the list row.
|
||||
}
|
||||
return task;
|
||||
};
|
||||
for (const task of tasks) {
|
||||
const eff = await mergeEffectiveSettings(this.store, task, settings);
|
||||
const fallback = eff.maxPostReviewFixes ?? 3;
|
||||
@@ -6276,15 +6325,28 @@ export class SelfHealingManager {
|
||||
rawMaxRevisions = undefined;
|
||||
}
|
||||
}
|
||||
const budget = resolveOptionalStepRevisionBudget(rawMaxRevisions, fallback);
|
||||
const maxRevisions = resolveOptionalReviewRevisionBudget({
|
||||
optionalGroupId: target?.workflowStepId ?? "",
|
||||
workflowSettings: eff as Record<string, unknown>,
|
||||
nodeMaxRevisions: rawMaxRevisions,
|
||||
fallbackMaxRevisions: fallback,
|
||||
});
|
||||
const budget = resolveOptionalStepRevisionBudget(maxRevisions, fallback);
|
||||
const key = optionalStepRevisionKey(target?.workflowStepId, target?.workflowStepName);
|
||||
const revisionAttemptSource = await loadRevisionAttemptSource(task);
|
||||
revisionBudgetByTask.set(task.id, {
|
||||
...budget,
|
||||
key,
|
||||
stepName: target?.workflowStepName,
|
||||
attempts: countOptionalStepRevisionAttempts(revisionAttemptSource, key, target?.workflowStepName),
|
||||
label: budget.unbounded ? "unbounded" : String(budget.max),
|
||||
});
|
||||
}
|
||||
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 revisionBudgetFor = (taskId: string): { unbounded: boolean; max: number; label: string; key: string; stepName?: string; attempts: number } => {
|
||||
const budget = revisionBudgetByTask.get(taskId);
|
||||
if (budget) return budget;
|
||||
const fallbackBudget = resolveOptionalStepRevisionBudget(undefined, 3);
|
||||
return { ...fallbackBudget, key: "pre-merge-optional-step", attempts: 0, label: fallbackBudget.unbounded ? "unbounded" : String(fallbackBudget.max) };
|
||||
};
|
||||
|
||||
const candidates = tasks.filter((task) => {
|
||||
@@ -6297,7 +6359,7 @@ export class SelfHealingManager {
|
||||
if (executingIds.has(task.id)) 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;
|
||||
if (!budget.unbounded && budget.attempts >= budget.max) return false;
|
||||
|
||||
// Must have at least one failed pre-merge workflow step result.
|
||||
if (!latestFailedPreMergeStep(task)) return false;
|
||||
@@ -6322,16 +6384,18 @@ export class SelfHealingManager {
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of candidates) {
|
||||
const nextCount = (task.postReviewFixCount ?? 0) + 1;
|
||||
const budget = revisionBudgetFor(task.id);
|
||||
const nextCount = budget.attempts + 1;
|
||||
const totalFixCount = (task.postReviewFixCount ?? 0) + 1;
|
||||
try {
|
||||
// Increment the counter BEFORE delegating so that even if the
|
||||
// executor path crashes or races, the budget is still consumed and
|
||||
// we can't enter an infinite revival loop.
|
||||
await this.store.updateTask(task.id, { postReviewFixCount: nextCount });
|
||||
await this.store.updateTask(task.id, { postReviewFixCount: totalFixCount });
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-reviving in-review task with failed pre-merge workflow step (attempt ${nextCount}/${budget.label})`,
|
||||
optionalStepRevisionLogOutcome(`Step: ${budget.stepName ?? budget.key}`, budget.key),
|
||||
);
|
||||
const sentBack = await recoverFn(task);
|
||||
if (sentBack) {
|
||||
|
||||
@@ -199,7 +199,10 @@ export interface WorkflowGraphExecutorDeps {
|
||||
recordWorkflowStepResult?: (taskId: string, result: WorkflowStepResult) => void | Promise<void>;
|
||||
/*
|
||||
* FNXC:WorkflowOptionalStepFix 2026-06-26-16:20:
|
||||
* 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.
|
||||
* 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 workflow-value caps, `maxPostReviewFixes`, or `"unbounded"`; absent or false preserves prior byte-inert behavior for in-memory tests and exhausted budgets.
|
||||
*
|
||||
* FNXC:WorkflowRevisionBudget 2026-06-30-20:46:
|
||||
* Forward the optional-group id for every failure context because Plan Review/spec and Code Review budget resolution is keyed by that id. The graph does not read workflow setting values directly; live execution and self-healing share the core resolver at the remediation boundary.
|
||||
*/
|
||||
requestPreMergeOptionalStepFix?: (taskId: string, info: {
|
||||
stepName: string;
|
||||
|
||||
Reference in New Issue
Block a user