[PR #3317] feat(managed): decode signature-owned custom attribute arguments #5717

New issue

Open

opened 2026-10-31 01:08:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3317
Author: @1052326311
Created: 10/14/2026
Status: 🔄 Open

Base: main ← Head: feat/managed-attribute-arguments

📝 Commits (9)

a8bcf94 feat(managed): decode signature-owned custom attribute arguments
89cc828 fix(managed)!: share member list facts and retain verification uncertainty
ec12008 fix(managed): derive attribute enums from shared member ownership
fb5c2f3 fix(managed): integrate attribute decoding with current ownership facts
cc56c1f fix(managed): reconcile attribute shapes with nominal signature digests
17c5632 fix(managed): bound enum lookup and retain partial attribute facts
9f63254 merge: retain upstream identities with bounded attribute decoding
8c585e1 merge: verify attribute decoding on current upstream main
e7bd335 fix: bound boxed attributes and reuse constructor signatures

📊 Changes

14 files changed (+1819 additions, -123 deletions)

View changed files

📝 docs/managed-code-analysis.md (+29 -0)
📝 src/domain/managed/managedArtifact.ts (+25 -0)
➕ src/dotnet/ManagedAttributeEnumFacts.ts (+208 -0)
➕ src/dotnet/ManagedCustomAttributeArguments.test.ts (+616 -0)
➕ src/dotnet/ManagedCustomAttributeArguments.ts (+534 -0)
📝 src/dotnet/ManagedDecodeBudget.test.ts (+16 -0)
📝 src/dotnet/ManagedDecodeBudget.ts (+14 -0)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+87 -35)
📝 src/dotnet/ManagedMetadataInventory.ts (+4 -2)
📝 src/dotnet/ManagedMetadataInventoryRows.ts (+20 -53)
📝 src/dotnet/ManagedPe.fixture.ts (+98 -30)
📝 tests/acceptance/managedCliOutput.test.ts (+116 -1)
📝 tests/boundary/mcp/managedMcp.test.ts (+24 -1)
📝 tests/boundary/providers/managed/managedDirectAnalysis.test.ts (+28 -1)

📄 Description

Summary

Related to #3154. Decode custom attribute fixed/named values from constructor signatures. Primitive/string/null/Type/boxed/array values and locally resolved enums remain static metadata inspection: no CLR load, attribute execution, referenced-assembly loading, mutation or network access. Decimal strings preserve 64-bit integers; raw blob length/hash and the TargetFramework compatibility projection remain available.

Review repairs

Current head: e7bd335c7cfef778d1902b6800a78ad3490baeb1, based on the already integrated 8c585e17 / main ab45bf64.

Every retained boxed object reserves shared capacity before reading its nested value. Repeated small blobs cannot accumulate uncharged nested objects before final inventory serialization.
Cache constructor parameter facts and expected parsing failures for each constructor token within its immutable metadata layout. Reserve cache bookkeeping and signature representation before parsing; that charge remains for the inspection lifetime. Different constructors/layouts remain independent, and attribute values are decoded anew. Remove the obsolete item-wide transient release so caller reservations remain live.
Earlier repairs pre-admit enum field parsing, share bounded enum identity/name/owned-field/width facts, and resolve enclosing/escaped identities. Ambiguous, foreign-qualified or cyclic names remain unknown. The upstream ManagedTypeNesting.ts prerequisite remains unchanged and is not claimed as this PR's work.
Failed arguments retain their original offset/type; named values retain name/kind and earlier decoded facts. Subsequent unknown-width offsets remain null.

Validation

Executed on this head, macOS arm64, pinned Node 24.18.0/npm 11.16.0:

344 tests / 12 suites pass: decoder/budget/metadata/ownership/signature comparison plus real static-provider Evidence, compiled CLI and MCP. The four latest regressions fail on 8c585e17 while its 42 existing attribute tests pass; the repaired attribute suite passes 46. Controls cover repeated 400-box values, scalar values, repeated valid/malformed constructor parsing, cache isolation and caller reservations.
npm run check, npm run docs:check, git diff --check, and npm run verify:managed (11 source-owned scenarios) pass. Optional operator manifest and ILSpy oracle are unconfigured.
Prior nine enum/partial-value regressions and escaped-name/cycle controls remain included. CLI assertions retain nested uint8 vs top-level int32 identity, value 42 and unresolved enum-array start offset 2.

These are executed fixture/static-provider workflows. Compiler-produced public framework-assembly oracles and Windows execution remain unverified. Hosted verification on e7bd335c is now complete: CI required passed, including source tests, static checks, build/docs, all five test shards, portable tests and the Linux native baseline. Windows native controls built successfully; the Windows capability/package lane and unrelated feature lanes were skipped, not executed. Greptile’s updated review reviewed this exact head, confirmed both latest P1 defects fixed and reported 5/5. All six review threads are resolved. These automated results do not establish human readiness or maintainer scope approval.

