[PR #1825] [CLOSED] fix(managed): surface exported (forwarded) types in member inspection #4843

New issue

Closed

opened 2026-10-31 00:55:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:55:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1825
Author: @JasonBuildAI
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/managed-exported-types-gmsv

📝 Commits (1)

588ae35 fix(managed): surface exported (forwarded) types in member inspection

📊 Changes

7 files changed (+648 additions, -27 deletions)

View changed files

📝 docs/managed-code-analysis.md (+8 -0)
📝 src/contracts/managed/managedWorkflowExamples.ts (+1 -0)
📝 src/domain/managed/managedArtifact.ts (+25 -0)
📝 src/dotnet/ManagedMemberInspector.test.ts (+208 -0)
📝 src/dotnet/ManagedMemberInspector.ts (+9 -1)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+235 -3)
📝 src/dotnet/ManagedPe.fixture.ts (+162 -23)

📄 Description

AI disclosure: this change was drafted with AI assistance at my request, under my direction and review.

Summary

Anyone inspecting a .NET facade / type-forwarding assembly with inspect-managed-members (CLI) or inspect_managed_members (MCP) currently gets a silently empty member inventory: System.Runtime.dll (861 ExportedType rows) and System.Web.Routing.dll (15 rows) both report only <Module> with coverage.state: "complete". This PR inventories the ExportedType table as exported_types, so the member view reports the forwarded/exported type surface with its implementation scope instead of claiming completeness over invisible rows.

Fixes #1812.

An independent review of the first revision found that derived nested names expanded without bound on adversarial chains (thousands of chained rows drove the whole inspection to unavailable after allocating hundreds of MB) and that the newly required field rejected evidence recorded by older builds. The current head bounds the walk, reserves name bytes before allocation, and defaults the field for older evidence; the review evidence is quoted below.

Problem and expected behavior

Smallest trigger: node scripts/rea.mjs inspect-managed-members "C:\Program Files\dotnet\shared\Microsoft.NETCore.App\7.0.20\System.Runtime.dll" --format json.

Prior behavior: types: [<Module>], no exported-type facts, coverage.state: "complete" — while inspect-managed-artifact reports ExportedType: 861 for the same file. The violated invariant is that member completeness ignores a populated metadata table.
Expected behavior: the member inventory surfaces the exported type rows (name, namespace, flags/forwarder, TypeDefId, implementation scope), and malformed rows remain visible as partial coverage.

Change and scope

src/domain/managed/managedArtifact.ts: adds exported_types to managedMemberInspectionSchema (default [], so evidence recorded before this change stays readable). Each row carries token, row_offset, flags, forwarder, type_def_id, namespace, name, full_name, and implementation (assembly-ref | file | exported-type | current-module | unresolved, with resolved token, name, version).
src/dotnet/ManagedMemberInspectorCore.ts: new parseExportedTypes reads table 39 and decodes the Implementation coded index per ECMA-335 (tag 0 File, tag 1 AssemblyRef, tag 2 ExportedType). Raw 0 becomes current-module; a coded index that names out-of-range or undefined rows is retained as unresolved with an invalid-row issue. Nested types join names outermost-first with /; the walk stops at a revisited row (issue) or at 64 nesting levels (issue, name joins the innermost levels) and reserves the joined name's JSON bytes in the decode budget before building it, because every level repeats inside each descendant.
src/dotnet/ManagedMemberInspector.ts: wires the parser and adds exported_types to both the success and unavailable() payloads.
src/dotnet/ManagedPe.fixture.ts and src/dotnet/ManagedMemberInspector.test.ts: fixture support for tables 38/39 and five tests (forwarder + nested chain; File and null scopes; invalid coded index; self-referential chain; 4000-level chain bounded without expansion).
docs/managed-code-analysis.md: documents the new field and the nesting bound.

Not included: no change to types/member semantics, no new CLI flags or dependencies; compare_managed_members still compares only fields and methods, which is outside this issue's scope.

Contract and boundary impact

