feat: gate review verdicts on finding severity and preserve remediation sessions

Review remediation loops were the dominant cost of task wall-clock: over 14 days,
tasks with >=5 post-review fix rounds were 22% of tasks but consumed 78% of all
task active time, and 311 of 331 recorded findings were spec-internal-consistency
complaints that changed no delivered behavior.

Two causes compounded. Plan/Code Review remediation was unbounded by default, and
the review policy ordered a full re-derivation of the artifact after every edit
("distrust the edit ... fresh holistic pass"), so each round surfaced a fresh crop
of previously-acceptable observations as new blockers.

Make the already-persisted WorkflowReviewFinding.severity load-bearing instead of
decorative: a REVISE only blocks when it carries a finding at or above the review
kind's threshold (plan: P0+P1, code: P0). Non-blocking findings are still parsed,
persisted, and handed to the implementer as advisory notes in PROMPT.md. Fails
closed — a REVISE with no findings, or with any unclassified finding, still blocks,
so prose-only and custom reviewers keep full blocking power. The gate only ever
relaxes a verdict, never promotes one.

Reviewer prompts now request the structured findings schema (Plan Review emitted
none before), define severity by consequence as P0/P1/P2, omit nits entirely rather
than filing them as low-severity findings, and use an incremental re-review contract.
Remediation renders findings grouped by priority and sanctions an explicit decline
with rationale, so a disputed finding has a terminal state.

Also preserve the implementation session across a review bounce: sendTaskBackForFix
no longer nulls sessionFile when preserving resume state, and the executor's finally
no longer clears it on a review handoff. Remediation rounds continue the conversation
instead of re-reading the repo and re-deriving the change they just wrote. The resume
prompt now directs a PROMPT.md re-read, without which a resumed agent would never see
the new findings.

New per-workflow settings planReviewBlockingSeverity / codeReviewBlockingSeverity;
set either to "any" to restore the previous behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-08-10 11:28:32 -07:00
parent e6b6223d30
commit 963dba6f80
26 changed files with 1120 additions and 48 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": minor
---
summary: Reviews now block only on high-priority findings, cutting repeated plan/code review fix rounds.
category: feature
dev: Adds `applyReviewSeverityGate`/`resolveReviewBlockingSeverity` (`packages/core/src/workflows/review-severity-gate.ts`) making the existing `WorkflowReviewFinding.severity` load-bearing. New per-workflow settings `planReviewBlockingSeverity` (default `high`) and `codeReviewBlockingSeverity` (default `critical`); set either to `any` to restore the previous behavior where every REVISE blocks. A REVISE carrying no finding at or above the threshold is recorded as APPROVE_WITH_NOTES and its findings are written to PROMPT.md as a non-blocking `## Review Advisory Notes` section. Fails closed: a REVISE with no findings, or with any unclassified finding, still blocks. Plan/Code Review prompts now request the structured findings schema, define the severity vocabulary as P0/P1/P2, suppress nits, and use an incremental re-review contract; remediation injection renders findings grouped by priority and sanctions an explicit decline with rationale.

View File

