fix: rejoin empty custom error handler at IF ELSE first activity

[hjotha]

Summary

• When a call activity carries an empty custom error handler (on error without rollback { }) and is followed by an IF that tests the call's output variable with both branches terminating, Studio Pro authors the error edge into the fallback path — into a merge shared with the split's false branch that feeds the first ELSE activity. The describer/builder now emits that topology instead of leaving the handler dangling.
• Previous attempt (fix: rejoin empty error handler at IF split when next IF reads skipVar #503, closed) wired the error edge straight to the IF split and triggered CE0108 "variable not in scope" on activities that referenced the call's output variable. This PR places the rejoin at the first ELSE activity via addEmptyErrorHandlerRejoinFlowFrom(splitID, errorOrigin, firstElseID), preserving scope through the split→merge edge.
• Falls back to the pre-existing phantom-EndEvent terminator when the ELSE body is empty, starts with a compound statement, or the follow-up IF does not both-return. No behavioural change outside the narrow capture condition.

Validation

• Unit test TestEmptyErrorHandlerRejoinsAtIfElseFirstActivity verifies: 2 EndEvents (no phantom), error flow from call reaches a merge, merge receives the split's false branch AND flows to the first ELSE activity.
• go test ./mdl/executor/ passes; go test -tags integration ./... passes.
• Targeted audit against the authored reproduction confirms mx check errors = 0 and mpr check errors = 0; residual roundtrip diff is cosmetic (10 lines, unrelated to this fix).

Test plan

• Unit test TestEmptyErrorHandlerRejoinsAtIfElseFirstActivity
• Full executor test suite
• Integration test suite (-tags integration)
• Targeted roundtrip audit on the authored reproduction — MPR validates (0 errors)

🤖 Generated with Claude Code

[github-actions]

AI Code Review

Critical Issues

None found.

Moderate Issues

• Missing MDL bug test script: The checklist requires bug fixes to include "an MDL test script in mdl-examples/bug-tests/ that reproduces the issue". While the PR adds a unit test in mdl/executor/error_handler_if_else_rejoin_test.go, it does not provide the corresponding MDL test script in mdl-examples/bug-tests/ as mandated by the Test coverage section. This is necessary for verifying the fix in Studio Pro and ensuring end-to-end validity.

Minor Issues

None found.

What Looks Good

• Well-scoped fix: The PR addresses a specific narrow case (empty custom error handler followed by terminating IF-ELSE) without altering behavior outside this condition.
• Comprehensive unit test: The new test TestEmptyErrorHandlerRejoinsAtIfElseFirstActivity thoroughly validates the expected microflow topology (2 EndEvents, error flow rejoin at merge, split→merge→first ELSE activity flow).
• Correct fallback logic: The solution properly defers to the phantom-EndEvent terminator when ELSE body is empty/compound or branches don't both-return, preserving existing behavior.
• Test suite validation: All executor and integration tests pass, confirming no regressions.
• Clear documentation: Inline comments explain the Studio Pro authoring pattern, the CE0108 scope issue, and the rejoin strategy.
• No syntax changes: Since this is a pure behavioral fix (no new MDL syntax), full-stack consistency checks (grammar/AST/visitor/etc.) are not applicable.

Recommendation

Request changes to add the missing MDL bug test script in mdl-examples/bug-tests/ per the checklist requirement for bug fixes. The reproduction case mentioned in the PR ("Targeted audit against the authored reproduction") should be formatted as an executable MDL test script there. Once added, the PR is ready for approval.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[ako]

Review: fix: rejoin empty custom error handler at IF ELSE first activity

Verdict: Approve with minor note

What the PR does

Follow-on to #503. That PR wired the error edge directly to the IF split, which caused CE0108 "variable not in scope" — because the error path bypasses the call activity that declares $R, and downstream activities reference $R. This PR routes the error edge to the first ELSE activity via a shared merge, matching the Studio Pro-authored topology:

Call → (normal) → IF Split → true: ThenBody → EndEvent
                           → false: Merge → firstElseActivity → ... → EndEvent
Call → (error) ────────────→ same Merge

The ELSE body is the fallback/error path that handles both "no value" and "error" cases without needing $R, so scope is preserved.

No blockers

Minor: bypasses the state abstraction rather than using setActivePendingErrorHandler

The capture block directly clears four individual fields:

fb.errorHandlerSource = ""
fb.errorHandlerTailFrom = ""
fb.errorHandlerSkipVar = ""
fb.errorHandlerTailIsSource = false

This is functionally equivalent to fb.setActivePendingErrorHandler(pendingErrorHandlerState{}) in this case — registerEmptyCustomErrorHandlerWithSkip for the skipVar != "" path sets exactly these four fields and leaves emptyErrorHandlerFrom untouched, so clearing just these four is correct right now. But direct field clearing is slightly fragile: if pendingErrorHandlerState gains a new field in the future, this block would miss it. Using setActivePendingErrorHandler(pendingErrorHandlerState{}) or at minimum a comment saying "emptyErrorHandlerFrom is not set by the skipVar path so it does not need clearing here" would be more robust.

Correctness notes

• The condition is appropriately strict: tailFrom == source && tailIsSource is the exact shape set by registerEmptyCustomErrorHandlerWithSkip for the skipVar != "" case — no other code path produces this combination
• bothReturn guard is important: without it, a continuation in THEN would create a dangling handler in the happy path
• firstElseIsLeafActivity correctly excludes compound-statement heads (IF, LOOP, WHILE, ENUM SPLIT) where rejoin plumbing would be more complex
• Fallback to phantom-EndEvent for all unmatched cases is explicit and preserves existing behaviour

Test

The test correctly verifies the three-edge topology: error flow → merge, split-false → merge, merge → firstElseActivity. The "second log in visitation order = else body log" heuristic is fine for this synthetic test shape. Clean PR, no bundled unrelated changes.