[PR #3064] [CLOSED] fix(verify): select exact managed manifest method facts #5588

New issue

Closed

opened 2026-10-31 01:07:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3064
Author: @Zhong-fan
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/2617-managed-manifest-selection

📝 Commits (3)

f9bad8a fix(verify): select exact managed manifest method facts
ad432b1 Merge commit 'ff56f78594' into fix/2617-managed-manifest-selection
693300c test(verify): cover selected managed method facts

📊 Changes

6 files changed (+300 additions, -114 deletions)

View changed files

📝 docs/managed-code-analysis.md (+12 -6)
📝 scripts/fixtures/managed/pe.d.mts (+2 -0)
📝 scripts/fixtures/managed/pe.mjs (+13 -4)
📝 scripts/verify/managed/manifest.mjs (+57 -52)
📝 scripts/verify/managed/oracles.mjs (+216 -38)
📝 scripts/verify/managed/support.mjs (+0 -14)

📄 Description

Summary

Fixes #2617. The managed manifest verifier now projects only the declared MethodDef's verified facts. Selecting Second from First, Second, and Third previously produced a three-method graph while recording an ignored one-method limit; it now produces a one-method graph with explicit post-inspection selection and partial coverage.

Problem and expected behavior

inspectManagedMembersBytes no longer accepts row-pagination options. The verifier still passed methodOffset/methodLimit, found the right token in the complete result, and supplied that complete result to graph projection. Its one-method self-test could not expose the extra methods. Selection parameters must describe the retained facts actually used by the graph.

Change and scope

Inspect members once per manifest, select each exact MethodDef token, verify its commitments, and reuse that same selected result for graph projection.
Retain only that method's body, outgoing call edges, field accesses, and loaded user strings. Preserve shared metadata, artifact/MVID identity, and existing diagnostic issues. Mark omitted methods as partial coverage.
Remove obsolete member-pagination configuration. Record method_tokens and selection_stage: "post-inspection" in verifier-owned Evidence; report its ID, selection, and graph coverage.
Replace the single-method self-test with three methods, first/second-row controls, and rejection checks. Give every method a distinct call target, an owned field access, and its own decoded A/B/C #US literal; check retained member facts directly at the shared selection producer. Correct the guide's obsolete paging claim.

This is a verifier fix within the existing contribution scope. A new-provider/target-execution maintainer decision is not applicable. Selection follows bounded full inspection; it does not add a public token selector or avoid decoding unlisted bodies.

Contract and boundary impact

Semantic owner and earliest changed stage: manifest verification in scripts/verify/managed/manifest.mjs, immediately after member inspection.
Contribution scope fit and linked maintainer decision: ordinary verifier/documentation correction; not applicable.
CLI and MCP/tool-catalog contract: unchanged. The verifier's compact report gains member Evidence identity, actual selection, and graph coverage.
Provider, bridge, target-format, or platform compatibility: existing execution-free PE/CLI inspection; no new provider or parsing limits.
Evidence, artifact, provenance, or reconstruction contract: exact target and method commitments remain checked; selected member Evidence replaces inaccurate pagination parameters and whole-assembly completeness.
Target selection, authority, upstream behavior, and cleanup: caller-selected manifest/target and existing temporary-fixture cleanup; selected assemblies are never loaded or executed.
Generated metadata, package, or installation impact: no tracked generated files, dependencies, versions, installers, or workflows changed. Ignored managed verification evidence was regenerated and checked.

Expected effects of running verification:

Read existing files or captured data.
Launch or attach to a process — existing Node verifier/test processes only.
Inject instrumentation or run scripts/code in a selected target.
Change files — owned temporary verification fixtures only.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Before the fix, npm run verify:managed with the three-method regression failed with 3 !== 1 at the graph method-count assertion.
The owning verifier self-test checks two graph selections, three direct member-fact selections, and 13 rejected requests: target digest/MVID/name/runtime/architecture, signature/IL commitments, missing/zero-row tokens, unselected seed leakage, and an undeclared trace method. The direct checks assert exact caller/callee tokens, owning-method/field tokens, and literal tokens/values; graph method counts do not observe those arrays.
Three isolated mutation probes remove the call-edge, field-access, and loaded-string filters independently. All three survived the original self-test; each now fails its corresponding retained-fact assertion. Four fixture variants without the new optional #US stream remain byte-identical to their pre-change output.
A separate source-owned, file-backed manifest exercised REA_MANAGED_APP_MANIFEST_PATH, including a relative target path. It retained only token 0x06000002 / Second, with summary.methods: 1 and coverage.status: "partial".

Representative executed verifier output:

{"method_token":"0x06000002","member_selection":{"method_tokens":["0x06000002"],"selection_stage":"post-inspection"},"summary":{"methods":1},"coverage":{"status":"partial"}}

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

Windows x64, pinned Node.js 24.18.0/npm 11.16.0; integrated upstream through ff56f785946c609b09c1c60232b28d7c34ac89a9, final tested head 693300c1b91ddd64e8d45882e4aaf6f777fda87b. The affected managed parser/verifier paths were also compared with main 41a2e846470bd2af40dd298d975b1a1d2fef5786 and were unchanged:

npm run verify:managed — passed, including the three direct selected-fact controls and existing selection/rejection checks.
REA_MANAGED_APP_MANIFEST_PATH=<source-owned manifest> npm run verify:managed — passed on the final head; configured manifest selected only Second.
npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts src/dotnet/ManagedCilDecoding.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts — 205 passed on the final head.
npm run check:ci — passed on the final head, including duplication/TODO checks and architecture guards; existing lint warnings remain visible.
npm run docs:check — passed on the final head, including five README consistency tests.
Post-commit npm run format:check and final git diff --check — passed.

A small four-method manifest without graph projection produced byte-identical compact results before/after. On this host, first calls took 16.0/12.0 ms; 50 warm calls took 159.4/119.0 ms and 204/125 ms process CPU, with peak process RSS 109,660/107,648 KiB. This is one diagnostic comparison after module imports, not a general performance claim. Agent-token cost and process-family memory were not measured.

Compatibility, safety, and release

Breaking changes or migration steps: none for public CLI/MCP. Verifier report fields are additive; Evidence identities reflect corrected scope.
Real Hopper/Ghidra, browser, or OS coverage: source-owned PE byte fixtures on Windows. No target assembly execution, real osu! binary, ILSpy oracle, or Linux/macOS run is claimed.
Package or release metadata impact: none.
Security, privacy, process, or containment review: existing bounded parsing and identity checks remain; no additional target authority or external service use.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved; no new-provider decision is required.
The PR has one focused outcome and a Conventional Commit title.
Related issue is linked.
Tests cover changed observable behavior and meaningful failure paths.
Owning documentation and ignored verification evidence are updated where needed.
Representative verifier output is included; CLI/MCP contracts are unchanged.
Final diff checked 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/3064 **Author:** [@Zhong-fan](https://github.com/Zhong-fan) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/2617-managed-manifest-selection` --- ### 📝 Commits (3) - [`f9bad8a`](https://github.com/morluto/rea/commit/f9bad8a171614953d50ed89611bd231ffaf36f96) fix(verify): select exact managed manifest method facts - [`ad432b1`](https://github.com/morluto/rea/commit/ad432b1d65add52ac6cdd5e048504e55f7f7b24a) Merge commit 'ff56f785946c609b09c1c60232b28d7c34ac89a9' into fix/2617-managed-manifest-selection - [`693300c`](https://github.com/morluto/rea/commit/693300c1b91ddd64e8d45882e4aaf6f777fda87b) test(verify): cover selected managed method facts ### 📊 Changes **6 files changed** (+300 additions, -114 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+12 -6) 📝 `scripts/fixtures/managed/pe.d.mts` (+2 -0) 📝 `scripts/fixtures/managed/pe.mjs` (+13 -4) 📝 `scripts/verify/managed/manifest.mjs` (+57 -52) 📝 `scripts/verify/managed/oracles.mjs` (+216 -38) 📝 `scripts/verify/managed/support.mjs` (+0 -14) </details> ### 📄 Description ## Summary Fixes #2617. The managed manifest verifier now projects only the declared MethodDef's verified facts. Selecting `Second` from `First`, `Second`, and `Third` previously produced a three-method graph while recording an ignored one-method limit; it now produces a one-method graph with explicit post-inspection selection and partial coverage. ## Problem and expected behavior `inspectManagedMembersBytes` no longer accepts row-pagination options. The verifier still passed `methodOffset`/`methodLimit`, found the right token in the complete result, and supplied that complete result to graph projection. Its one-method self-test could not expose the extra methods. Selection parameters must describe the retained facts actually used by the graph. ## Change and scope - Inspect members once per manifest, select each exact MethodDef token, verify its commitments, and reuse that same selected result for graph projection. - Retain only that method's body, outgoing call edges, field accesses, and loaded user strings. Preserve shared metadata, artifact/MVID identity, and existing diagnostic issues. Mark omitted methods as partial coverage. - Remove obsolete member-pagination configuration. Record `method_tokens` and `selection_stage: "post-inspection"` in verifier-owned Evidence; report its ID, selection, and graph coverage. - Replace the single-method self-test with three methods, first/second-row controls, and rejection checks. Give every method a distinct call target, an owned field access, and its own decoded A/B/C `#US` literal; check retained member facts directly at the shared selection producer. Correct the guide's obsolete paging claim. This is a verifier fix within the existing [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). A new-provider/target-execution maintainer decision is not applicable. Selection follows bounded full inspection; it does not add a public token selector or avoid decoding unlisted bodies. ## Contract and boundary impact - Semantic owner and earliest changed stage: manifest verification in `scripts/verify/managed/manifest.mjs`, immediately after member inspection. - Contribution scope fit and linked maintainer decision: ordinary verifier/documentation correction; not applicable. - CLI and MCP/tool-catalog contract: unchanged. The verifier's compact report gains member Evidence identity, actual selection, and graph coverage. - Provider, bridge, target-format, or platform compatibility: existing execution-free PE/CLI inspection; no new provider or parsing limits. - Evidence, artifact, provenance, or reconstruction contract: exact target and method commitments remain checked; selected member Evidence replaces inaccurate pagination parameters and whole-assembly completeness. - Target selection, authority, upstream behavior, and cleanup: caller-selected manifest/target and existing temporary-fixture cleanup; selected assemblies are never loaded or executed. - Generated metadata, package, or installation impact: no tracked generated files, dependencies, versions, installers, or workflows changed. Ignored managed verification evidence was regenerated and checked. Expected effects of running verification: - [x] Read existing files or captured data. - [x] Launch or attach to a process — existing Node verifier/test processes only. - [ ] Inject instrumentation or run scripts/code in a selected target. - [x] Change files — owned temporary verification fixtures only. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Before the fix, `npm run verify:managed` with the three-method regression failed with `3 !== 1` at the graph method-count assertion. - The owning verifier self-test checks two graph selections, three direct member-fact selections, and 13 rejected requests: target digest/MVID/name/runtime/architecture, signature/IL commitments, missing/zero-row tokens, unselected seed leakage, and an undeclared trace method. The direct checks assert exact caller/callee tokens, owning-method/field tokens, and literal tokens/values; graph method counts do not observe those arrays. - Three isolated mutation probes remove the call-edge, field-access, and loaded-string filters independently. All three survived the original self-test; each now fails its corresponding retained-fact assertion. Four fixture variants without the new optional `#US` stream remain byte-identical to their pre-change output. - A separate source-owned, file-backed manifest exercised `REA_MANAGED_APP_MANIFEST_PATH`, including a relative target path. It retained only token `0x06000002` / `Second`, with `summary.methods: 1` and `coverage.status: "partial"`. Representative executed verifier output: ```json {"method_token":"0x06000002","member_selection":{"method_tokens":["0x06000002"],"selection_stage":"post-inspection"},"summary":{"methods":1},"coverage":{"status":"partial"}} ``` - [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 Windows x64, pinned Node.js 24.18.0/npm 11.16.0; integrated upstream through `ff56f785946c609b09c1c60232b28d7c34ac89a9`, final tested head `693300c1b91ddd64e8d45882e4aaf6f777fda87b`. The affected managed parser/verifier paths were also compared with main `41a2e846470bd2af40dd298d975b1a1d2fef5786` and were unchanged: - `npm run verify:managed` — passed, including the three direct selected-fact controls and existing selection/rejection checks. - `REA_MANAGED_APP_MANIFEST_PATH=<source-owned manifest> npm run verify:managed` — passed on the final head; configured manifest selected only `Second`. - `npm run test:local -- src/dotnet/ManagedMemberInspector.test.ts src/dotnet/ManagedCilDecoding.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts` — 205 passed on the final head. - `npm run check:ci` — passed on the final head, including duplication/TODO checks and architecture guards; existing lint warnings remain visible. - `npm run docs:check` — passed on the final head, including five README consistency tests. - Post-commit `npm run format:check` and final `git diff --check` — passed. A small four-method manifest without graph projection produced byte-identical compact results before/after. On this host, first calls took 16.0/12.0 ms; 50 warm calls took 159.4/119.0 ms and 204/125 ms process CPU, with peak process RSS 109,660/107,648 KiB. This is one diagnostic comparison after module imports, not a general performance claim. Agent-token cost and process-family memory were not measured. ## Compatibility, safety, and release - Breaking changes or migration steps: none for public CLI/MCP. Verifier report fields are additive; Evidence identities reflect corrected scope. - Real Hopper/Ghidra, browser, or OS coverage: source-owned PE byte fixtures on Windows. No target assembly execution, real osu! binary, ILSpy oracle, or Linux/macOS run is claimed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: existing bounded parsing and identity checks remain; no additional target authority or external service use. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved; no new-provider decision is required. - [x] The PR has one focused outcome and a Conventional Commit title. - [x] Related issue is linked. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning documentation and ignored verification evidence are updated where needed. - [x] Representative verifier output is included; CLI/MCP contracts are unchanged. - [x] Final diff checked 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 01:07:47 +00:00