@@ -383,7 +383,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`, `planReviewMaxRevisions`, `codeReviewMaxRevisions`, `planReviewReplanCap`; project override: `planApprovalMode` |
| **Review / approval** | Workflow values: `requirePrApproval`, `requirePlanApproval`, `reviewHandoffPolicy`, `maxReviewerContextRetries`, `maxReviewerFallbackRetries`, `planReviewMaxRevisions`, `codeReviewMaxRevisions`, `planReviewBlockingSeverity`, `codeReviewBlockingSeverity`, `planReviewReplanCap`; project override: `planApprovalMode` |
| **Planner oversight** | `plannerOversightLevel` (workflow-native; values: `off`, `observe`, `steer`, `autonomous`); `plannerOversightNotificationLevel` (workflow-native; values: `silent`, `errors`, `important`, `all`); `plannerOverseerExecutorStuckAfterMs` (workflow-native; number, default `7200000` = 2h); `plannerOverseerAdvisorEnabled` (boolean, **default false**); `plannerOverseerAdvisorProvider` / `plannerOverseerAdvisorModelId` (session-advisor model; both required when enabled); `plannerHeartbeatPatrolEnabled` (workflow-native; boolean, default `true`, gates idle/no-task heartbeat patrol task creation) |
| **Per-phase model lanes** | `executionProvider`/`executionModelId` + `executionThinkingLevel`, `planningProvider`/`planningModelId` + `planningThinkingLevel` (+ fallbacks), `validatorProvider`/`validatorModelId` + `validatorThinkingLevel` (+ fallbacks). Thinking values accept `off`, `minimal`, `low`, `medium`, `high`, or `xhigh`; unset inherits. |
@@ -421,6 +421,8 @@ The built-in workflows also declare triage/spec policy settings that were **not*
| `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 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. |
| `planReviewBlockingSeverity` | `high` | Minimum finding severity that lets Plan Review block execution. A `REVISE` carrying no finding at or above this level is recorded as `APPROVE_WITH_NOTES` and its findings are written into PROMPT.md as non-blocking `## Review Advisory Notes` instead of forcing another planning round. Values: `critical` (P0 only), `high` (P0+P1, the default), `medium`, `low`, or `any` to restore the previous behavior where every `REVISE` blocks. **Fails closed:** a `REVISE` with no structured findings, or with any finding that omits `severity`, still blocks — so prose-only and custom reviewers keep their full blocking power. |
| `codeReviewBlockingSeverity` | `critical` | Same gate for Code Review, defaulting to `critical` (P0 only) because Code Review findings land against real code the implementer can address inline. Same values and the same fail-closed contract as `planReviewBlockingSeverity`. |
| `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). |

View File

@@ -775,6 +775,20 @@ Retryable graph failures at explicit remediation nodes (for example `code-review
During a live graph run, an enabled **pre-merge** optional step that returns `REVISE` (including the built-in **Code Review** / `code-review` and **Browser Verification** / `browser-verification` groups) sends the task back to the executor for a fix pass before the graph continues to review or merge. The workflow graph restarts on the next executor pass, re-launches task execution, and reopens the terminal verification/delivery suffix plus the nearest preceding implementation step so the verdict-demanded fix can be made rather than merely replaying a trivial trailing step. The optional step re-runs only after the executor drives those reopened steps back to `done`; the cycle repeats until the step returns `APPROVE` / `APPROVE_WITH_NOTES` or the resolved revision budget is exhausted. Generic optional gates use the workflow/project `maxPostReviewFixes` value (built-in default: 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.
### Severity-gated review verdicts
Revision budgets bound a remediation loop; they do not make it converge. The **blocking severity** gate shapes it at the source: a review-kind gate's `REVISE` only blocks when it carries at least one finding at or above the review kind's threshold (`planReviewBlockingSeverity`, default `high`/P0+P1; `codeReviewBlockingSeverity`, default `critical`/P0). A `REVISE` whose findings are all below the threshold is recorded as `APPROVE_WITH_NOTES`, the task proceeds, and the findings are written into PROMPT.md as a non-blocking `## Review Advisory Notes` section so the implementer still sees them.
The gate **fails closed** and only ever relaxes a verdict:
- a `REVISE` with no structured `findings` at all (prose-only, custom, or older reviewers) still blocks;
- a `REVISE` carrying any finding that omits `severity` still blocks;
- an `APPROVE`/`APPROVE_WITH_NOTES` is never promoted to a block, whatever its findings say.
Set either setting to `any` to restore the previous behavior where every `REVISE` blocks. The built-in Plan Review and Code Review prompts request the structured findings schema, define severity as P0/P1/P2 by consequence, instruct reviewers to omit nits entirely rather than file them as low-severity findings, and use an incremental re-review contract that forbids introducing new non-blocking findings as grounds for another round. Remediation instructions render findings grouped by priority — P0 must fix, P1 fix or explicitly decline with a rationale, P2 optional.
Remediation now also **preserves the implementation session**: a review bounce keeps `task.sessionFile`, so the next round continues the same conversation instead of re-reading the repository and re-deriving the change it just wrote. The resume prompt directs the agent to re-read PROMPT.md, which is where the new findings were written. Paths that genuinely need a fresh session (context overflow, stale continuation, worktree reacquisition, task-done refusal) still clear the session explicitly at their own site, and the resume guard re-validates the persisted worktree before reopening.
The same resolved per-step budget is used by self-healing when it revives an `in-review` task that is parked with a failed pre-merge workflow result. If the failed step's IR cannot be resolved, self-healing falls back to `maxPostReviewFixes` so existing behavior is preserved. `"unbounded"` relies on the optional step eventually approving; a step that always returns `REVISE` will continue cycling until a human intervenes or another guard (pause, worktree/lease, auto-merge policy, dependency blocker) stops recovery. The remediation instructions show `attempt/unbounded` and unlimited remaining retries for this policy rather than a misleading legacy `3/3` label. When the budget is exhausted or disabled, behavior falls through to the prior semantics: advisory results remain non-blocking and gate failures remain failed/parked.
Post-merge optional groups never trigger this send-back path because merge has already happened; their failures are recorded/logged as non-blocking post-merge results.

View File

@@ -686,9 +686,21 @@ describe("built-in workflows", () => {
const planReviewPrompt = String(planReviewInnerConfig(ir).prompt);
expect(planReviewPrompt).toContain("## Mandatory Plan Review Procedure");
expect(planReviewPrompt).toContain("all independently discoverable blocking findings");
expect(planReviewPrompt).toContain("prior-review ledger as a decision primer");
expect(planReviewPrompt).toContain("never demote a critical defect merely because it was missed before");
expect(planReviewPrompt).toContain("verdict notes must contain the complete blocking checklist");
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
The re-review contract moved out of the completeness policy into REVIEW_REREVIEW_POLICY, which
replaces the former "distrust the edit / fresh holistic pass" instruction with an incremental one.
Assert the interpolated severity + re-review policies rather than the retired wording: the prompt
must still forbid reopening settled findings and still refuse to demote a genuinely-missed P0.
*/
expect(planReviewPrompt).toContain("## Finding Priority");
expect(planReviewPrompt).toContain("## Do Not Report Nits");
expect(planReviewPrompt).toContain("## Re-Review (round 2 and later)");
expect(planReviewPrompt).toContain("Resolved items are settled");
expect(planReviewPrompt).toContain("say so plainly rather than demoting it");
expect(planReviewPrompt).toContain("Never introduce a new P1 or P2 finding as grounds for another revision round");
expect(planReviewPrompt).not.toContain("distrust the edit");
expect(byId.get("parse")?.column).toBe("in-progress");
expect(byId.get("steps")?.column).toBe("in-progress");
/*

View File

@@ -0,0 +1,175 @@
import { describe, expect, it } from "vitest";
import {
applyReviewSeverityGate,
formatFindingsByPriority,
isBlockingFinding,
resolveReviewBlockingSeverity,
DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
} from "../workflows/review-severity-gate.js";
import type { WorkflowReviewFinding } from "../types.js";
function finding(overrides: Partial<WorkflowReviewFinding> = {}): WorkflowReviewFinding {
return { id: "f1", title: "t", body: "b", ...overrides };
}
describe("resolveReviewBlockingSeverity", () => {
it("defaults plan review to high (P0+P1) and code review to critical (P0)", () => {
expect(resolveReviewBlockingSeverity({ reviewKind: "plan" })).toBe("high");
expect(resolveReviewBlockingSeverity({ reviewKind: "code" })).toBe("critical");
expect(DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY).toBe("high");
expect(DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY).toBe("critical");
});
it("prefers a stored workflow setting over the node value and the default", () => {
expect(resolveReviewBlockingSeverity({
reviewKind: "plan",
workflowSettings: { planReviewBlockingSeverity: "any" },
nodeBlockingSeverity: "critical",
})).toBe("any");
expect(resolveReviewBlockingSeverity({
reviewKind: "code",
workflowSettings: { codeReviewBlockingSeverity: "low" },
})).toBe("low");
});
it("falls back to the node value, then the default, ignoring invalid values", () => {
expect(resolveReviewBlockingSeverity({ reviewKind: "plan", nodeBlockingSeverity: "critical" })).toBe("critical");
expect(resolveReviewBlockingSeverity({
reviewKind: "plan",
workflowSettings: { planReviewBlockingSeverity: "bogus" },
nodeBlockingSeverity: 7,
})).toBe("high");
});
it("reads each review kind from its own setting key", () => {
// A plan-review override must not leak into code review's threshold.
const settings = { planReviewBlockingSeverity: "any" };
expect(resolveReviewBlockingSeverity({ reviewKind: "code", workflowSettings: settings })).toBe("critical");
});
});
describe("isBlockingFinding", () => {
it("blocks at the threshold and above", () => {
expect(isBlockingFinding(finding({ severity: "critical" }), "high")).toBe(true);
expect(isBlockingFinding(finding({ severity: "high" }), "high")).toBe(true);
expect(isBlockingFinding(finding({ severity: "medium" }), "high")).toBe(false);
expect(isBlockingFinding(finding({ severity: "low" }), "high")).toBe(false);
expect(isBlockingFinding(finding({ severity: "high" }), "critical")).toBe(false);
});
it("treats an unclassified finding as blocking (fail closed)", () => {
expect(isBlockingFinding(finding({ severity: undefined }), "critical")).toBe(true);
});
it("blocks everything at threshold \"any\"", () => {
expect(isBlockingFinding(finding({ severity: "low" }), "any")).toBe(true);
});
});
describe("applyReviewSeverityGate", () => {
it("downgrades a REVISE whose findings are all below the threshold", () => {
const result = applyReviewSeverityGate({
verdict: "REVISE",
findings: [finding({ id: "a", severity: "medium" }), finding({ id: "b", severity: "low" })],
threshold: "high",
});
expect(result.verdict).toBe("APPROVE_WITH_NOTES");
expect(result.downgraded).toBe(true);
expect(result.blocking).toHaveLength(0);
expect(result.advisory.map((f) => f.id)).toEqual(["a", "b"]);
});
it("keeps a REVISE that carries a finding at the threshold", () => {
const result = applyReviewSeverityGate({
verdict: "REVISE",
findings: [finding({ id: "a", severity: "medium" }), finding({ id: "b", severity: "high" })],
threshold: "high",
});
expect(result.verdict).toBe("REVISE");
expect(result.downgraded).toBe(false);
expect(result.blocking.map((f) => f.id)).toEqual(["b"]);
expect(result.advisory.map((f) => f.id)).toEqual(["a"]);
});
it("applies the asymmetric built-in defaults: a high finding blocks plan review but not code review", () => {
const findings = [finding({ severity: "high" })];
expect(applyReviewSeverityGate({ verdict: "REVISE", findings, threshold: DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY }).verdict)
.toBe("REVISE");
expect(applyReviewSeverityGate({ verdict: "REVISE", findings, threshold: DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY }).verdict)
.toBe("APPROVE_WITH_NOTES");
});
/*
* The fail-closed contract is the reason this gate is safe to enable by default: every reviewer that
* does not opt into the structured findings schema keeps its full blocking power.
*/
it("never downgrades a REVISE with no findings at all (prose-only reviewer)", () => {
for (const findings of [undefined, []]) {
const result = applyReviewSeverityGate({ verdict: "REVISE", findings, threshold: "critical" });
expect(result.verdict).toBe("REVISE");
expect(result.downgraded).toBe(false);
}
});
it("never downgrades when any finding is unclassified", () => {
const result = applyReviewSeverityGate({
verdict: "REVISE",
findings: [finding({ id: "a", severity: "low" }), finding({ id: "b", severity: undefined })],
threshold: "critical",
});
expect(result.verdict).toBe("REVISE");
expect(result.downgraded).toBe(false);
expect(result.blocking.map((f) => f.id)).toEqual(["b"]);
});
it("restores pre-gate behavior at threshold \"any\"", () => {
const result = applyReviewSeverityGate({
verdict: "REVISE",
findings: [finding({ severity: "low" })],
threshold: "any",
});
expect(result.verdict).toBe("REVISE");
expect(result.downgraded).toBe(false);
});
it("only ever relaxes — an APPROVE carrying a critical finding is left alone", () => {
for (const verdict of ["APPROVE", "APPROVE_WITH_NOTES", "CLOSE_NO_OP", undefined]) {
const result = applyReviewSeverityGate({
verdict,
findings: [finding({ severity: "critical" })],
threshold: "high",
});
expect(result.verdict).toBe(verdict);
expect(result.downgraded).toBe(false);
}
});
});
describe("formatFindingsByPriority", () => {
it("groups by priority with an explicit obligation per group", () => {
const out = formatFindingsByPriority([
finding({ id: "a", title: "boom", body: "breaks", severity: "critical", filePath: "src/a.ts", line: 12 }),
finding({ id: "b", title: "maybe", body: "risky", severity: "high" }),
finding({ id: "c", title: "nit-ish", body: "minor", severity: "low" }),
]);
expect(out).toContain("### P0 — must fix");
expect(out).toContain("### P1 — should fix");
expect(out).toContain("### P2 — optional");
expect(out).toContain("(src/a.ts:12)");
expect(out).toContain("Skipping these is expected");
});
it("omits groups with no findings and returns empty for no findings", () => {
const out = formatFindingsByPriority([finding({ severity: "critical" })]);
expect(out).toContain("### P0 — must fix");
expect(out).not.toContain("### P1");
expect(out).not.toContain("### P2");
expect(formatFindingsByPriority([])).toBe("");
});
it("presents unclassified findings with the strongest obligation, matching the fail-closed gate", () => {
const out = formatFindingsByPriority([finding({ title: "unknown", body: "x" })]);
expect(out).toContain("### Unclassified — treat as must fix");
});
});

View File

@@ -3,6 +3,16 @@
* Planning and Plan Review share one completeness contract: planning researches
* and maps the full requirement ledger up front, while review evaluates the whole
* artifact and batches every independently discoverable blocker in one round.
*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Two clauses were removed from the review procedure because they manufactured revision rounds rather
* than finding defects. The former step 6 ordered a full re-derivation of the artifact after every edit
* ("distrust the edit ... rebuild the ledger ... fresh holistic pass"), which surfaced a new crop of
* previously-acceptable observations each round; the former step 3's "keep wording polish advisory"
* carve-out was too weak to stop them being filed as blockers. Re-review is now incremental (see
* REVIEW_REREVIEW_POLICY) and nits are suppressed at the source (see REVIEW_SEVERITY_POLICY).
* Measured driver: 311 of 331 findings over 14 days were spec-internal-consistency complaints, and
* tasks with >=5 remediation rounds consumed 78% of all task active time.
*/
export const PLANNING_COMPLETENESS_POLICY = `## Mandatory Planning Completeness Procedure
@@ -29,9 +39,8 @@ Before choosing a verdict:
1. Build one **review ledger** for the entire PROMPT.md: Original Description and user comments; Mission and Completion Criteria; Surface Enumeration and Symptom Verification when required; every implementation step, File Scope entry, dependency, risk, and test/verification promise.
2. Complete the full review before reporting. Check coherence and requirement traceability, feasibility against the current repository, scope discipline, execution ordering, and verification quality. When relevant, also inspect security/data integrity, state transitions, concurrency orderings, deployment/configuration boundaries, recovery competitors, and force/bypass paths.
3. Review at specification altitude. Block when a required behavior, surface, ordering, safety constraint, or proof is missing or the stated approach cannot work. Keep optional implementation detail, wording polish, and nonessential improvements advisory.
4. If REVISE is necessary, batch **all independently discoverable blocking findings** into this one verdict; do not stop after the first defect. Give each blocker a stable ID, cite the affected section or repository evidence, and state the concrete PROMPT.md correction. Put advisory observations in a separate list.
5. On re-review, use the supplied prior-review ledger as a decision primer. Verify every prior blocker, do not re-raise resolved or rejected semantic duplicates, and preserve accepted decisions. A newly blocking finding must say whether the revision introduced it, which prior blocker genuinely masked it, or why it is independently delivery-blocking for correctness, security, data safety, or executability. Record an earlier reviewer miss explicitly; never demote a critical defect merely because it was missed before.
6. After any same-session PROMPT.md edit, distrust the edit: reread the complete artifact, rebuild the ledger, and perform a fresh holistic pass before APPROVE.
3. Review at specification altitude. Block when a required behavior, surface, ordering, safety constraint, or proof is missing or the stated approach cannot work. A plan does not need to be internally seamless to be executable — judge whether a competent implementer would build the right thing from it, not whether every section agrees with every other section.
4. If REVISE is necessary, batch **all independently discoverable blocking findings** into this one verdict; do not stop after the first defect. Give each blocker a stable ID, cite the affected section or repository evidence, and state the concrete PROMPT.md correction.
5. After a same-session PROMPT.md edit, verify that the edit resolved what you asked for. Confine that check to the changed sections and their direct dependencies; a fresh whole-artifact re-derivation at this point produces new observations rather than new information.
APPROVE when the plan is executable and verifiable, not when it is cosmetically perfect. If returning REVISE, the verdict notes must contain the complete blocking checklist because those notes are the durable input to the next planning round.`;

View File

@@ -0,0 +1,67 @@
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
One shared severity taxonomy for every review-kind gate (Plan Review, Code Review). It exists because
the persisted `severity` field was previously requested from the model with no definition at all —
the system prompt asked for "low|medium|high|critical" and never said what they meant or what they did,
so reviewers classified arbitrarily and the engine ignored the answer.
The engine now gates the verdict on these values (see `workflows/review-severity-gate.ts`), so the
vocabulary is a behavioral contract, not documentation: a mis-classified finding either blocks delivery
that should have proceeded, or lets a real defect through. The definitions below are written in terms of
CONSEQUENCE (what breaks if unfixed) rather than effort or confidence, because consequence is the only
axis the gate can act on.
The no-nits rule is the load-bearing half. Measured over 14 days, 311 of 331 recorded findings were
spec-internal-consistency complaints that changed no delivered behavior, and each one forced a full
remediation round. Suppressing them at the source is cheaper than classifying and then discarding them,
and it keeps the reviewer's attention on defects.
*/
/** Severity definitions + no-nits rule shared by all review-kind gates. */
export const REVIEW_SEVERITY_POLICY = `## Finding Priority
Every finding you report MUST carry a \`severity\`. Classify by CONSEQUENCE — what breaks if this is never fixed — not by how confident you are or how easy the fix is.
- **critical (P0)** — delivery-blocking. The stated approach cannot work, a required behavior or safety constraint is missing, or shipping this causes incorrect behavior, data loss, a security hole, or a broken contract for an existing consumer.
- **high (P1)** — materially wrong but not fatal. A real defect, missing verification for a behavior being changed, or an ambiguity concrete enough that a competent implementer would likely resolve it the wrong way.
- **medium / low (P2)** — genuine but non-blocking. Worth knowing, safe to defer, and safe to decline.
## Do Not Report Nits
Assume the implementer is a competent engineer who resolves local detail correctly without being told. Do NOT report — at ANY severity, including as advisory notes — any of the following:
- Wording, phrasing, formatting, heading, ordering, or naming preferences.
- Internal numbering, counting, or cross-reference mismatches between sections that do not change what gets built (for example: a list says "13 sites" but enumerates 15, a step cites a section by the wrong label, two sections describe the same requirement in different words).
- Restating a requirement that is already satisfied elsewhere in the artifact, or asking for a detail to be repeated in a second location.
- Detail that is genuinely underspecified but that any reasonable implementation choice would satisfy.
- Speculative future concerns not reachable by the change under review, and improvements outside its stated scope.
- Requests to add defensive handling for conditions the surrounding code already prevents.
If a finding's only consequence is that the artifact reads less precisely, it is a nit. Omit it entirely. A review with no findings is a good outcome, and you are not expected to find something.
Report only what changes the delivered result. Prefer few, well-evidenced findings over exhaustive coverage.`;
/**
* Re-review rules for the second and subsequent rounds.
*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* The previous policy told the reviewer to "distrust the edit: reread the complete artifact, rebuild
* the ledger, and perform a fresh holistic pass" on every round. Combined with an unbounded revision
* budget that instruction was the churn engine — each round re-derived the artifact from scratch and
* surfaced a fresh crop of previously-acceptable observations as new blockers, so convergence depended
* on the reviewer running out of things to notice. Re-review is now INCREMENTAL and the bar for
* introducing a NEW blocker after round one is deliberately higher than for the first round.
*/
export const REVIEW_REREVIEW_POLICY = `## Re-Review (round 2 and later)
You are re-reviewing an artifact that was revised in response to your own earlier findings. Your job is to CONVERGE, not to re-derive the review.
1. Verify each prior blocking finding: resolved, partially resolved, or unresolved. Resolved items are settled — do not reopen them, and do not re-raise a semantic duplicate under a new ID.
2. Accept decisions the previous round already accepted. A choice you did not object to before is not a defect now merely because you are looking again.
3. You may raise a NEW blocking finding only when one of these is true, and you must state which:
- the revision INTRODUCED it;
- it is P0 (delivery-blocking) and was genuinely missed earlier — say so plainly rather than demoting it;
- a prior blocker was masking it.
A new finding that meets none of these is not blocking. Report it as P2 or omit it.
4. Never introduce a new P1 or P2 finding as grounds for another revision round. Late-arriving non-blocking observations belong in notes.
5. If every prior blocker is resolved, APPROVE. Do not withhold approval because a fresh read suggests further polish — approve when the artifact is executable and verifiable, not when it is beyond improvement.`;

View File

@@ -556,6 +556,28 @@ export {
type OptionalReviewRevisionBudget,
type ResolveOptionalReviewRevisionBudgetInput,
} from "./workflows/workflow-settings-resolver.js";
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
Keep in SYNC with the main barrel (index.ts) — the `engine-core` vitest project builds @fusion/core from
THIS file, so a gate export present only in index.ts resolves to `undefined` under engine-core alone.
*/
export {
applyReviewSeverityGate,
formatFindingsByPriority,
isBlockingFinding,
isReviewBlockingSeverity,
resolveReviewBlockingSeverity,
CODE_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
PLAN_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
REVIEW_BLOCKING_SEVERITIES,
SEVERITY_PRIORITY_LABEL,
type ResolveReviewBlockingSeverityInput,
type ReviewBlockingSeverity,
type ReviewSeverityGateInput,
type ReviewSeverityGateResult,
} from "./workflows/review-severity-gate.js";
export {
applyWorkflowSettingsOverlay,
type WorkflowSettingsOverlayInput,

View File

@@ -649,6 +649,23 @@ export {
type OptionalReviewRevisionBudget,
type ResolveOptionalReviewRevisionBudgetInput,
} from "./workflows/workflow-settings-resolver.js";
export {
applyReviewSeverityGate,
formatFindingsByPriority,
isBlockingFinding,
isReviewBlockingSeverity,
resolveReviewBlockingSeverity,
CODE_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
PLAN_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
REVIEW_BLOCKING_SEVERITIES,
SEVERITY_PRIORITY_LABEL,
type ResolveReviewBlockingSeverityInput,
type ReviewBlockingSeverity,
type ReviewSeverityGateInput,
type ReviewSeverityGateResult,
} from "./workflows/review-severity-gate.js";
export {
applyWorkflowSettingsOverlay,
type WorkflowSettingsOverlayInput,

View File

@@ -1,5 +1,6 @@
import type { WorkflowIrNode } from "./workflow-ir-types.js";
import { CODE_REVIEW_COMPLETENESS_POLICY } from "../agents/code-review-policy.js";
import { REVIEW_REREVIEW_POLICY, REVIEW_SEVERITY_POLICY } from "../agents/review-severity-policy.js";
/*
FNXC:CodeReviewStep 2026-06-25-15:00:
@@ -68,15 +69,20 @@ const CODE_REVIEW_PROMPT = `You are a senior code reviewer. Review the task's di
${CODE_REVIEW_COMPLETENESS_POLICY}
${REVIEW_SEVERITY_POLICY}
${REVIEW_REREVIEW_POLICY}
Be specific: cite \`file:line\` for every finding and explain the concrete failure it causes.
## Output Requirements
- Fast-bail: if the diff is trivial, generated, or out-of-scope for code review (e.g. pure docs/config/formatting with no logic), output {"verdict":"APPROVE","notes":"out of scope: code review"} immediately and stop.
- APPROVE: no correctness concerns; use empty or brief notes.
- APPROVE_WITH_NOTES: shippable, but include non-blocking advisories (with file:line) in notes.
- REVISE: a correctness bug, regression, or contract break requires changes; include file:line and the concrete failure plus remediation in notes.
- APPROVE_WITH_NOTES: shippable. Use this when your findings are all P1/P2 — they are recorded and handed to the implementer without another remediation round.
- REVISE: a correctness bug, regression, or contract break requires changes before merge. Requires at least one \`critical\` finding in \`findings\`; a REVISE without one will be treated as APPROVE_WITH_NOTES.
- Every blocking issue MUST appear as an entry in \`findings\` with its severity and \`filePath\`/\`line\`. Prose in \`notes\` alone does not block.
- Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose):
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}`;
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"...","findings":[{"id":"stable-id","title":"concise issue","body":"concrete failure and remediation","filePath":"path/to/file.ts","line":1,"severity":"critical|high|medium|low"}]}`;
/**
* Build the `code-review` optional-group node placed on a workflow's pre-merge path.

View File

@@ -1,5 +1,6 @@
import type { WorkflowIrNode } from "./workflow-ir-types.js";
import { PLAN_REVIEW_COMPLETENESS_POLICY } from "../agents/planning-review-policy.js";
import { REVIEW_REREVIEW_POLICY, REVIEW_SEVERITY_POLICY } from "../agents/review-severity-policy.js";
/*
FNXC:PlanReviewStep 2026-06-28-23:29:
@@ -32,15 +33,20 @@ const PLAN_REVIEW_PROMPT = `You are a senior plan reviewer. Review the task's PR
${PLAN_REVIEW_COMPLETENESS_POLICY}
${REVIEW_SEVERITY_POLICY}
${REVIEW_REREVIEW_POLICY}
Be specific: cite the plan section or file path for every finding and explain the concrete correction.
## Output Requirements
- APPROVE: the plan is ready for execution.
- APPROVE_WITH_NOTES: execution may proceed, but include non-blocking advisory notes.
- REVISE: the plan should be corrected before execution; include every blocking finding and needed change in the JSON notes, not only in preceding prose.
- APPROVE_WITH_NOTES: execution may proceed. Use this when your findings are all P2 — they are recorded and handed to the implementer without another planning round.
- REVISE: the plan must be corrected before execution. Requires at least one \`critical\` or \`high\` finding in \`findings\`; a REVISE whose findings are all P2 will be treated as APPROVE_WITH_NOTES.
- CLOSE_NO_OP: implementation must not proceed because the premise is stale, the work is already satisfied, redundant, or a duplicate. The notes field MUST begin with exactly one existing completion sentinel: PREMISE STALE:, NO-OP:, NOOP:, REDUNDANT:, or DUPLICATE:. For duplicates, use DUPLICATE: FN-NNNN ... when the canonical task is known.
- Every blocking issue MUST appear as an entry in \`findings\` with its severity. Prose in \`notes\` alone does not block, and is not a durable input to the next planning round.
- Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose):
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE|CLOSE_NO_OP","notes":"..."}`;
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE|CLOSE_NO_OP","notes":"...","findings":[{"id":"stable-id","title":"concise issue","body":"actionable correction","severity":"critical|high|medium|low"}]}`;
/*
FNXC:PlanReviewStep 2026-07-27-06:10:

View File

@@ -1,4 +1,8 @@
import { THINKING_LEVELS, type Settings } from "../types.js";
import {
DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
} from "./review-severity-gate.js";
import type { WorkflowSettingDefinition } from "./workflow-ir-types.js";
/**
@@ -546,6 +550,42 @@ export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [
description:
"Maximum automatic Code Review remediation attempts for this workflow. Leave unset to use the workflow's authored default; set 0 to disable automatic revision.",
},
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* The blocking threshold shapes review churn at its source; the revision caps above only truncate a
* loop once it is already running. Exposed per workflow so an operator can restore the pre-gate
* behavior ("any") for a high-assurance workflow without editing a read-only built-in.
*/
{
id: "planReviewBlockingSeverity",
name: "Plan Review blocking severity",
type: "enum",
options: [
{ value: "critical", label: "P0 only (critical)" },
{ value: "high", label: "P0 + P1 (high and above)" },
{ value: "medium", label: "P0 + P1 + P2 medium" },
{ value: "low", label: "Any classified finding" },
{ value: "any", label: "Every REVISE blocks" },
],
default: DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
description:
"Minimum finding severity that lets Plan Review block execution. A REVISE carrying no finding at or above this level is recorded as APPROVE_WITH_NOTES and its findings are handed to the implementer. Choose \"any\" to block on every REVISE.",
},
{
id: "codeReviewBlockingSeverity",
name: "Code Review blocking severity",
type: "enum",
options: [
{ value: "critical", label: "P0 only (critical)" },
{ value: "high", label: "P0 + P1 (high and above)" },
{ value: "medium", label: "P0 + P1 + P2 medium" },
{ value: "low", label: "Any classified finding" },
{ value: "any", label: "Every REVISE blocks" },
],
default: DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
description:
"Minimum finding severity that lets Code Review block merge. A REVISE carrying no finding at or above this level is recorded as APPROVE_WITH_NOTES and its findings are handed to the implementer. Choose \"any\" to block on every REVISE.",
},
{
id: "planReviewReplanCap",
name: "Plan Review replan cap",

View File

@@ -35,6 +35,7 @@ export * from "./workflow-lifecycle-validation.js";
export * from "./workflow-optional-steps.js";
export * from "./workflow-prompt-overrides.js";
export * from "./workflow-reconciliation.js";
export * from "./review-severity-gate.js";
export * from "./workflow-settings.js";
export * from "./workflow-settings-resolver.js";
export * from "./workflow-step-results.js";

View File

@@ -0,0 +1,194 @@
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
Review remediation loops were the dominant cost of task wall-clock: measured over 14 days, tasks with
>=5 post-review fix rounds were 22% of tasks but consumed 78% of all task active time, and 311 of 331
recorded review findings were spec-internal-consistency complaints ("Step 4 says X but Step 5 says Y",
"Surface Enumeration claims 13 but lists 15") rather than defects that change delivered behavior.
Every such finding forced a full REVISE bounce because the verdict token was taken at face value.
The fix is to make the ALREADY-PERSISTED `WorkflowReviewFinding.severity` field load-bearing instead of
decorative: a REVISE only blocks when it carries at least one finding at or above the review kind's
blocking threshold. Non-blocking findings are still parsed, persisted, and handed to the implementer —
they simply stop bouncing the task. This reduces churn at its source rather than truncating it with a
revision cap, which only converts a runaway loop into a hard stop.
Severity is mapped onto operator-facing priority labels (P0=critical, P1=high, P2=medium/low) so prompts
can speak in priorities without introducing a third severity vocabulary alongside this one and the
Compound Engineering skill's P0-P3 table.
FAIL-CLOSED CONTRACT (load-bearing, do not relax):
A REVISE that carries NO findings at all, or that carries a finding with NO severity, is never
downgraded. Prose-only reviewers (custom nodes, older workflows, malformed-JSON fallbacks) and reviewers
that decline to classify must keep their blocking power — otherwise this gate would silently disarm
every review that does not opt into the structured contract.
*/
import type { WorkflowReviewFinding, WorkflowReviewFindingSeverity, WorkflowReviewKind } from "../types.js";
/**
* Blocking threshold for a review gate. A severity value blocks at that level and above;
* `"any"` restores the pre-gate behavior where every REVISE blocks regardless of severity.
*/
export type ReviewBlockingSeverity = WorkflowReviewFindingSeverity | "any";
export const REVIEW_BLOCKING_SEVERITIES = ["any", "low", "medium", "high", "critical"] as const;
export const PLAN_REVIEW_BLOCKING_SEVERITY_SETTING_ID = "planReviewBlockingSeverity";
export const CODE_REVIEW_BLOCKING_SEVERITY_SETTING_ID = "codeReviewBlockingSeverity";
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
Defaults are asymmetric because the measured churn is asymmetric. Plan Review accounted for 185 recorded
prior attempts across 117 tasks in 14 days while Code Review recorded 0, so Plan Review blocks on P0+P1
(a plan defect that reaches implementation is expensive) while Code Review blocks on P0 only (its
findings land against real code the implementer can address inline or in a follow-up).
*/
export const DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY: ReviewBlockingSeverity = "high";
export const DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY: ReviewBlockingSeverity = "critical";
const SEVERITY_RANK: Record<WorkflowReviewFindingSeverity, number> = {
low: 0,
medium: 1,
high: 2,
critical: 3,
};
/** Operator/prompt-facing priority label for a persisted severity. */
export const SEVERITY_PRIORITY_LABEL: Record<WorkflowReviewFindingSeverity, string> = {
critical: "P0",
high: "P1",
medium: "P2",
low: "P2",
};
const BLOCKING_SEVERITY_SETTING_BY_REVIEW_KIND: Record<WorkflowReviewKind, string> = {
plan: PLAN_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
code: CODE_REVIEW_BLOCKING_SEVERITY_SETTING_ID,
};
const DEFAULT_BLOCKING_SEVERITY_BY_REVIEW_KIND: Record<WorkflowReviewKind, ReviewBlockingSeverity> = {
plan: DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
code: DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
};
export function isReviewBlockingSeverity(value: unknown): value is ReviewBlockingSeverity {
return typeof value === "string" && (REVIEW_BLOCKING_SEVERITIES as readonly string[]).includes(value);
}
export interface ResolveReviewBlockingSeverityInput {
reviewKind: WorkflowReviewKind;
/** Effective per-task workflow settings map (stored value ?? declaration default). */
workflowSettings?: Record<string, unknown>;
/** Authored node override, read from the review group's config. */
nodeBlockingSeverity?: unknown;
}
/**
* Resolve the blocking threshold for a review gate.
*
* Precedence mirrors {@link resolveOptionalReviewRevisionBudget}: a stored workflow setting wins first
* so an operator can restore `"any"` per workflow, then an authored node value keeps custom-workflow
* semantics, then the review kind's built-in default.
*/
export function resolveReviewBlockingSeverity({
reviewKind,
workflowSettings,
nodeBlockingSeverity,
}: ResolveReviewBlockingSeverityInput): ReviewBlockingSeverity {
const settingId = BLOCKING_SEVERITY_SETTING_BY_REVIEW_KIND[reviewKind];
const stored = workflowSettings?.[settingId];
if (isReviewBlockingSeverity(stored)) return stored;
if (isReviewBlockingSeverity(nodeBlockingSeverity)) return nodeBlockingSeverity;
return DEFAULT_BLOCKING_SEVERITY_BY_REVIEW_KIND[reviewKind];
}
/**
* Whether a single finding blocks at the given threshold.
*
* An UNCLASSIFIED finding always blocks — see the fail-closed contract in the module header.
*/
export function isBlockingFinding(finding: WorkflowReviewFinding, threshold: ReviewBlockingSeverity): boolean {
if (threshold === "any") return true;
if (!finding.severity) return true;
return SEVERITY_RANK[finding.severity] >= SEVERITY_RANK[threshold];
}
export interface ReviewSeverityGateInput {
verdict: string | undefined;
findings: WorkflowReviewFinding[] | undefined;
threshold: ReviewBlockingSeverity;
}
export interface ReviewSeverityGateResult<V = string | undefined> {
/** The effective verdict after the gate. `REVISE` becomes `APPROVE_WITH_NOTES` when nothing blocks. */
verdict: V;
/** True only when a REVISE was downgraded by this gate. */
downgraded: boolean;
/** Findings at or above the threshold (empty on a downgrade, by construction). */
blocking: WorkflowReviewFinding[];
/** Findings below the threshold. Still persisted and still handed to the implementer. */
advisory: WorkflowReviewFinding[];
}
/**
* Apply the severity gate to a parsed review verdict.
*
* Only ever RELAXES a REVISE; it never promotes an APPROVE into a block, so a reviewer that approves
* while attaching a critical finding is left alone (that combination is a reviewer contradiction the
* gate has no authority to resolve, and escalating it here would make approval non-deterministic).
*/
export function applyReviewSeverityGate({ verdict, findings, threshold }: ReviewSeverityGateInput): ReviewSeverityGateResult {
const all = findings ?? [];
const blocking = all.filter((finding) => isBlockingFinding(finding, threshold));
const advisory = all.filter((finding) => !isBlockingFinding(finding, threshold));
if (verdict !== "REVISE") return { verdict, downgraded: false, blocking, advisory };
// Fail closed: an unstructured REVISE keeps its blocking power.
if (all.length === 0) return { verdict, downgraded: false, blocking, advisory };
if (blocking.length > 0) return { verdict, downgraded: false, blocking, advisory };
return { verdict: "APPROVE_WITH_NOTES", downgraded: true, blocking, advisory };
}
/**
* Render findings for an agent-facing prompt, grouped by priority with explicit obligations.
*
* Shared by the remediation injection and the advisory carry-forward so the implementer sees one
* consistent shape whether the review blocked or was downgraded.
*/
export function formatFindingsByPriority(findings: WorkflowReviewFinding[]): string {
if (findings.length === 0) return "";
const groups: Array<{ label: string; obligation: string; severities: WorkflowReviewFindingSeverity[] }> = [
{ label: "P0 — must fix", obligation: "Fix every item in this group before returning.", severities: ["critical"] },
{
label: "P1 — should fix",
obligation: "Fix these unless you have a concrete reason not to. If you decline one, say which and why.",
severities: ["high"],
},
{
label: "P2 — optional",
obligation: "Address only if cheap and clearly correct. Skipping these is expected and requires no justification.",
severities: ["medium", "low"],
},
];
const sections: string[] = [];
for (const group of groups) {
const matching = findings.filter((finding) => finding.severity && group.severities.includes(finding.severity));
if (matching.length === 0) continue;
const items = matching.map((finding) => {
const location = finding.filePath ? ` (${finding.filePath}${finding.line ? `:${finding.line}` : ""})` : "";
return `- **${finding.title}**${location}\n ${finding.body}`;
});
sections.push(`### ${group.label}\n${group.obligation}\n\n${items.join("\n")}`);
}
// Unclassified findings block (fail-closed), so present them with the strongest obligation.
const unclassified = findings.filter((finding) => !finding.severity);
if (unclassified.length > 0) {
const items = unclassified.map((finding) => `- **${finding.title}**\n ${finding.body}`);
sections.push(`### Unclassified — treat as must fix\n${items.join("\n")}`);
}
return sections.join("\n\n");
}

View File

@@ -1842,20 +1842,25 @@ describe("swallowed async store failure observability", () => {
warnSpy.mockRestore();
});
it("logs warning when sessionFile clear fails on completion", async () => {
/*
FNXC:SessionResume 2026-08-10-17:33:
SUPERSEDES "logs warning when sessionFile clear fails on completion". That test asserted the executor
nulls `sessionFile` when a completed implementation hands off to review. It no longer does: a review
gate can bounce the card straight back for remediation in the same worktree, and clearing here forced
every one of those rounds to restart cold and re-derive the change it had just written. The clear now
happens only on genuinely terminal exits (and at the explicit fresh-session sites, which also null
worktree/branch). This asserts the replacement invariant on the same fixture: the handoff preserves the
conversation and attempts no clear at all.
*/
it("preserves sessionFile across the review handoff so remediation can resume the conversation", async () => {
const warnSpy = vi.spyOn(executorLog, "warn");
const store = createMockStore();
let capturedCustomTools: any[] = [];
/*
FNXC:EngineTests 2026-07-19-04:47 (U10b):
Only the sessionFile CLEAR may fail; the graph re-reads the card between nodes, so every other write must still land on the row or the run never reaches the completion that triggers the clear.
*/
const sessionFileClears: unknown[] = [];
const passThroughUpdateTask = store.updateTask.getMockImplementation()!;
store.updateTask.mockImplementation(async (taskId: string, patch: Record<string, unknown>) => {
if (patch?.sessionFile === null) {
throw new Error("session clear failed");
}
if (patch?.sessionFile === null) sessionFileClears.push(patch);
return passThroughUpdateTask(taskId, patch);
});
@@ -1891,16 +1896,16 @@ describe("swallowed async store failure observability", () => {
/*
FNXC:EngineTests 2026-07-19-03:19 (U10b):
A failed sessionFile clear must warn but must not block the run's handoff to review; that handoff is now the graph's merge boundary, so the move carries workflow-graph provenance.
The handoff to review is the graph's merge boundary, so the move carries workflow-graph provenance.
*/
expect(store.moveTask).toHaveBeenCalledWith(
"FN-001",
"in-review",
expect.objectContaining({ workflowMoveSource: "workflow-graph" }),
);
expect(warnSpy).toHaveBeenCalledWith(
expect.stringContaining("FN-001 failed to clear sessionFile: session clear failed"),
);
// The conversation survives the handoff — nothing nulls sessionFile on this path.
expect(sessionFileClears).toEqual([]);
expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("failed to clear sessionFile"));
warnSpy.mockRestore();
});

View File

@@ -960,11 +960,14 @@ describe("Workflow Steps Execution", () => {
// Code Review budget). A hard-failure exhaustion passes the bounded
// MAX_WORKFLOW_STEP_RETRIES budget (currently 3), so the injected
// PROMPT.md note shows "3/3 (0 remaining)".
// FNXC:ReviewSeverityGate 2026-08-10-17:33: a trailing `findings` arg now carries structured
// review findings into the injection; a prompt-mode hard failure has none, so it is `undefined`.
expect(injectSpy).toHaveBeenCalledWith(
mutableTask,
feedback,
stepName,
{ attempt: 3, max: 3 },
undefined,
);
// The scheduleWorkflowRerun stub above never registers the 15 s

View File

@@ -0,0 +1,131 @@
import { describe, expect, it, beforeEach, afterEach } from "vitest";
import { mkdtemp, mkdir, readFile, writeFile } from "node:fs/promises";
import { rm } from "node:fs/promises";
import { join } from "node:path";
import { tmpdir } from "node:os";
import type { Task, WorkflowReviewFinding } from "@fusion/core";
import {
injectReviewAdvisoryNotes,
injectWorkflowStepFailureInstructions,
} from "../executor/workflow-step-failure-injection.js";
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
These pin the IMPLEMENTER-FACING contract of the severity work. The gate only pays off if the
implementer can tell a blocking defect from an optional note: before this, remediation injected one
undifferentiated prose blob, so every observation read as mandatory and a single REVISE turned into a
multi-round negotiation. Assert the priority grouping, the explicit obligations, and the sanctioned
decline path — those are the behaviors that make the loop converge.
*/
const task = { id: "FN-TEST" } as Task;
let dir: string;
let promptPath: string;
const store = {
getFusionDir: () => dir,
};
function finding(overrides: Partial<WorkflowReviewFinding>): WorkflowReviewFinding {
return { id: "f", title: "t", body: "b", ...overrides };
}
beforeEach(async () => {
dir = await mkdtemp(join(tmpdir(), "fusion-review-injection-"));
await mkdir(join(dir, "tasks", task.id), { recursive: true });
promptPath = join(dir, "tasks", task.id, "PROMPT.md");
await writeFile(promptPath, "# Task\n\nOriginal plan body.\n");
});
afterEach(async () => {
await rm(dir, { recursive: true, force: true });
});
describe("injectWorkflowStepFailureInstructions", () => {
it("renders findings grouped by priority with per-group obligations", async () => {
await injectWorkflowStepFailureInstructions(
store,
task,
"prose feedback",
"Code Review",
{ attempt: 1, max: 3 },
[
finding({ id: "a", title: "null deref", body: "crashes", severity: "critical", filePath: "src/a.ts", line: 9 }),
finding({ id: "b", title: "weak test", body: "no coverage", severity: "high" }),
finding({ id: "c", title: "naming", body: "minor", severity: "low" }),
],
);
const content = await readFile(promptPath, "utf-8");
expect(content).toContain("## Workflow Step Failure");
expect(content).toContain("### P0 — must fix");
expect(content).toContain("### P1 — should fix");
expect(content).toContain("### P2 — optional");
expect(content).toContain("src/a.ts:9");
// The decline path is what lets a disputed finding terminate instead of ping-ponging.
expect(content).toContain("a recorded decline is a valid resolution");
expect(content).toContain("P2 items are optional");
// Structured findings REPLACE the prose blob rather than appearing alongside it.
expect(content).not.toContain("prose feedback");
expect(content).toContain("Original plan body.");
});
it("falls back to prose feedback when the step produced no structured findings", async () => {
await injectWorkflowStepFailureInstructions(store, task, "prose feedback", "Verification", { attempt: 1, max: 2 });
const content = await readFile(promptPath, "utf-8");
expect(content).toContain("**Failure Feedback:**");
expect(content).toContain("prose feedback");
expect(content).not.toContain("### P0 — must fix");
});
it("replaces its own section instead of appending on a second remediation round", async () => {
await injectWorkflowStepFailureInstructions(store, task, "first", "Code Review", { attempt: 1, max: 3 });
await injectWorkflowStepFailureInstructions(store, task, "second", "Code Review", { attempt: 2, max: 3 });
const content = await readFile(promptPath, "utf-8");
expect(content.match(/## Workflow Step Failure/g)).toHaveLength(1);
expect(content).toContain("second");
expect(content).not.toContain("first");
});
});
describe("injectReviewAdvisoryNotes", () => {
it("labels downgraded findings as non-blocking and requiring no remediation", async () => {
await injectReviewAdvisoryNotes(store, task, "Plan Review", [
finding({ id: "a", title: "consider caching", body: "optional", severity: "medium" }),
]);
const content = await readFile(promptPath, "utf-8");
expect(content).toContain("## Review Advisory Notes");
expect(content).toContain("NON-BLOCKING");
expect(content).toContain("require no remediation round");
expect(content).toContain("### P2 — optional");
expect(content).toContain("Original plan body.");
});
it("replaces its own section across repeated reviews so PROMPT.md cannot grow without bound", async () => {
await injectReviewAdvisoryNotes(store, task, "Plan Review", [finding({ title: "first note", body: "x", severity: "low" })]);
await injectReviewAdvisoryNotes(store, task, "Plan Review", [finding({ title: "second note", body: "y", severity: "low" })]);
const content = await readFile(promptPath, "utf-8");
expect(content.match(/## Review Advisory Notes/g)).toHaveLength(1);
expect(content).toContain("second note");
expect(content).not.toContain("first note");
});
it("is a no-op when there are no advisory findings", async () => {
const before = await readFile(promptPath, "utf-8");
await injectReviewAdvisoryNotes(store, task, "Plan Review", []);
expect(await readFile(promptPath, "utf-8")).toBe(before);
});
it("does not throw when PROMPT.md is absent", async () => {
await rm(promptPath);
await expect(
injectReviewAdvisoryNotes(store, task, "Plan Review", [finding({ severity: "low" })]),
).resolves.toBeUndefined();
});
});

View File

@@ -0,0 +1,100 @@
import { describe, expect, it, vi } from "vitest";
import type { Task } from "@fusion/core";
import { sendTaskBackForFix } from "../executor/send-task-back-for-fix.js";
/*
FNXC:SessionResume 2026-08-10-17:33:
The remediation bounce used to null `sessionFile` unconditionally, so every review round-trip restarted
the implementation agent cold — it re-read the repository and re-derived the change it had just written,
once per round. Combined with unbounded review remediation that was the dominant cost of task wall-clock.
These pin the contract that `preserveResumeState` preserves the CONVERSATION, and that a caller which
explicitly opts out still gets a cold session.
*/
function task(overrides: Partial<Task> = {}): Task {
return {
id: "FN-RESUME",
worktree: "/tmp/fusion/fn-resume",
sessionFile: "/tmp/fusion/fn-resume/.session.jsonl",
...overrides,
} as Task;
}
function createDeps(live: Task) {
const store = {
addTaskComment: vi.fn().mockResolvedValue(undefined),
logEntry: vi.fn().mockResolvedValue(undefined),
updateTask: vi.fn().mockResolvedValue(undefined),
getTask: vi.fn().mockResolvedValue(live),
getSettings: vi.fn().mockResolvedValue({}),
};
return {
store: store as never,
getRunContextFor: () => undefined,
clearCompletedTaskWatchdog: vi.fn(),
injectWorkflowStepFailureInstructions: vi.fn().mockResolvedValue(undefined),
reopenLastStepForRevision: vi.fn().mockResolvedValue(undefined),
scheduleWorkflowRerun: vi.fn(),
maxWorkflowStepRetries: 3,
_store: store,
};
}
function sessionFilePatches(store: { updateTask: ReturnType<typeof vi.fn> }) {
return store.updateTask.mock.calls
.map(([, patch]) => patch)
.filter((patch: Record<string, unknown>) => patch && "sessionFile" in patch);
}
describe("sendTaskBackForFix session preservation", () => {
it("preserves sessionFile so a remediation round continues the implementation conversation", async () => {
const live = task();
const deps = createDeps(live);
await sendTaskBackForFix(deps as never, live, live.worktree!, "fix it", "Code Review", "revision requested", true);
// No patch may null the session — that is what forced a cold restart every round.
expect(sessionFilePatches(deps._store)).toEqual([]);
const statusPatch = deps._store.updateTask.mock.calls.find(([, patch]) => patch && "workflowStepRetries" in patch);
expect(statusPatch?.[1]).toEqual({ status: null, error: null, workflowStepRetries: 0 });
});
it("still clears sessionFile when the caller explicitly opts out of resume state", async () => {
const live = task();
const deps = createDeps(live);
await sendTaskBackForFix(deps as never, live, live.worktree!, "fix it", "Code Review", "revision requested", false);
expect(sessionFilePatches(deps._store)).toEqual([
{ sessionFile: null, status: null, error: null, workflowStepRetries: 0 },
]);
});
it("forwards structured findings to the PROMPT.md injection", async () => {
const live = task();
const deps = createDeps(live);
const findings = [{ id: "a", title: "t", body: "b", severity: "critical" as const }];
await sendTaskBackForFix(
deps as never,
live,
live.worktree!,
"fix it",
"Code Review",
"revision requested",
true,
false,
{ attempt: 1, max: 3 },
findings,
);
expect(deps.injectWorkflowStepFailureInstructions).toHaveBeenCalledWith(
live,
"fix it",
"Code Review",
{ attempt: 1, max: 3 },
findings,
);
});
});

View File

@@ -254,6 +254,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
true,
false,
{ attempt: 1, max: 3 },
undefined,
);
}
});
@@ -312,6 +313,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
true,
false,
{ attempt: 1, max: 2 },
undefined,
);
expect(store.updateTask.mock.invocationCallOrder[0]).toBeLessThan(sendBack.mock.invocationCallOrder[0]);
});
@@ -809,6 +811,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
true,
false,
{ attempt: count + 1, max: undefined },
undefined,
);
}
});
@@ -893,6 +896,51 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
true,
false,
{ attempt: 4, max: undefined },
undefined,
);
});
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
Self-healing recovery must forward the PERSISTED structured findings, not just the prose `output`.
Without this the implementer sees an undifferentiated blob on a restart-recovered bounce and cannot
tell a P0 from an optional note — the exact ambiguity that turned single REVISE verdicts into
multi-round negotiations.
*/
it("forwards persisted review findings into failed-step recovery remediation", async () => {
const store = createMockStore();
const findings = [
{ id: "f-blocking", title: "guard missing", body: "null deref", severity: "critical" as const },
{ id: "f-advisory", title: "naming", body: "minor", severity: "low" as const },
];
const liveTask = task({
column: "in-review",
workflowStepResults: [{
workflowStepId: "code-review",
workflowStepName: "Code Review",
phase: "pre-merge",
status: "failed",
output: "Fix the review finding.",
findings,
completedAt: new Date().toISOString(),
}],
});
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 3 });
const executor = new TaskExecutor(store, "/tmp/test");
const sendBack = vi.spyOn(executor as any, "sendTaskBackForFix").mockResolvedValue(undefined);
await expect(executor.recoverFailedPreMergeWorkflowStep(liveTask)).resolves.toBe(true);
expect(sendBack).toHaveBeenCalledWith(
liveTask,
liveTask.worktree,
"Fix the review finding.",
"Code Review",
expect.any(String),
true,
false,
expect.anything(),
findings,
);
});

