[Birfy]

Owner: Dev
Reviewer: Reviewer
Written-by: Dev

Fixes #1526

What changed

Before: A Workflow node failure stopped the run (dead end); there was no way to return upstream and retry (e.g. Review → Rework).

After:

• onFail edges { from, to, onFail: true, max: 3 } send failure back to an upstream node and re-queue the failing node
• Attempts tracked per edge (max 1–8, default 3); after exhaustion the node fails permanently as before
• Stage narrate shows recovery; failOnce and success-path loop edges unchanged

Verification at the tip

Tip SHA: 9a31805
Ran: cd plugins/workflow && npm test → 99/99 pass (5 new failure-recovery tests; +1 from #1528)
Not run: full npm run verify (workflow-only change); smoke (no index.js / src/host/); Desktop

Checkpoint

{
  "pr": 1529,
  "claim": "Dev",
  "issue": 1526,
  "summary": "Workflow nodes can recover from failure by following onFail edges back upstream and retrying, instead of ending the run as a dead end. Attempts are bounded per edge (1–8, default 3); after exhaustion the node fails permanently as before. Stage messages show recovery, and existing failOnce and success-path loop edges stay unchanged. Rebased onto main after #1528 so concurrency Settings and HOST_CAP removal remain intact.",
  "files": ["plugins/workflow/src/graph.js", "plugins/workflow/src/service.js", "plugins/workflow/src/narrate.js", "plugins/workflow/test/failure-recovery.test.js", "plugins/workflow/README.md", "docs/subsystems/workflow.md", "changelog.d/1526.added.md"],
  "checks": { "ran": "plugins/workflow npm test", "result": "green", "at": "9a31805bcbaca46a559eca51bee1e09f2d0d378c" },
  "next": "none — ready for review"
}

[cursor]

Verdict: FAIL at 2b970ff
Written-by: Reviewer
Attestation: 2b970ff role=Reviewer

Ran: cd plugins/workflow && npm test (98/98 PASS)
Not run: full npm run verify (oxlint missing, but workflow tests are the relevant scope), smoke test (no host changes), manual UI testing

Findings

FAIL: PR body format blocks merge

The PR body has two defects that will cause enqueue-guard to refuse this pull request:

1. Owner/Reviewer lines use markdown headers: The body has ## Owner: Dev and ## Reviewer: Reviewer, but enqueue-guard.mjs requires plain lines matching /^\s*Owner:/i and /^\s*Reviewer:/i. The ## characters are not whitespace, so these lines will NOT match. Fix: change to plain Owner: Dev and Reviewer: Reviewer (no ## prefix).

2. Missing checkpoint block: The PR body has no <!-- flybot:checkpoint --> section. The checkpoint is required at queue entry for non-fork, non-release PRs (enqueue-rules.mjs checkpointReasons). Fix: add the checkpoint block from .github/pull_request_template.md with all required fields (pr, claim, issue, summary, files, checks, next).

Implementation review: PASS

The failure recovery implementation itself is correct:

• Edge validation: cleanEdge properly validates onFail: true with max bounded 1–8, default 3
• Recovery logic: failRecoveryDecision checks attempts < max; failNode uses loopBody to find nodes between recovery target and failing node, resets target to 'running' and downstream to 'queued'
• Attempt tracking: failAttempts is persisted in service.js, incremented per edge
• Exhaustion: After max attempts, node fails permanently as before
• Compatibility: Does not interfere with failOnce retry or success-path loop edges (service.js line 310-311 checks both)
• Tests: 5 new tests in failure-recovery.test.js verify the above behaviors; all pass
• Docs: subsystems/workflow.md, plugins/workflow/README.md updated; changelog.d/1526.added.md present
• Narrate: Clear messages for recovery events in Chinese ("失败，已返回 [target] 重试", "↺ 失败恢复 → [target]")

Next: Fix the PR body format (plain Owner:/Reviewer: lines + checkpoint block), then re-request review at the same tip.

[cursor]

Verdict: PASS at 9a31805
Written-by: Reviewer
Attestation: 9a31805 role=Reviewer

Ran: cd plugins/workflow && npm test → 99/99 pass (5 new failure-recovery tests, including max-attempts exhaustion and coexistence with failOnce/loop edges)
Not run: full npm run verify (workflow-only change confirmed); smoke test (no index.js / src/host/ changes); Desktop

Findings

✓ onFail recovery implemented correctly:

• failRecoveryDecision() checks for onFail: true edges and tracks attempts per edge
• failNode() returns { recovered: true, reset: [...], started: [...] } when recovery is possible
• Attempts bounded 1–8 (default 3); after exhaustion the node fails permanently
• Recovery resets the loop body from the target back to the failing node
• service.js passes failAttempts context and checks recovered === true to avoid stopping the run
• narrate.js shows recovery messages: ↺ 失败恢复 → <target>

✓ #1528 concurrency changes intact:

• maxConcurrent per-workflow setting present (default 2, max 8)
• setConcurrency() endpoint exists and works
• maxOf(item) reads per-workflow cap
• No HOST_CAP regression (confirmed removed)

✓ PR body correct:

• Owner: Dev ✓
• Reviewer: Reviewer ✓
• Checkpoint block valid (pr: 1529, claim: Dev, issue: 1526, all required fields present)

✓ Test coverage:

• 5 new tests in failure-recovery.test.js
• Covers: recovery path, max attempts exhaustion, failOnce coexistence, loop edge coexistence, no-recovery baseline
• All 99 tests pass

✓ Changed files match the checkpoint:

• plugins/workflow/src/graph.js (failRecoveryDecision, failNode recovery path)
• plugins/workflow/src/service.js (failAttempts context, recovered check)
• plugins/workflow/src/narrate.js (recovery messages)
• plugins/workflow/test/failure-recovery.test.js (5 new tests)
• plugins/workflow/README.md, docs/subsystems/workflow.md, changelog.d/1526.added.md

No regressions, clean implementation. Ready to merge.

[Birfy]

Verdict: PASS at 9a31805
Written-by: Reviewer
Attestation: 9a31805 role=Reviewer
Signed-by: Reviewer
Nonce: e1dbed1d3f1c1a6e
Signature: (not enabled — .github/agent/signers.json does not exist; see AGENTS.md #1110/#1111)

Ran:

• gh checks on tip 9a31805: plugins (workflow), tests-ok, package, merge-gate, ubuntu-latest Node 24 DSH → pass
• (Reviewer session) workflow npm test at 9a31805 → 99/99 pass

Not run:

• Desktop / LIVE smoke — onFail recovery covered by failure-recovery unit tests
• smoke test — no index.js / src/host/ changes

Findings

No actionable findings. onFail edges reset to upstream and re-queue with bounded attempts (1–8, default 3); exhaustion fails permanently; failOnce and loop edges unchanged. Rebase onto #1528 kept HOST_CAP removal and Settings concurrency. Checkpoint tip matches. Matches owner ask for fail→upstream retry (Review→Rework).