fix(FN-WF): make Documentation a reporter, and refuse every bounce with no work

Observed on a live card (mult-021), where the log tells the whole story: Documentation
returned an advisory REVISE asking for implementation work, the card was "moved back to
in-progress for remediation", and 467ms later Code Review started again. No step was ever
created, no executor session ran, and the demand was never implemented — the card merged
when the second Documentation pass happened to pass.

Two separate defects produced that.

FIRST, the reporter could hold the merge. An advisory REVISE records `advisory_failure`,
and `resolveRequiredPreMergeStepIds` included the Documentation group, so
`evaluatePreMergeApprovals` read it as "not-approved". `gateMode: "advisory"` only stops
the node blocking traversal; it says nothing to the merge door.

SECOND, the reporter could bounce. `requestPreMergeOptionalStepFix` accepts
`advisory_failure`, and under this workflow's named-remediation policy the resulting
`sendTaskBackForFix` reopens NOTHING. With no pending step the foreach answered
`already-expanded` and the walk replayed the review lane over an unchanged tree. The
budget was 1/10, so it could have burned ten rounds of two model calls each.

New opt-in `reportingOnly` on an optional group states the contract once — no approval to
withhold, no remediation to request — and both doors read it. It is set only on
Documentation, so advisory gates that DO own remediation (browser verification) keep their
behaviour exactly.

Plus the general invariant that would have caught both: under `stepReopenPolicy: "none"`,
a bounce that appended no named steps is refused and logged on the card. Only the gates
that can APPEND work may send a card back. Code Review REVISE and the deterministic
verification failure still produce named fix steps — unchanged, still covered.

pnpm lint 0 errors, test:gate green, core + engine typecheck clean, pipeline-smoke 90/90.
This commit is contained in:
Fusion Agent
2026-08-26 06:54:08 +00:00
parent 8328b458b0
commit 56ee1622df
13 changed files with 247 additions and 8 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Documentation now only documents — it can no longer hold a merge or send a card back with nothing to do.
category: fix
dev: Observed on a live card: the Documentation milestone returned an advisory REVISE, which recorded `advisory_failure`. `resolveRequiredPreMergeStepIds` included the group, so `evaluatePreMergeApprovals` read it as "not-approved" and held the merge door; the same REVISE also reached `requestPreMergeOptionalStepFix`, which bounced the card to `in-progress` where `sendTaskBackForFix` reopens nothing under the named-remediation policy — no pending step, foreach `already-expanded`, Code Review replayed over an unchanged tree, and the card merged when the second Documentation pass happened to pass. New opt-in `WorkflowOptionalGroupConfig.reportingOnly`, surfaced on `ResolvedWorkflowOptionalStep` and set only on `documentationDeliveryOptionalGroupNode`, excludes a reporting group from the required pre-merge approval set and refuses executor remediation for it. A general guard now also refuses any `stepReopenPolicy: "none"` bounce that appended no named steps, logging it on the card instead of looping. Code Review REVISE and the deterministic verification failure keep producing named fix steps; advisory gates that own remediation (browser verification) are untouched.

View File

