fix(FN-WF): a blocking gate must not approve without a usable verdict
Restores FN-6582's rule, which a later operator request had relaxed — deleting its test
along with it.
Operator decision, now carrying the reason the first reversal lacked: the only legitimate
reason to stop a task is an LLM problem; everything else is fixed at the source, or the AI
is made unable to return anything but what is expected — and if it does anyway, restart
cleanly.
Restarting cleanly already happens, twice, inside executeWorkflowStep: a malformed primary
retries on the fallback model, or self-retries once on the primary when no fallback is
configured. So `malformed` reaching this decision does not mean "one fumbled response" — it
means the reviewer failed to return a usable verdict across every attempt. That IS the
LLM-class condition an operator accepts as a legitimate stop. What it must never mean is
approval.
Measured: 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 ("revise",
"reject", "must fix" all absent) because it was a factual statement of absence. Only the
ABSENCE of a verdict is detectable, so absence must not approve.
Advisory gates keep the relaxation: a step that was never allowed to hold a card does not
start holding one, which is where the original operator ask actually applies.
pnpm lint 0 errors, test:gate green, engine typecheck clean, pipeline-smoke 90/90.
This commit is contained in:
7
.changeset/blocking-gate-requires-verdict.md
Normal file
7
.changeset/blocking-gate-requires-verdict.md
Normal file
@@ -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.
|
||||
@@ -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({
|
||||
|
||||
@@ -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 } : {}),
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user