Fix external type resolution failing when the type's dependencies are not deployed with the generator - #11465
Fix external type resolution failing when the type's dependencies are not deployed with the generator#11465rhurey wants to merge 1 commit into
Conversation
… not deployed with the generator
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
commit: |
|
No changes needing a change description found. |
| // same simple name in the process, and any type they both declare (System.Memory.Data's | ||
| // System.BinaryData, for example) would exist twice, so signatures mentioning it fail to bind | ||
| // with a MissingMethodException. Reusing the host's copy keeps exactly one identity per name. | ||
| var assembly = UseHostAssembly(simpleName) ?? Probe(globalPackagesFolder, simpleName, name.Version); |
There was a problem hiding this comment.
This unconditionally prefers the host assembly by simple name, even when the package closure selected a newer/incompatible version. That can recreate the same MissingMethodException class of failure this change is intended to fix—for example, the PR notes that Extensions requests System.Memory.Data 10.0.0.9 while the host carries 10.0.0.3. If maintaining a single type identity requires using the host copy, please verify compatibility with the closure requirement and report a clear resolution failure when it is not compatible rather than silently substituting it.
--generated by Copilot
| } | ||
|
|
||
| [Test] | ||
| public void TryResolve_ResolvesTypeWhoseBaseTypeLivesInAnotherPackage() |
There was a problem hiding this comment.
This regression test does not exercise RegisterPackageClosure/WalkPackage: FakeNuGetPackage.Create writes only the DLL under lib/netstandard2.0 and does not create a .nuspec, so closure walking immediately takes the missing-nuspec fallback. Because both fake package and assembly versions are also 1.0.0, the test passes through the assembly-version fallback. Please add a dependency group to a fake .nuspec and deliberately make the dependency package version differ from its assembly version (for example package 2.0.0 containing assembly 1.0.0.0) so the new closure/version-selection path is actually required for the test to pass.
--generated by Copilot
There was a problem hiding this comment.
To clarify the requested scope: please add only the hermetic unit test described above. We can leave validation against real published NuGet packages as a manual E2E check for now.
--generated by Copilot
| /// type whose base type lives in a dependency. The external type then appears unresolvable and is | ||
| /// generated instead of referenced. | ||
| /// </remarks> | ||
| internal static class NugetAssemblyResolver |
There was a problem hiding this comment.
Keeping assembly resolution as a separate type makes sense, but can this be a per-generator instance owned by ExternalTypeReferenceResolver's cache state rather than process-wide static state? The closure versions, resolved assemblies, in-progress set, load-context hook, and debug callback all belong to one generation run. Instance ownership would prevent state/type-binding decisions from leaking between sequential generator invocations and would make teardown and isolated testing explicit instead of relying on every static collection and event subscription being reset correctly.
--generated by Copilot
2 participants
Problem
@@alternateType(..., { identity, package, minVersion }, "csharp") silently had no effect whenever the referenced type's base type (or any type in its signature) lived in a package the generator does not itself deploy. Instead of referencing the external type, the generator emitted its own copy of the model.
Reproducing with Azure.AI.Projects.Agents , 12 tool models mapped to Azure.AI.Extensions.OpenAI were all still generated, each reporting:
unsupported-external-type: External type 'Azure.AI.Extensions.OpenAI.BingGroundingTool' could not be resolved:
package 'Azure.AI.Extensions.OpenAI' (>= 3.0.0-alpha.20260729.1) was not found in the NuGet cache or any configured feed.
The diagnostic was misleading — the package was present and was found. ExternalTypeReferenceResolver loads the package assembly with Assembly.Load(byte[]) into the default AssemblyLoadContext , which resolves dependencies only from the generator's own trusted-platform-assemblies list. BingGroundingTool derives from OpenAI.Responses.ResponseTool , and OpenAI.dll is not deployed with the generator, so Assembly.GetType(identity, throwOnError: false) returned null without throwing. That null was interpreted as "type not found", and generation proceeded as if no external mapping existed.
Fix
Adds NugetAssemblyResolver , which satisfies the transitive references of external-type assemblies from the NuGet global packages folder via an AssemblyLoadContext.Default.Resolving hook. Two details drive the design:
Dependencies are pinned from the package dependency closure, not from assembly versions. An assembly reference only carries an assembly version, which for many packages is deliberately lower than the package version that ships it — System.ClientModel 1.14.0 ships assembly version 1.9.0.0 . Resolving that as a package version selects the unrelated 1.9.0 package, and the resulting type-load failure surfaces as MissingMethodException: Method not found: 'System.BinaryData IPersistableModel.Write(ModelReaderWriterOptions)' . The resolver instead walks the package's .nuspec transitively, picking the framework-nearest dependency group and unifying duplicates by highest version, mirroring what NuGet would select.
An assembly the host already provides is preferred over the cache, regardless of version. The default context only asks the hook to resolve a name it could not satisfy itself, but it will still satisfy other, lower-versioned requests for that same name from its own deployment. Loading a second copy from the cache therefore leaves two assemblies with the same simple name in the process, and any type they both declare exists twice. Concretely: Azure.AI.Extensions.OpenAI requests System.Memory.Data 10.0.0.9 while the generator deploys 10.0.0.3; loading 10.0.9 from the cache produces two System.BinaryData types, and every signature mentioning it fails to bind. Reusing the host's copy keeps exactly one identity per simple name.
Resolved dependencies are also registered as Roslyn metadata references so generated code that references them still binds in the generated-code workspace.
Two supporting changes:
• NugetPackageResolver.FindPackageAssemblyInVersion looks up one exact installed version and selects the lib/ asset closest to the framework the generator is running on. The existing FindPackageAssembly probes PreferredDotNetFrameworkVersions and so prefers netstandard2.0 , which is correct for collecting compile-time references but not for assemblies loaded into a live .NET process, where a netstandard2.0 asset can bind against compatibility shims that duplicate shared-framework types.
• ExternalTypeReferenceResolver now records a specific failure reason per resolution path, and TypeFactory reports it in unsupported-external-type . The previous message claimed the package was missing regardless of the actual cause; failures now distinguish "package not found", "assembly could not be read/loaded", and "assembly was loaded but does not declare the type, or one of the type's dependencies could not be resolved".
Validation
• New TryResolve_ResolvesTypeWhoseBaseTypeLivesInAnotherPackage builds two interdependent fake NuGet packages and asserts the derived type resolves with a readable BaseType . Confirmed to fail without the fix, with exactly the symptom above.
• New TryResolve_ReportsFailureReasonWhenTypeMissingFromAssembly , plus a GetFailureReason assertion on the existing unknown-package test.
• Microsoft.TypeSpec.Generator.Tests 1783/1783; Microsoft.TypeSpec.Generator.ClientModel.Tests 1536/1536.
• End-to-end against Azure.AI.Projects.Agents : all 12 unsupported-external-type warnings are gone and none of the 12 tool models are generated.
Notes for reviewers
• FakeNuGetPackage.Create gained an optional referencedAssemblyPaths parameter so tests can build packages that reference each other.
• ExternalTypeReferenceResolver.Reset() now tolerates InvalidOperationException . It dereferences CodeModelGenerator.Instance , which is not available in test SetUp before a mock generator is loaded — pre-existing fragility, surfaced by the new tests.
• The resolver keys its cache on assembly simple name and returns cached failures, so a missing dependency is probed once per process.