CsWin32 follow-up: CLR metadata + TypeLib interop migration

[JeremyKuhne]

CsWin32 follow-up: CLR metadata + TypeLib interop migration

Continues the CsWin32 struct-based COM migration from #13705 and #13746,
this time covering the CLR metadata APIs (IMetaDataDispenser /
IMetaDataImport2 / IMetaDataAssemblyImport) consumed by AssemblyInformation
and ManifestUtil.MetadataReader, plus the ICreateTypeLib path used by
RegisterAssembly.

New files

• src/Tasks/AssemblyDependency/Metadata/
  • Metadata.cs folder header / namespace docs
  • IMetaDataDispenser.cs OpenScope with typed CorOpenFlags
  • IMetaDataImport2.cs GetCustomAttributeByName + GetPEKind
    with canonical inherited 3-72 slot map
  • IMetaDataAssemblyImport.cs GetAssemblyProps / GetAssemblyRefProps /
    GetFileProps / EnumAssemblyRefs / EnumFiles /
    GetAssemblyFromScope / CloseEnum
  • ASSEMBLYMETADATA.cs blittable struct from cor.h
  • OSINFO.cs blittable struct from cor.h
  • CorOpenFlags.cs typed [Flags] enum
  • CorAssemblyFlags.cs typed [Flags] enum
  • CorTokenType.cs 26 mdt* table-type tags
  • Tokens.cs MdToken / MdAssembly / MdAssemblyRef / MdFile
    readonly structs with validating constructors
• src/Tasks/TypeLib/ICreateTypeLib.cs struct-based wrapper for
  RegisterAssembly
• src/Tasks.UnitTests/MetadataReader_Tests.cs 12 tests / 24 cross-TFM
  (PEReader on net10, COM on
  net472), verifies identity
  parity for Accessibility.dll

Modifications

• AssemblyInformation.cs uses MdAssembly / BufferScope / typed
  CorAssemblyFlags; OpenScope asks for
  IMetaDataImport2 directly (skips one QI);
  AllocAsmMeta / FreeAsmMeta eliminated
• MetadataReader.cs net472 path migrated from [ComImport] to
  struct-based COM via AgileComPointer; failure
  to OpenScope returns null (per-file contract)
• RegisterAssembly.cs uses the new ICreateTypeLib struct
• Microsoft.Build.Tasks.csproj wildcard for the
  Fusion / Metadata / TypeLib folders
  (FeatureWindowsInterop-gated)
• NativeMethods.cs legacy [ComImport] declarations removed;
  only oleaut32 typelib P/Invokes remain;
  System.Runtime.Versioning using gated on
  FEATURE_WINDOWSINTEROP for source-build

Skills

• .github/skills/cswin32-com/SKILL.md adds 'Strongly-typed handle /
  token wrappers' with CLR
  validation table; pair-pointer
  to cswin32-interop; error-
  handling-parity guidance;
  IComIID polyfill heading rename
• .github/skills/cswin32-interop/SKILL.md reorganized: Rules / Blittable
  signatures / Infrastructure /
  BufferScope / Gotchas;
  source-build verification note

Verified

• Normal build: 0 warnings, 0 errors (net472 + net10.0)
• Source build: 0 warnings, 0 errors
  (DotNetBuildSourceOnly=true DotNetBuild=true)
• Tests: 24/24 pass on both TFMs

[github-actions]

🔍 Skill Validator Results

⚠️ Warnings or advisories found

Scope	Checked
Skills	2
Agents	0
Total	2
Severity	Count
---	---:
❌ Errors	0
⚠️ Warnings	2
ℹ️ Advisories	0

Summary

Level	Finding
ℹ️	Found 2 skill(s)
ℹ️	[cswin32-interop] 📊 cswin32-interop: 3,402 BPE tokens [chars/4: 3,195] (standard ~), 14 sections, 3 code blocks
ℹ️	[cswin32-interop] ⚠ Skill is 3,402 BPE tokens (chars/4 estimate: 3,195) — approaching "comprehensive" range where gains diminish.
ℹ️	[cswin32-com] 📊 cswin32-com: 4,200 BPE tokens [chars/4: 4,075] (standard ~), 13 sections, 6 code blocks
ℹ️	[cswin32-com] ⚠ Skill is 4,200 BPE tokens (chars/4 estimate: 4,075) — approaching "comprehensive" range where gains diminish.
ℹ️	✅ All checks passed (2 skill(s))

