Skip to content

fix: Avoid dependency conflicts from bundled metadata DLLs - #1288

Merged
hatayama merged 6 commits into
mainfrom
fix/issue-1284-memorypack-unsafe
Jun 12, 2026
Merged

fix: Avoid dependency conflicts from bundled metadata DLLs#1288
hatayama merged 6 commits into
mainfrom
fix/issue-1284-memorypack-unsafe

Conversation

@hatayama

@hatayama hatayama commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Prevent consuming Unity projects from implicitly referencing bundled metadata validation DLLs from Unity CLI Loop.
  • Keep Unity CLI Loop metadata validation working through explicit editor-only package references.

User Impact

  • Projects that install packages such as MemoryPack can keep their own System.Runtime.CompilerServices.Unsafe dependency without Unity CLI Loop bundled DLLs being selected accidentally.
  • This reduces dependency clashes without changing the public workflow.

Changes

  • Move the bundled metadata validation dependency DLLs under the editor metadata validation package area.
  • Mark those DLLs as explicitly referenced only so Unity does not expose them as implicit project references.
  • Give the bundled dependency assemblies private package-specific identities and update their internal references to match.
  • Remove the orphaned Plugins metadata left after moving the DLLs.
  • Add package metadata tests for the explicit-reference flags, private assembly identities, and orphaned metadata removal.

Verification

  • Unity compile via the project-local CLI: 0 errors, 0 warnings
  • Kensho project compile via the project-local CLI: 0 errors, 0 warnings
  • npm --prefix Packages/src/Cli~ run lint
  • npm --prefix Packages/src/Cli~ run build
  • npm --prefix Packages/src/Cli~ run test:cli
  • npm --prefix Packages/src/Cli~ run test:cli -- --runTestsByPath src/tests/package-metadata.test.ts
  • codex-review --mode branch main

Closes #1284

Summary

This PR adds comprehensive test coverage for the bundled metadata validation DLLs under the editor-only metadata validation package, ensuring they remain private and explicitly referenced only.

Changes Made

File: Packages/src/Cli~/src/__tests__/package-metadata.test.ts (+103 lines)

New Test Infrastructure

  • Helper functions: Added loadUnityPackageText(relativePath) and loadUnityPackageBytes(relativePath) to read checked-in Unity asset metadata and DLL binaries from stable repo-relative paths
  • Test data structures: Defined arrays for hard-coded metadata validation dependency paths and private assembly configuration:
    • METADATA_VALIDATION_DEPENDENCY_META_PATHS: Three .dll.meta file paths
    • METADATA_VALIDATION_PRIVATE_ASSEMBLIES: Assembly configurations with name, path, and reference mappings

New Test Cases

  1. Explicit Reference Flag Verification (keeps bundled metadata validation dependencies out of implicit project references): Verifies each metadata validation dependency .meta file contains isExplicitlyReferenced: 1, preventing implicit project references

  2. Private Assembly Identity Verification (uses private assembly identities for metadata validation dependencies): Confirms each dependency DLL contains the correct private assembly name (with uLoopMCP prefix) and all declared assembly references within its binary

  3. Strong-Name Signature Verification (does not claim strong-name signing on unsigned metadata validation dependencies): Parses PE/CLI headers to verify the StrongNameSigned flag is cleared on all three dependency DLLs, preventing BadImageFormatException on CoreCLR when strong-name signatures were stripped during identity rewriting

  4. Orphaned Metadata Cleanup (does not keep orphaned plugin folder metadata): Confirms the Plugins.meta file was removed after moving DLLs

Technical Details

  • Implements PE/CLI header parsing (readCorHeaderFlags) to inspect the COR20 header and verify the COMIMAGE_FLAGS_STRONGNAMESIGNED flag state
  • Validates three rewritten dependency assemblies:
    • uLoopMCP.System.Runtime.CompilerServices.Unsafe.dll (no references)
    • uLoopMCP.System.Collections.Immutable.dll (references Unsafe)
    • uLoopMCP.System.Reflection.Metadata.dll (references both Unsafe and Collections.Immutable)

Impact

  • Prevents regression of the fix for issue #1284 where consuming projects (e.g., those using MemoryPack) would encounter implicit dependency conflicts with unity-cli-loop's bundled metadata validation DLLs
  • Ensures metadata validation functionality continues to work through explicit editor-only references while keeping bundled DLLs hidden from consuming projects
  • No changes to exported/public entity signatures (test-only additions)

Keep bundled CodeAnalysis dependencies explicit-only so consuming Unity projects do not receive uLoop's System.Runtime.CompilerServices.Unsafe.dll through implicit plugin references.
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e9d5c8af-ebbb-43e1-81fa-29d83ecdf36d

📥 Commits

