[PR #5235] [MERGED] fix(windows): align path handling across Hopper, Electron, and ADB #6829

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5235
Author: @reynold-hu
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/windows-cross-platform


📝 Commits (8)

  • 717c280 fix(hopper): keep macOS and Linux launcher paths POSIX
  • c333fa4 fix(hopper): keep Linux and macOS bundle paths POSIX
  • 1f6ec02 fix(electron): follow Windows symlink targets with native separators
  • a8da99f test(windows): align host-specific path and lifecycle expectations
  • 9bbe846 test(windows): close review coverage gaps
  • 9139658 Merge origin/main into fix/windows-cross-platform
  • 6683a3d Merge origin/main into fix/windows-cross-platform
  • 646e23a fix(windows): retain fixture ownership while reconciling main

📊 Changes

18 files changed (+403 additions, -163 deletions)

View changed files

📝 src/adb/AdbDeviceOutput.test.ts (+1 -0)
📝 src/adb/AdbDeviceOutput.ts (+2 -1)
📝 src/adb/AdbHostFileName.test.ts (+4 -1)
📝 src/application/LinuxHopper.ts (+7 -2)
📝 src/application/MacHopper.ts (+13 -11)
📝 src/application/MacHopperPaths.test.ts (+17 -4)
📝 src/application/MacHopperPaths.ts (+9 -6)
📝 src/application/Setup.planning.test.ts (+7 -3)
📝 src/artifacts/inventory/ArtifactOccurrenceIdentity.test.ts (+25 -22)
📝 src/electron/ElectronFusePath.ts (+7 -3)
📝 src/electron/ElectronFuseProvider.test.ts (+39 -2)
📝 src/ghidra/GhidraLanguageOverride.test.ts (+14 -3)
📝 src/ghidra/GhidraPowerPcProfile.test.ts (+17 -5)
📝 src/ghidra/GhidraSnapshotCache.test.ts (+62 -3)
📝 src/hopper/HopperProviderDiscovery.test.ts (+51 -34)
📝 src/hopper/HopperTargetLease.test.ts (+34 -26)
📝 src/wasm/WabtArtifactProvider.test.ts (+89 -32)
📝 tests/composition/reverseEngineering.test.ts (+5 -5)

📄 Description

Summary

Windows x64 source runs exposed three host-dependent path and separator assumptions that make target-platform paths wrong or weaken symlink containment:

  • Hopper installer and launcher paths for macOS/Linux were built with the host path.join, so a Windows test host produced backslash-separated paths for those targets.
  • Electron fuse path walking split readlink targets only on /, but Windows returns \-separated relative targets. A nested link was treated as one component and a relative escaping target could pass the framework-root check without being walked.
  • ADB device-path validation implicitly used the host platform, so Windows test hosts rejected valid POSIX device paths.

This PR builds macOS/Linux target paths with path.posix, follows Windows symlink targets using native separators, and lets parsePmPathOutput take an explicit platform for validation. Affected tests now compare against native joins, POSIX-only cases are skipped on Windows with explicit Windows assertions where behavior differs, and a regression covers an escaping Windows symlink target.

No linked issue: found while running the source suite on Windows x64.

Problem and expected behavior

On Windows:

  • defaultHopperLauncherPath("linux" | "darwin", ...) returned \fixture\home\... instead of /fixture/home/....
  • macOS/Linux Hopper bundle and archive paths mixed native separators into target-platform paths.
  • readlink returned ..\..\Other; splitting only on / left it as one component, so containment was not established by walking.
  • parsePmPathOutput validated /data/local/tmp/... against Windows filename rules.

Expected: paths that describe a macOS/Linux target stay POSIX on any host; Windows symlink targets are walked with Windows separator semantics; ADB tests can select the validation platform explicitly.

Change and scope

Production changes:

  • src/application/MacHopperPaths.ts, src/application/MacHopper.ts, src/application/LinuxHopper.ts: use path.posix for macOS/Linux launcher, bundle, staging, and archive paths.
  • src/electron/ElectronFusePath.ts: split relative link targets on \ and / on Windows, preserving the existing ./.. walk and escape checks.
  • src/adb/AdbDeviceOutput.ts: add an optional platform parameter to parsePmPathOutput, defaulting to process.platform; pass it to adbHostFileName.

Test changes align native path joins, bind ADB tests to explicit platforms, prepare simulated POSIX Ghidra roots on Windows, cover the Windows Ghidra native-source admission step before snapshot replay, and skip POSIX-only Hopper lease, permission-bit, and symlink-parent cases on Windows. Windows still asserts the unsupported Hopper state.

Review follow-up (9bbe846):

  • The Windows Electron regression now uses a nested nested\subdir link target; a plain ..\..\..\Other target was already rejected lexically before the separator splitter ran.
  • Ghidra language fixtures are created on the checkout drive and keep the native path for cleanup, so a TEMP on another drive cannot redirect writes away from the owned directory.
  • Windows snapshot replay tests admit the native source with a different query, so the imported entry, not a fresh live cache entry for the same query, is under test.
  • WABT asserts the primary read error on Windows through diagnostics.primary_error; the Mach-O occurrence group now runs on Windows with executable: false instead of being skipped.

Not included: no native Windows Job Object build, no new provider, no target execution/injection, and no change to POSIX behavior (path.posix matches the host on macOS/Linux).

Contract and boundary impact

  • Semantic owner and earliest changed stage: Hopper path construction, Electron fuse path walking, and ADB device-path parsing.
  • Contribution scope fit and linked maintainer decision: ordinary fixes within existing boundaries; no new provider or authority; not applicable.
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: Windows x64 source tests; macOS/Linux path output unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: no change.
  • Target selection, authority, upstream behavior, and cleanup details: no target interaction; not applicable.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects:

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

Evidence and regression coverage

  • Tests added or updated: 12 source test files plus tests/composition/reverseEngineering.test.ts.
  • Base reproduction: on 23f984b (Windows x64), npm run test:local -- src/config.test.ts src/application/MacHopperPaths.test.ts failed 4 tests because native path.join produced backslashes for macOS/Linux paths.
  • Windows symlink regression: a nested nested\subdir target that reaches an outside directory is rejected as escaping; the link target needs per-component walking to expose it.
  • Windows Ghidra replay: admits the native source with a different query, then asserts the imported query returns the saved relationship coverage and snapshot limitation without another provider call.
  • POSIX-only skips on Windows: 15 Hopper lease tests, 2 Hopper discovery tests, and 1 artifact permission-change test. The Hopper discovery case is replaced by a Windows unsupported_host assertion; the Mach-O occurrence group now runs on Windows.
  • Remaining proof gaps: real Hopper and Ghidra provider runs were not executed; the Windows native addon used for local builds was restored from the packaged rea-agents@6.7.0 tarball (source checkouts do not ship it and cannot build it on Windows). Full check:pr is not a Windows gate here because it includes unavailable real-provider and POSIX-only lanes; it was stopped before completion.

Validation performed

  • Full source suite on a8da99f (owner verification comment): 609 files passed / 7 skipped; 9,924 tests passed / 218 skipped.
  • npm run test:local -- <6 review-affected test files> on 9bbe846 — 105 passed, 1 skipped (the remaining skip is the POSIX permission-change case).
  • npm run test:focused -- tests/composition/reverseEngineering.test.ts — 14 passed.
  • npm run check:fast — 2 tasks successful.
  • npm run build:cached — 3 tasks successful after restoring the packaged Windows native artifact.
  • npm run check — 6 tasks successful; architecture guards passed across 1506 source files.
  • Full npm run check:pr — attempted on Windows x64 but stopped before completion; it includes real-provider and POSIX-only lanes that are unavailable on this host, so it is not used as a Windows gate.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows x64 source/boundary tests only; no real provider run.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the Electron change makes Windows symlink escape detection stricter; the regression covers an escaping backslash target.

Merge/review follow-up at 646e23aa7: integrated main e83e3cead, preserving in-root absolute Electron links and Windows component-by-component symlink walking. The MacHopper scan fixture now keeps its native owned path for filesystem I/O and cleanup and uses a checkout-relative POSIX home on Windows, avoiding UNC-prefix collapse. Verified the existing nested-link regression and drive-safe Ghidra fixture corrections; all three formerly open review threads now have implementation/validation replies and are resolved. The 13-file focused batch passed 246 tests with 2 Windows-only skips on macOS; check:ci:fast passed all five tasks. Native Windows/UNC and real-provider behavior were not run locally. Hosted checks remain pending.

Author readiness

Human owner: @reynold-hu

  • 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/5235 **Author:** [@reynold-hu](https://github.com/reynold-hu) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/windows-cross-platform` --- ### 📝 Commits (8) - [`717c280`](https://github.com/morluto/rea/commit/717c2809763293fb663508ce0d83e42965e1ea79) fix(hopper): keep macOS and Linux launcher paths POSIX - [`c333fa4`](https://github.com/morluto/rea/commit/c333fa4aa4963b5e9bf8e99bc07eb9198bd05c2b) fix(hopper): keep Linux and macOS bundle paths POSIX - [`1f6ec02`](https://github.com/morluto/rea/commit/1f6ec02b471573c6fa9c93a4c092cce164fbbfc3) fix(electron): follow Windows symlink targets with native separators - [`a8da99f`](https://github.com/morluto/rea/commit/a8da99faa2d8bd97e2630d842d938013e05ade27) test(windows): align host-specific path and lifecycle expectations - [`9bbe846`](https://github.com/morluto/rea/commit/9bbe8460a2311c56894f74622480a62500e4709c) test(windows): close review coverage gaps - [`9139658`](https://github.com/morluto/rea/commit/9139658fc0c51973c421e1aaaed71b8351bde0a3) Merge origin/main into fix/windows-cross-platform - [`6683a3d`](https://github.com/morluto/rea/commit/6683a3d4acb7fe6b66f77ecadbd5432ecbde9ca9) Merge origin/main into fix/windows-cross-platform - [`646e23a`](https://github.com/morluto/rea/commit/646e23aa7065cd30677b4b7701a9e8a8f095597b) fix(windows): retain fixture ownership while reconciling main ### 📊 Changes **18 files changed** (+403 additions, -163 deletions) <details> <summary>View changed files</summary> 📝 `src/adb/AdbDeviceOutput.test.ts` (+1 -0) 📝 `src/adb/AdbDeviceOutput.ts` (+2 -1) 📝 `src/adb/AdbHostFileName.test.ts` (+4 -1) 📝 `src/application/LinuxHopper.ts` (+7 -2) 📝 `src/application/MacHopper.ts` (+13 -11) 📝 `src/application/MacHopperPaths.test.ts` (+17 -4) 📝 `src/application/MacHopperPaths.ts` (+9 -6) 📝 `src/application/Setup.planning.test.ts` (+7 -3) 📝 `src/artifacts/inventory/ArtifactOccurrenceIdentity.test.ts` (+25 -22) 📝 `src/electron/ElectronFusePath.ts` (+7 -3) 📝 `src/electron/ElectronFuseProvider.test.ts` (+39 -2) 📝 `src/ghidra/GhidraLanguageOverride.test.ts` (+14 -3) 📝 `src/ghidra/GhidraPowerPcProfile.test.ts` (+17 -5) 📝 `src/ghidra/GhidraSnapshotCache.test.ts` (+62 -3) 📝 `src/hopper/HopperProviderDiscovery.test.ts` (+51 -34) 📝 `src/hopper/HopperTargetLease.test.ts` (+34 -26) 📝 `src/wasm/WabtArtifactProvider.test.ts` (+89 -32) 📝 `tests/composition/reverseEngineering.test.ts` (+5 -5) </details> ### 📄 Description ## Summary Windows x64 source runs exposed three host-dependent path and separator assumptions that make target-platform paths wrong or weaken symlink containment: - Hopper installer and launcher paths for macOS/Linux were built with the host `path.join`, so a Windows test host produced backslash-separated paths for those targets. - Electron fuse path walking split `readlink` targets only on `/`, but Windows returns `\`-separated relative targets. A nested link was treated as one component and a relative escaping target could pass the framework-root check without being walked. - ADB device-path validation implicitly used the host platform, so Windows test hosts rejected valid POSIX device paths. This PR builds macOS/Linux target paths with `path.posix`, follows Windows symlink targets using native separators, and lets `parsePmPathOutput` take an explicit platform for validation. Affected tests now compare against native joins, POSIX-only cases are skipped on Windows with explicit Windows assertions where behavior differs, and a regression covers an escaping Windows symlink target. No linked issue: found while running the source suite on Windows x64. ## Problem and expected behavior On Windows: - `defaultHopperLauncherPath("linux" | "darwin", ...)` returned `\fixture\home\...` instead of `/fixture/home/...`. - macOS/Linux Hopper bundle and archive paths mixed native separators into target-platform paths. - `readlink` returned `..\..\Other`; splitting only on `/` left it as one component, so containment was not established by walking. - `parsePmPathOutput` validated `/data/local/tmp/...` against Windows filename rules. Expected: paths that describe a macOS/Linux target stay POSIX on any host; Windows symlink targets are walked with Windows separator semantics; ADB tests can select the validation platform explicitly. ## Change and scope Production changes: - `src/application/MacHopperPaths.ts`, `src/application/MacHopper.ts`, `src/application/LinuxHopper.ts`: use `path.posix` for macOS/Linux launcher, bundle, staging, and archive paths. - `src/electron/ElectronFusePath.ts`: split relative link targets on `\` and `/` on Windows, preserving the existing `.`/`..` walk and escape checks. - `src/adb/AdbDeviceOutput.ts`: add an optional `platform` parameter to `parsePmPathOutput`, defaulting to `process.platform`; pass it to `adbHostFileName`. Test changes align native path joins, bind ADB tests to explicit platforms, prepare simulated POSIX Ghidra roots on Windows, cover the Windows Ghidra native-source admission step before snapshot replay, and skip POSIX-only Hopper lease, permission-bit, and symlink-parent cases on Windows. Windows still asserts the unsupported Hopper state. Review follow-up (`9bbe846`): - The Windows Electron regression now uses a nested `nested\subdir` link target; a plain `..\..\..\Other` target was already rejected lexically before the separator splitter ran. - Ghidra language fixtures are created on the checkout drive and keep the native path for cleanup, so a `TEMP` on another drive cannot redirect writes away from the owned directory. - Windows snapshot replay tests admit the native source with a different query, so the imported entry, not a fresh live cache entry for the same query, is under test. - WABT asserts the primary read error on Windows through `diagnostics.primary_error`; the Mach-O occurrence group now runs on Windows with `executable: false` instead of being skipped. Not included: no native Windows Job Object build, no new provider, no target execution/injection, and no change to POSIX behavior (`path.posix` matches the host on macOS/Linux). ## Contract and boundary impact - Semantic owner and earliest changed stage: Hopper path construction, Electron fuse path walking, and ADB device-path parsing. - Contribution scope fit and linked maintainer decision: ordinary fixes within existing boundaries; no new provider or authority; not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: Windows x64 source tests; macOS/Linux path output unchanged. - Evidence, artifact, provenance, or reconstruction contract: no change. - Target selection, authority, upstream behavior, and cleanup details: no target interaction; not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects: - [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. ## Evidence and regression coverage - Tests added or updated: 12 source test files plus `tests/composition/reverseEngineering.test.ts`. - Base reproduction: on `23f984b` (Windows x64), `npm run test:local -- src/config.test.ts src/application/MacHopperPaths.test.ts` failed 4 tests because native `path.join` produced backslashes for macOS/Linux paths. - Windows symlink regression: a nested `nested\subdir` target that reaches an outside directory is rejected as escaping; the link target needs per-component walking to expose it. - Windows Ghidra replay: admits the native source with a different query, then asserts the imported query returns the saved relationship coverage and snapshot limitation without another provider call. - POSIX-only skips on Windows: 15 Hopper lease tests, 2 Hopper discovery tests, and 1 artifact permission-change test. The Hopper discovery case is replaced by a Windows `unsupported_host` assertion; the Mach-O occurrence group now runs on Windows. - Remaining proof gaps: real Hopper and Ghidra provider runs were not executed; the Windows native addon used for local builds was restored from the packaged `rea-agents@6.7.0` tarball (source checkouts do not ship it and cannot build it on Windows). Full `check:pr` is not a Windows gate here because it includes unavailable real-provider and POSIX-only lanes; it was stopped before completion. ## Validation performed - Full source suite on `a8da99f` (owner verification comment): 609 files passed / 7 skipped; 9,924 tests passed / 218 skipped. - `npm run test:local -- <6 review-affected test files>` on `9bbe846` — 105 passed, 1 skipped (the remaining skip is the POSIX permission-change case). - `npm run test:focused -- tests/composition/reverseEngineering.test.ts` — 14 passed. - `npm run check:fast` — 2 tasks successful. - `npm run build:cached` — 3 tasks successful after restoring the packaged Windows native artifact. - `npm run check` — 6 tasks successful; architecture guards passed across 1506 source files. - Full `npm run check:pr` — attempted on Windows x64 but stopped before completion; it includes real-provider and POSIX-only lanes that are unavailable on this host, so it is not used as a Windows gate. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Windows x64 source/boundary tests only; no real provider run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the Electron change makes Windows symlink escape detection stricter; the regression covers an escaping backslash target. Merge/review follow-up at `646e23aa7`: integrated main `e83e3cead`, preserving in-root absolute Electron links and Windows component-by-component symlink walking. The MacHopper scan fixture now keeps its native owned path for filesystem I/O and cleanup and uses a checkout-relative POSIX home on Windows, avoiding UNC-prefix collapse. Verified the existing nested-link regression and drive-safe Ghidra fixture corrections; all three formerly open review threads now have implementation/validation replies and are resolved. The 13-file focused batch passed 246 tests with 2 Windows-only skips on macOS; `check:ci:fast` passed all five tasks. Native Windows/UNC and real-provider behavior were not run locally. Hosted checks remain pending. ## Author readiness Human owner: @reynold-hu - [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 - [ ] 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. --- <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:20 +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#6829
No description provided.