[PR #6927] [CLOSED] fix(managed): scope P/Invoke spelling candidates to PE #7450

Closed
opened 2026-11-01 21:47:57 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6927
Author: @gensokyo-dev
Created: 10/24/2026
Status: ❌ Closed

Base: main ← Head: fix/6926-pinvoke-platform-decorations-20261023


📝 Commits (2)

  • ae38e0c fix(managed): scope P/Invoke spelling candidates to PE
  • 9d22729 Merge remote-tracking branch 'origin/main' into fix/6926-pinvoke-platform-decorations-20261023

📊 Changes

4 files changed (+285 additions, -95 deletions)

View changed files

📝 docs/managed-code-analysis.md (+9 -5)
📝 src/domain/managed/managedNativeVerificationMatch.ts (+5 -9)
📝 tests/boundary/mcp/managedNativeVerificationAdapters.test.ts (+171 -79)
📝 tests/conformance/providers/managed/managedNativeVerification.test.ts (+100 -2)

📄 Description

Status: superseded by main. Commit 64fa1db1309218283df19f3ee4dba61f5fbd2a67 independently fixes #6926. Current main 76500f12b passed 44 conformance/public adapter tests and the same four real-artifact requests, with identical corrected PE/ELF normalized results. Closing this overlapping PR; no merge or bypass is needed. Main retains its intentional unknown-platform behavior and additional identity/Evidence controls.

Summary

Fixes #6926. P/Invoke verification now adds Windows A/W and stdcall spelling candidates only with supplied PE context. Explicit ANSI/Unicode declarations against ELF, Mach-O, or unknown native formats keep their declared name; supported PE candidates remain inferred.

Problem and expected behavior

An ELF import Example with explicit ANSI or Unicode incorrectly acquired ExampleA/ExampleW as a candidate. Real .NET 10 Linux controls fail to find those declarations while exact-name controls return 11/22. Charset marshalling does not establish Windows entry-point probing on Unix.

Change and scope

Move the PE condition to the shared candidate-generation boundary, covering charset and stdcall variants together. Preserve per-format candidate reuse, ExactSpelling, source names and the separate Mach-O export underscore interpretation. Split public adapter cases into independent tests with their existing per-case time budget rather than extending a shared timeout.

Contract and boundary impact

  • Semantic owner: managedNativeVerificationMatch.candidateNames; both public adapters consume this result. No schema/catalog changes.
  • Contribution scope: ordinary correction within existing static analysis; new execution/mutation/provider authority is not applicable.
  • Compatibility: removes unsupported non-PE inferred correspondences; exact-name matching and current PE behavior are preserved. No runtime binding or ABI equivalence is claimed.
  • Evidence: original declarations, native observations, coverage and top-level provenance remain present. Corrected rows have content-derived IDs and match links recomputed by the existing owner.
  • Target selection, resource ownership, packaging and installation: unchanged.

Expected effects:

  • Read existing files or captured data.

Evidence and regression coverage

  • Base fc2dddc1: the new naming matrix reproduced 12 failures before the fix (41 passes at that stage).

  • Coverage: PE/ELF/Mach-O/unknown-format charset and stdcall matrix, Mach-O export linkage controls, and compiled CLI/stdio MCP parity through actual managed-byte inspection. Native Evidence in these tests is explicitly synthetic.

  • Real producer check: the same four SHA-256-validated .NET/Clang artifacts described in #6926, with 8 declarations per request and 4 verification requests. Native-name Evidence was supplied from LLVM export names/addresses, with unavailable bodies. PE normalized results are identical before/after; ELF changes only ANSI/Unicode rows plus their derived summary/identifiers. The other 6 ELF rows are identical. Complete CLI/MCP normalized results agree.

  • Observed ELF output: inferred / decorated-name-candidate / ExampleA|ExampleW becomes unresolved / no-native-candidate / matched_native: null / candidates: [] / confidence: unknown.

  • Limitations: Windows execution and real Hopper/Ghidra/IDA body/ABI analysis were not run. The separate Linux runtime controls used owned artifacts, not target execution by REA.

  • 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

At the initial fix before the unrelated main reconciliation:

  • npm run test:focused -- tests/conformance/providers/managed/managedNativeVerification.test.ts tests/boundary/mcp/managedNativeVerificationAdapters.test.ts — 67 passed.
  • npm run check — passed, including typecheck, lint, formatting, knip, metadata and architecture.
  • npm run docs:check — passed, including 9 README consistency tests.
  • Same-input compiled CLI/stdio MCP real-artifact workflow — passed before/after. Fresh processes, warm filesystem, 2 CPU affinity: elapsed 7.06 → 7.19 s; CPU 8.45 → 8.82 s; sampled process-family peak RSS 835.1 → 832.9 MiB. Tokenx estimate for complete arguments and replies: 29,571 → 28,339. One paired observation; no speed improvement established. Actual model tokens, schema/protocol negotiation, artifact production/build costs and OS-cold runs are unmeasured/excluded.

Latest main 6b3eee1 was merged without conflicts. The integrated head 9d22729 passed the same 67 focused tests and npm run check again. GitHub CI checks out merge d95e602 against actual main base f90d407; its build, static checks and documentation jobs passed. Remaining CI/review results will be recorded before merge.

Compatibility, safety, and release

Static result correction; no new capability, migration or release metadata change. No native provider or Windows runtime verification is represented. Test compilation and owned runtime controls are separate from REA authority.

Author readiness

The requesting maintainer explicitly authorized continued work, ready PR creation and merge, and waived human-only confirmation gates in this session. This PR is ready under that instruction. The technical diff/evidence review is AI-assisted; no independent human review of this final diff is attested here.

  • 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.

These human-only declarations are left unchecked under the explicit waiver; they are not evidence of technical validation or a claim that a human independently inspected this diff.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved; a new capability scope decision is not applicable.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning documentation is updated; no generated contract change.
  • Representative CLI/MCP output is recorded above.
  • Technical diff review found no secrets, unrelated cleanup, or 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/6927 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/24/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6926-pinvoke-platform-decorations-20261023` --- ### 📝 Commits (2) - [`ae38e0c`](https://github.com/morluto/rea/commit/ae38e0c73f21cf264c1dd2531de3a854f706597e) fix(managed): scope P/Invoke spelling candidates to PE - [`9d22729`](https://github.com/morluto/rea/commit/9d22729b85017aca359af9f0706036c136b99249) Merge remote-tracking branch 'origin/main' into fix/6926-pinvoke-platform-decorations-20261023 ### 📊 Changes **4 files changed** (+285 additions, -95 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+9 -5) 📝 `src/domain/managed/managedNativeVerificationMatch.ts` (+5 -9) 📝 `tests/boundary/mcp/managedNativeVerificationAdapters.test.ts` (+171 -79) 📝 `tests/conformance/providers/managed/managedNativeVerification.test.ts` (+100 -2) </details> ### 📄 Description **Status: superseded by main.** Commit `64fa1db1309218283df19f3ee4dba61f5fbd2a67` independently fixes #6926. Current main `76500f12b` passed 44 conformance/public adapter tests and the same four real-artifact requests, with identical corrected PE/ELF normalized results. Closing this overlapping PR; no merge or bypass is needed. Main retains its intentional unknown-platform behavior and additional identity/Evidence controls. ## Summary Fixes #6926. P/Invoke verification now adds Windows A/W and stdcall spelling candidates only with supplied PE context. Explicit ANSI/Unicode declarations against ELF, Mach-O, or unknown native formats keep their declared name; supported PE candidates remain inferred. ## Problem and expected behavior An ELF import `Example` with explicit ANSI or Unicode incorrectly acquired `ExampleA`/`ExampleW` as a candidate. Real .NET 10 Linux controls fail to find those declarations while exact-name controls return 11/22. Charset marshalling does not establish Windows entry-point probing on Unix. ## Change and scope Move the PE condition to the shared candidate-generation boundary, covering charset and stdcall variants together. Preserve per-format candidate reuse, ExactSpelling, source names and the separate Mach-O export underscore interpretation. Split public adapter cases into independent tests with their existing per-case time budget rather than extending a shared timeout. ## Contract and boundary impact - Semantic owner: `managedNativeVerificationMatch.candidateNames`; both public adapters consume this result. No schema/catalog changes. - Contribution scope: ordinary correction within existing static analysis; new execution/mutation/provider authority is not applicable. - Compatibility: removes unsupported non-PE inferred correspondences; exact-name matching and current PE behavior are preserved. No runtime binding or ABI equivalence is claimed. - Evidence: original declarations, native observations, coverage and top-level provenance remain present. Corrected rows have content-derived IDs and match links recomputed by the existing owner. - Target selection, resource ownership, packaging and installation: unchanged. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Base `fc2dddc1`: the new naming matrix reproduced 12 failures before the fix (41 passes at that stage). - Coverage: PE/ELF/Mach-O/unknown-format charset and stdcall matrix, Mach-O export linkage controls, and compiled CLI/stdio MCP parity through actual managed-byte inspection. Native Evidence in these tests is explicitly synthetic. - Real producer check: the same four SHA-256-validated .NET/Clang artifacts described in #6926, with 8 declarations per request and 4 verification requests. Native-name Evidence was supplied from LLVM export names/addresses, with unavailable bodies. PE normalized results are identical before/after; ELF changes only ANSI/Unicode rows plus their derived summary/identifiers. The other 6 ELF rows are identical. Complete CLI/MCP normalized results agree. - Observed ELF output: `inferred / decorated-name-candidate / ExampleA|ExampleW` becomes `unresolved / no-native-candidate / matched_native: null / candidates: [] / confidence: unknown`. - Limitations: Windows execution and real Hopper/Ghidra/IDA body/ABI analysis were not run. The separate Linux runtime controls used owned artifacts, not target execution by REA. - [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 At the initial fix before the unrelated main reconciliation: - `npm run test:focused -- tests/conformance/providers/managed/managedNativeVerification.test.ts tests/boundary/mcp/managedNativeVerificationAdapters.test.ts` — 67 passed. - `npm run check` — passed, including typecheck, lint, formatting, knip, metadata and architecture. - `npm run docs:check` — passed, including 9 README consistency tests. - Same-input compiled CLI/stdio MCP real-artifact workflow — passed before/after. Fresh processes, warm filesystem, 2 CPU affinity: elapsed 7.06 → 7.19 s; CPU 8.45 → 8.82 s; sampled process-family peak RSS 835.1 → 832.9 MiB. Tokenx estimate for complete arguments and replies: 29,571 → 28,339. One paired observation; no speed improvement established. Actual model tokens, schema/protocol negotiation, artifact production/build costs and OS-cold runs are unmeasured/excluded. Latest main `6b3eee1` was merged without conflicts. The integrated head `9d22729` passed the same 67 focused tests and `npm run check` again. GitHub CI checks out merge `d95e602` against actual main base `f90d407`; its build, static checks and documentation jobs passed. Remaining CI/review results will be recorded before merge. ## Compatibility, safety, and release Static result correction; no new capability, migration or release metadata change. No native provider or Windows runtime verification is represented. Test compilation and owned runtime controls are separate from REA authority. ## Author readiness The requesting maintainer explicitly authorized continued work, ready PR creation and merge, and waived human-only confirmation gates in this session. This PR is ready under that instruction. The technical diff/evidence review is AI-assisted; no independent human review of this final diff is attested here. - [ ] 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. These human-only declarations are left unchecked under the explicit waiver; they are not evidence of technical validation or a claim that a human independently inspected this diff. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved; a new capability scope decision is not applicable. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning documentation is updated; no generated contract change. - [x] Representative CLI/MCP output is recorded above. - [x] Technical diff review found no secrets, unrelated cleanup, or unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:47:57 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7450
No description provided.