Bounded synthetic performance evidence

A completed decoder stress workload retains 1,000 results sharing one 12,000-array constructor signature; the bounded interpretation returns unsupported. Comparing 8c585e17 with this repair in separate Node processes, cold/warm elapsed: 251.69/230.02 → 47.88/43.87 ms; CPU: 269.16/232.47 → 51.72/43.97 ms. Cumulative process peak RSS (including module loading), cold/warm: 237,328/238,176 → 110,896/111,392 KiB. Both arms/phases emit identical 263,001-byte JSON (SHA-256 5e95a8cb8ee9a86add537e6e6c71449a8ce61ce4a12a35adb0fe8e6ddf5c6cc2). One sample per arm; agent tokens, real assemblies and whole CLI/MCP task costs were not measured by this stress probe.

Original feature-revision evidence (not current-head performance): 4,096 attributes increased JSON 1,303,743 → 2,114,751 bytes and tokenx estimates 377,705 → 660,329; cold elapsed/CPU 18.75/39.53 → 32.01/68.67 ms, warm elapsed 10.30–13.95 → 17.26–22.53 ms, cumulative process peak RSS 154,640 → 215,072 KiB. Rich output still has a material representation cost.

Remaining decisions

#3154 still needs maintainer direction on compatibility-field retention/deprecation, opt-in rich decoding and compact empty arguments. This PR does not assert scope acceptance or human readiness. The branch was advanced normally without force; exact local Git object hashes were verified during publication and upstream workflow contents were preserved.

Author readiness

