fix(engine): implementation-incomplete merge failures fail-closed/resumable (#1991)

[Elit3-X]

What & why

Workflow graph merge failures classified implementation-incomplete (i.e. the merge node reports there is no implementation proof — no branch / no committed work) could still be routed to the no-op merge requester and false-complete the task as done. This hides genuinely unlanded work behind a green "merge" and is the merge-side sibling of the "(no feedback captured)" no-verdict dispatch defect.

Closes the truthfulness gap: an implementation-incomplete merge-graph failure now fails closed when there is no executable proof to resume, or requeues resumable parsed steps back to todo for execution — it is never handed to a no-branch no-op merge requester.

Refs #1991 (no-op merge truthfulness). Sibling of #1946 (no-verdict "(no feedback captured)" dispatch defect).

Change

• New classifier routeImplementationIncompleteMergeGraphFailure(live, failedNode):
  • clears paused-aborted state + active worktree,
  • requeues resumable parsed steps via the existing execution-resume router when the task still has non-terminal workflow steps,
  • otherwise fails closed (status: "failed" with a logged, explicit reason).
• Defense-in-depth: isRetryableBenignMergePauseAbort and the merge-requester route both short-circuit (return false) for implementation-incomplete, so this value can never reach the no-op merge requester.
• handleGraphFailure routes genuine (non-global-pause, non-completion-finalize, non-user-paused) implementation-incomplete merge-graph failures through the new classifier.
• Resume-eligibility predicate treats an implementation-incomplete merge failure with no incomplete steps as fail-closed, and keeps the premature-merge-with-incomplete-steps requeue path.

Legitimate noCommitsExpected no-op merges are explicitly preserved (regression test included).

Tests

New regression coverage in:

• packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts — parametrized across merge node ids: (a) no-proof implementation-incomplete fails closed without requesting a no-op merge; (b) resumable parsed steps are requeued to todo for execution resume, not no-op-merged.
• packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts — a legitimate noCommitsExpected builtin:coding merge is still allowed (guard does not over-block).

Verification (engine package):

pnpm --filter @fusion/engine exec vitest run \
  src/__tests__/executor-fast-mode-workflows.test.ts \
  src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts
# => 2 files, 69 tests, 0 failures

pnpm check:changesets            # pass
pnpm --filter @fusion/engine typecheck   # 0 errors

A patch changeset for @runfusion/fusion is included.

Summary by CodeRabbit

• Bug Fixes
  • Prevented “implementation-incomplete” workflow merge failures from being treated as successful no-op merges.
  • Ensured tasks with resumable implementation steps move back to execution to continue where they left off.
  • Ensured tasks without sufficient implementation evidence fail safely rather than entering misleading retry/no-op paths.
  • Improved paused/aborted merge-failure handling to avoid incorrect completion states.
• Tests
  • Added/expanded coverage for fast-mode coding merges and implementation-incomplete pause/abort retry classification.

[coderabbitai]

Warning

Review limit reached

@gsxdsm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bbda71f-c0e6-467e-9098-182a85823ae9

📥 Commits

Reviewing files that changed from the base of the PR and between b2ec782 and a556a5e.

📒 Files selected for processing (2)

• packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts
• packages/engine/src/executor.ts

📝 Walkthrough

Walkthrough

The executor now routes implementation-incomplete merge-graph failures to execution resume or terminal failure instead of no-op merge handling. Tests cover both failure classifications and valid noCommitsExpected no-op merges, with a patch changeset documenting the behavior.

Changes

Merge failure truthfulness

Layer / File(s)	Summary
Implementation-incomplete failure routing packages/engine/src/executor.ts	Merge-graph failures with incomplete implementation proof are excluded from retryable and merge-requester paths, then routed to execution resume when possible or failed closed otherwise.
Failure classification coverage packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts	Parameterized tests verify terminal failure without executable proof and requeue to todo when parsed steps can resume.
No-op merge validation and release metadata packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts, .changeset/fn-1165-noop-merge-truthfulness.md	Tests preserve valid noCommitsExpected no-op merges, and the changeset records the patch release behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GraphFailure
  participant TaskExecutor
  participant RecoveryRouting
  participant TaskState
  GraphFailure->>TaskExecutor: implementation-incomplete merge failure
  TaskExecutor->>RecoveryRouting: route failure
  RecoveryRouting->>TaskState: requeue resumable steps
  RecoveryRouting->>TaskState: or mark task failed without executable proof

Loading

Possibly related PRs

• Runfusion/Fusion#1697: Both modify pause-abort classification and task routing in executor.ts.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main engine change: implementation-incomplete merge failures now fail closed or resume safely.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR makes implementation-incomplete merge failures fail closed or resume execution instead of completing as no-op merges.

• Adds a classifier for implementation-incomplete merge graph failures.
• Requeues resumable parsed steps back to todo.
• Fails no-proof merge failures with an explicit error.
• Blocks implementation-incomplete failures from merge retry and no-op requester paths.
• Adds tests for merge-node aliases, system pauses, worktree tracking, and valid noCommitsExpected no-op merges.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
packages/engine/src/executor.ts	Routes implementation-incomplete merge failures through explicit fail-closed or execution-resume handling, with guards against no-op merge requester fallback.
packages/engine/src/tests/reliability-interactions/merge-node-paused-abort-retryable.test.ts	Adds coverage for implementation-incomplete merge-node handling across aliases, pause states, and active worktree tracking.
packages/engine/src/tests/executor-fast-mode-workflows.test.ts	Adds coverage that valid noCommitsExpected builtin coding merges still complete as no-op merges.
.changeset/fn-1165-noop-merge-truthfulness.md	Adds a patch changeset for the operator-facing merge truthfulness fix.

_{Reviews (4): Last reviewed commit: "fix(engine): use undefined for in-memory..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts`:
- Around line 431-436: Update the negative store.logEntry assertion in the
fast-mode workflow test to use undefined for the argument that may be undefined,
replacing the corresponding expect.anything() matcher so the assertion catches
emitted calls with an undefined value.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e897375e-5914-4d56-b041-80894fdbde88

📥 Commits

Reviewing files that changed from the base of the PR and between 03966ec and 2e59714.

📒 Files selected for processing (4)

• .changeset/fn-1165-noop-merge-truthfulness.md
• packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts
• packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts
• packages/engine/src/executor.ts

[Elit3-X]

Addressed the CodeRabbit review on packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts: the negative store.logEntry guard in the "noCommitsExpected builtin:coding merge … empty implementation steps" test now asserts undefined, undefined for the trailing args instead of expect.anything(). Since expect.anything() does not match undefined/null, the .not.toHaveBeenCalledWith(...) guard was vacuous; it now matches the real 4-arg call shape (taskId, message, undefined, undefined) (proven by the sibling positive assertion) so the negative guard is meaningful. Pushed as commit b2ec782. Verified: pnpm --filter @fusion/engine exec vitest run → 2 files / 69 tests passed; typecheck + check:changesets green.

[gsxdsm]

Thanks so much for this! Can you address the two greptile P1s? Will merge after that