Skip to content

fix(foundation): add execution event parity for workflow lifecycle#1563

Open
Bhanudahiyaa wants to merge 2 commits intomofa-org:mainfrom
Bhanudahiyaa:fix/workflow-execution-event-parity
Open

fix(foundation): add execution event parity for workflow lifecycle#1563
Bhanudahiyaa wants to merge 2 commits intomofa-org:mainfrom
Bhanudahiyaa:fix/workflow-execution-event-parity

Conversation

@Bhanudahiyaa
Copy link
Copy Markdown
Contributor

Summary

Align workflow runtime event emission with ExecutionEvent schema for retry/failure and pause/resume lifecycle paths.

Motivation

Observability/debugging currently misses key execution transitions, causing a mismatch between declared event contracts and emitted runtime data.

Changes

  • Added lifecycle event variants in ExecutionEvent:
    • WorkflowPaused
    • WorkflowResumed
  • Updated WorkflowExecutor to emit:
    • WorkflowPaused on wait/HITL pause
    • WorkflowResumed on resume_with_human_input and resume_from_checkpoint
    • NodeRetrying based on retry count
    • NodeFailed on terminal node failures
  • Corrected workflow terminal event logic:
    • WorkflowCompleted now emitted only when status is Completed (not Paused)
  • Added regression tests:
    • retry + eventual success emits retry + completed
    • terminal failure emits NodeFailed
    • pause/resume emits WorkflowPaused + WorkflowResumed

Related Issues

#1562

Testing

  • cargo test -p mofa-foundation workflow::execution_event -- --nocapture
  • cargo test -p mofa-foundation workflow::executor::tests:: -- --nocapture
  • cargo test -p mofa-foundation test_emits_ -- --nocapture
  • cargo check -p mofa-foundation
  • cargo clippy -p mofa-foundation --all-features -- -D errors
  • rustfmt --edition 2024 --check crates/mofa-foundation/src/workflow/executor.rs crates/mofa-foundation/src/workflow/execution_event.rs

Checklist

  • cargo fmt --check passes (for touched files)
  • cargo clippy --workspace --all-features -- -D errors equivalent attempted (mofa-foundation target; repo currently has known -D errors lint naming issue)
  • cargo test coverage for changed paths
  • Architecture layer rules respected
  • Small, focused PR scope

@Bhanudahiyaa
Copy link
Copy Markdown
Contributor Author

@yangrudan @lijingrs

This PR focuses on event contract parity at the workflow runtime boundary.

ExecutionEvent already modeled retry/failure semantics, but executor emission didn’t consistently include those transitions. I kept the change intentionally narrow to mofa-foundation workflow event schema + executor emission points, and added regression tests to lock behavior for:

  • retry + success
  • terminal failure
  • pause/resume lifecycle

@yabhimanyu2007-sys
Copy link
Copy Markdown

I reviewed the tests in this PR and they cover the main lifecycle paths well.

I’d like to contribute by adding a few additional edge case tests:

  • Event ordering validation (e.g., retry events occur before NodeFailed)
  • Ensuring no duplicate NodeFailed emissions
  • Behavior when max_retries = 0 (no retry case)

These should help strengthen correctness guarantees for execution event sequencing.

Please let me know if this direction works.

@Bhanudahiyaa
Copy link
Copy Markdown
Contributor Author

@yabhimanyu2007-sys
Thanks for taking a look, glad the coverage makes sense!

Those are good edge cases, especially around ordering and duplicate emissions. For this PR, I tried to keep the scope focused on establishing baseline lifecycle parity (retry/failure + pause/resume) and locking in the core transitions.

I think the cases you mentioned would be great as a follow-up focused on event sequencing guarantees (e.g., ordering + dedup invariants), since that’s slightly orthogonal to just emitting the missing events.

If you’re interested, feel free to open a separate PR building on top of this, happy to review 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants