From 8d6a557e1b57ed8698f3166144415561dfefa7a0 Mon Sep 17 00:00:00 2001 From: MBrekhof Date: Wed, 21 Oct 2026 22:16:00 +0200 Subject: [PATCH] Fix EvaluateRule action failing when the target workflow has GlobalParams (#626) The inputs an EvaluateRule action forwards come from RuleResultTree.Inputs, which already holds the source rule's evaluated GlobalParams. The target workflow then evaluates its own GlobalParams again, so chaining into any workflow with globals failed with "The identifier 'x' was defined more than once" (or a GlobalParam/input collision error). EvaluateRuleAction now drops forwarded params named like the target workflow's GlobalParams before the call; the target recomputes them from the remaining inputs. Names are compared the way the expression parser compares identifiers (IsExpressionCaseSensitive), nothing is dropped when EnableScopedParams is off, and additionalInputs are added after the drop so they behave as before. Co-Authored-By: Claude Opus 5.5 --- src/RulesEngine/Actions/EvaluateRuleAction.cs | 4 + src/RulesEngine/RulesCache.cs | 12 ++ src/RulesEngine/RulesEngine.cs | 12 ++ test/RulesEngine.UnitTest/Issue626Test.cs | 186 ++++++++++++++++++ 4 files changed, 214 insertions(+) create mode 100644 test/RulesEngine.UnitTest/Issue626Test.cs diff --git a/src/RulesEngine/Actions/EvaluateRuleAction.cs b/src/RulesEngine/Actions/EvaluateRuleAction.cs index 032308e8..0af10c58 100644 --- a/src/RulesEngine/Actions/EvaluateRuleAction.cs +++ b/src/RulesEngine/Actions/EvaluateRuleAction.cs @@ -47,6 +47,10 @@ public async override ValueTask Run(ActionContext context, RuleParameter { filteredRuleParameters = ruleParameters.Where(c => inputFilter.Contains(c.Name)).ToList(); } + // The forwarded inputs include the source rule's evaluated GlobalParams. The target workflow + // evaluates its own globals again, so a same-named forwarded value would be defined twice (#626). + var targetGlobals = _ruleEngine.GetEvaluatedGlobalParamNames(workflowName); + filteredRuleParameters.RemoveAll(c => targetGlobals.Contains(c.Name)); if (context.TryGetContext>("additionalInputs", out var additionalInputs)) { foreach(var additionalInput in additionalInputs) diff --git a/src/RulesEngine/RulesCache.cs b/src/RulesEngine/RulesCache.cs index 968d7b80..729f1d2f 100644 --- a/src/RulesEngine/RulesCache.cs +++ b/src/RulesEngine/RulesCache.cs @@ -56,6 +56,18 @@ public List GetAllWorkflowNames() return _workflow.Keys.ToList(); } + /// Names of the workflow's own GlobalParams; empty if the workflow is unknown. + /// Reads the stored workflow directly: GetWorkflow resolves WorkflowsToInject, which only adds rules, not globals. + internal IEnumerable GetGlobalParamNames(string workflowName) + { + if (string.IsNullOrEmpty(workflowName) || !_workflow.TryGetValue(workflowName, out var entry)) + { + return Enumerable.Empty(); + } + return entry.Item1.GlobalParams?.Where(p => !string.IsNullOrEmpty(p?.Name)).Select(p => p.Name) + ?? Enumerable.Empty(); + } + /// Adds the or update workflow rules. /// Name of the workflow. /// The rules. diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index b31bb88c..7c881f25 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -181,6 +181,18 @@ public async ValueTask ExecuteActionWorkflowAsync(string workf return actionResult; } + // GlobalParams the workflow evaluates itself, compared the way the expression parser compares identifiers. + // Used by EvaluateRuleAction to drop forwarded values the target workflow recomputes (#626). + internal ISet GetEvaluatedGlobalParamNames(string workflowName) + { + var comparer = _reSettings.IsExpressionCaseSensitive ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase; + if (!_reSettings.EnableScopedParams) + { + return new HashSet(comparer); + } + return new HashSet(_rulesCache.GetGlobalParamNames(workflowName), comparer); + } + // Invokes the supplied globals delegate (if any) and appends the results as RuleParameters. private static RuleParameter[] AppendGlobals(RuleParameter[] ruleParameters, Func> globalParamsDelegate) { diff --git a/test/RulesEngine.UnitTest/Issue626Test.cs b/test/RulesEngine.UnitTest/Issue626Test.cs new file mode 100644 index 00000000..e687ad24 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue626Test.cs @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + // An EvaluateRule action forwards the source rule's inputs, which already contain the + // evaluated GlobalParams. Chaining into a workflow that declares the same GlobalParams + // used to fail with "The identifier 'x' was defined more than once". + [Trait("Category", "Unit")] + [ExcludeFromCodeCoverage] + public class Issue626Test + { + private static Workflow GetWorkflow(string globalName = "g", string chainedExpression = "g == 1", + Dictionary extraContext = null) + { + var context = new Dictionary { + { "workflowName", "wf" }, + { "ruleName", "r1" } + }; + foreach (var kv in extraContext ?? new Dictionary()) + { + context[kv.Key] = kv.Value; + } + + return new Workflow { + WorkflowName = "wf", + GlobalParams = new[] { new ScopedParam { Name = globalName, Expression = "input1.X" } }, + Rules = new[] { + new Rule { + RuleName = "r1", + Expression = chainedExpression, + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "OutputExpression", + Context = new Dictionary { { "expression", globalName + " * 10" } } + } + } + }, + new Rule { + RuleName = "r2", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { Name = "EvaluateRule", Context = context } + } + } + } + }; + } + + private static RuleParameter Input(int x) => new RuleParameter("input1", new { X = x }); + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ExecuteActionWorkflowAsync_ChainIntoWorkflowWithGlobalParams_Succeeds(bool exceptionAsErrorMessage) + { + var engine = new RulesEngine(new[] { GetWorkflow() }, + new ReSettings { EnableExceptionAsErrorMessage = exceptionAsErrorMessage }); + + var result = await engine.ExecuteActionWorkflowAsync("wf", "r2", new[] { Input(1) }); + + Assert.Null(result.Exception); + Assert.Equal(10, result.Output); + Assert.All(result.Results, r => Assert.True(r.IsSuccess, r.ExceptionMessage)); + } + + [Fact] + public async Task ExecuteAllRulesAsync_ChainIntoWorkflowWithGlobalParams_Succeeds() + { + var engine = new RulesEngine(new[] { GetWorkflow() }); + + var results = await engine.ExecuteAllRulesAsync("wf", Input(1)); + + var chained = results.Single(r => r.Rule.RuleName == "r2"); + Assert.True(chained.IsSuccess, chained.ExceptionMessage); + Assert.Null(chained.ActionResult.Exception); + Assert.Equal(10, chained.ActionResult.Output); + } + + // A separate workflow with global "G" (= input1.X + 100) whose rule chains into targetWorkflow/targetRule. + private static Workflow GetSourceWorkflow(string targetWorkflow, string targetRule) + { + return new Workflow { + WorkflowName = "source", + GlobalParams = new[] { new ScopedParam { Name = "G", Expression = "input1.X + 100" } }, + Rules = new[] { + new Rule { + RuleName = "start", + Expression = "G == 101", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "EvaluateRule", + Context = new Dictionary { { "workflowName", targetWorkflow }, { "ruleName", targetRule } } + } + } + } + } + }; + } + + [Fact] + public async Task ExecuteActionWorkflowAsync_CaseInsensitiveGlobalName_TargetGlobalIsRecomputed() + { + // Default IsExpressionCaseSensitive = false: the forwarded "G" (101) collides with the + // target workflow's global "g" in the expression parser. + var engine = new RulesEngine(new[] { GetSourceWorkflow("wf", "r1"), GetWorkflow() }, + new ReSettings { EnableExceptionAsErrorMessage = false }); + + var result = await engine.ExecuteActionWorkflowAsync("source", "start", new[] { Input(1) }); + + // "G" is dropped in favour of the target workflow's own "g", recomputed from input1 (1, not 101). + Assert.Null(result.Exception); + Assert.Equal(10, result.Output); + } + + [Fact] + public async Task ExecuteActionWorkflowAsync_CaseSensitiveGlobalName_SourceGlobalIsKept() + { + // With IsExpressionCaseSensitive = true, "G" and "g" are different identifiers: keep "G". + var engine = new RulesEngine(new[] { GetSourceWorkflow("wf", "r1"), GetWorkflow(chainedExpression: "g == 1 && G == 101") }, + new ReSettings { IsExpressionCaseSensitive = true, EnableExceptionAsErrorMessage = false }); + + var result = await engine.ExecuteActionWorkflowAsync("source", "start", new[] { Input(1) }); + + Assert.Null(result.Exception); + Assert.Equal(10, result.Output); + } + + [Fact] + public async Task ExecuteActionWorkflowAsync_TargetWithoutThatGlobal_SourceGlobalIsForwarded() + { + var target = new Workflow { + WorkflowName = "plain", + Rules = new[] { new Rule { RuleName = "check", Expression = "G == 101" } } + }; + var engine = new RulesEngine(new[] { GetSourceWorkflow("plain", "check"), target }, + new ReSettings { EnableExceptionAsErrorMessage = false }); + + var result = await engine.ExecuteActionWorkflowAsync("source", "start", new[] { Input(1) }); + + Assert.Null(result.Exception); + var check = Assert.Single(result.Results, r => r.Rule.RuleName == "check"); + Assert.True(check.IsSuccess, check.ExceptionMessage); + } + + [Fact] + public async Task ExecuteActionWorkflowAsync_ScopedParamsDisabled_InputNamedLikeGlobalIsForwarded() + { + // With scoped params off the workflow's GlobalParams are never evaluated, so an input that + // happens to share a global's name is a real input and must reach the chained rule. + var workflow = GetWorkflow(chainedExpression: "g == 1"); + workflow.Rules.First().Actions = null; + var engine = new RulesEngine(new[] { workflow }, + new ReSettings { EnableScopedParams = false, EnableExceptionAsErrorMessage = false }); + + var result = await engine.ExecuteActionWorkflowAsync("wf", "r2", new[] { Input(1), new RuleParameter("g", 1) }); + + Assert.Null(result.Exception); + Assert.All(result.Results, r => Assert.True(r.IsSuccess, r.ExceptionMessage)); + } + + [Fact] + public async Task ExecuteActionWorkflowAsync_AdditionalInputsWithGlobalParams_StillWork() + { + var workflow = GetWorkflow( + chainedExpression: "g == 1 && extra == 2", + extraContext: new Dictionary { + { "additionalInputs", new List { new ScopedParam { Name = "extra", Expression = "g + 1" } } } + }); + // Default settings: r1 cannot compile for the top-level call (no "extra" there), only for the chained one. + var engine = new RulesEngine(new[] { workflow }); + + var result = await engine.ExecuteActionWorkflowAsync("wf", "r2", new[] { Input(1) }); + + Assert.Null(result.Exception); + Assert.Equal(10, result.Output); + } + } +}