morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4144] [CLOSED] fix(managed): match Windows module paths independently of the host #6084

New issue

Closed

opened 2026-10-31 01:11:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4144
Author: @hichipli
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/managed-native-module-paths

📝 Commits (2)

6fc19ba fix(managed): match Windows module paths independently of the host
5af6be0 fix(managed): preserve parsed native module filenames

📊 Changes

2 files changed (+97 additions, -17 deletions)

View changed files

📝 src/domain/managed/managedNativeVerificationMatch.ts (+27 -17)
📝 tests/conformance/providers/managed/managedNativeVerification.test.ts (+70 -0)

📄 Description

Superseded on 2026-10-24. #3351 was resolved independently on main. Its current platform-aware matcher preserves unknown full-path identity when only a basename is available; this PR's older basename-only contract should not be merged. Closing without merging. See the reporter's recheck and current matcher change. The description below records the historical proposal and validation.

Summary

Fixes #3351. Managed/native verification on a POSIX host now matches a Windows absolute module declaration such as C:\deps\helper.dll against native helper.dll, just as it does for C:/deps/helper.dll. A genuinely different module remains contradicted.

Problem and expected behavior

The shared matcher used the host's basename for target metadata. On macOS, Windows separators stayed in the module identity and produced a false module-mismatch even with an exact native function name.

Change and scope

Select Windows basename semantics for an absolute Windows spelling that is not a POSIX absolute path; use explicit POSIX semantics otherwise. Apply path interpretation once to declarations and native paths. Treat the producer-owned native module name as a literal filename; share stem/extension normalization without reparsing extracted names. This preserves literal backslashes in POSIX paths, existing extension/lib alias rules, original declarations and supplied Evidence identities.

This is a focused domain correction within existing boundaries; an advance maintainer scope decision is not applicable. Relative module strings are not newly reinterpreted as Windows paths.

Contract and boundary impact

Semantic owner: the shared managed/native module matcher, consumed by CLI and MCP.
Schemas, tool catalog, native provider protocols, dependencies and generated metadata: unchanged.
Evidence: removes false contradictions for matching target paths while retaining the original strings and actual mismatches. No CLR runtime binding or token-to-address mapping is inferred.
Existing workflow effects: inspect caller-supplied Evidence; no target execution, mutation, network or new authority.

Evidence and regression coverage

On base a40ee741, actual compiled CLI and stdio MCP calls return contradicted/module-mismatch for C:\deps\helper.dll, while the slash control verifies against the same synthetic native function Evidence. Four new drive/UNC regression cases fail on that base.

After the fix, both adapters return verified/exact-function-name for both spellings and retain contradicted/module-mismatch for C:\deps\other.dll. The generated PE fixture bytes remain unchanged. Conformance controls cover Windows paths on either side, UNC paths, POSIX literal backslashes and known mismatches.

Validation performed

Node 24.18.0 / npm 11.16.0, macOS arm64:

npm run test:focused -- tests/conformance/providers/managed/managedNativeVerification.test.ts tests/boundary/mcp/managedMcp.test.ts tests/boundary/providers/managed/managedNativeOsMachine.test.ts — 30 passed, including four review regressions that fail on the initial PR revision.
npm run check — passed typecheck, format, lint, unused-code, metadata, module boundaries and architecture guards; zero lint errors, existing repository warnings.
Actual compiled CLI and production stdio MCP — backslash/slash positive controls, different-module negative control, POSIX filenames with drive/rooted/UNC-looking prefixes, and a matching literal-name positive control passed; each independent MCP control uses a fresh session.
git diff --check — passed.

Compatibility, safety, and release

No schema, dependency, package or migration changes. Tests use self-generated managed fixtures and explicitly synthetic native Evidence; no real Ghidra/Hopper/IDA engine or CLR binding was executed. Linux/Windows execution and the full deterministic suite are left to CI.

