fix(workflow): failure recovery resets only the two endpoints, not the nodes on the way back #1541

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: 4c870815318f48acb5325299bb8fff59
Branch: dev4/fix/p78-onfail-recovery-body
Base: a406fee
At: 2026-10-28T03:58:25Z

What happens

plugins/workflow/src/graph.js, failNode()'s recovery branch (around line 661) calls

const body = loopBody(graph, recovery.edge.to, nodeId)

but loopBody(graph, source, target) (around line 539) means "nodes forward-reachable from target that can still reach source" — the sense completeNode's loop branch uses: loopBody(graph, nodeId, looping.edge.to). With the two arguments reversed the intersections is empty for any recovery target that is not adjacent to the failing node, so body is only the fallback at the end of loopBody:

body.add(source)
body.add(target)

The recovery therefore resets the recovery target and the failing node only. Every node between them keeps its done state and its output, so the run re-feeds the same rejected artefact and burns the remaining max without ever producing a different result.

Reproduction (measured on d6410d07)

Graph draft → copy → review, plus review → draft with { onFail: true, max: 3 }; completeNode(draft), completeNode(copy), then failNode(review, { failAttempts: {} }):

• now: reset: ["draft","review"], status: { draft:"running", copy:"done", review:"queued" } — copy never leaves done, and after completeNode(draft) again the next failNode still reports failAttempts["review>draft"] = 2 with copy untouched;
• with the arguments in the documented order (loopBody(graph, nodeId, recovery.edge.to)): reset: ["draft","copy","review"], status: { draft:"running", copy:"queued", review:"queued" }, which is the README's "runs through the failing node again".

The 2-node shape the current tests use (draft → review) cannot see it: there the fallback's two body.add calls happen to be the whole body.

Expected

A recovery resets the whole segment from the recovery target to the failing node — the target running, the nodes in between queued — so the failing step is reached again with freshly produced input.

Scope

• One line in plugins/workflow/src/graph.js: pass the arguments in the documented order. loopBody's contract and the loop branch stay unchanged.
• Tests: a ≥3-node chain asserting reset contains the middle node and that it is no longer done; plus one case through service's advance/applyOp asserting the middle node is really dispatched again. Both must fail on the base tree.
• The sentence in docs/subsystems/workflow.md describing recovery as resetting "both nodes" and the README line that promises a re-run through the failing node are corrected in the same change.

Out of scope

The max semantics of the recovery edge (a separate defect) and the Settings authoring surface for onFail (no UI entry exists; a separate request).