fix: traverse through unpaired ExclusiveMerge in describe (#281)

[hjotha]

Fixes #281.

Summary

DESCRIBE MICROFLOW silently truncated its output at an ExclusiveMerge that wasn't the matching end-of-branch point of an ExclusiveSplit. Every activity downstream of the merge was dropped — in the reporter's case, that was the REST CALL, the retry-decision split, and the retry branch.

Root cause

traverseFlow in mdl/executor/cmd_microflows_show_helpers.go returned unconditionally on every ExclusiveMerge, assuming the node would be handled by the matching split's branch traversal:

if _, isMerge := obj.(*microflows.ExclusiveMerge); isMerge {
    return
}

That's correct when the merge closes an IF/ELSE block, but not when it's a pure junction — e.g. the manual retry-loop pattern from issue #281 where a back-edge from "change retry count + delay" routes back into a merge that sits between the setup activities and the REST call. That merge isn't a value in splitMergeMap (no split points at it as its end-of-branch), so skipping it drops everything after.

Fix

Early-return only when the merge is paired with a split in splitMergeMap. Otherwise walk through it as pass-through, following its outgoing flow — the same behaviour traverseFlowUntilMerge already has for intermediate merges (line 348-354 of the same file, predating this PR).

A small isMergePairedWithSplit helper does the lookup.

Tests

Three focused regression tests in cmd_microflows_unpaired_merge_test.go:

• TestTraverseFlow_UnpairedMergeIsPassThrough — minimal repro (start → act1 → merge → act2 → end, empty splitMergeMap): asserts both activities appear in the output and no stray end if; is emitted.
• TestTraverseFlow_PairedMergeStillClosesIfElse — regression guard: an IF/ELSE still closes with exactly one end if;, neither zero nor duplicated.
• TestTraverseFlow_ManualRetryLoopPatternEmitsEverything — closer shape of the user's microflow (start → setup → merge → call → decide → [retry branch] → change → delay → merge), verifies every activity shows up in describe output.

Validation

• go test ./... passes
• go test -race ./mdl/executor/ passes
• go build -tags integration ./... builds clean

The three new tests each fail on origin/main with the expected "activity dropped" symptom and pass after the fix.

Test plan

• Minimal unpaired-merge case traverses through
• Regular IF/ELSE still emits exactly one end if;
• Manual retry-loop pattern (shape of issue DESCRIBE MICROFLOW does not describe the entire Microflow #281's repro) preserves every activity
• go test ./... clean
• -race clean
• Integration build clean

🤖 Generated with Claude Code

[github-actions]

AI Code Review

Critical Issues

• None found

Moderate Issues

• None found

Minor Issues

• None found

What Looks Good

• Root cause analysis is precise: The PR correctly identifies that the early-return on ExclusiveMerge was incorrect for unpaired merges (junctions) and provides a clear fix based on the existing splitMergeMap lookup.
• Fix is minimal and targeted: Only modifies the problematic conditional in traverseFlow and adds a small helper function (isMergePairedWithSplit), preserving all other logic.
• Tests are comprehensive:
  • Three focused regression tests cover the exact issue scenario (unpaired merge as pass-through), paired merge correctness (IF/ELSE block closure), and the real-world manual retry loop pattern from DESCRIBE MICROFLOW does not describe the entire Microflow #281.
  • Tests validate both activity presence and absence of erroneous end if; emissions.
  • All new tests fail on origin/main and pass after the fix, confirming they catch the regression.
• Validation is thorough:
  • go test ./... passes
  • go test -race ./mdl/executor/ passes
  • Integration build with -tags integration succeeds
  • Explicit test plan items are all checked off
• Code quality:
  • Fix aligns with existing pattern in traverseFlowUntilMerge (lines 348-354 referenced in comments)
  • Clear, detailed comments explain the distinction between paired/unpaired merges
  • No duplication or overlap with existing functionality
  • Maintains single-responsibility scope (only fixes the merge traversal logic)

Recommendation

Approve. The PR correctly resolves issue #281 with minimal, well-tested changes that follow the project's architectural patterns. No modifications to MDL syntax or full-stack pipeline are needed since this is an executor-level bug fix in an existing command. The test coverage is appropriate for the scope, and all validation passes. Ready for merge.

---

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

[github-actions]

AI Code Review

Critical Issues

• None found.

Moderate Issues

• None found.

Minor Issues

• None found.

What Looks Good

• The fix correctly addresses the root cause: unpaired ExclusiveMerge nodes (used as pure junctions) were causing premature termination in traverseFlow, dropping downstream activities. The solution only returns early for merges paired with a split (IF/ELSE closing), treating others as pass-through.
• The implementation is minimal and localized: adds a helper isMergePairedWithSplit and adjusts the merge handling logic in traverseFlow.
• Tests are comprehensive:
  • TestTraverseFlow_UnpairedMergeIsPassThrough validates the core fix (unpaired merge traversed, no spurious end if;).
  • TestTraverseFlow_PairedMergeStillClosesIfElse ensures IF/ELSE blocks still emit exactly one end if;.
  • TestTraverseFlow_ManualRetryLoopPatternEmitsEverything mirrors the user's exact scenario (retry loop) and verifies all activities appear.
• Validation steps pass: go test ./..., race tests, and integration builds.
• The PR follows the project's contribution workflow: links to issue DESCRIBE MICROFLOW does not describe the entire Microflow #281, includes focused regression tests, and validates locally.
• No MDL syntax changes are made, so full-stack consistency checks (grammar/AST/visitor/etc.) are not applicable—this is purely an executor bug fix.

Recommendation

Approve. The PR is ready for merge. It resolves issue #281 with a targeted fix, robust test coverage, and no regressions. No changes are needed.

---

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