feat(engine): signal a run that completed with node errors (#661 L1)

[oxoxDev]

Summary

Fixes #661 L1 (OpenCompany workflows audit): under on_error: "continue" (and route), a node that fails turns its failure into a data item and the run proceeds — but the run's terminal status stayed RunStatus::Completed, so a host read the run as an unqualified success while a node had actually failed. route is validated (it needs an error-port edge); continue had no signal at all. The failure was only discoverable by scanning every node's output items for an {error: …} shape.

This surfaces it on the run record the engine already assembles:

• RunStatus::CompletedWithErrors — new variant between Completed and Failed. The run reached a terminal node (so it did not Fail), but at least one node failed under a non-stop policy.
• The terminal status on the success path is now derived from the steps (engine.rs): any StepStatus::Error step ⇒ CompletedWithErrors, else Completed. stop-policy failures still bubble out as Err and assemble a Failed record, unchanged.
• Run::failed_node_ids() — the ids of the nodes that errored (the Error steps), so a host can act on which nodes failed without scanning the output. Empty for a clean Completed; non-empty for CompletedWithErrors/Failed.

The signal lands on observability::Run, delivered to every host via RunObserver::on_run_finish (the same hook OpenHuman/OpenCompany already use to persist per-node output), so it reaches consumers on the observed run path with no new wiring.

Scope notes for review

• RunOutcome unchanged. The signal is on the observed Run record, which is where the audit finding points ("WorkflowRun carries no failure signal") and where the host's existing observer reads it. Mirroring it onto RunOutcome/the resume return (which don't collect steps today) is a reasonable follow-up if a non-observing consumer needs it — called out rather than bolted on inconsistently.
• route counts as with-errors too. A routed failure still errored the node (StepStatus::Error is recorded); a downstream recovery branch handling it does not erase that the node itself failed. If you'd rather a fully-recovered route read clean, that's a one-line change in the derivation — your call.
• Semver. New public enum variant + method — additive, but breaks an exhaustive match RunStatus downstream. Left the crate at 0.6.1; bump to your preference.

Tests

• cargo fmt --all -- --check — clean
• cargo clippy --all-targets --all-features — clean
• cargo test --all-features — all suites green

New (engine, observer-captured Run.status):

• a_clean_run_is_completed_and_names_no_failed_node
• on_error_continue_marks_the_run_completed_with_errors (names the failing node)
• on_error_route_marks_the_run_completed_with_errors

Related

• OpenCompany #661 (workflows authoring & validation audit), finding L1. This is the last open finding on that tracker; the rest shipped.

Summary by CodeRabbit

• New Features

  • Runs that finish after handled node failures are now marked Completed with Errors.
  • Run details identify failed nodes in the order their errors occurred.
  • Completed runs retain all collected execution steps, including failures.

• Bug Fixes

  • Corrected status reporting for errors handled through continue or route policies.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 579a8d4c-ae65-4324-bd5d-b6c5c87b32a7

📥 Commits

Reviewing files that changed from the base of the PR and between ef20b5c and 734b53e.

📒 Files selected for processing (1)

• src/observability.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/observability.rs

---

📝 Walkthrough

Walkthrough

The change adds CompletedWithErrors for terminal runs with handled node failures. It preserves collected steps, exposes failed node IDs in completion order, and adds tests for clean, continue, and route execution.

Changes

Run status reporting

Layer / File(s)	Summary
Run status and failed-node contract src/observability.rs	RunStatus now includes CompletedWithErrors. Run::failed_node_ids() returns errored node IDs in completion order.
Terminal status integration and validation src/engine.rs	Terminal runs use CompletedWithErrors when collected steps contain errors. Tests cover clean, continue, and route runs.

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

Poem

I’m a rabbit, and errors now leave a clear trail,
Clean runs stay bright; handled faults do not fail.
Continue and route mark the path they have crossed,
Steps stay collected, so no trace is lost.
Hop by hop, the run tells what went wrong.

🚥 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 describes the main change: reporting completed runs that contain node errors.
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.

---

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.}

[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 `@src/observability.rs`:
- Around line 115-118: Update the documentation for failed_node_ids() to remove
the unconditional guarantee that RunStatus::Failed produces a non-empty result;
clarify that it is non-empty only for node failures, while driver, recursion,
and checkpointer failures may return an empty vector.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14d419b4-9233-4d98-8854-6303383859e8

📥 Commits

Reviewing files that changed from the base of the PR and between 4357473 and ef20b5c.

📒 Files selected for processing (2)

• src/engine.rs
• src/observability.rs