[PR #549] [MERGED] refactor(host): inject platform and environment instead of reading them ambiently #2066

New issue

Closed

opened 2026-10-16 21:13:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/549
Author: @morluto
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: cleanup/host-input-threading

📝 Commits (1)

59e42d5 refactor(host): inject platform and environment instead of reading them ambiently

📊 Changes

3 files changed (+90 additions, -7 deletions)

View changed files

📝 src/cliBrowserContext.ts (+14 -3)
➕ src/cliHostInputs.test.ts (+65 -0)
📝 src/hopper/HopperProvider.ts (+11 -4)

📄 Description

B-2 from the cleanup sweep: finish the pattern #505 established for the Windows batch probe.

Hopper hard-gates on the ambient platform

HopperProvider read process.platform in three places — the availability gate, the diagnostic it reports, and the Linux launch-mode selection. That made a real provider gate untestable off-platform: on a Linux CI machine there was no way to assert the Windows behaviour, so the only thing the suite could check was the branch the runner happened to be on. GhidraProvider already threads installationHost.platform ?? process.platform, so this is the same fix applied to the sibling that lacked it.

`browserContext` re-read `process.env` on every call

It is called from three CLI modules, and each call re-parsed the ambient environment. There is no second way to supply the environment, so configuration-driven behaviour could only be tested by mutating process.env for the duration of a test — global, order-dependent, and racy under a parallel runner.

environment now defaults to process.env, so every existing caller is unchanged, but a caller may pass one explicitly. That makes the helper's behaviour a function of its input rather than of where it happens to run, which is the interface the test surface needs.

Scope, stated honestly

This is the slice of B-2 that pays for itself, not the whole list. The sweep also found parseConfig(process.env) at 13 sites and evidence-stamping that reads process.platform in ~10 modules. Those are left for a follow-up deliberately: each needs its own call-site sweep and its own review, and bundling 20+ unrelated files into one change would make it unreviewable. browserContext was chosen as the representative because it has real call sites and a clean seam.

On the error projection

The first draft of the environment test asserted that a disabled browser policy surfaces the string "browser observation is disabled". It does not: projectAnalysisError redacts specifics into a category-level message. That is existing, intentional behaviour, so the test now asserts the outcome difference — same helper, supplied environment decides failure versus success — which is what actually proves the injection works.

Tests: cliHostInputs.test.ts covers Hopper availability for an injected unsupported platform, both supported platforms, and the environment-driven enable/disable difference. Full suite green, typecheck and lint clean.

_{🔄 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/549 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cleanup/host-input-threading` --- ### 📝 Commits (1) - [`59e42d5`](https://github.com/morluto/rea/commit/59e42d553729e5b655a247e86a33b2c9bd405358) refactor(host): inject platform and environment instead of reading them ambiently ### 📊 Changes **3 files changed** (+90 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/cliBrowserContext.ts` (+14 -3) ➕ `src/cliHostInputs.test.ts` (+65 -0) 📝 `src/hopper/HopperProvider.ts` (+11 -4) </details> ### 📄 Description B-2 from the cleanup sweep: finish the pattern #505 established for the Windows batch probe. ## Hopper hard-gates on the ambient platform `HopperProvider` read `process.platform` in three places — the availability gate, the diagnostic it reports, and the Linux launch-mode selection. That made a real provider gate untestable off-platform: on a Linux CI machine there was no way to assert the Windows behaviour, so the only thing the suite could check was the branch the runner happened to be on. `GhidraProvider` already threads `installationHost.platform ?? process.platform`, so this is the same fix applied to the sibling that lacked it. ## `browserContext` re-read `process.env` on every call It is called from three CLI modules, and each call re-parsed the *ambient* environment. There is no second way to supply the environment, so configuration-driven behaviour could only be tested by mutating `process.env` for the duration of a test — global, order-dependent, and racy under a parallel runner. `environment` now defaults to `process.env`, so every existing caller is unchanged, but a caller may pass one explicitly. That makes the helper's behaviour a function of its input rather than of where it happens to run, which is the interface the test surface needs. ## Scope, stated honestly This is the **slice** of B-2 that pays for itself, not the whole list. The sweep also found `parseConfig(process.env)` at 13 sites and evidence-stamping that reads `process.platform` in ~10 modules. Those are left for a follow-up deliberately: each needs its own call-site sweep and its own review, and bundling 20+ unrelated files into one change would make it unreviewable. `browserContext` was chosen as the representative because it has real call sites and a clean seam. ## On the error projection The first draft of the environment test asserted that a disabled browser policy surfaces the string "browser observation is disabled". It does not: `projectAnalysisError` redacts specifics into a category-level message. That is existing, intentional behaviour, so the test now asserts the outcome difference — same helper, supplied environment decides failure versus success — which is what actually proves the injection works. Tests: `cliHostInputs.test.ts` covers Hopper availability for an injected unsupported platform, both supported platforms, and the environment-driven enable/disable difference. Full suite green, typecheck and lint clean. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:53 +00:00