morluto/rea

Fork 0

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

[GH-ISSUE #3635] [Bug] Windows package runner identity shadowed by ambient variables & ConPTY test failures #1359

New issue

Closed

opened 2026-10-16 21:04:38 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-16 21:04:38 +00:00

Owner

Originally created by @Swatantra-66 on GitHub (Oct 14, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3635

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Setup, packaging, or upgrade

Observed behavior

On Windows, when invokingPackageRunner(environment, "win32") evaluates an environment where an ambient uppercase variable (such as NPM_COMMAND="run") exists alongside an overriding lowercase entry ({ ...process.env, npm_command: "exec" }), environmentValue selects the earlier ambient key instead of the caller's override. Furthermore, candidate keys were compared directly against name rather than name.toLowerCase().

Additionally, in tests/boundary/cli/cliSetup.test.ts, the child pseudoterminal in runJourney isolates HOME but omits USERPROFILE, leaking into the host user's actual profile directory on Windows. Windows ConPTY also emits ANSI cursor restore escape codes (\x1b[?25h) and clear-line sequences trailing the decision output, causing runJourney's JSON parser (.split(/\r?\n/u, 1)[0]) to fail with:
SyntaxError: Unexpected non-whitespace character after JSON at position ...

Reproduction

In Node on Windows, run:
node -e "const { invokingPackageRunner } = require('./dist/application/ClientRegistrationIdentity.js'); console.log(invokingPackageRunner({ NPM_COMMAND: 'run', npm_command: 'exec', NPM_CONFIG_USER_AGENT: 'npm/10.9.0' }, 'win32'))"
Observe that undefined is returned instead of "npx".
Run the focused setup boundary tests on Windows:
npm run test:focused -- tests/boundary/cli/cliSetup.test.ts
Observe test failures due to ConPTY escape sequence JSON parsing errors and non-isolated USERPROFILE.

Expected behavior

On Windows, environmentValue should resolve environment variables case-insensitively while preserving JavaScript property order precedence so later properties override earlier ones (findLast).
tests/boundary/cli/cliSetup.test.ts should isolate USERPROFILE alongside HOME and extract the JSON payload bounded by delimiter braces { and } to prevent trailing ConPTY ANSI control codes from corrupting the JSON parse.

Impact and scope

Package runner detection on Windows can falsely fail when launched from scripts or package managers that populate uppercase ambient npm variables.
Windows developer workstations fail setup boundary tests and risk writing temporary configuration files into the host machine's USERPROFILE.

REA version

6.5.0

Runtime and platform

Node 24.18.0; Windows 11 x64 (ConPTY)

Target and artifact context

Windows CLI setup and test runner

Interface

CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Command or MCP request actually run:
npm run test:focused -- tests/boundary/cli/cliSetup.test.ts

Relevant result, error, or artifact:
SyntaxError: Unexpected non-whitespace character after JSON at position 63
at JSON.parse ()
at tests/boundary/cli/cliSetup.test.ts:484:37

And environment case collision returning undefined:
invokingPackageRunner({ NPM_COMMAND: "run", npm_command: "exec", NPM_CONFIG_USER_AGENT: "npm/10.9.0" }, "win32") === undefined

For source-only findings, link the file/revision and relevant code:

src/application/ClientRegistrationIdentity.ts: environmentValue uses Object.keys(environment).find() which matches ambient uppercase keys first on Windows.
tests/boundary/cli/cliSetup.test.ts: runJourney isolates HOME but omits USERPROFILE, leaking host environment.

Missing evidence or remaining uncertainty:
None. Root cause and regression test isolation confirmed on Windows 11.

Typed outcome or recovery expected

invokingPackageRunner should resolve "npx" (or "bunx") based on the caller's explicit environment entries, and runJourney should return a parsed JourneyResult without JSON parse errors.

Analysis or suspected cause

In src/application/ClientRegistrationIdentity.ts, environmentValue uses Object.keys(environment).find(). On Windows, when ambient uppercase variables like NPM_COMMAND="run" exist alongside explicit { npm_command: "exec" }, find() returns the earlier ambient key instead of the caller's override. Changing to findLast() with case-normalized target comparison preserves JavaScript property order precedence.
In tests/boundary/cli/cliSetup.test.ts, homeDirectoryFromEnvironment prioritizes USERPROFILE over HOME on Windows, so tests omitting USERPROFILE leak into the host user profile. Additionally, Windows ConPTY emits ANSI cursor visibility and clear codes after the decision marker, so extracting JSON between delimiter braces { and } avoids syntax errors.

Additional context

A fix and full regression tests have been prepared and verified on Windows 11 with Node 24.18.0. I am opening a PR with the fix and test coverage.

Originally created by @Swatantra-66 on GitHub (Oct 14, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3635 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Setup, packaging, or upgrade ### Observed behavior On Windows, when `invokingPackageRunner(environment, "win32")` evaluates an environment where an ambient uppercase variable (such as `NPM_COMMAND="run"`) exists alongside an overriding lowercase entry (`{ ...process.env, npm_command: "exec" }`), `environmentValue` selects the earlier ambient key instead of the caller's override. Furthermore, candidate keys were compared directly against `name` rather than `name.toLowerCase()`. Additionally, in `tests/boundary/cli/cliSetup.test.ts`, the child pseudoterminal in `runJourney` isolates `HOME` but omits `USERPROFILE`, leaking into the host user's actual profile directory on Windows. Windows ConPTY also emits ANSI cursor restore escape codes (`\x1b[?25h`) and clear-line sequences trailing the decision output, causing `runJourney`'s JSON parser (`.split(/\r?\n/u, 1)[0]`) to fail with: `SyntaxError: Unexpected non-whitespace character after JSON at position ...` ### Reproduction 1. In Node on Windows, run: node -e "const { invokingPackageRunner } = require('./dist/application/ClientRegistrationIdentity.js'); console.log(invokingPackageRunner({ NPM_COMMAND: 'run', npm_command: 'exec', NPM_CONFIG_USER_AGENT: 'npm/10.9.0' }, 'win32'))" 2. Observe that undefined is returned instead of "npx". 3. Run the focused setup boundary tests on Windows: npm run test:focused -- tests/boundary/cli/cliSetup.test.ts 4. Observe test failures due to ConPTY escape sequence JSON parsing errors and non-isolated USERPROFILE. ### Expected behavior 1. On Windows, `environmentValue` should resolve environment variables case-insensitively while preserving JavaScript property order precedence so later properties override earlier ones (`findLast`). 2. `tests/boundary/cli/cliSetup.test.ts` should isolate `USERPROFILE` alongside `HOME` and extract the JSON payload bounded by delimiter braces `{` and `}` to prevent trailing ConPTY ANSI control codes from corrupting the JSON parse. ### Impact and scope - Package runner detection on Windows can falsely fail when launched from scripts or package managers that populate uppercase ambient npm variables. - Windows developer workstations fail setup boundary tests and risk writing temporary configuration files into the host machine's `USERPROFILE`. ### REA version 6.5.0 ### Runtime and platform Node 24.18.0; Windows 11 x64 (ConPTY) ### Target and artifact context Windows CLI setup and test runner ### Interface CLI ### Earliest failure stage Setup or provider admission ### Relevant output or evidence Command or MCP request actually run: npm run test:focused -- tests/boundary/cli/cliSetup.test.ts Relevant result, error, or artifact: SyntaxError: Unexpected non-whitespace character after JSON at position 63 at JSON.parse (<anonymous>) at tests/boundary/cli/cliSetup.test.ts:484:37 And environment case collision returning undefined: invokingPackageRunner({ NPM_COMMAND: "run", npm_command: "exec", NPM_CONFIG_USER_AGENT: "npm/10.9.0" }, "win32") === undefined For source-only findings, link the file/revision and relevant code: - src/application/ClientRegistrationIdentity.ts: `environmentValue` uses `Object.keys(environment).find()` which matches ambient uppercase keys first on Windows. - tests/boundary/cli/cliSetup.test.ts: `runJourney` isolates `HOME` but omits `USERPROFILE`, leaking host environment. Missing evidence or remaining uncertainty: None. Root cause and regression test isolation confirmed on Windows 11. ### Typed outcome or recovery expected invokingPackageRunner should resolve "npx" (or "bunx") based on the caller's explicit environment entries, and runJourney should return a parsed JourneyResult without JSON parse errors. ### Analysis or suspected cause 1. In `src/application/ClientRegistrationIdentity.ts`, `environmentValue` uses `Object.keys(environment).find()`. On Windows, when ambient uppercase variables like `NPM_COMMAND="run"` exist alongside explicit `{ npm_command: "exec" }`, `find()` returns the earlier ambient key instead of the caller's override. Changing to `findLast()` with case-normalized target comparison preserves JavaScript property order precedence. 2. In `tests/boundary/cli/cliSetup.test.ts`, `homeDirectoryFromEnvironment` prioritizes `USERPROFILE` over `HOME` on Windows, so tests omitting `USERPROFILE` leak into the host user profile. Additionally, Windows ConPTY emits ANSI cursor visibility and clear codes after the decision marker, so extracting JSON between delimiter braces `{` and `}` avoids syntax errors. ### Additional context A fix and full regression tests have been prepared and verified on Windows 11 with Node 24.18.0. I am opening a PR with the fix and test coverage.

bsakaguchi

2026-10-16 21:04:38 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-16 21:04:39 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Delivered on main in fffb07419.

Integrated the contributor fix from #3639, then fixed the earlier environment snapshot boundary that still discarded overrides before lookup. Windows normalization now retains the last caller-selected alias, including explicit undefined; the regression follows normalization through runner selection and setup registration. PTY fixtures isolate USERPROFILE and tolerate ConPTY suffixes.

Validation: Snapshot/setup composition tests and interactive setup journeys. Hosted Windows qualification covers the curated native lane; the contributor reports Windows ConPTY journey validation. No new local Windows ConPTY run is claimed. Full exact-revision qualification passed.

@morluto commented on GitHub (Oct 14, 2026): Delivered on main in [fffb07419](https://github.com/morluto/rea/commit/fffb0741936067530d1ce9777461cd53e87398c2). Integrated the contributor fix from #3639, then fixed the earlier environment snapshot boundary that still discarded overrides before lookup. Windows normalization now retains the last caller-selected alias, including explicit undefined; the regression follows normalization through runner selection and setup registration. PTY fixtures isolate USERPROFILE and tolerate ConPTY suffixes. Validation: Snapshot/setup composition tests and interactive setup journeys. Hosted Windows qualification covers the curated native lane; the contributor reports Windows ConPTY journey validation. No new local Windows ConPTY run is claimed. Full exact-revision [qualification](https://github.com/morluto/rea/actions/runs/38629313022) passed.

bsakaguchi referenced this issue

2026-10-17 21:10:44 +00:00

[PR #1359] [MERGED] feat: add Frida instrumentation support #2825

bsakaguchi referenced this issue

2026-10-17 21:12:21 +00:00

[PR #1552] [MERGED] feat(adb): complete ADB device support for Android reverse engineering #2965