fix(roslyn): install missing System.Runtime.CompilerServices.Unsafe v6 + surface inner errors#1116
Open
sMartz1 wants to merge 2 commits intoCoplayDev:betafrom
Conversation
…nd surface inner errors
The RoslynInstaller downloads only 4 NuGet packages but Microsoft.CodeAnalysis 4.12.0
on netstandard2.0 also references System.Runtime.CompilerServices.Unsafe v6.0.0.0,
which is NOT what Unity ships (Unity bundles v4.x). The reference is unresolved at
runtime, so Roslyn's StringTable static cctor throws FileNotFoundException, which
in turn poisons CSharpSyntaxTree's cctor: every parse / compile attempt then
throws TypeInitializationException.
The error is invisible because RoslynCompiler.Compile's catch block only logs
e.Message, and for TargetInvocationException that string is the generic
"Exception has been thrown by the target of an invocation." — the real cause
in InnerException is silently dropped.
Repro:
1. Trigger Tools > MCP for Unity > Install Roslyn on a fresh project.
2. Ask the MCP execute_code tool to compile any Roslyn-only snippet.
3. Observe: "Compilation failed: Roslyn compilation error: Exception has been
thrown by the target of an invocation." — with no further detail.
4. Drilling via reflection reveals:
TypeInitializationException for CSharpSyntaxTree
└─ TypeInitializationException for Roslyn.Utilities.StringTable
└─ FileNotFoundException: Could not load file or assembly
'System.Runtime.CompilerServices.Unsafe, Version=6.0.0.0'
Fix:
* RoslynInstaller.NuGetEntries: add the missing dependency so a fresh install
drops all 5 DLLs into Assets/Plugins/Roslyn/.
* RoslynCompiler.Compile catch: walk the InnerException chain so a future
bootstrap regression surfaces the actual cause (type + message) instead of
the generic invocation wrapper.
Verified locally: after dropping System.Runtime.CompilerServices.Unsafe.dll v6
into the plugins folder and forcing a domain reload, the execute_code tool
compiles C# 7+ snippets via the Roslyn backend successfully.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a transitive NuGet dependency required for Roslyn integration ( ChangesRoslyn Integration Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MCPForUnity/Editor/Setup/RoslynInstaller.cs`:
- Around line 16-23: The IsInstalled() check currently treats the presence of
System.Runtime.CompilerServices.Unsafe.dll as sufficient; change IsInstalled()
to also verify the assembly file's version is at least 6.0.0.0 (the required v6)
before returning true: locate the entry for
("system.runtime.compilerservices.unsafe","6.0.0", ...) and in IsInstalled() use
System.Reflection.AssemblyName.GetAssemblyName(path).Version to read the file's
Version and compare to Version(6,0,0,0); if the file exists but version is less
than 6.0.0.0, treat it as not installed. Add using System.Reflection; at the
top. Optionally apply the same version check pattern to the other package tuples
for defense-in-depth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c1a8aeb-9dd5-478f-86fd-3fefb2c9c9ef
📒 Files selected for processing (2)
MCPForUnity/Editor/Setup/RoslynInstaller.csMCPForUnity/Editor/Tools/ExecuteCode.cs
Address PR review: file-existence alone treats a stale v4 of System.Runtime.CompilerServices.Unsafe (or any other downgraded plugin) as "installed" and silently masks the bootstrap failure this PR is meant to fix. Read each on-disk DLL's manifest via AssemblyName.GetAssemblyName(...).Version and compare to the NuGet entry's declared version; if the file is older than declared or its manifest can't be read, return false so Install() rewrites it. Applied uniformly to all entries (defense-in-depth, per the optional suggestion in the review). Notes: - System.Version comparison handles "6.0.0" (Revision=-1) vs "6.0.0.0" (Revision=0) correctly: the higher-revision actual passes; a stale v4 fails. - AssemblyName.GetAssemblyName only reads the manifest, no assembly load into the AppDomain, so this is cheap and side-effect-free. - Validated locally: with the v6 DLL in Plugins/Roslyn/, IsInstalled returns true; manually swapping in a v4 placeholder causes it to return false and Install() proceeds.
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
1 participant
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.
Summary
The
RoslynInstallerships only 4 NuGet packages butMicrosoft.CodeAnalysis4.12.0 on netstandard2.0 also referencesSystem.Runtime.CompilerServices.Unsafev6.0.0.0, which is not what Unity ships (Unity bundles a v4.x of that assembly). The reference is unresolved at runtime, so:Worse, the failure is invisible:
RoslynCompiler.Compile's catch block logs onlye.Message, and forTargetInvocationExceptionthat string is the generic "Exception has been thrown by the target of an invocation." — the real cause inInnerExceptionis silently dropped, so the install looks fine andIsAvailablereturnstrue, but every actual compile fails with a useless message.Repro
execute_codetool withcompiler="roslyn"(orauto) on any C# 7+ snippet.Compilation failed: Roslyn compilation error: Exception has been thrown by the target of an invocation.— no other detail.Compilestep-by-step with properInnerExceptionwalking) reveals the real chain shown above.Fix
MCPForUnity/Editor/Setup/RoslynInstaller.cs— add the missing transitive dependencysystem.runtime.compilerservices.unsafev6.0.0 toNuGetEntriesso a fresh install drops 5 DLLs intoAssets/Plugins/Roslyn/instead of 4. This makesIsInstalled()accurately reflect "Roslyn will actually load".MCPForUnity/Editor/Tools/ExecuteCode.cs— inRoslynCompiler.Compile's outer catch, walk theInnerExceptionchain to the deepest cause and reportType.Name + Messageof that root, instead of the generic wrapper. This makes future bootstrap regressions debuggable from the tool output alone.Verification
Tested on Unity 2022.3 / Windows / netstandard2.0 build target:
IsAvailable=True, every parse fails with the generic message.Assets/Plugins/Roslyn/, ran the patched installer → 5 DLLs present.TypeInitializationExceptionwould otherwise persist for the AppDomain lifetime).execute_codewithcompiler="roslyn"compiles C# 7+ snippets (var,?.,??, string interpolation) and runs them successfully.Notes
System.Reflection.Metadata8.0.0 andSystem.Memory, but the Roslyn IL references the specificVersion=6.0.0.0ref-assembly — there is no binding redirect, and Unity's bundledSystem.Runtime.CompilerServices.Unsafedoesn't satisfy a strong-named v6 reference.catchchange is purely diagnostic; it never alters whether compilation succeeds, only what gets surfaced when it doesn't.Test plan
Assets/Plugins/Roslyn/), run the installer → confirm 5 DLLs are present.execute_code→ confirm success.System.Runtime.CompilerServices.Unsafe.dlland try again → confirm the error message now reportsFileNotFoundExceptiondirectly instead of the generic invocation wrapper.Summary by CodeRabbit
Bug Fixes
Chores