Reviewing files that changed from the base of the PR and between 891b1c4 and 3c186ab.

⛔ Files ignored due to path filters (5)
  • Packages/src/Editor/MetadataValidation/Dependencies/uLoopMCP.System.Collections.Immutable.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/MetadataValidation/Dependencies/uLoopMCP.System.Reflection.Metadata.dll is excluded by !**/*.dll and included by none
  • Packages/src/Editor/MetadataValidation/Dependencies/uLoopMCP.System.Runtime.CompilerServices.Unsafe.dll is excluded by !**/*.dll and included by none
  • Packages/src/TypeScriptServer~/dist/server.bundle.js is excluded by !**/dist/** and included by **/*.js
  • Packages/src/TypeScriptServer~/dist/server.bundle.js.map is excluded by !**/dist/**, !**/*.map and included by none
📒 Files selected for processing (1)
  • Packages/src/Cli~/src/__tests__/package-metadata.test.ts

📝 Walkthrough

Walkthrough

Adds test-only constants for Unity dependency .dll.meta paths and private assembly identities, repo-relative helpers to read Unity asset text and DLL bytes, and Jest tests that (1) assert each dependency meta file contains isExplicitlyReferenced: 1, (2) verify DLL bytes include declared assembly reference names and lack the strong-name flag, and (3) expect loading an orphaned Plugins.meta path to throw.

Changes

Package metadata tests

Layer / File(s) Summary
Test data: meta paths and private assembly list
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Defines explicit dependency .dll.meta paths and a structured list of private assembly identities with expected referenced assembly names.
PE/CLI header parsing helpers
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Adds readCorHeaderFlags and constants to inspect PE/CLI headers for the strong-name signing flag.
Repo-relative Unity asset loaders
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Adds loadUnityPackageText(relativePath) and loadUnityPackageBytes(relativePath) helpers to read checked-in Unity asset text and raw DLL bytes from stable repo-relative locations.
Meta explicit-reference and DLL content tests
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Adds Jest tests asserting each listed .dll.meta contains isExplicitlyReferenced: 1 and that each private dependency DLL bytes contain the primary assembly identity and all declared referenced assembly names.
Strong-name signing flag test
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Adds a Jest test that inspects each configured dependency DLL's PE/CLI header to assert the strong-name signing flag is not set.
Orphaned Plugins.meta load error test
Packages/src/Cli~/src/__tests__/package-metadata.test.ts
Adds a Jest test expecting loadUnityPackageText('Plugins.meta') to throw when attempting to load an orphaned plugin folder metadata file via the repo-relative loader.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses issue #1284 by marking bundled CodeAnalysis plugin DLLs as explicitly referenced only and adding metadata regression tests, preventing implicit exposure that caused compilation conflicts.
Out of Scope Changes check ✅ Passed All changes are scoped to test additions verifying metadata validation settings for bundled DLLs, directly supporting the linked issue objective of preventing package conflicts.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title mentions 'bundled metadata DLLs' but the PR's primary objective is to prevent package conflicts by marking CodeAnalysis DLLs as explicitly referenced only, not about avoiding dependency conflicts in general.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1284-memorypack-unsafe

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

hatayama added 3 commits June 5, 2026 23:34
Move the bundled System.* metadata validation assemblies under the owning editor assembly with uLoop-specific file names, so consumer asmdefs that reference System.Runtime.CompilerServices.Unsafe.dll resolve their own package dependency instead of uLoop's private copy.
Rename the bundled metadata validation dependency assembly identities to match their uLoop-specific DLL names, so Unity and Burst do not resolve them as System.* assemblies while consumer packages keep using their own dependencies.
Delete the empty package Plugins folder metadata left after moving the metadata validation DLLs, and add package metadata coverage so clean Unity imports do not recreate an empty folder or warn about the orphan.
@hatayama hatayama changed the title fix: Avoid package conflicts from bundled CodeAnalysis DLLs fix: Avoid dependency conflicts from bundled metadata DLLs Jun 5, 2026
hatayama and others added 2 commits June 13, 2026 00:28
The identity rewrite removed the strong-name signature and public key
from the bundled metadata validation dependencies but left the
StrongNameSigned bit set in the COR20 header. CoreCLR rejects such
images with BadImageFormatException ("Bad IL format"), so the DLLs
would fail to load outside Mono-based editors.

- Clear the StrongNameSigned CorFlag in all three dependency DLLs
- Add a package metadata test that parses the COR20 header and guards
  against the flag regressing
@hatayama
hatayama merged commit 636439d into main Jun 12, 2026
1 of 2 checks passed
@hatayama
hatayama deleted the fix/issue-1284-memorypack-unsafe branch June 12, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A Error about Unsafe.dll.

2 participants