fix(dotnet): make InternalsVisibleTo work under strong-naming - #2296
Open
1aifanatic wants to merge 1 commit into
Open
fix(dotnet): make InternalsVisibleTo work under strong-naming#22961aifanatic wants to merge 1 commit into
1aifanatic wants to merge 1 commit into
Conversation
`dotnet test tests/<Project>` — the command AGENTS.md documents — failed to compile on a clean checkout with a wall of CS0122 "inaccessible due to its protection level" errors. Only `-p:SignAssembly=false` made it work, and nothing in the error output pointed at signing. Two things combined to cause it: - `SignAssembly`/`AssemblyOriginatorKeyFile` lived in the `'$(IsPackable)' == 'true'` PropertyGroup, so the product assemblies were strong-named but the test assemblies were not. A strong-named assembly can only grant InternalsVisibleTo to another strong-named assembly. - Each `InternalsVisibleTo` ItemGroup was gated on `'$(SignAssembly)' != 'true'`, so a default build emitted no friend attribute at all. Signing now applies to every project, and the public key of AGUI.snk is set as `$(PublicKey)`. The .NET SDK appends `PublicKey=$(PublicKey)` to any InternalsVisibleTo item that carries no explicit Key metadata (Microsoft.NET.GenerateAssemblyInfo.targets), so friend references stay valid while signing is on. `$(PublicKey)` is conditioned on `SignAssembly`, so `-p:SignAssembly=false` still emits the attribute without a key and unsigned builds keep working. With the flag no longer needed, the CI workflows drop it and now exercise the same signed configuration developers build locally. Verified: `dotnet test tests/<Project> -c Release` (no flag) passes for all five unit-test projects across net8.0/net9.0/net10.0 and for the ASP.NET Core integration tests, and the packable assemblies keep public key token d5950b1d09108385 on every target framework after a clean rebuild. Fixes ag-ui-protocol#2165
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.
Fixes #2165
I hit this the first time I ran the .NET tests, before I'd read the issue —
dotnet test tests/AGUI.Server.UnitTestson a cleanmaingave me a wall ofCS0122errors pointing at test files, with nothing suggesting the real cause was assembly signing.Root cause
Two things combine, and the issue only named one of them:
InternalsVisibleTowas gated off under signing. Each ItemGroup carriedCondition="'$(SignAssembly)' != 'true'", so a default build emitted no friend attribute at all.SignAssembly/AssemblyOriginatorKeyFilelived inside the'$(IsPackable)' == 'true'PropertyGroup, so only the five product assemblies were strong-named. A strong-named assembly can only grantInternalsVisibleToto another strong-named assembly, so just deleting theCondition(option 1 as literally worded) would have tradedCS0122forCS1726.Fix — option 1, done properly
Signing moves out of the
IsPackablegroup so every project is signed withAGUI.snk, including the test projects that consume internals.$(PublicKey)is set to the public key ofAGUI.snk. The SDK appendsPublicKey=$(PublicKey)to anyInternalsVisibleToitem without explicitKeymetadata — this is the built-in behaviour the issue guessed at, inMicrosoft.NET.GenerateAssemblyInfo.targets:The SDK does not derive
$(PublicKey)from the.snkon its own — it has to be set, which is what was missing.$(PublicKey)is conditioned on'$(SignAssembly)' == 'true', so-p:SignAssembly=falsestill lands on the third branch above: attribute emitted with no key, everything unsigned, exactly as today. The escape hatch keeps working.The four
Condition="'$(SignAssembly)' != 'true'"gates are removed.Since the flag is no longer needed, both workflows drop it (13 call sites). That's deliberate rather than cosmetic: with the flag, CI never built the configuration anyone actually ships or develops against — it only ever tested the unsigned one. Now CI, local
dotnet test, and the release build all agree.Verification
dotnet test tests/<Project> -c Releasewith no flag — the command that fails onmain:AGUI.Abstractions.UnitTestsAGUI.Client.UnitTestsAGUI.Server.UnitTestsAGUI.Protobuf.UnitTestsAGUI.Formatting.UnitTestsPlus
AGUI.Hosting.AspNetCore.IntegrationTests— 117 ✅ (net10.0).Also checked:
-c Release -p:SignAssembly=falsepasses (the old CI command).d5950b1d09108385onnet10.0andnetstandard2.0, and the newly-signed test assemblies carry the same token.net10.0;net9.0;net8.0;netstandard2.0;net472), 0 warnings underTreatWarningsAsErrors.CrossLanguage.TestServer(a friend ofAGUI.Protobuf) builds unflagged. I couldn't runAGUI.CrossLanguage.IntegrationTestslocally — it needs the TypeScript fake-agent server on :8092, which is a pnpm/vitest setup I didn't stand up — but that's a runtime dependency, not a compile one, and the project compiles fine without the flag.Note on scope
I went with option 1 rather than 2 or 3 because it's the only one that leaves a single configuration: option 2 would make Debug and Release produce differently-identified assemblies, and option 3 documents a papercut instead of removing it. If you'd rather keep
-p:SignAssembly=falsein CI so the unsigned path stays covered, I'm happy to restore those lines and keep only theDirectory.Build.props+.csprojchanges.