[Martin Brekhof (MBrekhof)]

Problem

1. In-place mutation of cached Workflow.Rules:
   In RulesCache.GetWorkflow(string workflowName), when workflow.WorkflowsToInject?.Any() == true, workflow.Rules = workflow.Rules.Union(injectedWorkflow.Rules).ToList() mutates the cached Workflow instance stored in _workflowRules (and the caller's original Workflow reference if passed in memory).

   • Calling GetWorkflow (or ExecuteAllRulesAsync) returns a Workflow whose Rules collection has been permanently overwritten.
   • If an injected workflow is later updated via AddOrUpdateWorkflow, the parent workflow continues to hold stale rules from the earlier injection alongside newly appended ones.

2. Unbounded recursion on circular WorkflowsToInject:
   If workflow A injects B and B injects A, GetWorkflow recurses infinitely and crashes with StackOverflowException.

Solution

• Return a cloned Workflow instance when resolving WorkflowsToInject so the cached Workflow.Rules in _workflowRules is never modified in-place.
• Track visited workflow names during GetWorkflow resolution and throw an InvalidOperationException when a cycle in WorkflowsToInject is detected.
• Added unit tests in est/RulesEngine.UnitTest/WorkflowsToInjectMutationTest.cs covering non-mutation, updating an injected workflow via AddOrUpdateWorkflow, and cycle detection.

[Martin Brekhof (MBrekhof)]

@microsoft-github-policy-service agree

[Martin Brekhof (MBrekhof)]

@microsoft-github-policy-service rerun

[YogeshPraj]

Thanks, this is a real fix. On main the injected rules were appended to the cached workflow on every call, so the rule list grew without bound. In my benchmark that was about 4 ms and 7 MB per call, against about 4.5 µs with this change. The cycle detection and the copy of all Workflow properties look correct, and all existing tests pass on net8, net9 and net10.

Change needed: the BOM on RulesCache.cs was stripped. Please restore the original UTF-8 BOM so the diff only shows the intended change.

Optional: the merged workflow is rebuilt on every call (about 14 KB each). Caching it until the workflow is updated would remove that cost.

[Martin Brekhof (MBrekhof)]

Thanks for the review and the benchmark numbers.

• Restored the UTF-8 BOM on RulesCache.cs (5f9a884). The diff now only contains the GetWorkflow change and the new test.
• I left the optional caching of the merged workflow out of this PR, to keep it focused on the mutation fix. Happy to send it as a follow-up if you'd like it.

🤖 Generated with Claude Code

[YogeshPraj]

Approved. Encoding fix confirmed and the diff is now only the intended change. Tests pass on net8/net9/net10 and the injected-workflow growth is gone in my benchmarks. Release note: a circular WorkflowsToInject now throws InvalidOperationException instead of overflowing the stack.