[PR #5133] [CLOSED] fix(managed): distinguish truncated compressed signature integers #6781

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5133
Author: @David-the-happy-piglet
Created: 10/19/2026
Status: ❌ Closed

Base: main ← Head: fix/5102-truncated-compressed-integers


📝 Commits (4)

  • 2750a21 fix(managed): distinguish truncated compressed signature integers
  • ef109d6 test(snapshot): align replay questions with evidence identity
  • af341f5 chore: sync main to keep signature fix focused
  • a25d75b test(managed): cover MethodSpec compressed integer diagnostics

📊 Changes

4 files changed (+182 additions, -27 deletions)

View changed files

➕ src/dotnet/ManagedCompressedInteger.test.ts (+52 -0)
📝 src/dotnet/ManagedMemberInspector.test.ts (+52 -0)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+18 -27)
➕ tests/acceptance/managedSignatureDiagnostics.test.ts (+60 -0)

📄 Description

Summary

Fixes #5102. Managed member inspection now reports truncated compressed integer when a valid two- or four-byte signature prefix is missing continuation bytes. Previously those inputs were incorrectly described as reserved encodings.

Problem and expected behavior

A MethodDef signature such as 00 80 or 00 c0 00 00 contains a truncated parameter count. inspect-managed-members and inspect_managed_members returned signature.issue: "reserved compressed integer". The prefix determines the encoding width; absent continuation bytes should report truncation. Truly reserved prefixes such as e0 still report reserved. Malformed status and partial coverage remain unchanged.

Change and scope

Separate prefix recognition from byte-availability checks in the shared readCompressed producer and reuse the existing typed signature-failure helper. Preserve decoded values, next offsets and error locations. Add compiled CLI/stdio MCP regressions, producer boundary cases, and MethodSpec coverage for row-owned diagnostics and readable call targets. The four-file delta contains no snapshot expectation changes. This is a routine fix within existing static parsing scope; new-capability preapproval is not applicable.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Shared managed-signature compressed integer decoder, before signature projection and Evidence mapping.
  • Contribution scope fit and linked maintainer decision: Routine static parsing fix within existing boundaries; advance maintainer scope decision is not applicable.
  • CLI and MCP/tool-catalog contract: No schema or argument changes; incomplete encodings now report the correct diagnostic text.
  • Provider, bridge, target-format, or platform compatibility: Existing static PE/CLI reader; no bridge or target-execution changes.
  • Evidence, artifact, provenance, or reconstruction contract: Artifact digest, method token, malformed/partial status, typed failure and signature-relative error offsets are preserved.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): Existing read-only file analysis; selection, authority and cleanup are unchanged.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): None; this patch does not change authored contracts, catalogs, packages or installation.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • 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.

The effect checklist describes the changed product behavior. Tests launch the CLI/MCP processes and write temporary fixtures, but never execute the inspected target.

Evidence and regression coverage

  • Tests added or updated: src/dotnet/ManagedCompressedInteger.test.ts, tests/acceptance/managedSignatureDiagnostics.test.ts, and the MethodSpec cases in src/dotnet/ManagedMemberInspector.test.ts add 22 regressions. The three new MethodSpec inputs (valid, truncated and reserved) preserve precise row-owned diagnostics, complete/partial coverage, and readable call targets. These cover valid integer boundaries/cursors, typed failure offsets, all continuation-byte cut-off points through compiled CLI/stdio MCP, a generic count, and valid/reserved controls with artifact identity and partial Evidence.
  • Base reproduction or other evidence: Executed on macOS 26.5.2 arm64, Node 24.18.0, npm 11.16.0. On 2026-10-20, an isolated main@8443692e tree with the two regression files and the original parser failed seven cases (30 passed, two Windows-only skips across the regression and unchanged snapshot files). With the fix, those same three files passed 37 cases with the same two skips.
  • User-visible CLI/MCP output (if applicable): For 00 80, both adapters return signature.parse_status: "malformed", signature.issue: "truncated compressed integer", and coverage.state: "partial", preserving artifact digest and method identity. 00 e0 remains reserved; 00 00 01 remains decoded with complete coverage.
  • Remaining proof gaps: Full suite/coverage, Windows/Linux execution, real customer assemblies and external Hopper/Ghidra engines were not exercised. Hosted CI and PR-title workflows still require maintainer approval on the published revision; at validation time, this follow-up had not yet been pushed or reviewed by those services.

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

