[PR #554] [MERGED] refactor(config): take the environment as an input at every configuration read #2069

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/windows-scenario-paths-and-permission-remediation

📝 Commits (3)

72bd8e1 fix(platform): accept absolute paths for any host and stop blaming the ceiling for a missing grant
d10fbc1 refactor(config): take the environment as an input at every configuration read
b7a66e5 style(cli): apply prettier formatting to the runtime observation command

📊 Changes

23 files changed (+350 additions, -56 deletions)

View changed files

📝 docs/product-catalog.json (+2 -2)
📝 docs/verification/managed-conformance-manifest.json (+2 -2)
📝 skills/reverse-engineer-anything/SKILL.md (+1 -1)
📝 src/application/DirectAnalysis.ts (+9 -1)
📝 src/application/DirectAnalysisStatus.ts (+10 -5)
📝 src/application/PermissionAuthority.ts (+11 -3)
➕ src/application/PermissionAuthorityRemediation.test.ts (+99 -0)
📝 src/application/ProcessCli.ts (+5 -2)
📝 src/cli.ts (+12 -10)
📝 src/cli/managedCommands.ts (+4 -2)
📝 src/cli/utilityCommands.ts (+4 -2)
📝 src/cliApplicationCommands.ts (+14 -6)
📝 src/cliBrowserScenarioCommands.ts (+2 -1)
📝 src/cliElectronCommands.ts (+13 -6)
➕ src/cliEnvironmentSeam.test.ts (+38 -0)
📝 src/cliJavaScriptRuntimeCommands.ts (+10 -3)
📝 src/cliPolicyCommands.ts (+2 -1)
📝 src/doctorRuntime.ts (+8 -4)
📝 src/domain/analysisErrorPresentation.ts (+2 -0)
📝 src/domain/errors.ts (+13 -1)

...and 3 more files

📄 Description

Completes B-2. Every parseConfig(process.env) site now reads from an injected environment.

The twelve sites were not equivalent

Treating them as one number would have hidden the only three that were actual defects.

src/application/ (3 sites) were real bugs. runSessionStatus already injected its logger as a default parameter but hardcoded process.env for configuration — so the MCP path's dependencies.env never reached it, and the injection was half-done inside a single function. captureProcessScenarioFile and runAnalysis had the same shape. runAnalysis now takes environment on the options object it already had, alongside logger, signal, permissionAuthority and providerId, because that is where its injected dependencies live.

The CLI command layer (9 sites) is a documented boundary, so process.env there is legitimate per the guidelines. What was not legitimate was nine independent ambient reads with no way to substitute one. They now share a single seam: createCli(environment) passes it to the seven registrars that need configuration, and the nested helpers (electronContext, runtimeContext, loadConfiguredAuthority, inspectJavaScriptReplay) take it explicitly.

Every parameter defaults to `process.env`

Production behaviour is byte-identical and no existing call site changed. The difference is that the ambient read is now a documented default at a boundary rather than a hidden dependency buried in the middle of behaviour. process.env survives in src/ only in those default positions — nothing reads it at the point of use, which is what makes the remaining ones safe to change in one place.

Tests

cliEnvironmentSeam.test.ts covers the behaviour that matters rather than the shape of the change: the CLI and doctor host build from a supplied environment, still build with none, a process-scenario failure is produced from a supplied environment, and an invalid configuration surfaces through the caller's environment rather than the ambient one.

I deliberately did not add a test that scans sources for process.env. It would pass, but it locks an implementation detail rather than a behaviour, and would fail on any legitimate refactor — the kind of test the guidelines warn against.

Full suite green (1994 passed); typecheck, lint and prettier 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/554 **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:** `fix/windows-scenario-paths-and-permission-remediation` --- ### 📝 Commits (3) - [`72bd8e1`](https://github.com/morluto/rea/commit/72bd8e109c27e81ed409368cda8bfdcd6bf561b1) fix(platform): accept absolute paths for any host and stop blaming the ceiling for a missing grant - [`d10fbc1`](https://github.com/morluto/rea/commit/d10fbc1e529b3d74e28b37554206b0c6e8ea15e4) refactor(config): take the environment as an input at every configuration read - [`b7a66e5`](https://github.com/morluto/rea/commit/b7a66e50caf35ffd62c607c14adadf5d8bb29b95) style(cli): apply prettier formatting to the runtime observation command ### 📊 Changes **23 files changed** (+350 additions, -56 deletions) <details> <summary>View changed files</summary> 📝 `docs/product-catalog.json` (+2 -2) 📝 `docs/verification/managed-conformance-manifest.json` (+2 -2) 📝 `skills/reverse-engineer-anything/SKILL.md` (+1 -1) 📝 `src/application/DirectAnalysis.ts` (+9 -1) 📝 `src/application/DirectAnalysisStatus.ts` (+10 -5) 📝 `src/application/PermissionAuthority.ts` (+11 -3) ➕ `src/application/PermissionAuthorityRemediation.test.ts` (+99 -0) 📝 `src/application/ProcessCli.ts` (+5 -2) 📝 `src/cli.ts` (+12 -10) 📝 `src/cli/managedCommands.ts` (+4 -2) 📝 `src/cli/utilityCommands.ts` (+4 -2) 📝 `src/cliApplicationCommands.ts` (+14 -6) 📝 `src/cliBrowserScenarioCommands.ts` (+2 -1) 📝 `src/cliElectronCommands.ts` (+13 -6) ➕ `src/cliEnvironmentSeam.test.ts` (+38 -0) 📝 `src/cliJavaScriptRuntimeCommands.ts` (+10 -3) 📝 `src/cliPolicyCommands.ts` (+2 -1) 📝 `src/doctorRuntime.ts` (+8 -4) 📝 `src/domain/analysisErrorPresentation.ts` (+2 -0) 📝 `src/domain/errors.ts` (+13 -1) _...and 3 more files_ </details> ### 📄 Description Completes B-2. Every `parseConfig(process.env)` site now reads from an injected environment. ## The twelve sites were not equivalent Treating them as one number would have hidden the only three that were actual defects. **`src/application/` (3 sites) were real bugs.** `runSessionStatus` already injected its `logger` as a default parameter but hardcoded `process.env` for configuration — so the MCP path's `dependencies.env` never reached it, and the injection was half-done inside a single function. `captureProcessScenarioFile` and `runAnalysis` had the same shape. `runAnalysis` now takes `environment` on the options object it already had, alongside `logger`, `signal`, `permissionAuthority` and `providerId`, because that is where its injected dependencies live. **The CLI command layer (9 sites) is a documented boundary,** so `process.env` there is legitimate per the guidelines. What was *not* legitimate was nine independent ambient reads with no way to substitute one. They now share a single seam: `createCli(environment)` passes it to the seven registrars that need configuration, and the nested helpers (`electronContext`, `runtimeContext`, `loadConfiguredAuthority`, `inspectJavaScriptReplay`) take it explicitly. ## Every parameter defaults to `process.env` Production behaviour is byte-identical and no existing call site changed. The difference is that the ambient read is now a *documented default at a boundary* rather than a hidden dependency buried in the middle of behaviour. `process.env` survives in `src/` only in those default positions — nothing reads it at the point of use, which is what makes the remaining ones safe to change in one place. ## Tests `cliEnvironmentSeam.test.ts` covers the behaviour that matters rather than the shape of the change: the CLI and doctor host build from a supplied environment, still build with none, a process-scenario failure is produced from a supplied environment, and an invalid configuration surfaces through the caller's environment rather than the ambient one. I deliberately did **not** add a test that scans sources for `process.env`. It would pass, but it locks an implementation detail rather than a behaviour, and would fail on any legitimate refactor — the kind of test the guidelines warn against. Full suite green (1994 passed); typecheck, lint and prettier 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:54 +00:00