Full validator output

```text Found 2 skill(s) [cswin32-interop] 📊 cswin32-interop: 3,402 BPE tokens [chars/4: 3,195] (standard ~), 14 sections, 3 code blocks [cswin32-interop] ⚠ Skill is 3,402 BPE tokens (chars/4 estimate: 3,195) — approaching "comprehensive" range where gains diminish. [cswin32-com] 📊 cswin32-com: 4,200 BPE tokens [chars/4: 4,075] (standard ~), 13 sections, 6 code blocks [cswin32-com] ⚠ Skill is 4,200 BPE tokens (chars/4 estimate: 4,075) — approaching "comprehensive" range where gains diminish. ✅ All checks passed (2 skill(s)) ```

[Copilot]

Pull request overview

Migrates CLR metadata (IMetaData*) and typelib authoring (ICreateTypeLib) interop from [ComImport]/RCW-based calls to CsWin32-style struct-based COM, improving blittability and aligning Tasks/ManifestUtil with the newer ComScope<T> / AgileComPointer<T> infrastructure.

Changes:

• Adds new struct-based COM definitions for CLR metadata APIs and strongly-typed metadata token wrappers (MdAssembly, MdAssemblyRef, etc.).
• Updates AssemblyInformation and ManifestUtil.MetadataReader to use CoCreateInstance, ComScope<T>, and AgileComPointer<T> instead of [ComImport] interfaces and manual marshalling.
• Migrates RegisterAssembly’s ICreateTypeLib.SaveAllChanges usage to a struct-based COM call and removes legacy declarations from NativeMethods.cs; adds unit tests for MetadataReader parity.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Tasks/TypeLib/ICreateTypeLib.cs	New struct-based COM wrapper for ICreateTypeLib (typelib persistence).
src/Tasks/RegisterAssembly.cs	Uses QI + struct-based ICreateTypeLib to call SaveAllChanges().
src/Tasks/NativeMethods.cs	Removes legacy [ComImport] declarations for metadata + typelib interop.
src/Tasks/Microsoft.Build.Tasks.csproj	Switches to wildcard Compile includes for Fusion/Metadata/TypeLib interop folders under FeatureWindowsInterop.
src/Tasks/ManifestUtil/MetadataReader.cs	Replaces net472 COM RCW usage with CoCreateInstance + ComScope<T> + AgileComPointer<T>.
src/Tasks/AssemblyDependency/Metadata/Tokens.cs	Adds strongly-typed CLR metadata token wrappers and conversions.
src/Tasks/AssemblyDependency/Metadata/OSINFO.cs	Adds blittable OSINFO struct for CLR metadata APIs.
src/Tasks/AssemblyDependency/Metadata/Metadata.cs	Adds CorMetadata helper (CLSID constants) for activation.
src/Tasks/AssemblyDependency/Metadata/IMetaDataImport2.cs	Adds struct-based COM for IMetaDataImport2 with vtable slot mapping.
src/Tasks/AssemblyDependency/Metadata/IMetaDataDispenser.cs	Adds struct-based COM for IMetaDataDispenser::OpenScope.
src/Tasks/AssemblyDependency/Metadata/IMetaDataAssemblyImport.cs	Adds struct-based COM for IMetaDataAssemblyImport methods used by RAR/ManifestUtil.
src/Tasks/AssemblyDependency/Metadata/CorTokenType.cs	Adds typed CorTokenType table-tag enum for token decoding/validation.
src/Tasks/AssemblyDependency/Metadata/CorOpenFlags.cs	Adds typed CorOpenFlags for OpenScope options.
src/Tasks/AssemblyDependency/Metadata/CorAssemblyFlags.cs	Adds typed CorAssemblyFlags for assembly/ref flags.
src/Tasks/AssemblyDependency/Metadata/ASSEMBLYMETADATA.cs	Adds blittable ASSEMBLYMETADATA struct and allocation contract docs.
src/Tasks/AssemblyDependency/AssemblyInformation.cs	Uses new metadata COM structs + AgileComPointer<T>; removes AllocAsmMeta/FreeAsmMeta.
src/Tasks.UnitTests/MetadataReader_Tests.cs	New parity/regression tests for MetadataReader across TFMs.
.github/skills/cswin32-interop/SKILL.md	Documentation updates for interop rules and parity guidance.
.github/skills/cswin32-com/SKILL.md	Documentation updates for COM patterns, token wrappers, and error-handling parity.