Human owner: @1052326311

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 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/3317 **Author:** [@1052326311](https://github.com/1052326311) **Created:** 10/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/managed-attribute-arguments` --- ### 📝 Commits (9) - [`a8bcf94`](https://github.com/morluto/rea/commit/a8bcf943a16ef3c284ee65f6af8c79d45bb95161) feat(managed): decode signature-owned custom attribute arguments - [`89cc828`](https://github.com/morluto/rea/commit/89cc8284f699a63146e0e597e3b4d27f69bbc23b) fix(managed)!: share member list facts and retain verification uncertainty - [`ec12008`](https://github.com/morluto/rea/commit/ec120080f1996a9578c9d087922ed674304243b7) fix(managed): derive attribute enums from shared member ownership - [`fb5c2f3`](https://github.com/morluto/rea/commit/fb5c2f3b004f36f82d8505a7de62c3eb8b4634b6) fix(managed): integrate attribute decoding with current ownership facts - [`cc56c1f`](https://github.com/morluto/rea/commit/cc56c1f178ad67d9eb151ad3a7a8411079209018) fix(managed): reconcile attribute shapes with nominal signature digests - [`17c5632`](https://github.com/morluto/rea/commit/17c5632c2e43ae24bcd4ae1ce4b84e6708494cb5) fix(managed): bound enum lookup and retain partial attribute facts - [`9f63254`](https://github.com/morluto/rea/commit/9f632543072b73c697d8c820f2d32d5461dbc50e) merge: retain upstream identities with bounded attribute decoding - [`8c585e1`](https://github.com/morluto/rea/commit/8c585e17693db29d0e8dc89fe4ee2119f7711846) merge: verify attribute decoding on current upstream main - [`e7bd335`](https://github.com/morluto/rea/commit/e7bd335c7cfef778d1902b6800a78ad3490baeb1) fix: bound boxed attributes and reuse constructor signatures ### 📊 Changes **14 files changed** (+1819 additions, -123 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+29 -0) 📝 `src/domain/managed/managedArtifact.ts` (+25 -0) ➕ `src/dotnet/ManagedAttributeEnumFacts.ts` (+208 -0) ➕ `src/dotnet/ManagedCustomAttributeArguments.test.ts` (+616 -0) ➕ `src/dotnet/ManagedCustomAttributeArguments.ts` (+534 -0) 📝 `src/dotnet/ManagedDecodeBudget.test.ts` (+16 -0) 📝 `src/dotnet/ManagedDecodeBudget.ts` (+14 -0) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+87 -35) 📝 `src/dotnet/ManagedMetadataInventory.ts` (+4 -2) 📝 `src/dotnet/ManagedMetadataInventoryRows.ts` (+20 -53) 📝 `src/dotnet/ManagedPe.fixture.ts` (+98 -30) 📝 `tests/acceptance/managedCliOutput.test.ts` (+116 -1) 📝 `tests/boundary/mcp/managedMcp.test.ts` (+24 -1) 📝 `tests/boundary/providers/managed/managedDirectAnalysis.test.ts` (+28 -1) </details> ### 📄 Description ## Summary Related to #3154. Decode custom attribute fixed/named values from constructor signatures. Primitive/string/null/Type/boxed/array values and locally resolved enums remain static metadata inspection: no CLR load, attribute execution, referenced-assembly loading, mutation or network access. Decimal strings preserve 64-bit integers; raw blob length/hash and the TargetFramework compatibility projection remain available. ## Review repairs Current head: `e7bd335c7cfef778d1902b6800a78ad3490baeb1`, based on the already integrated `8c585e17` / main `ab45bf64`. - Every retained boxed object reserves shared capacity before reading its nested value. Repeated small blobs cannot accumulate uncharged nested objects before final inventory serialization. - Cache constructor parameter facts and expected parsing failures for each constructor token within its immutable metadata layout. Reserve cache bookkeeping and signature representation before parsing; that charge remains for the inspection lifetime. Different constructors/layouts remain independent, and attribute values are decoded anew. Remove the obsolete item-wide transient release so caller reservations remain live. - Earlier repairs pre-admit enum field parsing, share bounded enum identity/name/owned-field/width facts, and resolve enclosing/escaped identities. Ambiguous, foreign-qualified or cyclic names remain unknown. The upstream `ManagedTypeNesting.ts` prerequisite remains unchanged and is not claimed as this PR's work. - Failed arguments retain their original offset/type; named values retain name/kind and earlier decoded facts. Subsequent unknown-width offsets remain null. ## Validation Executed on this head, macOS arm64, pinned Node 24.18.0/npm 11.16.0: - **344 tests / 12 suites pass**: decoder/budget/metadata/ownership/signature comparison plus real static-provider Evidence, compiled CLI and MCP. The four latest regressions fail on `8c585e17` while its 42 existing attribute tests pass; the repaired attribute suite passes 46. Controls cover repeated 400-box values, scalar values, repeated valid/malformed constructor parsing, cache isolation and caller reservations. - `npm run check`, `npm run docs:check`, `git diff --check`, and `npm run verify:managed` (11 source-owned scenarios) pass. Optional operator manifest and ILSpy oracle are unconfigured. - Prior nine enum/partial-value regressions and escaped-name/cycle controls remain included. CLI assertions retain nested uint8 vs top-level int32 identity, value 42 and unresolved enum-array start offset 2. These are executed fixture/static-provider workflows. Compiler-produced public framework-assembly oracles and Windows execution remain unverified. Hosted verification on `e7bd335c` is now complete: [CI required](https://github.com/morluto/rea/actions/runs/38698196536) passed, including source tests, static checks, build/docs, all five test shards, portable tests and the Linux native baseline. Windows native controls built successfully; the Windows capability/package lane and unrelated feature lanes were skipped, not executed. [Greptile’s updated review](https://github.com/morluto/rea/pull/3317#issuecomment-6185976986) reviewed this exact head, confirmed both latest P1 defects fixed and reported 5/5. All six review threads are resolved. These automated results do not establish human readiness or maintainer scope approval. <details><summary>Bounded synthetic performance evidence</summary> A completed decoder stress workload retains 1,000 results sharing one 12,000-array constructor signature; the bounded interpretation returns unsupported. Comparing `8c585e17` with this repair in separate Node processes, cold/warm elapsed: 251.69/230.02 → 47.88/43.87 ms; CPU: 269.16/232.47 → 51.72/43.97 ms. Cumulative process peak RSS (including module loading), cold/warm: 237,328/238,176 → 110,896/111,392 KiB. Both arms/phases emit identical 263,001-byte JSON (SHA-256 `5e95a8cb8ee9a86add537e6e6c71449a8ce61ce4a12a35adb0fe8e6ddf5c6cc2`). One sample per arm; agent tokens, real assemblies and whole CLI/MCP task costs were not measured by this stress probe. Original feature-revision evidence (not current-head performance): 4,096 attributes increased JSON 1,303,743 → 2,114,751 bytes and tokenx estimates 377,705 → 660,329; cold elapsed/CPU 18.75/39.53 → 32.01/68.67 ms, warm elapsed 10.30–13.95 → 17.26–22.53 ms, cumulative process peak RSS 154,640 → 215,072 KiB. Rich output still has a material representation cost. </details> ## Remaining decisions #3154 still needs maintainer direction on compatibility-field retention/deprecation, opt-in rich decoding and compact empty arguments. This PR does not assert scope acceptance or human readiness. The branch was advanced normally without force; exact local Git object hashes were verified during publication and upstream workflow contents were preserved. ## Author readiness Human owner: @1052326311 - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:08:53 +00:00

bsakaguchi referenced this issue

2026-11-01 21:10:53 +00:00

[GH-ISSUE #3582] [Tracker] Ready to close or merge: issues and pull requests (October 2026) #1330

bsakaguchi referenced this issue

2026-11-01 21:44:55 +00:00

[PR #5755] fix(native): keep ELF layout independent of pwntools kernel probes #7040