Fix the same shape without a parent service field, bump to 6.2.3

The replacer fix alone left `srv.Get(new Key(ctx.A, ctx.B))` broken on a plain service field with nothing above it - it turned "An item with the same key has already been added" into "Argument types do not match", because the extracted expression is still the whole construction. Walk through New/MemberInit in ExpressionExtractor.ProcessPotentialLeaf so each member read credits itself, which is what the class doc-comment already describes. Keep the ExpressionReplacer change: the extractor still emits a repeat for a conditional argument, which hits the identical crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lukemurray · Sep 8, 2026 · Sep 8, 2026 · Sep 8, 2026 · Sep 8, 2026 · dcd4a251103f571189c4fb732a9f621ec2d6dd6b
commit dcd4a251103f571189c4fb732a9f621ec2d6dd6b
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,13 @@
+# 6.2.3
+
+## Fixes
+
+- Fixed `An item with the same key has already been added` when a service field's resolver builds an object from more than one context member as an argument to the service call - `(ctx, srv) => srv.Get(new Key(ctx.A, ctx.B))`. `ExpressionExtractor` credited both member reads to the enclosing construction rather than to themselves, so it emitted the same `Expression` instance twice under the one name and `ExpressionReplacer` added each of them to a dictionary keyed by node identity. A construction is now walked through rather than treated as a leaf, so each read is extracted on its own. `ExpressionReplacer` also tolerates a repeated expression, which the extractor still emits for other shapes (a conditional argument, where the branch reads are both credited to the conditional).
+
 # 6.2.2
 
 ## Fixes
 
-- Fixed `An item with the same key has already been added` when a service field's resolver builds an object from more than one context member as an argument to the service call (`(ctx, srv) => srv.Get(new Key(ctx.A, ctx.B))`) and that field is selected below another service field. `ExpressionExtractor` credits both member reads to the enclosing construction rather than to themselves, so it emits the same `Expression` instance twice under the one name - which its own contract allows - and `ExpressionReplacer` was adding each of them to a dictionary keyed by node identity.
 - Fixed a field whose dotnet type is a nullable value type (`Instant?`, `DateTime?`, `int?`) resolving to `default(T)` instead of `null`, for a query that also selects an async field. Results are rebuilt after any async field is awaited, and each member's new type was taken from the resolved value's runtime type.
 
 # 6.2.1

diff --git a/src/EntityGraphQL.AspNet/EntityGraphQL.AspNet.csproj b/src/EntityGraphQL.AspNet/EntityGraphQL.AspNet.csproj
@@ -4,7 +4,7 @@
     <AssemblyName>EntityGraphQL.AspNet</AssemblyName>
     <PackageId>EntityGraphQL.AspNet</PackageId>
     <LangVersion>14</LangVersion>
-    <PackageVersion>6.2.2</PackageVersion>
+    <PackageVersion>6.2.3</PackageVersion>
     <Description>Contains ASP.NET extensions and middleware for EntityGraphQL</Description>
     <Authors>Luke Murray</Authors>
     <PackageProjectUrl>https://github.com/lukemurray/EntityGraphQL</PackageProjectUrl>

diff --git a/src/EntityGraphQL/Compiler/Util/ExpressionExtractor.cs b/src/EntityGraphQL/Compiler/Util/ExpressionExtractor.cs
@@ -87,7 +87,13 @@ protected override Expression VisitBinary(BinaryExpression node)
 
     private void ProcessPotentialLeaf(Expression node)
     {
-        var shouldAddNotAdd = node.NodeType == ExpressionType.MemberAccess && ((MemberExpression)node).Expression?.Type.IsNullableType() == true;
+        // A construction is not a leaf - it is a wrapper around the reads that build it. Visiting it with itself on
+        // the stack credits every one of those reads to the construction, so `srv.Get(new Key(ctx.A, ctx.B))` extracts
+        // that one node twice instead of ctx.A and ctx.B. Visit it without pushing so each read credits itself.
+        var shouldAddNotAdd =
+            (node.NodeType == ExpressionType.MemberAccess && ((MemberExpression)node).Expression?.Type.IsNullableType() == true)
+            || node.NodeType == ExpressionType.New
+            || node.NodeType == ExpressionType.MemberInit;
         if (shouldAddNotAdd)
             Visit(node);
         else

diff --git a/src/EntityGraphQL/EntityGraphQL.csproj b/src/EntityGraphQL/EntityGraphQL.csproj
@@ -4,7 +4,7 @@
     <LangVersion>14.0</LangVersion>
     <AssemblyName>EntityGraphQL</AssemblyName>
     <PackageId>EntityGraphQL</PackageId>
-    <PackageVersion>6.2.2</PackageVersion>
+    <PackageVersion>6.2.3</PackageVersion>
     <Description>A GraphQL library for .NET. Compiles queries into .NET Expressions (LinqProvider) for runtime execution against object graphs. E.g. against an ORM data model (EntityFramework or others) or just an in-memory object.</Description>
     <Authors>Luke Murray</Authors>
     <PackageProjectUrl>https://github.com/lukemurray/EntityGraphQL</PackageProjectUrl>

diff --git a/src/tests/EntityGraphQL.Tests/QueryTests/ServiceFieldTests.cs b/src/tests/EntityGraphQL.Tests/QueryTests/ServiceFieldTests.cs
@@ -1377,10 +1377,56 @@ public void TestCollectionWithServiceUsingNewInExpression()
     }
 
     /// <summary>
