Validate duplicate param names and input/global collisions with clear errors#743
Merged
YogeshPraj merged 2 commits intoJun 11, 2026
Merged
Conversation
… errors PR microsoft#402 surfaced two cases where the engine threw cryptic "An item with the same key has already been added. Key: …" deep inside RuleResultTree construction: 1. Two GlobalParams in the same workflow sharing a name. 2. Two LocalParams on the same rule sharing a name. 3. A caller-supplied RuleParameter whose Name matches a workflow's GlobalParam name. All three now surface clear, actionable errors: - WorkflowsValidator catches case 1 at AddWorkflow time and throws RuleValidationException with "Duplicate GlobalParam name '<name>'". - RuleValidator catches case 2 similarly with "Duplicate LocalParam name '<name>'". - RulesEngine.AppendGlobals catches case 3 at execution time and throws RuleException with "GlobalParam name '<name>' collides with an input RuleParameter of the same name". The exception flows through the existing per-rule error-surfacing path in ExecuteAllRuleByWorkflow, so callers see the explanatory message on each rule's ExceptionMessage — the same pattern other scoped-params errors already use. Constants in Constants.cs make the messages reusable and tested. Regression test: DuplicateParamNameValidationTest covers all three failure modes plus a sanity case where distinct names continue to work. All 174 unit tests pass on net6 / net8 / net9 / net10.
5fff2ef to
fa2f73e
Compare
sagarambilpure
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cleans up three cryptic "An item with the same key has already been added. Key: …" failures users hit when they accidentally introduce a name collision. Surfaced while triaging stale PR #402 — the underlying concerns it identified are real bugs even though that PR's proposed approach wasn't a fit.
The three cases
GlobalParamsin the same workflow with the sameNameAddWorkflowif eagerly used; otherwise at executionDictionary.ToDictionarythrows"An item with the same key has already been added. Key: dup"WorkflowsValidatorthrowsRuleValidationException("Duplicate GlobalParam name 'dup'. GlobalParam names must be unique within a workflow.")LocalParamson the same rule with the sameNameRuleValidatorthrowsRuleValidationException("Duplicate LocalParam name 'dup'. …")RuleParameter("foo", …)against a workflow withGlobalParam("foo", …)RulesEngine.AppendGlobalsthrowsRuleException("GlobalParam name 'foo' collides with an input RuleParameter of the same name. Rename either the global or the input to disambiguate.")— flowed through the existing per-rule error-surfacing path so each rule'sExceptionMessagecarries the explanationWhy this shape
WorkflowsValidator/RuleValidator).AppendGlobals) and uses the sametry/catchsurfacing pattern other scoped-params errors already use inExecuteAllRuleByWorkflow, so the behavior is consistent: each rule'sExceptionMessageexplains the collision andIsSuccess = false.HelperFunctions/Constants.csalongside the existing validator strings — same convention.Not in this PR
The other claims in PR #402 either don't reproduce on current
main(Dynamic.Corenew (…)syntax works withoutAllowNewToEvaluateAnyType) or are design debates rather than bugs (chained-rule result visibility, PostActions timing).Test plan
DuplicateParamNameValidationTest.cscovering:GlobalParams→RuleValidationExceptionwith "duplicate" + offending nameLocalParams→ sameExceptionMessagecontains "collide" + offending name; does NOT contain "same key has already been added"