@@ -139,6 +139,36 @@ describe("builtin:coding-ideas-v2", () => {
}
});
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
The advisory failure edge above was NOT enough, measured on a real card: Documentation's REVISE
recorded `advisory_failure`, which the required-approval set read as "no current approval" — so the
reporter held the merge door shut — while the same REVISE also bounced the card to implementation.
`reportingOnly` states the contract once, and BOTH doors read it.
*/
it("cannot hold the merge: Documentation carries no approval", () => {
const documentation = BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === "documentation-delivery");
expect(documentation?.config?.reportingOnly).toBe(true);
const required = resolveRequiredPreMergeStepIds(BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR, undefined);
expect([...required].sort()).toEqual(["code-review", "plan-review"]);
expect(required.has("documentation-delivery"), "a reporter must never gate the merge").toBe(false);
// Enabling it explicitly must not turn it into a gate either.
expect(resolveRequiredPreMergeStepIds(
BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR,
["plan-review", "code-review", "documentation-delivery"],
).has("documentation-delivery")).toBe(false);
// The gates that DO carry approval are untouched, here and on the inherited board.
expect([...resolveRequiredPreMergeStepIds(BUILTIN_CODING_IDEAS_WORKFLOW_IR, undefined)].sort())
.toEqual(["code-review", "plan-review"]);
for (const groupId of ["plan-review", "code-review"]) {
expect(BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.nodes.find((node) => node.id === groupId)?.config?.reportingOnly)
.toBeUndefined();
}
});
it("returns a rejected review to in-progress as named work", () => {
expect(BUILTIN_CODING_IDEAS_V2_WORKFLOW_IR.edges).toEqual(expect.arrayContaining([
{ from: "code-review", to: "code-review-remediation", condition: "failure" },

View File

@@ -293,6 +293,7 @@ export {
resolveWorkflowOptionalSteps,
resolveDefaultOnOptionalGroupIds,
isWorkflowOptionalGroupEnabled,
isReportingOnlyOptionalGroup,
} from "./workflows/workflow-optional-steps.js";
export type { ResolvedWorkflowOptionalStep } from "./workflows/workflow-optional-steps.js";
export { resolveRequiredPreMergeStepIds, resolvePreMergeGateForTask } from "./merge/required-pre-merge-steps.js";

View File

@@ -349,6 +349,7 @@ export {
resolveWorkflowOptionalSteps,
resolveDefaultOnOptionalGroupIds,
isWorkflowOptionalGroupEnabled,
isReportingOnlyOptionalGroup,
} from "./workflows/workflow-optional-steps.js";
export type { ResolvedWorkflowOptionalStep } from "./workflows/workflow-optional-steps.js";
export { resolveRequiredPreMergeStepIds, resolvePreMergeGateForTask } from "./merge/required-pre-merge-steps.js";

View File

@@ -23,6 +23,14 @@ export function resolveRequiredPreMergeStepIds(
return new Set(
resolveWorkflowOptionalSteps(ir)
.filter((step) => step.phase === "pre-merge")
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
A reporting group records what it observed; it carries no approval, so requiring one from it can
only produce a false blocker. Measured: the Documentation milestone returned an advisory REVISE,
which records `advisory_failure`, which this set turned into "task has enabled pre-merge
workflow steps without a current approval" — a reporter holding the merge door shut.
*/
.filter((step) => !step.reportingOnly)
.filter((step) => isWorkflowOptionalGroupEnabled(enabledWorkflowSteps, step.templateId, step.defaultOn))
.map((step) => step.templateId),
);

View File

@@ -32,6 +32,15 @@ export function documentationDeliveryOptionalGroupNode(column: string): Workflow
config: {
name: "Documentation",
defaultOn: true,
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
Documentation ONLY documents. `gateMode: "advisory"` on the inner node was not enough and the
gap was measured on a real card: its REVISE recorded `advisory_failure`, which held the merge
door ("no current approval") AND bounced the card to implementation with no work to do, where
it re-ran Code Review against an unchanged tree and merged on the second pass by luck.
`reportingOnly` states the contract once: no approval to withhold, no remediation to request.
*/
reportingOnly: true,
template: {
nodes: [{
id: "documentation-delivery-step",

View File

@@ -261,6 +261,22 @@ export interface WorkflowOptionalGroupConfig {
`WorkflowStepResult.phase` and `[post-merge]` logs follow this value.
*/
phase?: "pre-merge" | "post-merge";
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
A REPORTING group observes accepted work and records what it found. It carries no approval and can
never hold or reopen a card, so it is excluded from the required pre-merge approval set and may
not schedule executor remediation.
`gateMode: "advisory"` on the inner node was NOT enough, and the gap was measured on a real card:
an advisory REVISE records `advisory_failure`, which `evaluatePreMergeApprovals` reads as
"not-approved" — so the merge door held — AND still reached `requestPreMergeOptionalStepFix`, which
bounced the card to implementation. The Documentation milestone therefore both blocked the merge
and demanded work, while its own contract says it reports and never vetoes.
Deliberately opt-in and absent everywhere else: advisory gates that DO own remediation (browser
verification) keep their existing blocking behaviour untouched.
*/
reportingOnly?: boolean;
template: {
nodes: WorkflowIrNode[];
edges: WorkflowIrEdge[];

View File

@@ -12,6 +12,13 @@ export interface ResolvedWorkflowOptionalStep {
icon?: string;
phase: NonNullable<WorkflowStepTemplate["phase"]>;
defaultOn: boolean;
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
A reporting group observes accepted work: it holds no approval and never reopens implementation.
Surfaced here so merge admission and the remediation seam read ONE declaration instead of each
re-deriving "is this thing allowed to stop the card".
*/
reportingOnly: boolean;
}
/** Resolve one optional group's effective state consistently across runtime and
@@ -114,6 +121,7 @@ export function resolveWorkflowOptionalSteps(
*/
phase: config.phase === "post-merge" ? "post-merge" : "pre-merge",
defaultOn: config.defaultOn === true,
reportingOnly: config.reportingOnly === true,
/*
FNXC:WorkflowDefinitionSteps 2026-06-29-00:41:
Definition/task creation surfaces must order optional groups by graph execution position, not raw node-array order. Derived built-ins can insert Plan Review between planning and parse while appending its node object, and operators still need the step list to show Plan Review before execution.
@@ -146,3 +154,14 @@ Every optional-group node id in a workflow, regardless of `defaultOn`. These ids
export function resolveAllOptionalGroupIds(ir: WorkflowIr): string[] {
return resolveWorkflowOptionalSteps(ir).map((step) => step.templateId);
}
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
Resolve whether a node id names a REPORTING optional group — one that observes accepted work and
records what it found, holding no approval and owning no remediation. Shared by merge admission and
the remediation seam so "is this allowed to stop the card" has exactly one answer.
*/
export function isReportingOnlyOptionalGroup(ir: WorkflowIr, nodeId: string | undefined): boolean {
if (!nodeId) return false;
return resolveWorkflowOptionalSteps(ir).some((step) => step.templateId === nodeId && step.reportingOnly);
}

View File

@@ -121,6 +121,28 @@ describe("fix steps appear on the card when a gate fails", () => {
expect(sendTaskBackForFix).toHaveBeenCalledTimes(1);
});
/*
FNXC:VerificationRemediation 2026-08-26-06:31:
`performWorkflowRerunBounce` PERSISTS the bounce path onto `task.worktree`. A caller holding the
live checkout must therefore hand it over rather than let an unset task record supply "": that
would wipe the pointer the remediation is about to run in, render the card "Unassigned", and stop
self-healing reclaiming the worktree as idle.
*/
it("bounces into the checkout it was given, not an unset task record", async () => {
const { task, sendTaskBackForFix, store } = harness();
task.worktree = undefined;
await appendReviewRemediationSteps(
{ store: store as never, readTaskArtifact: async () => task.prompt, sendTaskBackForFix },
task,
{ stepName: "Verification (test)", feedback: FAILING_OUTPUT, phase: "pre-merge", status: "failed", nodeId: "verification" },
{ worktreePath: "/tmp/live-checkout" },
);
expect(sendTaskBackForFix).toHaveBeenCalledTimes(1);
expect(sendTaskBackForFix.mock.calls[0]?.[1]).toBe("/tmp/live-checkout");
});
it("turns a Code Review REVISE into named work on the card", async () => {
const { deps, task, pending, sendTaskBackForFix } = harness();
@@ -193,6 +215,57 @@ describe("fix steps appear on the card when a gate fails", () => {
expect(sendTaskBackForFix).not.toHaveBeenCalled();
});
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
Documentation only documents. Measured on a real card (mult-021): its advisory REVISE asked for
implementation work, this seam bounced the card to `in-progress`, and under this workflow's
named-remediation policy `sendTaskBackForFix` reopened NOTHING — no pending step, foreach
`already-expanded`, Code Review replayed over an unchanged tree, and the card merged when the
second Documentation pass happened to pass. The demand was never implemented and two model calls
were spent proving nothing.
*/
it("records Documentation feedback without reopening implementation", async () => {
const { deps, task, store, pending, sendTaskBackForFix } = harness();
const scheduled = await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
stepName: "Documentation",
feedback: "Implement the scoped removal and absence regression contract before documenting completion.",
phase: "pre-merge",
status: "advisory_failure",
verdict: "REVISE",
nodeId: "documentation-delivery",
});
expect(scheduled, "a reporter schedules no work").toBe(false);
expect(pending()).toHaveLength(0);
expect(sendTaskBackForFix, "the card must not bounce with an empty step list").not.toHaveBeenCalled();
// The feedback still reaches the operator on the card.
expect(store.logEntry.mock.calls.some(([, title]) => String(title).includes("cannot reopen implementation"))).toBe(true);
});
/*
FNXC:EmptyBounceGuard 2026-08-26-06:56:
The general invariant behind that fix: under named-remediation policy, only the gates that can
APPEND work may bounce. Any other node reaching the bounce would send the card back with nothing
to do, which is indistinguishable from a hang and re-reviews an unchanged tree on a loop.
*/
it("refuses to bounce a card for a gate that owns no remediation", async () => {
const { deps, task, pending, sendTaskBackForFix } = harness();
const scheduled = await requestPreMergeOptionalStepFix(deps as never, task.id, task, {
stepName: "Some Custom Gate",
feedback: "please change something",
phase: "pre-merge",
status: "failed",
verdict: "REVISE",
nodeId: "some-custom-gate",
});
expect(scheduled).toBe(false);
expect(pending()).toHaveLength(0);
expect(sendTaskBackForFix).not.toHaveBeenCalled();
});
/*
The appended shape is selected by the WORKFLOW, not by this seam: Coding (Ideas) reopens its
trailing step instead, and must keep doing so.

View File

@@ -80,6 +80,14 @@ describe("deterministic verification failure → named remediation", () => {
status: "failed",
phase: "pre-merge",
}),
/*
FNXC:VerificationRemediation 2026-08-26-06:31:
The checkout this gate just verified is handed over explicitly. `performWorkflowRerunBounce`
PERSISTS the path it receives onto `task.worktree`, so falling back to an empty task record
would wipe the pointer the remediation is about to run in — the card renders "Unassigned" and
self-healing can no longer reclaim the worktree. The legacy bounce below always passed it.
*/
{ worktreePath: "/tmp/fn-vr-1" },
);
// Remediation performs the bounce itself; a second one would double-dispatch the executor.
expect(deps.sendTaskBackForFix).not.toHaveBeenCalled();

View File

@@ -14,10 +14,20 @@ export type AppendReviewRemediationStepsDeps = {
* refuses a blind return to implementation: no candidate, out-of-scope evidence, duplicate-only
* work, or the fourth wave is a human hold rather than an empty executor dispatch.
*/
/*
FNXC:VerificationRemediation 2026-08-26-06:31:
`worktreePath` lets a caller that already HOLDS the live checkout hand it in instead of falling back
to `task.worktree`. The executor's deterministic-verification gate is such a caller, and the fallback
is not safe for it: `performWorkflowRerunBounce` persists whatever path it receives back onto
`task.worktree`, so an empty fallback WIPES the pointer — the card renders "Unassigned" and
self-healing can no longer reclaim the worktree as idle. Graph-driven callers (the Code Review
remediation node) have no such path in hand and keep the task-record fallback.
*/
export async function appendReviewRemediationSteps(
deps: AppendReviewRemediationStepsDeps,
task: Task,
info: RequestPreMergeOptionalStepFixInfo,
options: { worktreePath?: string } = {},
): Promise<boolean> {
const gate = info.nodeId === "verification" ? "Verification" : info.nodeId === "code-review" ? "Code Review" : undefined;
if (!gate) return false;
@@ -47,7 +57,7 @@ export async function appendReviewRemediationSteps(
await widenPromptFileScope(deps.store, task.id, prompt, remediationDeclaredFiles(appended.appended));
await deps.sendTaskBackForFix(
live,
live.worktree ?? "",
options.worktreePath?.trim() || live.worktree || "",
info.feedback,
info.stepName,
`Review gate ${gate} requested named remediation`,

View File

@@ -53,6 +53,7 @@ export type BounceVerificationFailureDeps = {
status: "failed";
nodeId: string;
},
options?: { worktreePath?: string },
) => Promise<boolean>;
sendTaskBackForFix: (
task: Task,
@@ -96,13 +97,22 @@ export async function bounceVerificationFailure(
* classify the executor's own just-written files as upstream work and park instead of fixing.
*/
const liveTask = await deps.store.getTask(task.id).catch(() => task);
const appended = await deps.appendReviewRemediationSteps(liveTask ?? task, {
stepName,
feedback,
phase: "pre-merge",
status: "failed",
nodeId: "verification",
});
/*
* Hand over the checkout this gate just verified. Falling back to `task.worktree` would let an
* empty pointer reach `performWorkflowRerunBounce`, which persists it — wiping the worktree the
* remediation is about to run in. The legacy branch below has always passed this same path.
*/
const appended = await deps.appendReviewRemediationSteps(
liveTask ?? task,
{
stepName,
feedback,
phase: "pre-merge",
status: "failed",
nodeId: "verification",
},
{ worktreePath },
);
if (appended) return "named-remediation";
/* Parked: drop the completed-task watchdog the bounce would otherwise have cleared. */
deps.clearCompletedTaskWatchdog(task.id);

View File

@@ -40,6 +40,7 @@ import {
resolveOptionalReviewRevisionBudget,
resolveOptionalStepRevisionBudget,
resolveStepReopenPolicy,
isReportingOnlyOptionalGroup,
resolveWorkflowIrForTask,
} from "@fusion/core";
import { mergeEffectiveSettings } from "../project/effective-settings.js";
@@ -383,6 +384,52 @@ export async function requestPreMergeOptionalStepFix(
return deps.appendReviewRemediationSteps(liveTask, info);
}
/*
FNXC:ReportingOnlyGroup 2026-08-26-06:56:
A REPORTING group cannot reopen implementation. It observes accepted work and records what it
found; it has no verdict to enforce, so a revision request from it is feedback, not a work order.
Measured on a real card: the Documentation milestone returned an advisory REVISE asking for
implementation work. This seam bounced the card to `in-progress` — where, under this workflow's
named-remediation policy, `sendTaskBackForFix` reopens NOTHING. With no pending step the foreach
answered `already-expanded`, the walk replayed Code Review over an UNCHANGED tree, and the card
merged when the second Documentation pass happened to pass. The reviewer's demand was never
implemented, and 2 model calls plus a worktree acquisition were spent proving nothing.
Returning false routes traversal down the node's own failure edge, which for a reporter reaches
the merge gate — the behaviour its contract always described. The feedback stays on the card.
*/
if (workflowIr && isReportingOnlyOptionalGroup(workflowIr, info.nodeId)) {
await deps.store.logEntry(
taskId,
`${info.stepName} recorded feedback but cannot reopen implementation`,
`${info.stepName} reports on accepted work and carries no approval, so no executor remediation was scheduled. Feedback:\n${info.feedback}`,
deps.getRunContextFor(taskId),
);
return false;
}
/*
FNXC:EmptyBounceGuard 2026-08-26-06:56:
Last line of defence for the named-remediation policy: under `none`, `sendTaskBackForFix` reopens
no step, so every bounce that did not append named work is a card sent back with nothing to do.
The two paths that CAN append have already returned above; anything still here would bounce empty.
Refuse visibly instead — a parked card an operator can see beats a silent loop that re-reviews an
unchanged tree until a non-deterministic verdict lets it through.
*/
if (resolveStepReopenPolicy(workflowIr) === "none") {
executorLog.warn(
`${taskId}: pre-merge remediation NOT scheduled for step "${info.stepName}" — this workflow appends named remediation steps, and no gate owns that for node "${info.nodeId ?? "unknown"}". Card left parked.`,
);
await deps.store.logEntry(
taskId,
`${info.stepName} requested changes but no remediation owner exists for it`,
`This workflow reopens implementation only through named remediation steps, which are produced by the Verification and Code Review gates. Node "${info.nodeId ?? "unknown"}" has no such owner, so the card was not bounced with an empty step list. Feedback:\n${info.feedback}`,
deps.getRunContextFor(taskId),
);
return false;
}
if (info.verdict !== "REVISE") {
// FNXC:RemediationVisibility 2026-07-26-19:20: a hard-failed gate with no parsed REVISE
// verdict schedules nothing, so the remediation node fails and the card parks. Say so.