-    /// A resolver that builds an object from more than one context member, as an argument to the service call,
-    /// has both member reads credited to the enclosing new/init expression - so ExpressionExtractor records that
-    /// single node instance twice. Below another service field the bulk path is unavailable and ExpressionReplacer
-    /// is built from those expressions, which used to throw "An item with the same key has already been added".
+    /// A resolver that builds an object from more than one context member, as an argument to the service call.
+    /// ExpressionExtractor used to credit both member reads to the enclosing new/init expression, recording that
+    /// single node instance twice - which threw "An item with the same key has already been added" when
+    /// ExpressionReplacer was built from those expressions, and left the second pass unable to rebuild the call
+    /// once the replacer tolerated the repeat ("Argument types do not match"). The construction is now walked
+    /// through, so each member read is extracted on its own.
+    /// </summary>
+    [Fact]
+    public void TestServiceFieldWithNewInServiceCallArg()
+    {
+        var schema = SchemaBuilder.FromObject<TestDataContext>();
+        schema.AddType<ProjectConfig>("ProjectConfig").AddAllFields();
+        schema.Type<User>().AddField("config", "Config for the user").Resolve<ConfigService>((user, srv) => srv.Get(new Project { Id = user.Id, Name = user.Field2 }));
+
+        var gql = new QueryRequest { Query = "{ users { id config { type } } }" };
+
+        var context = new TestDataContext().FillWithTestData();
+        var sp = new ServiceCollection().AddSingleton(new ConfigService()).BuildServiceProvider();
+
+        var res = schema.ExecuteRequestWithContext(gql, context, sp, null);
+        Assert.Null(res.Errors);
+        dynamic users = res.Data!["users"]!;
+        Assert.Equal("Something", users[0].config.type);
+    }
+
+    /// <summary>
+    /// Same shape, but the argument is a conditional. There is no VisitConditional override in the extractor so the
+    /// ifTrue/ifFalse reads are still both credited to the conditional - the repeat ExpressionReplacer has to tolerate.
+    /// </summary>
+    [Fact]
+    public void TestServiceFieldWithConditionalInServiceCallArg()
+    {
+        var schema = SchemaBuilder.FromObject<TestDataContext>();
+        schema.AddType<ProjectConfig>("ProjectConfig").AddAllFields();
+        schema.Type<User>().AddField("config", "Config for the user").Resolve<ConfigService>((user, srv) => srv.Get(user.Id > 1 ? user.Id : user.Field2.Length));
+
+        var gql = new QueryRequest { Query = "{ users { id config { type } } }" };
+
+        var context = new TestDataContext().FillWithTestData();
+        var sp = new ServiceCollection().AddSingleton(new ConfigService()).BuildServiceProvider();
+
+        var res = schema.ExecuteRequestWithContext(gql, context, sp, null);
+        Assert.Null(res.Errors);
+        dynamic users = res.Data!["users"]!;
+        Assert.Equal("Something", users[0].config.type);
+    }
+
+    /// <summary>
+    /// As above but selected below another service field, where the bulk path is unavailable and the per-item
+    /// resolver runs instead.
     /// </summary>
     [Fact]
     public void TestServiceFieldWithNewInServiceCallArgUnderServiceField()

diff --git a/src/tests/EntityGraphQL.Tests/Util/ExpressionExtractorTests.cs b/src/tests/EntityGraphQL.Tests/Util/ExpressionExtractorTests.cs
@@ -53,16 +53,34 @@ public void ExtractLongMemberExpressionSameNameInMethod()
     [Fact]
     public void ExtractObjectBuiltFromMultipleMembersInMethodArg()
     {
-        // Building an object from 2 context members as an argument to the service call - both member reads are
-        // credited to the enclosing expression, so the same node instance is extracted twice under the one name.
-        // ExpressionReplacer relies on tolerating that repeat - see TestServiceFieldWithNewInServiceCallArgUnderServiceField
+        // Building an object from 2 context members as an argument to the service call - the construction is not a
+        // leaf, so each member read is extracted on its own rather than both being credited to the construction.
         Expression<Func<User, ConfigService, ProjectConfig>> expression = (user, srv) => srv.Get(new Project { Id = user.Id, Name = user.Field2 });
         var extractor = new ExpressionExtractor();
         var extracted = extractor.Extract(expression.Body, expression.Parameters[0], false);
         Assert.NotNull(extracted);
-        Assert.Single(extracted);
+        Assert.Equal(2, extracted.Count);
+        var init = (MemberInitExpression)((MethodCallExpression)expression.Body).Arguments[0];
+        Assert.Equal("egql__user_Id", extracted.First().Key);
+        Assert.Equal(((MemberAssignment)init.Bindings[0]).Expression, extracted.First().Value.Single());
+        Assert.Equal("egql__user_Field2", extracted.ElementAt(1).Key);
+        Assert.Equal(((MemberAssignment)init.Bindings[1]).Expression, extracted.ElementAt(1).Value.Single());
+    }
+
+    [Fact]
+    public void ExtractConditionalInMethodArg()
+    {
+        // No VisitConditional override - the ifTrue/ifFalse reads are both credited to the conditional, so that one
+        // node is extracted twice under the one name. ExpressionReplacer tolerates that repeat by design.
+        // (the test is a binary, which VisitBinary walks into, so user.Id is also extracted on its own)
+        Expression<Func<User, ConfigService, ProjectConfig>> expression = (user, srv) => srv.Get(user.Id > 1 ? user.Id : user.Field2.Length);
+        var extractor = new ExpressionExtractor();
+        var extracted = extractor.Extract(expression.Body, expression.Parameters[0], false);
+        Assert.NotNull(extracted);
+        Assert.Equal(2, extracted.Count);
+        Assert.Equal("egql__user_Id", extracted.First().Key);
         var arg = ((MethodCallExpression)expression.Body).Arguments[0];
-        Assert.Equal([arg, arg], extracted.First().Value);
+        Assert.Equal([arg, arg], extracted.ElementAt(1).Value);
     }
 
     [Fact]