View File

@@ -13,12 +13,15 @@ import type {
Settings,
Task,
TaskStore,
WorkflowReviewKind,
WorkflowStep,
} from "@fusion/core";
import {
applyReviewSeverityGate,
finalizePlanningSegment,
resolveExecutorFallbackModel,
resolvePersistAgentThinkingLog,
resolveReviewBlockingSeverity,
resolveValidatorFallbackModel,
startPlanningSegment,
} from "@fusion/core";
@@ -60,6 +63,8 @@ import {
filterCustomToolsForReadonly,
} from "../workflows/workflow-step-tool-policy.js";
import { executorLog } from "../logger.js";
import { mergeEffectiveSettings } from "../project/effective-settings.js";
import { injectReviewAdvisoryNotes } from "./workflow-step-failure-injection.js";
import { parseAwaitInputQuestionToolCall } from "./await-input-parse.js";
import {
augmentSessionSkillsForBrowserStep,
@@ -308,6 +313,32 @@ export async function executeWorkflowStep(
const isSummaryProjectionStep = (workflowStep as WorkflowStep & { summaryTarget?: string }).summaryTarget === "task";
const requireVerdict = !isSummaryProjectionStep && (workflowStep.gateMode === "gate" || !isSkillStep);
const reviewFindingsContract = workflowStepMetadata.reviewKind === "plan" || workflowStepMetadata.reviewKind === "code";
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Severity is now the gate input, not decoration — state the threshold in the prompt so the reviewer
* knows which classifications actually block. Telling it the exact rule is what makes the
* classification honest; when severity had no stated consequence, reviewers marked everything
* blocking and every nit forced a remediation round.
*
* `settings` here is the RAW project map: the graph run loads it via `store.getSettings()` and never
* merges per-workflow values (see execute-workflow-graph.ts). Reading the threshold off it directly
* would silently ignore an operator's Workflow Editor override and always use the built-in default.
* Merge the per-task effective workflow settings first, exactly as the remediation path does — the
* merge is scoped to review-kind nodes so non-review steps pay nothing.
*/
const reviewBlockingSeverity = reviewFindingsContract
? resolveReviewBlockingSeverity({
reviewKind: workflowStepMetadata.reviewKind as WorkflowReviewKind,
workflowSettings: await mergeEffectiveSettings(deps.store, task, settings)
.catch(() => settings) as unknown as Record<string, unknown>,
nodeBlockingSeverity: (workflowStep as WorkflowStep & { blockingSeverity?: unknown }).blockingSeverity,
})
: undefined;
const blockingSeverityRule = reviewBlockingSeverity === undefined || reviewBlockingSeverity === "any"
? ""
: reviewBlockingSeverity === "critical"
? "\n - REVISE requires at least one `critical` (P0) finding. A REVISE without one is recorded as APPROVE_WITH_NOTES and its findings are handed to the implementer without another review round."
: "\n - REVISE requires at least one `critical` (P0) or `high` (P1) finding. A REVISE without one is recorded as APPROVE_WITH_NOTES and its findings are handed to the implementer without another review round.";
const verdictBlock = requireVerdict
? `
@@ -323,7 +354,7 @@ export async function executeWorkflowStep(
- Output exactly one trailing JSON object and stop.
- verdict must be exactly APPROVE, APPROVE_WITH_NOTES, or REVISE.
- notes should be concise and actionable. Use an empty string when there are no notes.
- For out-of-scope fast-bail responses, use: {"verdict":"APPROVE","notes":"out of scope: no UI files changed"}
- For out-of-scope fast-bail responses, use: {"verdict":"APPROVE","notes":"out of scope: no UI files changed"}${reviewFindingsContract ? "\n - Every finding MUST carry a `severity`. Put each blocking issue in `findings` — prose in `notes` alone does not block." : ""}${blockingSeverityRule}
Backward compat fallback: if JSON is unavailable, you may still begin output with REQUEST REVISION to request changes.`
: `
@@ -358,8 +389,10 @@ export async function executeWorkflowStep(
Your role:
- Execute this workflow step exactly as scoped.
- Prioritize high-impact correctness/risk findings over stylistic nits.
- Report only what changes the delivered result. Do NOT report nits — wording, formatting, naming or ordering preferences, internal numbering/cross-reference mismatches that do not change what gets built, or detail any reasonable implementation choice would satisfy. Omit them entirely rather than filing them as low-severity findings.
- Assume the implementer is a competent engineer who resolves local detail correctly without being told.
- Keep feedback actionable and directly tied to evidence in files/outputs.
- Finding nothing is a valid and common outcome. Do not manufacture findings to justify the review.
Your Instructions:
${workflowStep.prompt}
@@ -744,15 +777,42 @@ export async function executeWorkflowStep(
? parseWorkflowStepOutput(output, { optionalGroupId })
: parseWorkflowStepOutput(output, { requireVerdict: false, optionalGroupId });
if (parsed.verdict) {
const revisionRequested = parsed.verdict === "REVISE";
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Apply the severity gate HERE, at the single parse boundary, so the rewritten verdict is what
* every downstream consumer sees (step-result status mapping, remediation routing, Review tab,
* merge blocking). Downgrading later would leave the persisted verdict disagreeing with the
* routing decision. Only a REVISE with no finding at or above the threshold is relaxed; a
* prose-only or unclassified REVISE still blocks (fail-closed contract in review-severity-gate.ts).
*/
const gated = reviewBlockingSeverity
? applyReviewSeverityGate({
verdict: parsed.verdict,
findings: parsed.findings,
threshold: reviewBlockingSeverity,
})
: undefined;
const effectiveVerdict = (gated?.verdict ?? parsed.verdict) as typeof parsed.verdict;
if (gated?.downgraded) {
await deps.store.logEntry(
task.id,
`[pre-merge] ${workflowStep.name} returned REVISE with no finding at or above "${reviewBlockingSeverity}" — recorded as APPROVE_WITH_NOTES; ${gated.advisory.length} advisory finding(s) handed to the implementer.`,
);
// Non-fatal: losing the advisory carry-forward must never fail the step itself.
await injectReviewAdvisoryNotes(deps.store, task, workflowStep.name, gated.advisory).catch((err: unknown) => {
const msg = err instanceof Error ? err.message : String(err);
executorLog.warn(`${task.id}: failed to carry forward advisory review findings: ${msg}`);
});
}
const revisionRequested = effectiveVerdict === "REVISE";
if (workflowStep.requiresBrowser === true) {
await logBrowserVerificationActivity(`[browser-verification] finished browser verification for task ${task.id}: verdict ${parsed.verdict}`);
await logBrowserVerificationActivity(`[browser-verification] finished browser verification for task ${task.id}: verdict ${effectiveVerdict}`);
}
return {
success: !revisionRequested,
revisionRequested,
output: parsed.output,
verdict: parsed.verdict,
verdict: effectiveVerdict,
notes: parsed.notes,
...(parsed.findings ? { findings: parsed.findings } : {}),
};

View File

@@ -49,6 +49,7 @@ export type RecoverFailedPreMergeStepDeps = {
preserveResumeState?: boolean,
mergeVerificationFailure?: boolean,
retryPresentation?: { attempt: number; max?: number },
findings?: CoreWorkflowStepResult["findings"],
) => Promise<void>;
};
@@ -131,6 +132,14 @@ export async function recoverFailedPreMergeWorkflowStep(
true,
false,
{ attempt: budget.attempts + 1, max: budget.unbounded ? undefined : budget.max },
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Self-healing recovery of a failed pre-merge review step is the SAME remediation the executor
* schedules inline, so it must hand the implementer the same priority-grouped findings. Reading
* them off the persisted step result (rather than re-deriving from prose) is what keeps a
* restart-recovered bounce indistinguishable from a live one.
*/
target.findings,
);
return true;
} catch (err: unknown) {

View File

@@ -25,7 +25,7 @@
* FNXC:RemediationVisibility 2026-07-26-19:20:
* Unscheduled remediation (zero budget, non-REVISE hard fail) must log loudly, never silently park.
*/
import type { Task, TaskStore, WorkflowStepResult as CoreWorkflowStepResult } from "@fusion/core";
import type { Task, TaskStore, WorkflowReviewFinding, WorkflowStepResult as CoreWorkflowStepResult } from "@fusion/core";
import {
DEFAULT_MAX_POST_REVIEW_FIXES,
hasPreMergeRemediationAutoMergeHold,
@@ -60,6 +60,12 @@ export type RequestPreMergeOptionalStepFixInfo = {
failureValue?: string;
nodeId?: string;
maxRevisions?: unknown;
/**
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Structured findings from a review-kind gate, carried so remediation can present them grouped by
* priority instead of as one undifferentiated prose blob. Absent for prose-only / non-review steps.
*/
findings?: WorkflowReviewFinding[];
};
export type RequestPreMergeOptionalStepFixDeps = {
@@ -87,6 +93,7 @@ export type RequestPreMergeOptionalStepFixDeps = {
preserveResumeState: boolean,
mergeVerificationFailure: boolean,
retryPresentation?: { attempt: number; max?: number },
findings?: WorkflowReviewFinding[],
) => Promise<void>;
};
@@ -322,6 +329,7 @@ export async function requestPreMergeOptionalStepFix(
true,
false,
{ attempt: nextCount, max: budget.unbounded ? undefined : budget.max },
info.findings,
);
return true;
}

View File

@@ -1444,6 +1444,7 @@ export async function runImplementation(
deps.clearCompletedTaskWatchdog(task.id);
executorLog.log(`✓ ${task.id} implementation complete — graph interpreter owns the remaining lifecycle`);
const liveModified = (await deps.store.getTask(task.id).catch(() => task)).modifiedFiles ?? [];
handedOffForReview = true;
reportImplementationExit?.("complete-from-live-files");
graphCompletion({ modifiedFiles: liveModified });
return;
@@ -1666,6 +1667,22 @@ export async function runImplementation(
const codeReviewVerdicts = new Map<number, ReviewVerdict>();
let wasPaused = false;
/*
FNXC:SessionResume 2026-08-10-17:33:
Set when the run ends by handing the COMPLETED implementation to the graph for review, rather than
by finishing or failing the task. The distinction matters because a review gate can bounce the card
straight back here for remediation in the SAME worktree: before this flag the `finally` below nulled
`sessionFile` on every non-paused exit, so pause -> unpause was the only path that ever resumed a
conversation and every remediation round restarted cold — re-reading the repo and re-deriving the
change it had just written, once per round. Preserving the session across the review round-trip is
what makes a bounce a follow-up turn instead of a fresh investigation.
Scoped deliberately to the handoff exits, NOT to every non-terminal exit: paths that require a fresh
session (context overflow, stale assistant continuation, worktree reacquisition, non-continuable
session, task-done retry) clear `sessionFile` explicitly and synchronously at their own site, and the
resume guard re-validates the persisted worktree before reopening. Those defenses stay authoritative.
*/
let handedOffForReview = false;
// Mutable ref — populated after createFnAgent, tools access lazily via closure
const sessionRef: { current: AgentSession | null } = { current: null };
/*
@@ -2233,12 +2250,26 @@ export async function runImplementation(
executorLog.debug(`${task.id}: calling promptWithFallback()...`);
if (isResuming) {
// Session already has full conversation history — just tell the
// agent it was paused and should pick up where it left off.
/*
* Session already has full conversation history — re-prompt with a short continuation
* instead of the full execution prompt.
*
* FNXC:SessionResume 2026-08-10-17:33:
* A resume is no longer only a pause/unpause: a review gate can bounce a completed
* implementation back here for remediation with the same session. The remediation findings
* are written into PROMPT.md (`## Workflow Step Failure`) by sendTaskBackForFix AFTER this
* conversation's last turn, so the agent has never seen them. This prompt must therefore
* direct a re-read of PROMPT.md unconditionally — the previous wording ("you were paused,
* pick up where you left off") would silently skip the findings and the remediation round
* would do nothing, bouncing again on the next review.
*/
await promptWithFallback(session, [
"Your session was paused and has now been resumed.",
"Continue working on the task from where you left off.",
"Review the current state of your worktree and proceed with the next pending step.",
"Your session was resumed.",
"PROMPT.md may have been UPDATED since your last turn — re-read it now before doing anything else.",
"If it contains a `## Workflow Step Failure` section, a review gate requested changes: address those findings. Fix every P0; fix P1 unless you have a concrete reason not to, and say which you declined and why. P2 items are optional.",
"If it contains a `## Review Advisory Notes` section, those are non-blocking suggestions — address them only if cheap and clearly correct.",
"Otherwise continue the task from where you left off.",
"Review the current state of your worktree, then proceed with the next pending step.",
].join("\n"));
} else {
const customFieldDefs = await deps.resolveTaskCustomFieldDefs(task.id);
@@ -2431,6 +2462,7 @@ export async function runImplementation(
// at the implementation-complete boundary and hand control back.
deps.clearCompletedTaskWatchdog(task.id);
executorLog.log(`✓ ${task.id} implementation complete — graph interpreter owns the remaining lifecycle`);
handedOffForReview = true;
reportImplementationExit?.("complete");
graphCompletion({ modifiedFiles });
return;
@@ -2507,6 +2539,7 @@ export async function runImplementation(
deadlocks the merge queue) now lives with the node in the IR, where the routing
decision is.
*/
handedOffForReview = true;
reportImplementationExit?.("review-handoff-pending-review");
pendingReviewParked = true;
break;
@@ -2726,6 +2759,7 @@ export async function runImplementation(
// executeWorkflowGraph, KTD-5) — nothing to gate before handoff.
deps.clearCompletedTaskWatchdog(task.id);
executorLog.log(`✓ ${task.id} implementation complete (retry) — graph interpreter owns the remaining lifecycle`);
handedOffForReview = true;
reportImplementationExit?.("complete-after-retry");
graphCompletion({ modifiedFiles });
return;
@@ -2810,11 +2844,19 @@ export async function runImplementation(
session.dispose();
// Terminate all spawned child agents when parent session ends
await deps.terminateAllChildren(task.id);
// Clear session file when task completes or fails (not when paused —
// the file is preserved so unpause can resume the conversation).
// Check both the local flag (graceful exit) and the instance set
// (error path where dispose caused prompt to throw).
if (!wasPaused && !deps.pausedAborted.has(task.id)) {
/*
* Clear session file when task completes or fails (not when paused —
* the file is preserved so unpause can resume the conversation).
* Check both the local flag (graceful exit) and the instance set
* (error path where dispose caused prompt to throw).
*
* FNXC:SessionResume 2026-08-10-17:33:
* Also preserved across a review handoff (`handedOffForReview`): a review gate may bounce the
* card back here for remediation in the same worktree, and that round should continue the
* conversation instead of re-deriving the change from scratch. See the flag's declaration for
* why this is scoped to the handoff exits rather than to every non-terminal exit.
*/
if (!wasPaused && !handedOffForReview && !deps.pausedAborted.has(task.id)) {
deps.store.updateTask(task.id, { sessionFile: null }).catch((err: unknown) => {
const msg = err instanceof Error ? err.message : String(err);
executorLog.warn(`${task.id} failed to clear sessionFile: ${msg}`);

View File

@@ -6,7 +6,7 @@
* FNXC:ExternalExecutionCheckout 2026-08-09-22:43:
* Remediation reuses the live external checkout path and must not persist it as task.worktree.
*/
import type { Task, TaskStore } from "@fusion/core";
import type { Task, TaskStore, WorkflowReviewFinding } from "@fusion/core";
import type { EngineRunContext } from "../util/run-audit.js";
import { resolveAuthoritativeExternalExecutionRoute } from "./resolve-authoritative-external-execution-route.js";
@@ -19,6 +19,7 @@ export type SendTaskBackForFixDeps = {
failureFeedback: string,
stepName: string,
retry: { attempt: number; max?: number },
findings?: WorkflowReviewFinding[],
) => Promise<void>;
reopenLastStepForRevision: (
taskId: string,
@@ -44,6 +45,7 @@ export async function sendTaskBackForFix(
preserveResumeState: boolean = true,
mergeVerificationFailure: boolean = false,
retryPresentation?: { attempt: number; max?: number },
findings?: WorkflowReviewFinding[],
): Promise<void> {
const taskId = task.id;
deps.clearCompletedTaskWatchdog(taskId);
@@ -92,6 +94,7 @@ export async function sendTaskBackForFix(
failureFeedback,
stepName,
retryPresentation ?? { attempt: deps.maxWorkflowStepRetries, max: deps.maxWorkflowStepRetries },
findings,
);
// 4. Re-open only the last step for a single in-place fix pass. Earlier
@@ -107,10 +110,22 @@ export async function sendTaskBackForFix(
// task is still in-review would drop the merge blocker during the async
// bounce window and let a concurrent auto-merge sweep merge an
// empty-`steps` graph-native task with the gate failure unaddressed.
/*
FNXC:SessionResume 2026-08-10-17:33:
`preserveResumeState` now also preserves the CONVERSATION, not just step progress. Previously this
unconditionally nulled `sessionFile`, so every remediation round re-read the repository and re-derived
the change it had just written. The remediation instructions live in PROMPT.md, and the resume prompt
in run-implementation.ts directs the agent to re-read it, so a resumed session sees the new findings as
a follow-up turn — which is how a review round-trip actually works.
A caller that explicitly does NOT preserve resume state still gets a cold session, and the resume guard
re-validates the persisted worktree before reopening, so a remediation routed to a different checkout
(external execution route) starts fresh rather than resuming against the wrong tree.
*/
await deps.store.updateTask(taskId, {
status: mergeVerificationFailure ? "merging-fix" : null,
error: null,
sessionFile: null,
...(preserveResumeState ? {} : { sessionFile: null }),
workflowStepRetries: 0,
});

View File

@@ -5,18 +5,77 @@
*/
import { join } from "node:path";
import { readFile, writeFile } from "node:fs/promises";
import type { Task, TaskStore } from "@fusion/core";
import type { Task, TaskStore, WorkflowReviewFinding } from "@fusion/core";
import { formatFindingsByPriority } from "@fusion/core";
import { executorLog } from "../logger.js";
import { buildWorkflowFailureScopeGuard } from "./workflow-failure-scope-guard.js";
export type WorkflowStepFailureInjectionStore = Pick<TaskStore, "getFusionDir">;
/*
FNXC:ReviewSeverityGate 2026-08-10-17:33:
Structured findings are rendered grouped by priority with an EXPLICIT obligation per group, replacing a
flat prose blob that gave the implementer no way to tell a blocking defect from an optional note. The
implementer needs the distinction to converge: without it, every remediation round tried to satisfy every
observation, which is what turned a single REVISE into a multi-round negotiation. `findings` is optional
so prose-only reviewers (custom nodes, older workflows) keep working unchanged.
*/
const ADVISORY_SECTION_HEADER = "## Review Advisory Notes";
/**
* Write non-blocking review findings into PROMPT.md.
*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* When the severity gate downgrades a REVISE, the task proceeds — but the findings must not silently
* vanish into the Review tab, or the gate would trade churn for lost signal. Plan Review is the
* load-bearing case: its downgrade happens BEFORE implementation, so these notes reach the implementer
* as optional context on the very next run. The section is replaced (not appended) on each write so
* repeated reviews cannot grow PROMPT.md without bound, and it is explicitly labeled non-blocking so
* the implementer does not treat it as a remediation obligation.
*/
export async function injectReviewAdvisoryNotes(
store: WorkflowStepFailureInjectionStore,
task: Task,
stepName: string,
findings: WorkflowReviewFinding[],
): Promise<void> {
if (findings.length === 0) return;
const promptPath = join(store.getFusionDir(), "tasks", task.id, "PROMPT.md");
let content: string;
try {
content = await readFile(promptPath, "utf-8");
} catch {
executorLog.warn(`${task.id}: PROMPT.md not found at ${promptPath}, skipping review advisory injection`);
return;
}
const section = `${ADVISORY_SECTION_HEADER}
${stepName} raised the following NON-BLOCKING observations. They did not block this task and require no remediation round. Address them only if cheap and clearly correct while doing the work you were already going to do; skipping them is expected and needs no justification.
${formatFindingsByPriority(findings)}
`;
const sectionRegex = new RegExp(`${ADVISORY_SECTION_HEADER}[\\s\\S]*?(?=\\n## |\\n# |$)`, "i");
const newContent = sectionRegex.test(content) ? content.replace(sectionRegex, section) : `${content}\n${section}`;
try {
await writeFile(promptPath, newContent);
executorLog.log(`${task.id}: injected ${findings.length} advisory review finding(s) into PROMPT.md`);
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
executorLog.error(`${task.id}: failed to inject review advisory notes: ${errorMessage}`);
}
}
export async function injectWorkflowStepFailureInstructions(
store: WorkflowStepFailureInjectionStore,
task: Task,
failureFeedback: string,
stepName: string,
retry: { attempt: number; max?: number },
findings?: WorkflowReviewFinding[],
): Promise<void> {
const promptPath = join(store.getFusionDir(), "tasks", task.id, "PROMPT.md");
@@ -33,20 +92,33 @@ export async function injectWorkflowStepFailureInstructions(
const remainingRetries = retry.max === undefined ? "unlimited" : String(Math.max(0, retry.max - retry.attempt));
const failureSectionHeader = "## Workflow Step Failure";
const scopeGuard = buildWorkflowFailureScopeGuard(task, content);
const prioritized = findings?.length ? formatFindingsByPriority(findings) : "";
const feedbackBlock = prioritized
? `**Findings:**
${prioritized}`
: `**Failure Feedback:**
${failureFeedback}`;
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* The closing instruction sanctions an explicit DECLINE with rationale. Previously the only sanctioned
* response was "fix the issues", so an implementer that disagreed with a finding had no way to close
* the loop except to comply or stall — and a reviewer re-raising the same disputed point produced an
* unbounded ping-pong. A recorded decline is a terminal answer the next review round can accept.
*/
const failureSectionContent = `${failureSectionHeader}
The following workflow step failed and requires implementation fixes:
The following workflow step returned findings that require implementation fixes:
**Step:** ${stepName}
**Failure Feedback:**
${failureFeedback}
${feedbackBlock}
${scopeGuard}
**Retry:** ${retry.attempt}/${retryLabel} (${remainingRetries} remaining)
**Important:** This is a workflow step failure — fix the issues above by making the necessary code changes. The task has been sent back to in-progress for remediation. The executor will attempt to fix the issues on the next pass.
**Important:** This is a workflow step failure — address the findings above by making the necessary code changes. The task has been sent back to in-progress for remediation. Fix every P0. Fix P1 unless you have a concrete reason not to; if you decline one, state which and why in your summary — a recorded decline is a valid resolution and the next review round will treat it as settled. P2 items are optional and need no justification if skipped. Do not make unrelated changes while remediating.
`;

View File

@@ -1257,6 +1257,13 @@ export class WorkflowGraphExecutor {
...(parseRequiredArtifactMissingValue(verdictRaw) ? { failureValue: verdictRaw } : {}),
nodeId: node.id,
maxRevisions: node.config?.maxRevisions,
/*
* FNXC:ReviewSeverityGate 2026-08-10-17:33:
* Carry the structured findings into remediation so PROMPT.md can present them grouped by
* priority. Without this the implementer only ever saw `feedback` prose and could not tell
* a blocking defect from an optional note.
*/
...(stepFindings?.length ? { findings: stepFindings } : {}),
};
context[optionalStepFailureContextKey(node.id)] = failureContext;
/*