fix(http-client-csharp): resolve System.ClientModel types as framework types - #11678
Conversation
…k types TypeFactory.CreateFrameworkType falls back to Type.GetType, which only probes corlib and the assembly declaring the call (Microsoft.TypeSpec.Generator). System.ClientModel is referenced by Microsoft.TypeSpec.Generator.ClientModel, so "System.ClientModel.FileBinaryContent" never resolved and last-contract symbols for it produced a non-framework CSharpType. Back-compat property type preservation then replaced the generated FileBinaryContent framework type with the symbol-backed look-alike, which broke IsFileBinaryContentType and dropped the multipart convenience constructors, the [Experimental] attributes and the correct MultiPartFormContent.Add overload. Override CreateFrameworkType in ScmTypeFactory to fall back to Assembly.GetType scoped to the System.ClientModel assembly. Fixes microsoft#11676 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 33840ead-d95e-4c5d-91eb-8765e25089bc
commit: |
|
No changes needing a change description found. |
There was a problem hiding this comment.
Pull request overview
This PR fixes nondeterministic framework-type resolution for System.ClientModel types in the http-client-csharp generator by ensuring System.ClientModel.* types can be resolved as framework types during external type binding, preventing back-compat from “downgrading” them into symbol-backed non-framework lookalikes.
Changes:
- Override framework-type resolution in
ScmTypeFactoryto fall back to resolving types via theSystem.ClientModelassembly. - Add a regression test ensuring a last-contract
FileBinaryContentproperty remains a framework type and generated output stays stable. - Add last-contract and expected-output test assets for the new regression.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/ScmTypeFactory.cs | Adds a CreateFrameworkType override to resolve System.ClientModel types via Assembly.GetType. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs | Adds a regression test validating back-compat preserves FileBinaryContent as a framework type. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/TestMultipartFormDataModel_LastContractFileType_KeepsFileBinaryContentFrameworkType/MultiPartRequest.cs | Adds the last-contract source file used to reproduce the type-resolution scenario. |
| packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/TestMultipartFormDataModel_LastContractFileType_KeepsFileBinaryContentFrameworkType.cs | Adds the expected generated output baseline for the regression. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Verified against the real
|
StacAssetData.cs |
StacAssetData.Serialization.Multipart.cs |
PlanetaryComputerModelFactory.cs |
|
|---|---|---|---|
| without fix | −47 lines (lost [Experimental("SCME0004")] + string/Stream/BinaryData ctors) |
content.Add("file", File) → content.Add<FileBinaryContent>(...) |
−2 lines |
| with fix | clean | clean | clean |
With the fix the only remaining diff is Internal/ModelSerializationExtensions.cs (+55, a WriteBase64StringValue helper) — unrelated version skew between main and the pinned 1.0.0-alpha.20260813.5, and identical in both arms. The regenerated library compiles with 0 errors (it previously failed with 12).
Important
One gotcha worth flagging for the Azure side: AzureTypeFactory already declares protected override Type? CreateFrameworkType(...) ending in base.CreateFrameworkType(...). Since ScmTypeFactory had no such member when Azure.Generator was last compiled, that base. call was emitted as a non-virtual call TypeFactory::CreateFrameworkType and bypasses the new override. Azure.Generator therefore has to be recompiled against a generator build containing this fix — a plain DLL swap is not enough. The normal UnbrandedGeneratorVersion bump handles this, so no action is needed beyond the usual dependency flow.
--generated by Copilot
Fixes #11676
Root cause
Not PR #11585 (the auto-triage hypothesis in the issue). The real cause is framework-type resolution.
TypeFactory.CreateFrameworkTypeultimately falls back toType.GetType(fullyQualifiedTypeName). An unqualifiedType.GetTypeonly probes corlib and the assembly that declares the calling method — hereMicrosoft.TypeSpec.Generator.dll, which does not referenceSystem.ClientModel. So"System.ClientModel.FileBinaryContent"always resolved tonull. (This is the same reasonSystem.BinaryData,System.Uri,System.Text.Json.JsonElementandSystem.Net.IPAddressalready needed hard-coded special cases there.)Verified that this is not fixable by moving the call or by adding metadata references:
Even from an assembly that directly references and has loaded
System.ClientModel,Type.GetTypereturnsnull.CodeModelGenerator.AdditionalMetadataReferencesdoes not help either — that is a Roslyn/symbol-binding concern and has no effect on CLR reflection.How that produced the reported symptoms
TypeSymbolExtensions.GetCSharpTypecallsCreateFrameworkType, getsnull, and builds a symbol-backedCSharpType { Name = "FileBinaryContent", Namespace = "System.ClientModel", IsFrameworkType = false }.ModelProvider.BuildPropertiesback-compat handling sees!lastContractPropertyType.Equals(outputProperty.Type)(framework-backed vs symbol-backed), concludes the contract changed, and overwrites the property type with the symbol-backed look-alike.ScmModel.IsFileBinaryContentTypethen returnsfalse, so:BuildMultipartFileConstructorsreturnsnull→ thestring/Stream/BinaryDataconvenience constructors disappear[Experimental("SCME0004")]attributes on the constructor, property and model-factory method are dropped, along withusing System.Diagnostics.CodeAnalysis;MultipartFormDataSerializationDefinition.BuildScalarAddpicks the modelAdd<T>overload instead of theFileBinaryContentoneWhy it looked intermittent
The bad path only runs when a last contract is actually resolved (
SourceInputModel.FindForTypeInLastContract). Runs without a last-contract assembly available never hit it, which is why byte-identical inputs produced different output across CI runs. Thedotnet msbuildversion delta noted in the issue is a proxy for that, not the cause.Fix
Override
CreateFrameworkTypeinScmTypeFactoryand fall back toAssembly.GetTypescoped to theSystem.ClientModelassembly:This is deterministic (the assembly is a compile-time reference of
Microsoft.TypeSpec.Generator.ClientModel) and fixes everySystem.ClientModeltype, not justFileBinaryContent. Deliberately avoided broad probing such asAppDomain.CurrentDomain.GetAssemblies(), which would reintroduce load-order nondeterminism — exactly the class of problem this issue is about.Tests
Added
TestMultipartFormDataModel_LastContractFileType_KeepsFileBinaryContentFrameworkType, which loads a last contract declaringpublic FileBinaryContent ProfileImage { get; }and asserts the property stays a framework type, thatIsFileBinaryContentTyperecognizes it, and that the emitted model is byte-identical to the no-last-contract output. It fails onmainwithExpected: True But was: Falseand passes with this change.Validation
Microsoft.TypeSpec.Generator.Tests— 1895/1895 passedMicrosoft.TypeSpec.Generator.ClientModel.Tests— 1571/1571 passedeng/scripts/Generate.ps1— regenerated all libraries, no output driftnpm run cop—cop checks passed.Follow-up
Azure.GeneratorinAzure/azure-sdk-for-netderives fromScmTypeFactory, so it picks upSystem.ClientModelresolution from this change automatically. It will still need the equivalent one-line fallback for its ownAzure.Core/Azure.ResourceManagertypes.