Semantic owner and earliest changed stage: managed member inspection domain schema (managedArtifact.ts) and the src/dotnet metadata adapter that produces it.
CLI and MCP/tool-catalog contract: inspect-managed-members / inspect_managed_members output gains the additive exported_types array; the workflow example is updated in managedWorkflowExamples.ts. Advertised JSON schemas validate after SDK conversion (toolSchemaValidity, outputSchemaSemanticConstraints pass).
Provider, bridge, target-format, or platform compatibility: pure managed-metadata reads; no provider- or OS-specific behavior.
Evidence, artifact, provenance, or reconstruction contract: rows stay bound to the artifact digest/MVID via identity_scope; unresolved scopes keep the row instead of dropping it; pre-change evidence without the field parses and defaults to [].
Process execution, authorization, cleanup, or containment impact: none; existing bounded-read primitives and decode budget are reused, with derived names charged before allocation.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none authored; generated catalogs stay CI-owned, npm run docs:check passes.

Evidence and regression coverage

Tests added or updated: describe("managed exported types") and describe("managed exported type implementation scopes") in src/dotnet/ManagedMemberInspector.test.ts (5 cases).
Base reproduction (only the producer files reverted to 43dafb5a, fixture and tests kept):

 FAIL  |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported types > inventories forwarded and nested exported types that TypeDef omits
 FAIL  |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > resolves File and null Implementation scopes without inventing a target
 FAIL  |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > retains an exported type whose Implementation coded index is invalid
 FAIL  |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > stops a self-referential nested Implementation chain without hanging
 Test Files  1 failed (1)
      Tests  4 failed | 25 passed (29)

Review regression (deep chain, only ManagedMemberInspectorCore.ts reverted to the pre-review revision):

 FAIL  |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > bounds a deep Implementation chain instead of expanding it quadratically
 AssertionError: expected [] to have a length of 4000 but got +0
 Test Files  1 failed (1)
      Tests  1 failed | 29 skipped (30)

With the fix, the same 4000-level chain completes in ~0.6 s, retains all 4000 rows, and reports truncation at the nesting bound as partial coverage.

User-visible CLI output on System.Runtime.dll after the change: exported_types: 861 ({"assembly-ref": 831, "exported-type": 30}), coverage.state: "complete"; first forwarded row Microsoft.Win32.SafeHandles.CriticalHandleMinusOneIsInvalid -> 0x23000001 System.Private.CoreLib 7.0.0.0; nested example System.ArraySegment1/Enumerator -> 0x27000029 System.ArraySegment1. System.Web.Routing.dll: exported_types: 15, all assembly-ref (e.g. System.Web.Routing.HttpMethodConstraint -> System.Web 4.0.0.0).
Older evidence: parsing a recorded inspect_managed_members result with exported_types removed succeeds and yields exported_types: [].

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts — 30 passed (25 pre-existing + 5 new)
npm run test:local -- <all 8 src/dotnet test files> — 98 passed
npm run test:local -- src/contracts/workflowInputContracts.test.ts src/domain/managed/managedReconstruction.test.ts — 4 passed
npm run test:focused -- tests/boundary/mcp/toolSchemaValidity.test.ts tests/boundary/mcp/outputSchemaSemanticConstraints.test.ts — 9 passed
npm run check:fast — typecheck and lint successful
npm run build:cached — 3 successful
npm run docs:check — 6 successful
node scripts/rea.mjs inspect-managed-members ... — output quoted above

Compatibility, safety, and release