An initial reused-session probe encountered the separately tracked unknown-registry revision_conflict (#3333) on a later negative control. Fresh-session controls pass; that independent session defect is outside this change. The review regression exposed a second path interpretation after filename extraction. Three mismatched POSIX filenames now remain contradicted, while an identical literal filename still verifies. No performance improvement is claimed.

Author readiness

Human owner: @hichipli

On 2026-10-15, @hichipli confirmed review of the initially submitted diff, validation evidence and limitations, accepted responsibility, and explicitly declared this PR ready for initial maintainer review. Subsequent review fixes keep the description and validation current; the repository policy retains that initial readiness declaration.

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 within the existing contribution scope.
The PR has one focused outcome and links its issue.
Regression coverage preserves mismatches, raw identities and uncertain outcomes.
No owning contract, documentation or generated-metadata update is required.
Final diff checked for secrets, unrelated changes 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/4144 **Author:** [@hichipli](https://github.com/hichipli) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/managed-native-module-paths` --- ### 📝 Commits (2) - [`6fc19ba`](https://github.com/morluto/rea/commit/6fc19baf426340fca749578985964c64d7b678f5) fix(managed): match Windows module paths independently of the host - [`5af6be0`](https://github.com/morluto/rea/commit/5af6be0309a791af605e4c182c4aa1faa10f0b33) fix(managed): preserve parsed native module filenames ### 📊 Changes **2 files changed** (+97 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/managed/managedNativeVerificationMatch.ts` (+27 -17) 📝 `tests/conformance/providers/managed/managedNativeVerification.test.ts` (+70 -0) </details> ### 📄 Description > **Superseded on 2026-10-24.** #3351 was resolved independently on main. Its current platform-aware matcher preserves unknown full-path identity when only a basename is available; this PR's older basename-only contract should not be merged. Closing without merging. See the [reporter's recheck](https://github.com/morluto/rea/issues/3351#issuecomment-6380756641) and [current matcher change](https://github.com/morluto/rea/commit/64fa1db1309218283df19f3ee4dba61f5fbd2a67). The description below records the historical proposal and validation. ## Summary Fixes #3351. Managed/native verification on a POSIX host now matches a Windows absolute module declaration such as `C:\deps\helper.dll` against native `helper.dll`, just as it does for `C:/deps/helper.dll`. A genuinely different module remains contradicted. ## Problem and expected behavior The shared matcher used the host's `basename` for target metadata. On macOS, Windows separators stayed in the module identity and produced a false `module-mismatch` even with an exact native function name. ## Change and scope Select Windows basename semantics for an absolute Windows spelling that is not a POSIX absolute path; use explicit POSIX semantics otherwise. Apply path interpretation once to declarations and native paths. Treat the producer-owned native module name as a literal filename; share stem/extension normalization without reparsing extracted names. This preserves literal backslashes in POSIX paths, existing extension/lib alias rules, original declarations and supplied Evidence identities. This is a focused domain correction within existing boundaries; an advance maintainer scope decision is not applicable. Relative module strings are not newly reinterpreted as Windows paths. ## Contract and boundary impact - Semantic owner: the shared managed/native module matcher, consumed by CLI and MCP. - Schemas, tool catalog, native provider protocols, dependencies and generated metadata: unchanged. - Evidence: removes false contradictions for matching target paths while retaining the original strings and actual mismatches. No CLR runtime binding or token-to-address mapping is inferred. - Existing workflow effects: inspect caller-supplied Evidence; no target execution, mutation, network or new authority. ## Evidence and regression coverage On base `a40ee741`, actual compiled CLI and stdio MCP calls return `contradicted/module-mismatch` for `C:\deps\helper.dll`, while the slash control verifies against the same synthetic native function Evidence. Four new drive/UNC regression cases fail on that base. After the fix, both adapters return `verified/exact-function-name` for both spellings and retain `contradicted/module-mismatch` for `C:\deps\other.dll`. The generated PE fixture bytes remain unchanged. Conformance controls cover Windows paths on either side, UNC paths, POSIX literal backslashes and known mismatches. ## Validation performed Node 24.18.0 / npm 11.16.0, macOS arm64: - `npm run test:focused -- tests/conformance/providers/managed/managedNativeVerification.test.ts tests/boundary/mcp/managedMcp.test.ts tests/boundary/providers/managed/managedNativeOsMachine.test.ts` — 30 passed, including four review regressions that fail on the initial PR revision. - `npm run check` — passed typecheck, format, lint, unused-code, metadata, module boundaries and architecture guards; zero lint errors, existing repository warnings. - Actual compiled CLI and production stdio MCP — backslash/slash positive controls, different-module negative control, POSIX filenames with drive/rooted/UNC-looking prefixes, and a matching literal-name positive control passed; each independent MCP control uses a fresh session. - `git diff --check` — passed. ## Compatibility, safety, and release No schema, dependency, package or migration changes. Tests use self-generated managed fixtures and explicitly synthetic native Evidence; no real Ghidra/Hopper/IDA engine or CLR binding was executed. Linux/Windows execution and the full deterministic suite are left to CI. An initial reused-session probe encountered the separately tracked unknown-registry `revision_conflict` (#3333) on a later negative control. Fresh-session controls pass; that independent session defect is outside this change. The review regression exposed a second path interpretation after filename extraction. Three mismatched POSIX filenames now remain contradicted, while an identical literal filename still verifies. No performance improvement is claimed. ## Author readiness Human owner: @hichipli On 2026-10-15, @hichipli confirmed review of the initially submitted diff, validation evidence and limitations, accepted responsibility, and explicitly declared this PR ready for initial maintainer review. Subsequent review fixes keep the description and validation current; the repository policy retains that initial readiness declaration. - [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 - [x] This PR addresses a concrete problem within the existing contribution scope. - [x] The PR has one focused outcome and links its issue. - [x] Regression coverage preserves mismatches, raw identities and uncertain outcomes. - [x] No owning contract, documentation or generated-metadata update is required. - [x] Final diff checked for secrets, unrelated changes 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:11:43 +00:00

closed this issue
added the
pull-request
label

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#6084

No description provided.

Rows
Columns