diff --git a/.changeset/blocking-gate-requires-verdict.md b/.changeset/blocking-gate-requires-verdict.md new file mode 100644 index 0000000000..67602de5ae --- /dev/null +++ b/.changeset/blocking-gate-requires-verdict.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A blocking review gate no longer approves when the reviewer never returned a usable verdict. +category: fix +dev: Restores FN-6582's blocking-gate rule, reversing the later relaxation that treated malformed gate output as a non-blocking advisory. `executeWorkflowStep` already restarts cleanly twice on malformed output (fallback-model retry, or a self-retry on the primary when no fallback is configured), so `malformed` reaching the graph decision means the reviewer failed across every attempt — the LLM-class condition an operator accepts as a legitimate stop, and never grounds to record approval. Measured cost of the relaxation: a reviewer reported in prose that the deliverables were absent, carried no verdict JSON, and the gate recorded success, merging unreviewed work on a rejection nobody could see. A prose classifier cannot close this — that text contained no rejection marker at all — so only the absence of a verdict is detectable and absence must not approve. Advisory gates keep the relaxation: a step that was never allowed to hold a card does not start holding one. `runGraphCustomNode` now maps `success || (!blocking && verdict !== "UNAVAILABLE")`, and the malformed→block assertion the relaxation deleted is restored. diff --git a/packages/engine/src/__tests__/workflow-malformed-verdict-gate.test.ts b/packages/engine/src/__tests__/workflow-malformed-verdict-gate.test.ts index 1d331bd979..c8beaee8a9 100644 --- a/packages/engine/src/__tests__/workflow-malformed-verdict-gate.test.ts +++ b/packages/engine/src/__tests__/workflow-malformed-verdict-gate.test.ts @@ -10,7 +10,9 @@ FNXC:WorkflowGates 2026-06-17-18:27: FN-6582 requires malformed workflow-step verdicts to remain explicit failures for blocking gates while advisory gates may record a non-blocking advisory failure. These tests pin the shared imperative parser seam and the graph handler path so malformed output cannot be mistaken for APPROVE. FNXC:ReviewLeniency 2026-07-02-00:30: -POLICY CHANGE (operator request): malformed gate output (no parseable verdict, even after the executeWorkflowStep fallback-model retry) is now treated as a NON-BLOCKING advisory, relaxing the FN-6582 hard block. The real mapping lives in runGraphCustomNode (`outcome: success || !blocking || malformed ? "success" : "failure"`). A genuine PARSED non-pass verdict (REVISE) still blocks. The parser seam still classifies unparseable text as `malformed` (it is NOT silently promoted to APPROVE) — only the downstream blocking decision was relaxed. These handler/executor tests mock the node result to pin the graph PLUMBING for a genuine-failure verdict; they intentionally do not re-assert a malformed→block mapping that no longer exists. +POLICY CHANGE (operator request): malformed gate output was treated as a NON-BLOCKING advisory, relaxing the FN-6582 hard block, and the malformed→block assertion was removed with it. + +POLICY CHANGE REVERSED (operator request, 2026-08-26): a BLOCKING gate no longer approves on malformed output, and the assertion is restored below. The stated rule is that the only legitimate stop is an LLM problem; since `executeWorkflowStep` already retries a malformed response (fallback model, or a self-retry on the primary when none is configured), reaching this decision means the reviewer failed across every attempt — which is that LLM-class condition, and is never grounds to record approval. Advisory gates keep the relaxation, because a step that was never allowed to hold a card must not start holding one. The real mapping lives in runGraphCustomNode (`outcome: success || (!blocking && verdict !== "UNAVAILABLE") ? "success" : "failure"`). */ const task = { id: "FN-6582" } as TaskDetail; @@ -114,6 +116,47 @@ describe("workflow malformed-verdict gate", () => { expect(result.contextPatch).toEqual({ "workflow:gate:malformed": true, "workflow:gate:advisory": true }); }); + /* + FNXC:ReviewLeniency 2026-08-26-09:34: + FN-6582's blocking-gate rule is RESTORED, and this is the test the relaxation deliberately removed. + + Operator decision, with the reason the first reversal lacked: "the only valid reason a task can be + blocked is an LLM problem (429, 503); everything else is fixed at the source, or the AI is made + unable to return anything other than what is expected — and if it does anyway, restart cleanly". + + Restarting cleanly already happens twice inside `executeWorkflowStep` (fallback-model retry, or a + self-retry on the primary when no fallback is configured), so `malformed` reaching this decision + means the reviewer failed across every attempt — exactly the LLM-class condition an operator accepts + as a legitimate stop, and never a reason to record approval. + + Measured cost of the relaxation: a reviewer reported in prose that the deliverables were absent, + carried no verdict JSON, and the gate recorded success — unreviewed work merged on a rejection + nobody could see. A prose classifier cannot close this; that text held no rejection marker at all, + because it was a factual statement of absence. Only the ABSENCE of a verdict is detectable. + */ + it("keeps a blocking gate from passing on malformed output", async () => { + const malformed = parseWorkflowStepOutput( + "The requested repo1.txt and repo2.txt files are not present in the worktree. No modified files were detected.", + ); + // The reviewer's text carries no rejection marker at all — absence of a verdict is the only signal. + expect(malformed.malformed).toBe(true); + expect(malformed.verdict).toBeUndefined(); + + const handlers = createDefaultNodeHandlers(noopSeams(), async () => ({ + outcome: "failure", + value: "advisory_failure", + contextPatch: { "workflow:gate:malformed": true }, + })); + + const result = await handlers.gate( + { id: "code-review-step", kind: "gate", config: { prompt: "Return APPROVE or REVISE", gateMode: "gate" } }, + { task, settings: undefined, context: {} }, + ); + + expect(result.outcome, "a blocking gate must not approve without a usable verdict").toBe("failure"); + expect(result.value).toBe("advisory_failure"); + }); + it("terminates a graph run as failed when a blocking gate returns REVISE", async () => { const revise = parseWorkflowStepOutput("REQUEST REVISION\nfix the gate"); const executor = new WorkflowGraphExecutor({ diff --git a/packages/engine/src/executor/run-graph-custom-node.ts b/packages/engine/src/executor/run-graph-custom-node.ts index 171e6dec01..b687ee5aae 100644 --- a/packages/engine/src/executor/run-graph-custom-node.ts +++ b/packages/engine/src/executor/run-graph-custom-node.ts @@ -815,8 +815,30 @@ export async function runGraphCustomNode( const malformed = (outcome as { malformed?: boolean }).malformed === true; const advisoryFailureValue = malformed ? "advisory_failure" : "failed"; /* - FNXC:ReviewLeniency 2026-07-02-00:30: - Malformed review output (no parseable verdict, even after the fallback-model retry in executeWorkflowStep) is treated as a NON-BLOCKING advisory rather than a hard gate failure. Operators asked that an unparseable reviewer response not block a task in review — a genuine REVISE (parsed verdict) still blocks, and the advisory_failure value keeps the malformed result visible on the Workflow tab. Only `malformed` relaxes a gate; every parsed non-pass verdict continues to block exactly as before. + FNXC:ReviewLeniency 2026-07-02-00:30 (SUPERSEDED for blocking gates — see below): + Malformed review output (no parseable verdict, even after the fallback-model retry in executeWorkflowStep) was treated as a NON-BLOCKING advisory rather than a hard gate failure. Operators asked that an unparseable reviewer response not block a task in review — a genuine REVISE (parsed verdict) still blocks, and the advisory_failure value keeps the malformed result visible on the Workflow tab. + + FNXC:ReviewLeniency 2026-08-26-09:34: + A BLOCKING gate no longer approves on malformed output. Operator decision, reversing the line + above with the reason it was missing: "the only valid reason a task can be blocked is an LLM + problem (429, 503); everything else is fixed at the source, or the AI is made unable to return + anything other than what is expected — and if it does anyway, restart cleanly". + + Restarting cleanly is ALREADY implemented, twice: `executeWorkflowStep` retries a malformed + primary on the fallback model, or self-retries once on the primary when no fallback is + configured. `malformed` therefore does not mean "one fumbled response" — it means the reviewer + failed to return a usable verdict across every attempt, which IS the LLM-class condition the + operator accepts as a legitimate stop. + + What it must never mean is APPROVAL. Measured on a real card: a reviewer reported in prose that + the deliverables were absent, carried no verdict JSON, and the gate recorded success — unreviewed + work merged on a rejection nobody could see. The prose classifier cannot close this: that text + contained no rejection marker at all (no "revise", "reject", "must fix"), because it was a + factual statement of absence. Only the ABSENCE of a verdict is detectable, so absence must not + approve. + + Advisory gates are untouched: `!blocking` still passes, keeping the original operator ask exactly + where it applies — a step that was never allowed to hold a card cannot start holding one. */ return { /* @@ -825,7 +847,7 @@ export async function runGraphCustomNode( UNAVAILABLE result is never a pass. Returning success here would persist it as passed and admit an obsolete Code Review edge. */ - outcome: outcome.success || ((!blocking || malformed) && verdict !== "UNAVAILABLE") ? "success" : "failure", + outcome: outcome.success || (!blocking && verdict !== "UNAVAILABLE") ? "success" : "failure", value: (outcome as WorkflowStepOutcome).failureValue ?? verdict ?? (outcome.success ? "passed" : advisoryFailureValue), ...(Object.keys(contextPatch).length > 0 ? { contextPatch } : {}), };