Validate duplicate param names and input/global collisions with clear errors

src/RulesEngine/HelperFunctions/Constants.cs

@@ -21,6 +21,9 @@ public static class Constants
         public const string LAMBDA_EXPRESSION_EXPRESSION_NULL_ERRMSG = "Expression cannot be null or empty when RuleExpressionType is LambdaExpression";
         public const string LAMBDA_EXPRESSION_OPERATOR_ERRMSG = "Cannot use Operator field when RuleExpressionType is LambdaExpression";
         public const string LAMBDA_EXPRESSION_RULES_ERRMSG = "Cannot use Rules field when RuleExpressionType is LambdaExpression";
+        public const string DUPLICATE_GLOBAL_PARAM_NAME_ERRMSG = "Duplicate GlobalParam name '{0}'. GlobalParam names must be unique within a workflow.";
+        public const string DUPLICATE_LOCAL_PARAM_NAME_ERRMSG = "Duplicate LocalParam name '{0}'. LocalParam names must be unique within a rule.";
+        public const string GLOBAL_PARAM_INPUT_COLLISION_ERRMSG = "GlobalParam name '{0}' collides with an input RuleParameter of the same name. Rename either the global or the input to disambiguate.";
 
     }
 }

src/RulesEngine/RulesEngine.cs

@@ -190,6 +190,18 @@ private static RuleParameter[] AppendGlobals(RuleParameter[] ruleParameters, Fun
             }
             var inputs = ruleParameters.Select(c => c.Value).ToArray();
             var evaluated = globalParamsDelegate(inputs);
+
+            // Catch input/global name collisions here with a clear error instead of letting
+            // Helpers.ToResultTree's ToDictionary throw the cryptic "key already added" message.
+            var inputNames = new HashSet<string>(ruleParameters.Select(c => c.Name), StringComparer.Ordinal);
+            foreach (var key in evaluated.Keys)
+            {
+                if (inputNames.Contains(key))
+                {
+                    throw new RuleException(string.Format(Constants.GLOBAL_PARAM_INPUT_COLLISION_ERRMSG, key));
+                }
+            }
+
             var globals = evaluated.Select(kv => new RuleParameter(kv.Key, kv.Value));
             return ruleParameters.Concat(globals).ToArray();
         }

src/RulesEngine/Validators/RuleValidator.cs

@@ -33,6 +33,14 @@ public RuleValidator()
                 });
             });
             RegisterExpressionTypeRules();
+
+            // Surface duplicate LocalParam names with a clear message instead of a cryptic
+            // "An item with the same key has already been added" failure later at execution.
+            RuleFor(c => c)
+                .Must(rule => WorkflowsValidator.FindDuplicateName(rule.LocalParams?.Select(p => p?.Name)) == null)
+                .WithMessage(rule => string.Format(
+                    Constants.DUPLICATE_LOCAL_PARAM_NAME_ERRMSG,
+                    WorkflowsValidator.FindDuplicateName(rule.LocalParams?.Select(p => p?.Name))));
         }
 
         private void RegisterExpressionTypeRules()

src/RulesEngine/Validators/WorkflowRulesValidator.cs

@@ -19,6 +19,26 @@ public WorkflowsValidator()
                 var ruleValidator = new RuleValidator();
                 RuleForEach(c => c.Rules).SetValidator(ruleValidator);
             });
+
+            // Surface duplicate GlobalParam names with a clear message instead of a cryptic
+            // "An item with the same key has already been added" failure later at execution.
+            RuleFor(c => c)
+                .Must(workflow => FindDuplicateName(workflow.GlobalParams?.Select(p => p?.Name)) == null)
+                .WithMessage(workflow => string.Format(
+                    Constants.DUPLICATE_GLOBAL_PARAM_NAME_ERRMSG,
+                    FindDuplicateName(workflow.GlobalParams?.Select(p => p?.Name))));
+        }
+
+        internal static string FindDuplicateName(System.Collections.Generic.IEnumerable<string> names)
+        {
+            if (names == null) return null;
+            var seen = new System.Collections.Generic.HashSet<string>(System.StringComparer.Ordinal);
+            foreach (var name in names)
+            {
+                if (string.IsNullOrEmpty(name)) continue;
+                if (!seen.Add(name)) return name;
+            }
+            return null;
         }
     }
 }

test/RulesEngine.UnitTest/DuplicateParamNameValidationTest.cs