Executed on macOS 26.5.2 arm64 with npm 11.16.0. The local follow-up a25d75bd adds only three MethodSpec tests; production code is unchanged from the published fix.

  • Regression proof: The final MethodSpec test file, unchanged, ran against an isolated copy with the exact original parser from 8443692e: one failure / 134 passes. Only 0a 80 failed with actual reserved compressed integer instead of expected truncated compressed integer; valid and reserved controls passed. The fixed file passed all 135 cases. An independent reviewer repeated both runs.
  • Local commit, Node 24.18.0: npm run test:focused with the member-inspection, signature-resolution, compressed-integer and signature CLI/MCP files — 174 passed. npm run check passed (0 lint errors, 1936 warnings). npm run verify:test-discovery found all 1815 test files exactly once. Normal pre-commit hooks passed without changing the reviewed patch.
  • Latest-main compatibility, Node 24.22.1: An isolated trial merge of a25d75bd with main@592c46b0 is conflict-free. It preserves all upstream changes and the same four-file, +182/-27 patch; directAnalysisSnapshotReplay.test.ts is absent from the delta and the trial merge retains main's version unchanged. Exact pinned toolchain and locked dependencies were used; the task branch was not merged or rebased.
  • npm run test:focused on that combined tree with 13 explicit managed producer, metadata, compiled CLI/stdio MCP, workflow Evidence, snapshot and managed application graph test files — 296 passed, two existing Windows-only skips.
  • npm run check on the combined tree — passed (0 lint errors, 2375 warnings; six tasks succeeded, two served from the local build cache). npm run verify:test-discovery — all 2160 test files discovered exactly once, using a separate Git index for the exact trial-merge tree.
  • A separate production-inspection probe on the combined tree passed five LocalVarSig cases: normal, pinned, incomplete two-/four-byte counts and a reserved prefix. It verified complete/partial coverage, exact diagnostic ownership and absolute byte offsets, and retained CIL. This is a synthetic source-entrypoint probe, not an additional committed regression or external-engine verification.
  • Independent review found no actionable issues in the test addition, unchanged core fix or trial-merge delta. git diff --check passed. The earlier original-parser CLI/MCP failure evidence is recorded above; no full-suite or platform coverage is inferred from focused passes.

Remote snapshot (2026-10-23 10:39 PDT), for published af341f5b only: CI and PR title remain action_required, attempt 1, with zero jobs executed. Codex completed without findings; Greptile and PR readiness succeeded. These historical af341f5b statuses do not validate a25d75bd or establish CI success. Required CI/title checks must execute successfully before merge.

Compatibility, safety, and release

  • Breaking changes or migration steps: None; schema, values and offsets are unchanged. The intentional correction is the diagnostic for an incomplete two- or four-byte compressed integer.
  • Real Hopper/Ghidra, browser, or OS coverage: Local macOS arm64 only. Synthetic PE fixtures exercise the production static .NET provider through compiled CLI and stdio MCP. Windows/Linux, real customer assemblies, browser workflows and external Hopper/Ghidra engines were not exercised.
  • Package or release metadata impact: None; no package, installation or release metadata changes.
  • Security, privacy, process, or containment review: The decoder reads existing bytes; it introduces no target execution, network access or mutation. Test harnesses launch REA CLI/MCP processes and use temporary fixtures; they never execute the inspected target.

Author readiness

