From fac0545c09ccc379242d36785c550bf7a70ac884 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:30:47 +0000 Subject: [PATCH 1/4] Initial plan From d840bc14d74c7b28d1e5ef0bf2c36206bbe7a76b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:48:13 +0000 Subject: [PATCH 2/4] fix: required client initialization parameters should not trigger separate ClientOptions type In UseSingletonInstance, skip required parameters (those without DefaultValue) when determining if a client needs a separate client-specific options type. Required parameters are inlined as constructor parameters on the client and never become properties on the ClientOptions class, so they should not trigger the creation of a separate client-specific options type. Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/6c0324cb-28eb-47ca-b77e-05f5375bcea7 Co-authored-by: JonathanCrd <17486462+JonathanCrd@users.noreply.github.com> --- .../src/Providers/ClientOptionsProvider.cs | 12 +++++-- .../Providers/ClientOptionsProviderTests.cs | 36 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs index e0a0bf9edf8..de2c76a405c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs @@ -85,7 +85,9 @@ public static ClientOptionsProvider CreateClientOptionsProvider(InputClient inpu } /// - /// Determines if a client has only standard parameters (ApiVersion and Endpoint). + /// Determines if a client has only standard parameters (ApiVersion, Endpoint, and required non-optional parameters). + /// Only optional parameters with default values that would become properties on the options class + /// should trigger a separate client-specific options type. /// /// The input client to check. /// True if the client has only standard parameters, false otherwise. @@ -111,11 +113,15 @@ private static bool UseSingletonInstance(InputClient inputClient) return false; // Found a non-standard endpoint parameter } } - else + else if (parameter.DefaultValue != null) { - // Found a non-ApiVersion, non-Endpoint parameter + // Found a non-ApiVersion, non-Endpoint optional parameter that will become + // a property on the options class — requires a separate client-specific options type. return false; } + // Required parameters (DefaultValue == null) are inlined as constructor parameters + // on the client and do not become properties on the options class, + // so they should not trigger a separate client-specific options type. } } return true; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientOptionsProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientOptionsProviderTests.cs index 93664c6310b..7372b6e8f23 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientOptionsProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientOptionsProviderTests.cs @@ -466,6 +466,42 @@ public void MultipleClientsWithCustomParametersCreateSeparateOptions() Assert.AreEqual("SampleClientOptions", options2!.Name); } + [Test] + public void MultipleClientsWithRequiredCustomParametersShareSingletonOptions() + { + // Required parameters (no DefaultValue) should NOT trigger a separate client-specific options type. + // They are inlined as constructor parameters on the client, not as properties on ClientOptions. + var requiredParam = InputFactory.MethodParameter( + "knowledgeBaseName", + InputPrimitiveType.String, + isRequired: true, + scope: InputParameterScope.Client); + + var client1 = InputFactory.Client("KnowledgeBaseRetrievalClient", clientNamespace: "TestNamespace", parameters: [requiredParam]); + var client2 = InputFactory.Client("SearchClient", clientNamespace: "TestNamespace"); + + MockHelpers.LoadMockGenerator(clients: () => [client1, client2]); + + var clientProvider1 = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(client1); + var clientProvider2 = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(client2); + + Assert.IsNotNull(clientProvider1); + Assert.IsNotNull(clientProvider2); + + var options1 = clientProvider1!.ClientOptions; + var options2 = clientProvider2!.ClientOptions; + + Assert.IsNotNull(options1); + Assert.IsNotNull(options2); + + // Both clients should share the same singleton ClientOptions instance + // because the required parameter does not become a property on the options class + Assert.AreSame(options1, options2); + + // The name should be based on the InputNamespace (singleton naming) + Assert.AreEqual("SampleClientOptions", options1!.Name); + } + [Test] public void NamespaceLastSegmentIsUsedForSingletonName() { From 8e98ea60a9b7abd3b2ee184e1db8f2a46d7712d7 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 16 Apr 2026 16:00:40 -0700 Subject: [PATCH 3/4] fix: narrow UseSingletonInstance to only skip required InputMethodParameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DefaultValue check was too broad — it also skipped InputPathParameters like subscriptionId (required, no default), incorrectly allowing multi-service clients to share the singleton options type. Narrow the check to only apply to InputMethodParameter (from @@clientInitialization), preserving the original behavior for other parameter types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Providers/ClientOptionsProvider.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs index de2c76a405c..09dcc44ea55 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs @@ -85,9 +85,10 @@ public static ClientOptionsProvider CreateClientOptionsProvider(InputClient inpu } /// - /// Determines if a client has only standard parameters (ApiVersion, Endpoint, and required non-optional parameters). - /// Only optional parameters with default values that would become properties on the options class - /// should trigger a separate client-specific options type. + /// Determines if a client has only standard parameters (ApiVersion, Endpoint) and/or + /// required method parameters from @@clientInitialization that are inlined as constructor parameters. + /// Only parameters that would become properties on the options class should trigger + /// a separate client-specific options type. /// /// The input client to check. /// True if the client has only standard parameters, false otherwise. @@ -113,15 +114,18 @@ private static bool UseSingletonInstance(InputClient inputClient) return false; // Found a non-standard endpoint parameter } } - else if (parameter.DefaultValue != null) + else if (parameter is InputMethodParameter && parameter.DefaultValue == null) { - // Found a non-ApiVersion, non-Endpoint optional parameter that will become - // a property on the options class — requires a separate client-specific options type. + // Required method parameters (from @@clientInitialization) are inlined as + // constructor parameters on the client and do not become properties on + // the options class, so they should not trigger a separate options type. + } + else + { + // Found a non-ApiVersion, non-Endpoint parameter that may require + // a separate client-specific options type. return false; } - // Required parameters (DefaultValue == null) are inlined as constructor parameters - // on the client and do not become properties on the options class, - // so they should not trigger a separate client-specific options type. } } return true; From 367bc44fe18230a14a6e86f9a7d16be85370c559 Mon Sep 17 00:00:00 2001 From: jolov Date: Thu, 16 Apr 2026 16:08:41 -0700 Subject: [PATCH 4/4] fix: multi-service clients always use their own ClientOptions type Multi-service clients need client-specific options for service-specific API version properties, so UseSingletonInstance now returns false for them explicitly. Also adds singleton reset to ClientProviderTests.SetUp to prevent state leaking between tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Providers/ClientOptionsProvider.cs | 31 ++++++++++--------- .../ClientProviders/ClientProviderTests.cs | 5 +++ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs index 09dcc44ea55..d8f29f20071 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs @@ -85,13 +85,13 @@ public static ClientOptionsProvider CreateClientOptionsProvider(InputClient inpu } /// - /// Determines if a client has only standard parameters (ApiVersion, Endpoint) and/or - /// required method parameters from @@clientInitialization that are inlined as constructor parameters. - /// Only parameters that would become properties on the options class should trigger - /// a separate client-specific options type. + /// Determines if a client can share the singleton options instance. + /// Multi-service clients always need their own options type for service-specific API version properties. + /// Only optional parameters with default values that become properties on the options class + /// should trigger a separate client-specific options type. /// /// The input client to check. - /// True if the client has only standard parameters, false otherwise. + /// True if the client can share the singleton options instance, false otherwise. private static bool UseSingletonInstance(InputClient inputClient) { var rootClients = ScmCodeModelGenerator.Instance.InputLibrary.InputNamespace.RootClients; @@ -101,6 +101,12 @@ private static bool UseSingletonInstance(InputClient inputClient) return false; } + // Multi-service clients need their own options type for service-specific API version properties + if (inputClient.IsMultiServiceClient) + { + return false; + } + foreach (var parameter in inputClient.Parameters) { // Check if parameter is NOT an ApiVersion or Endpoint parameter @@ -114,18 +120,15 @@ private static bool UseSingletonInstance(InputClient inputClient) return false; // Found a non-standard endpoint parameter } } - else if (parameter is InputMethodParameter && parameter.DefaultValue == null) - { - // Required method parameters (from @@clientInitialization) are inlined as - // constructor parameters on the client and do not become properties on - // the options class, so they should not trigger a separate options type. - } - else + else if (parameter.DefaultValue != null) { - // Found a non-ApiVersion, non-Endpoint parameter that may require - // a separate client-specific options type. + // Found a non-ApiVersion, non-Endpoint optional parameter that will become + // a property on the options class — requires a separate client-specific options type. return false; } + // Required parameters (DefaultValue == null) are inlined as constructor parameters + // on the client and do not become properties on the options class, + // so they should not trigger a separate client-specific options type. } } return true; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs index c18cf79e9ff..4af2c5add7f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs @@ -6,6 +6,7 @@ using System.ClientModel.Primitives; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using System.Threading.Tasks; @@ -69,6 +70,10 @@ public bool ValidateIsLastNamespaceSegmentTheSame(string left, string right) [SetUp] public void SetUp() { + // Reset the singleton instance before each test to prevent state leaking + var singletonField = typeof(ClientOptionsProvider).GetField("_singletonInstance", BindingFlags.Static | BindingFlags.NonPublic); + singletonField?.SetValue(null, null); + var categories = TestContext.CurrentContext.Test?.Properties["Category"]; _containsSubClients = categories?.Contains(SubClientsCategory) ?? false; _hasKeyAuth = categories?.Contains(KeyAuthCategory) ?? false;