@@ -0,0 +1,115 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using RulesEngine.Exceptions;
+using RulesEngine.Models;
+using System;
+using System.Diagnostics.CodeAnalysis;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace RulesEngine.UnitTest
+{
+    [ExcludeFromCodeCoverage]
+    public class DuplicateParamNameValidationTest
+    {
+        // Two GlobalParams with the same Name in the same workflow used to crash deep inside
+        // RuleResultTree construction with "An item with the same key has already been added.
+        // Key: dup". This change validates the workflow at AddWorkflow time and surfaces a clear
+        // error pointing at the offending name.
+        [Fact]
+        public void AddWorkflow_DuplicateGlobalParamNames_ThrowsClearValidationError()
+        {
+            var workflow = new Workflow
+            {
+                WorkflowName = "wf",
+                GlobalParams = new[]
+                {
+                    new ScopedParam { Name = "dup", Expression = "1" },
+                    new ScopedParam { Name = "dup", Expression = "2" }
+                },
+                Rules = new[] { new Rule { RuleName = "R", Expression = "dup == 1" } }
+            };
+
+            var engine = new RulesEngine();
+            var ex = Assert.Throws<RuleValidationException>(() => engine.AddWorkflow(workflow));
+            Assert.Contains("dup", ex.Message);
+            Assert.Contains("duplicate", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+
+        [Fact]
+        public void AddWorkflow_DuplicateLocalParamNames_ThrowsClearValidationError()
+        {
+            // Same protection for per-rule LocalParams.
+            var workflow = new Workflow
+            {
+                WorkflowName = "wf",
+                Rules = new[] {
+                    new Rule {
+                        RuleName = "R",
+                        Expression = "dup == 1",
+                        LocalParams = new[]
+                        {
+                            new ScopedParam { Name = "dup", Expression = "1" },
+                            new ScopedParam { Name = "dup", Expression = "2" }
+                        }
+                    }
+                }
+            };
+
+            var engine = new RulesEngine();
+            var ex = Assert.Throws<RuleValidationException>(() => engine.AddWorkflow(workflow));
+            Assert.Contains("dup", ex.Message);
+            Assert.Contains("duplicate", ex.Message, StringComparison.OrdinalIgnoreCase);
+        }
+
+        // A caller-supplied RuleParameter whose Name collides with a workflow GlobalParam's Name
+        // used to surface as a cryptic "An item with the same key has already been added" deep
+        // inside result-tree construction. After this change the per-rule ExceptionMessage
+        // explicitly names the colliding identifier — same surfacing pattern as other
+        // scoped-params errors that ExecuteAllRuleByWorkflow already catches and reports per rule.
+        [Fact]
+        public async Task ExecuteAllRulesAsync_CallerInputCollidesWithGlobalParam_SurfacesClearCollisionError()
+        {
+            var workflow = new Workflow
+            {
+                WorkflowName = "wf",
+                GlobalParams = new[]
+                {
+                    new ScopedParam { Name = "foo", Expression = "99" }
+                },
+                Rules = new[] { new Rule { RuleName = "R", Expression = "foo == 99" } }
+            };
+            var engine = new RulesEngine(new[] { workflow });
+
+            var results = await engine.ExecuteAllRulesAsync("wf",
+                new[] { RuleParameter.Create("foo", 42) });
+
+            Assert.False(results[0].IsSuccess);
+            Assert.Contains("foo", results[0].ExceptionMessage);
+            Assert.Contains("collide", results[0].ExceptionMessage, StringComparison.OrdinalIgnoreCase);
+            Assert.DoesNotContain("same key has already been added", results[0].ExceptionMessage);
+        }
+
+        // Sanity: no collision when names are distinct (regression guard for the validation logic).
+        [Fact]
+        public async Task ExecuteAllRulesAsync_DistinctNames_StillWorks()
+        {
+            var workflow = new Workflow
+            {
+                WorkflowName = "wf",
+                GlobalParams = new[]
+                {
+                    new ScopedParam { Name = "fromGlobal", Expression = "99" }
+                },
+                Rules = new[] { new Rule { RuleName = "R", Expression = "fromInput == 42 && fromGlobal == 99" } }
+            };
+            var engine = new RulesEngine(new[] { workflow });
+
+            var results = await engine.ExecuteAllRulesAsync("wf",
+                new[] { RuleParameter.Create("fromInput", 42) });
+
+            Assert.True(results[0].IsSuccess);
+        }
+    }
+}