Human owner: @David-the-happy-piglet

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

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • 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/5133 **Author:** [@David-the-happy-piglet](https://github.com/David-the-happy-piglet) **Created:** 10/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/5102-truncated-compressed-integers` --- ### 📝 Commits (4) - [`2750a21`](https://github.com/morluto/rea/commit/2750a219f20cf72582b43c0d6ca334ec7b54b9db) fix(managed): distinguish truncated compressed signature integers - [`ef109d6`](https://github.com/morluto/rea/commit/ef109d6c576b371cf1acdfbb7aa194c81e26a6c0) test(snapshot): align replay questions with evidence identity - [`af341f5`](https://github.com/morluto/rea/commit/af341f5bcb75ea93c19d15a54f574c1250df1e3b) chore: sync main to keep signature fix focused - [`a25d75b`](https://github.com/morluto/rea/commit/a25d75bd61c440a0138b4631d1955c39695942fc) test(managed): cover MethodSpec compressed integer diagnostics ### 📊 Changes **4 files changed** (+182 additions, -27 deletions) <details> <summary>View changed files</summary> ➕ `src/dotnet/ManagedCompressedInteger.test.ts` (+52 -0) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+52 -0) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+18 -27) ➕ `tests/acceptance/managedSignatureDiagnostics.test.ts` (+60 -0) </details> ### 📄 Description <!-- PR title: type(optional-scope): imperative outcome The title becomes the squash-merge commit subject and generated changelog entry. --> ## Summary <!-- What user, agent, or maintainer problem does this solve? Link the issue with "Fixes #123" when applicable. Keep the prior behavior, expected contract, and new behavior understandable without private context. --> Fixes #5102. Managed member inspection now reports `truncated compressed integer` when a valid two- or four-byte signature prefix is missing continuation bytes. Previously those inputs were incorrectly described as reserved encodings. ## Problem and expected behavior <!-- For fixes, give the smallest trigger and violated invariant. For features, describe the use case and observable outcome. --> A MethodDef signature such as `00 80` or `00 c0 00 00` contains a truncated parameter count. `inspect-managed-members` and `inspect_managed_members` returned `signature.issue: "reserved compressed integer"`. The prefix determines the encoding width; absent continuation bytes should report truncation. Truly reserved prefixes such as `e0` still report reserved. Malformed status and partial coverage remain unchanged. ## Change and scope <!-- Explain the chosen approach, why it fits REA's architecture, and what is intentionally not included. If adding complexity for a speculative or vanishingly unlikely race or security edge case, record the concrete failure mode, likelihood, complexity cost, and the user's buy-in obtained before implementation. Do not silently expand scope for theoretical robustness. --> Separate prefix recognition from byte-availability checks in the shared `readCompressed` producer and reuse the existing typed signature-failure helper. Preserve decoded values, next offsets and error locations. Add compiled CLI/stdio MCP regressions, producer boundary cases, and MethodSpec coverage for row-owned diagnostics and readable call targets. The four-file delta contains no snapshot expectation changes. This is a routine fix within existing static parsing scope; new-capability preapproval is not applicable. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact <!-- Short answers are welcome. Complete applicable lines; mark unresolved facts as "unknown". Use "none" or "not applicable" only when established. --> - Semantic owner and earliest changed stage: Shared managed-signature compressed integer decoder, before signature projection and Evidence mapping. - Contribution scope fit and linked maintainer decision: Routine static parsing fix within existing boundaries; advance maintainer scope decision is not applicable. - CLI and MCP/tool-catalog contract: No schema or argument changes; incomplete encodings now report the correct diagnostic text. - Provider, bridge, target-format, or platform compatibility: Existing static PE/CLI reader; no bridge or target-execution changes. - Evidence, artifact, provenance, or reconstruction contract: Artifact digest, method token, malformed/partial status, typed failure and signature-relative error offsets are preserved. - Target selection, authority, upstream behavior, and cleanup details (if applicable): Existing read-only file analysis; selection, authority and cleanup are unchanged. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): None; this patch does not change authored contracts, catalogs, packages or installation. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] 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. The effect checklist describes the changed product behavior. Tests launch the CLI/MCP processes and write temporary fixtures, but never execute the inspected target. ## Evidence and regression coverage <!-- State whether evidence is executed, source-derived, or proposed. For fixes, show the original failure and explain how the regression would fail on the affected base revision when practical. Include the most useful proof inline; link long logs or artifacts, or put them in optional details. Documentation-only changes can use source, link, or rendered-document checks. --> - Tests added or updated: `src/dotnet/ManagedCompressedInteger.test.ts`, `tests/acceptance/managedSignatureDiagnostics.test.ts`, and the MethodSpec cases in `src/dotnet/ManagedMemberInspector.test.ts` add 22 regressions. The three new MethodSpec inputs (valid, truncated and reserved) preserve precise row-owned diagnostics, complete/partial coverage, and readable call targets. These cover valid integer boundaries/cursors, typed failure offsets, all continuation-byte cut-off points through compiled CLI/stdio MCP, a generic count, and valid/reserved controls with artifact identity and partial Evidence. - Base reproduction or other evidence: Executed on macOS 26.5.2 arm64, Node 24.18.0, npm 11.16.0. On 2026-10-20, an isolated `main@8443692e` tree with the two regression files and the original parser failed seven cases (30 passed, two Windows-only skips across the regression and unchanged snapshot files). With the fix, those same three files passed 37 cases with the same two skips. - User-visible CLI/MCP output (if applicable): For `00 80`, both adapters return `signature.parse_status: "malformed"`, `signature.issue: "truncated compressed integer"`, and `coverage.state: "partial"`, preserving artifact digest and method identity. `00 e0` remains reserved; `00 00 01` remains decoded with complete coverage. - Remaining proof gaps: Full suite/coverage, Windows/Linux execution, real customer assemblies and external Hopper/Ghidra engines were not exercised. Hosted CI and PR-title workflows still require maintainer approval on the published revision; at validation time, this follow-up had not yet been pushed or reviewed by those services. 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 <!-- List only commands that actually ran, with observed results. Include the relevant real-provider or platform workflow, or explain why it was unavailable. Keep these results current after changes; a command without its result is incomplete. --> Executed on macOS 26.5.2 arm64 with npm 11.16.0. The local follow-up `a25d75bd` adds only three MethodSpec tests; production code is unchanged from the published fix. - **Regression proof:** The final MethodSpec test file, unchanged, ran against an isolated copy with the exact original parser from `8443692e`: one failure / 134 passes. Only `0a 80` failed with actual `reserved compressed integer` instead of expected `truncated compressed integer`; valid and reserved controls passed. The fixed file passed all 135 cases. An independent reviewer repeated both runs. - **Local commit, Node 24.18.0:** `npm run test:focused` with the member-inspection, signature-resolution, compressed-integer and signature CLI/MCP files — 174 passed. `npm run check` passed (0 lint errors, 1936 warnings). `npm run verify:test-discovery` found all 1815 test files exactly once. Normal pre-commit hooks passed without changing the reviewed patch. - **Latest-main compatibility, Node 24.22.1:** An isolated trial merge of `a25d75bd` with `main@592c46b0` is conflict-free. It preserves all upstream changes and the same four-file, +182/-27 patch; `directAnalysisSnapshotReplay.test.ts` is absent from the delta and the trial merge retains main's version unchanged. Exact pinned toolchain and locked dependencies were used; the task branch was not merged or rebased. - `npm run test:focused` on that combined tree with 13 explicit managed producer, metadata, compiled CLI/stdio MCP, workflow Evidence, snapshot and managed application graph test files — 296 passed, two existing Windows-only skips. - `npm run check` on the combined tree — passed (0 lint errors, 2375 warnings; six tasks succeeded, two served from the local build cache). `npm run verify:test-discovery` — all 2160 test files discovered exactly once, using a separate Git index for the exact trial-merge tree. - A separate production-inspection probe on the combined tree passed five LocalVarSig cases: normal, pinned, incomplete two-/four-byte counts and a reserved prefix. It verified complete/partial coverage, exact diagnostic ownership and absolute byte offsets, and retained CIL. This is a synthetic source-entrypoint probe, not an additional committed regression or external-engine verification. - Independent review found no actionable issues in the test addition, unchanged core fix or trial-merge delta. `git diff --check` passed. The earlier original-parser CLI/MCP failure evidence is recorded above; no full-suite or platform coverage is inferred from focused passes. Remote snapshot (2026-10-23 10:39 PDT), for published `af341f5b` only: [CI](https://github.com/morluto/rea/actions/runs/39450960261) and [PR title](https://github.com/morluto/rea/actions/runs/39450968088) remain `action_required`, attempt 1, with zero jobs executed. Codex completed without findings; Greptile and PR readiness succeeded. These historical `af341f5b` statuses do not validate `a25d75bd` or establish CI success. Required CI/title checks must execute successfully before merge. ## Compatibility, safety, and release <!-- Call out breaking changes, migrations, package effects, platform/provider limits, security/privacy/containment implications, and meaningful proof gaps. --> - Breaking changes or migration steps: None; schema, values and offsets are unchanged. The intentional correction is the diagnostic for an incomplete two- or four-byte compressed integer. <!-- For ! / BREAKING CHANGE: show a previously valid call/configuration, its new behavior and migration, and why compatibility/deprecation cannot preserve it. Every release increments minor, including breaking changes; keep breaking markers and migration notes so callers can assess compatibility. See docs/releasing.md. --> - Real Hopper/Ghidra, browser, or OS coverage: Local macOS arm64 only. Synthetic PE fixtures exercise the production static .NET provider through compiled CLI and stdio MCP. Windows/Linux, real customer assemblies, browser workflows and external Hopper/Ghidra engines were not exercised. - Package or release metadata impact: None; no package, installation or release metadata changes. - Security, privacy, process, or containment review: The decoder reads existing bytes; it introduces no target execution, network access or mutation. Test harnesses launch REA CLI/MCP processes and use temporary fixtures; they never execute the inspected target. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @David-the-happy-piglet - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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-11-01 21:43:00 +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#6781
No description provided.