[github-actions]

Review Summary: COM Interop Migration to CsWin32 Struct-Based Patterns

This is a well-executed migration from legacy [ComImport] / Marshal.GetComInterfaceForObject() patterns to CsWin32 struct-based COM vtable patterns. The change is mechanically sound and follows the same patterns established in the WMI and Fusion interop code in this repo.

✅ Verified Correct

• Vtable indices: All slot numbers verified against the removed [ComImport] declarations (IMetaDataAssemblyImport slots 3-15, IMetaDataImport2 slots 60/70, ICreateTypeLib slot 12).
• ComScope / AgileComPointer lifecycle: The takeOwnership: false semantics are correctly used — GIT AddRefs, ComScope's using releases the transient ref, leaving GIT as the sole owner.
• DisposeManagedResources vs DisposeUnmanagedResources: Correct switch since AgileComPointer<T> is a managed class with its own finalizer.
• Memory safety in RegisterAssembly: The bridge between RCW (ITypeLib) and struct-based COM (ICreateTypeLib) is properly structured with nested try/finally.
• HRESULT checking: Every COM call checks the return via .ThrowOnFailure() — an improvement over the old pattern where some calls silently swallowed errors.
• Test coverage: The new MetadataReader_Tests.cs provides good coverage including threading, idempotent disposal, negative paths, and cross-assembly validation.
• Token types: The strongly-typed MdAssembly/MdAssemblyRef/MdFile wrappers with documented bypass semantics for native out-params are well-designed.

⚠️ One Performance Suggestion (Non-Blocking)

GetStringCustomAttribute acquires its own ComScope<IMetaDataImport2> via a GIT round-trip on every call, even when the caller (GetAssemblyMetadata) already holds one. This introduces ~4 unnecessary GIT lookups per GetAssemblyMetadata() invocation. The old code passed IMetaDataImport2 as a parameter. Consider accepting a pointer parameter in a follow-up to eliminate this overhead for the RAR hot path.

Assessment

No blocking issues. The vtable layouts, memory management, disposal patterns, and behavioral contracts are all correct. The one actionable suggestion (GIT round-trip reduction) is a minor performance optimization, not a correctness concern. This PR is ready to merge from a technical review perspective.

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

• #13853 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (on open) for issue #13853 · ● 14M

[DustinCampbell]

Looks good!

[ViktorHofer]

@JeremyKuhne this looks good. I reviewed the changes manually and all looks good. I only had one observation on the testing front. AssemblyInformation.cs would benefit of some additional testing. Here's me asking LLM to find what to actually test:

● Yes — I’d add one or two targeted AssemblyInformation tests, mainly because the PR’s new tests cover ManifestUtil.MetadataReader, but the largest risky migration is actually AssemblyInformation.cs.

  Best additions:

   1. net472-only real GetAssemblyMetadata() parity test
    - Construct new AssemblyInformation(typeof(SomeTestType).Assembly.Location).
    - Call internal GetAssemblyMetadata().
    - Verify key fields against AssemblyName.GetAssemblyName(path) / assembly attributes:
    AssemblyName, version parts, DefaultAlias, PublicHexKey, TargetFrameworkMoniker, PeKind != 0.
    - This directly exercises CoCreateInstance, OpenScope, QueryInterface, GetAssemblyProps, ASSEMBLYMETADATA, GetCustomAttributeByName, and GetPEKind.
   2. net472-only GetTargetFrameworkAttribute() real-file test
    - Compare AssemblyInformation.GetTargetFrameworkAttribute(path) with the assembly’s actual TargetFrameworkAttribute.FrameworkName.
    - This covers the borrowed IMetaDataImport2* path outside GetAssemblyMetadata().

  Existing coverage already has RealGetAssemblyNameIncludesCulture() for Dependencies, so I wouldn’t add much there unless you want extra confidence around EnumAssemblyRefs/locale handling. I also wouldn’t bother testing Files unless the repo already has an easy multi-module assembly fixture; manufacturing one is probably not worth it for this PR