Breaking changes or migration steps: additive JSON field on inspect_managed_members output; existing fields are unchanged and older evidence stays readable.
Real Hopper/Ghidra, browser, or OS coverage: not applicable to .NET metadata parsing; verified against Windows in-box .NET Framework 4.8 and .NET 7 assemblies.
Package or release metadata impact: none authored; generated catalogs are produced by CI.
Security, privacy, process, or containment review: read-only parsing of caller-provided files; derived names are charged against the existing 64 MiB representation budget before allocation; no new process execution.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1825 **Author:** [@JasonBuildAI](https://github.com/JasonBuildAI) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/managed-exported-types-gmsv` --- ### 📝 Commits (1) - [`588ae35`](https://github.com/morluto/rea/commit/588ae352b5ea6171edb1f8b58d801b3efb6595cc) fix(managed): surface exported (forwarded) types in member inspection ### 📊 Changes **7 files changed** (+648 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+8 -0) 📝 `src/contracts/managed/managedWorkflowExamples.ts` (+1 -0) 📝 `src/domain/managed/managedArtifact.ts` (+25 -0) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+208 -0) 📝 `src/dotnet/ManagedMemberInspector.ts` (+9 -1) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+235 -3) 📝 `src/dotnet/ManagedPe.fixture.ts` (+162 -23) </details> ### 📄 Description > AI disclosure: this change was drafted with AI assistance at my request, under my direction and review. ## Summary Anyone inspecting a .NET facade / type-forwarding assembly with `inspect-managed-members` (CLI) or `inspect_managed_members` (MCP) currently gets a silently empty member inventory: `System.Runtime.dll` (861 `ExportedType` rows) and `System.Web.Routing.dll` (15 rows) both report only `<Module>` with `coverage.state: "complete"`. This PR inventories the `ExportedType` table as `exported_types`, so the member view reports the forwarded/exported type surface with its implementation scope instead of claiming completeness over invisible rows. Fixes #1812. An independent review of the first revision found that derived nested names expanded without bound on adversarial chains (thousands of chained rows drove the whole inspection to `unavailable` after allocating hundreds of MB) and that the newly required field rejected evidence recorded by older builds. The current head bounds the walk, reserves name bytes before allocation, and defaults the field for older evidence; the review evidence is quoted below. ## Problem and expected behavior Smallest trigger: `node scripts/rea.mjs inspect-managed-members "C:\Program Files\dotnet\shared\Microsoft.NETCore.App\7.0.20\System.Runtime.dll" --format json`. - Prior behavior: `types: [<Module>]`, no exported-type facts, `coverage.state: "complete"` — while `inspect-managed-artifact` reports `ExportedType: 861` for the same file. The violated invariant is that member completeness ignores a populated metadata table. - Expected behavior: the member inventory surfaces the exported type rows (name, namespace, flags/forwarder, TypeDefId, implementation scope), and malformed rows remain visible as partial coverage. ## Change and scope - `src/domain/managed/managedArtifact.ts`: adds `exported_types` to `managedMemberInspectionSchema` (default `[]`, so evidence recorded before this change stays readable). Each row carries `token`, `row_offset`, `flags`, `forwarder`, `type_def_id`, `namespace`, `name`, `full_name`, and `implementation` (`assembly-ref` | `file` | `exported-type` | `current-module` | `unresolved`, with resolved `token`, `name`, `version`). - `src/dotnet/ManagedMemberInspectorCore.ts`: new `parseExportedTypes` reads table 39 and decodes the `Implementation` coded index per ECMA-335 (tag 0 File, tag 1 AssemblyRef, tag 2 ExportedType). Raw `0` becomes `current-module`; a coded index that names out-of-range or undefined rows is retained as `unresolved` with an `invalid-row` issue. Nested types join names outermost-first with `/`; the walk stops at a revisited row (issue) or at 64 nesting levels (issue, name joins the innermost levels) and reserves the joined name's JSON bytes in the decode budget before building it, because every level repeats inside each descendant. - `src/dotnet/ManagedMemberInspector.ts`: wires the parser and adds `exported_types` to both the success and `unavailable()` payloads. - `src/dotnet/ManagedPe.fixture.ts` and `src/dotnet/ManagedMemberInspector.test.ts`: fixture support for tables 38/39 and five tests (forwarder + nested chain; File and null scopes; invalid coded index; self-referential chain; 4000-level chain bounded without expansion). - `docs/managed-code-analysis.md`: documents the new field and the nesting bound. Not included: no change to `types`/member semantics, no new CLI flags or dependencies; `compare_managed_members` still compares only fields and methods, which is outside this issue's scope. ## Contract and boundary impact - Semantic owner and earliest changed stage: managed member inspection domain schema (`managedArtifact.ts`) and the `src/dotnet` metadata adapter that produces it. - CLI and MCP/tool-catalog contract: `inspect-managed-members` / `inspect_managed_members` output gains the additive `exported_types` array; the workflow example is updated in `managedWorkflowExamples.ts`. Advertised JSON schemas validate after SDK conversion (`toolSchemaValidity`, `outputSchemaSemanticConstraints` pass). - Provider, bridge, target-format, or platform compatibility: pure managed-metadata reads; no provider- or OS-specific behavior. - Evidence, artifact, provenance, or reconstruction contract: rows stay bound to the artifact digest/MVID via `identity_scope`; unresolved scopes keep the row instead of dropping it; pre-change evidence without the field parses and defaults to `[]`. - Process execution, authorization, cleanup, or containment impact: none; existing bounded-read primitives and decode budget are reused, with derived names charged before allocation. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none authored; generated catalogs stay CI-owned, `npm run docs:check` passes. ## Evidence and regression coverage - Tests added or updated: `describe("managed exported types")` and `describe("managed exported type implementation scopes")` in `src/dotnet/ManagedMemberInspector.test.ts` (5 cases). - Base reproduction (only the producer files reverted to `43dafb5a`, fixture and tests kept): ``` FAIL |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported types > inventories forwarded and nested exported types that TypeDef omits FAIL |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > resolves File and null Implementation scopes without inventing a target FAIL |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > retains an exported type whose Implementation coded index is invalid FAIL |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > stops a self-referential nested Implementation chain without hanging Test Files 1 failed (1) Tests 4 failed | 25 passed (29) ``` - Review regression (deep chain, only `ManagedMemberInspectorCore.ts` reverted to the pre-review revision): ``` FAIL |adapters| src/dotnet/ManagedMemberInspector.test.ts > managed exported type implementation scopes > bounds a deep Implementation chain instead of expanding it quadratically AssertionError: expected [] to have a length of 4000 but got +0 Test Files 1 failed (1) Tests 1 failed | 29 skipped (30) ``` With the fix, the same 4000-level chain completes in ~0.6 s, retains all 4000 rows, and reports truncation at the nesting bound as `partial` coverage. - User-visible CLI output on `System.Runtime.dll` after the change: `exported_types: 861` (`{"assembly-ref": 831, "exported-type": 30}`), `coverage.state: "complete"`; first forwarded row `Microsoft.Win32.SafeHandles.CriticalHandleMinusOneIsInvalid -> 0x23000001 System.Private.CoreLib 7.0.0.0`; nested example `System.ArraySegment`1/Enumerator -> 0x27000029 System.ArraySegment`1`. `System.Web.Routing.dll`: `exported_types: 15`, all `assembly-ref` (e.g. `System.Web.Routing.HttpMethodConstraint -> System.Web 4.0.0.0`). - Older evidence: parsing a recorded `inspect_managed_members` result with `exported_types` removed succeeds and yields `exported_types: []`. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts` — 30 passed (25 pre-existing + 5 new) - `npm run test:local -- <all 8 src/dotnet test files>` — 98 passed - `npm run test:local -- src/contracts/workflowInputContracts.test.ts src/domain/managed/managedReconstruction.test.ts` — 4 passed - `npm run test:focused -- tests/boundary/mcp/toolSchemaValidity.test.ts tests/boundary/mcp/outputSchemaSemanticConstraints.test.ts` — 9 passed - `npm run check:fast` — typecheck and lint successful - `npm run build:cached` — 3 successful - `npm run docs:check` — 6 successful - `node scripts/rea.mjs inspect-managed-members ...` — output quoted above ## Compatibility, safety, and release - Breaking changes or migration steps: additive JSON field on `inspect_managed_members` output; existing fields are unchanged and older evidence stays readable. - Real Hopper/Ghidra, browser, or OS coverage: not applicable to .NET metadata parsing; verified against Windows in-box .NET Framework 4.8 and .NET 7 assemblies. - Package or release metadata impact: none authored; generated catalogs are produced by CI. - Security, privacy, process, or containment review: read-only parsing of caller-provided files; derived names are charged against the existing 64 MiB representation budget before allocation; no new process execution. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:55:52 +00:00