Stop the mock environment mutating the real process environment (#490)

CHANGELOG.md

@@ -10,6 +10,18 @@
   `pt-BR`, `pt-PT`, `ro`, `ru`, `sv`, `th`, `tr`, `uk`, `vi`, `zh-Hans` or
   `zh-Hant`, with `en-US` remaining the source and fallback locale
   ([#466](https://github.com/leynos/netsuke/issues/466))
+- Add [docs/v0-1-0-migration-guide.md](docs/v0-1-0-migration-guide.md)
+  signposting the child-environment API additions, and recording that every
+  Rust API surface outside the Netsukefile format and the graph export is
+  private in intent and unstable.
+- Export `runner::CommandEnv` together with the `runner::NinjaBuildRequest` and
+  `runner::NinjaToolRequest` bundles and the `runner::run_ninja_with` and
+  `runner::run_ninja_tool_with` entry points, so an embedder can set the
+  environment of the spawned Ninja process — `PATH` included — without
+  mutating its own. Overrides are additive and `CommandEnv::inherit()`
+  reproduces the existing behaviour, so `run_ninja` and `run_ninja_tool` keep
+  their signatures and no embedder needs to change
+  ([#490](https://github.com/leynos/netsuke/issues/490))
 
 ### Changed
 

docs/adr-002-replace-cucumber-with-rstest-bdd.md

@@ -99,7 +99,9 @@ for unit tests.
   `tests/bdd/fixtures.rs` (or similar) and is backed by `Arc<Mutex<...>>` when
   interior mutability is required.
 - Provide `#[rstest::fixture]` constructors for shared resources:
-  - Temporary workspace plus `PathGuard` handling for PATH edits.
+  - Temporary workspace plus explicit command-environment composition: the
+    child's `PATH` is composed as data and carried on the world rather than
+    edited on the parent process.
   - HTTP server guard that mirrors `start_http_server`/`shutdown_http_server`
     semantics.
   - CLI/manifest/ninja contexts seeded with sensible defaults.

docs/contents.md

@@ -54,6 +54,9 @@ operator, user, and contributor references are easier to find.
 
 - [quickstart.md](quickstart.md): First-run walkthrough for building with
   Netsuke.
+- [v0-1-0-migration-guide.md](v0-1-0-migration-guide.md): Migration notes for
+  the v0.1.0 child-environment API additions, and the stability caveat that
+  covers them.
 - [users-guide.md](users-guide.md): End-user reference for authoring and
   running Netsuke manifests, including executable discovery and
   `command_available` branch selection.

docs/developers-guide.md

@@ -2113,7 +2113,7 @@ Table: Scenario state groups and fields
 | Manifest state     | `manifest`, `manifest_error`                                                                                                                                                                                                             | Parsed manifest and error capture.                         |
 | IR state           | `build_graph`, `removed_action_id`, `generation_error`                                                                                                                                                                                   | Build graph, negative-test identifiers, generation errors. |
 | Ninja state        | `ninja_content`, `ninja_error`                                                                                                                                                                                                           | Generated Ninja file content and errors.                   |
-| Process state      | `run_status`, `run_error`, `command_stdout`, `command_stderr`, `temp_dir`, `workspace_path`                                                                                                                                              | Process results and temporary workspace paths.             |
+| Process state      | `run_status`, `run_error`, `command_stdout`, `command_stderr`, `temp_dir`, `workspace_path`, `command_env`                                                                                                                               | Process results, workspace paths, child environment.       |
 | Stdlib state       | `stdlib_root`, `stdlib_output`, `stdlib_error`, `stdlib_state`, `stdlib_command`, `stdlib_policy`, `stdlib_path_override`, `stdlib_fetch_max_bytes`, `stdlib_command_max_output_bytes`, `stdlib_command_stream_max_bytes`, `stdlib_text` | Stdlib rendering, network policy, and size constraints.    |
 | Localization state | `localization_lock`, `localization_guard`, `locale_config`, `locale_env`, `locale_cli_override`, `locale_system`, `resolved_locale`, `locale_message`                                                                                    | Scenario-level localizer overrides and resolution state.   |
 | HTTP server state  | `http_server`, `stdlib_url`                                                                                                                                                                                                              | Test HTTP server fixture for fetch scenarios.              |
@@ -2794,6 +2794,86 @@ constructed by `BuildTargets::new` and read through `as_slice`. It exposes no
 workspace, so it was removed; call `as_slice().is_empty()` where that
 question needs asking.
 
+### Module: `runner::process::command_env`
+
+`src/runner/process/command_env.rs` composes the environment applied to a
+spawned Ninja command as data, rather than by mutating the parent process.
+
+`CommandEnv` carries overrides as a list of key/value pairs:
+
+- `CommandEnv::inherit()` sets no overrides, which is production behaviour:
+  the child receives the parent's environment unchanged.
+- `with_var(key, value)` and the `with_path(path)` convenience it is built on
+  are last-write-wins per key, so composing an environment twice for the same
+  key cannot leave it carrying two values.
+- "The same key" follows the target's own rule, via the module-private
+  `env_names_eq`: exact on Unix, where `Path` and `PATH` are two different
+  variables, and ASCII case-insensitive on Windows, where they are one. Match
+  Unix's rule on Windows and a `CommandEnv` would hold two entries the child
+  collapses into one, with `std` rather than the last `with_var` call choosing
+  the survivor; match Windows's rule on Unix and naming `Path` would silently
+  rewrite `PATH`. Replacement keeps the casing first recorded, as the
+  platform's own environment block does.
+- `get(key)` reports only what this `CommandEnv` overrides, never the
+  parent's value, so `None` means "inherited", not "unset". It matches keys by
+  the same rule, so a lookup answers with the value the child would receive.
+- `Debug` is implemented by hand rather than derived, and prints only
+  `override_count` and `path_overridden`. Override names and values may hold
+  secrets, and a `CommandEnv` reaches a log by any route that formats a struct
+  containing one — not only through the runner's own logging — so the derived
+  form would defeat the redaction contract the span fields keep.
+- `apply` writes each override onto the `Command` with `Command::env`,
+  deliberately additive rather than `env_clear`: Ninja needs the ambient
+  environment to function, and clearing it would make a test environment
+  diverge from production in ways unrelated to what the test is pinning.
+
+The `ninja_subprocess` span and its spawn/exit events carry
+`env_override_count` and `path_overridden`, derived from the prepared
+`Command` rather than from `CommandEnv`, so an environment-caused failure is
+diagnosable from the logs alone. Both fields are bounded and carry no variable
+name or value: override names and values may hold secrets, and a count plus a
+`PATH` flag is the most that can be logged safely. The flag uses the same
+target-aware name comparison, so a Unix variable merely named `Path` does not
+raise it. Production runs use `CommandEnv::inherit()`, so they report `0` and
+`false`.
+
+`PATH` values are composed with `test_support::env::prepend_path_value`, a
+pure function that places a directory ahead of an explicitly supplied prior
+value. It takes the starting value rather than reading the process, so the
+result depends only on its inputs. An absent prior value yields just the new
+directory, and — by the helper's contract, which its tests pin — a wholly
+empty prior value is treated the same way; empty entries inside a non-empty
+value survive composition. It returns an error when an entry cannot be
+represented in a `PATH`, which `std::env::join_paths` itself reports: Unix
+rejects an entry containing `:` because entries cannot be quoted, whereas
+Windows can quote `;` and instead rejects the quoting character `"`.
+
+Nothing in this seam reads or writes the process `PATH`. The guarantee that
+an injected `PATH` cannot select Ninja itself holds only when
+`NinjaBuildRequest.program`/`NinjaToolRequest.program` is an absolute or
+otherwise resolved path: `program` is handed to `Command::new` as given, so a
+bare relative name such as `ninja` is looked up in the child's `PATH` on
+Unix, injected directories included. Callers that must not let the injected
+`PATH` select the executable therefore pass an absolute or otherwise resolved
+program path; when that isolation does not matter, a relative name resolving
+through the child `PATH` is acceptable. What the injected `PATH` always
+governs is the environment Ninja's own child commands see when it shells out.
+
+The explicit request APIs compose on top of `CommandEnv`: `NinjaBuildRequest`/
+`NinjaToolRequest` carry an `env: &CommandEnv` field alongside the program, CLI
+settings, and build file, and are consumed by `run_ninja_with`/
+`run_ninja_tool_with`. The convenience wrappers `run_ninja`/`run_ninja_tool`
+call these with `CommandEnv::inherit()`, reproducing production behaviour;
+tests reach for `run_ninja_with`/`run_ninja_tool_with` directly to supply a
+`CommandEnv` built with `with_path` instead. Section 6.1 of the
+[design document](netsuke-design.md) records the same architecture from the
+process-management side.
+
+Property coverage for this seam lives in `tests/env_path_property_tests.rs`,
+which Cargo builds as its own integration-test target; Proptest therefore
+persists its failing seeds to `env_path_property_tests.proptest-regressions`
+beside it. The named cases sit in `tests/env_path_tests.rs`.
+
 ## IR cycle detection
 
 ### Module: `ir::cycle`

docs/netsuke-design.md

@@ -2022,36 +2022,68 @@ securely.
 
 ### 6.1 Invoking Ninja
 
-Netsuke will use Rust's standard library `std::process::Command` API to
-configure and spawn the `ninja` process.[^24] This provides fine-grained
-control over the child process's execution environment.
-
-The command construction will follow this pattern:
-
-1. A new `Command` is created via `Command::new("ninja")`. Netsuke will assume
-   `ninja` is available in the system's `PATH`.
-
-2. Arguments passed to Netsuke's own CLI will be translated and forwarded to
-   Ninja. For example, a `Netsuke build my_target` command would result in
-   `Command::new("ninja").arg("my_target")`. Flags like `-j` for parallelism
-   will also be passed through.[^8]
-
-3. The working directory for the Ninja process will be set using
-   `.current_dir()`. When the user supplies a `-C` flag, Netsuke canonicalizes
-   the path and applies it via `current_dir` rather than forwarding the flag to
-   Ninja.
-
-4. Standard I/O streams (`stdin`, `stdout`, `stderr`) will be configured using
+Netsuke uses Rust's standard library `std::process::Command` API to configure
+and spawn the `ninja` process.[^24] This provides fine-grained control over the
+child process's execution environment.
+
+Every invocation is described by a borrowed request bundle rather than a long
+parameter list: `NinjaBuildRequest` for a build and `NinjaToolRequest` for
+`ninja -t <tool>`. Each names the resolved program, the parsed CLI settings,
+the generated build file, the targets or tool, and a `&CommandEnv` describing
+the child's environment. `run_ninja_with` and `run_ninja_tool_with` consume
+these; the convenience wrappers `run_ninja` and `run_ninja_tool` call them with
+`CommandEnv::inherit()`, which is production behaviour.
+
+The command construction follows this pattern:
+
+1. A new `Command` is created via `Command::new(request.program)`. The program
+   is *resolved before* the request is built — from the `ninja` default, from
+   the `NETSUKE_NINJA` override, or from a path an embedder supplies — and is
+   handed to `Command::new` exactly as given. A bare relative name such as
+   `ninja` is therefore looked up in the *child's* `PATH`, so a caller that
+   must not let an injected `PATH` choose the executable passes an absolute or
+   otherwise resolved path.
+
+2. Arguments passed to Netsuke's own CLI are translated and forwarded to Ninja.
+   For example, a `netsuke build my_target` command results in
+   `Command::new(program).arg("my_target")`. Flags like `-j` for parallelism
+   are also passed through.[^8]
+
+3. The working directory for the Ninja process is set using `.current_dir()`.
+   When the user supplies a `-C` flag, Netsuke canonicalizes the path and
+   applies it via `current_dir` rather than forwarding the flag to Ninja.
+
+4. The request's `CommandEnv` is applied. Overrides are **additive**: each pair
+   is written with `Command::env` and nothing is cleared, because Ninja needs
+   the ambient environment to function and `env_clear` would make a test
+   environment diverge from production in ways unrelated to what the test
+   pins. `CommandEnv::inherit()` sets nothing at all, so production spawns
+   exactly the environment Netsuke itself received. This is the only supported
+   way to vary a child's environment: Netsuke never mutates its own process
+   environment to influence a subprocess.
+
+5. Standard I/O streams (`stdin`, `stdout`, `stderr`) are configured using
    `.stdout(Stdio::piped())` and `.stderr(Stdio::piped())`.[^24] This allows
-   Netsuke to capture the real-time output from Ninja, which can then be
-   streamed to the user's console, potentially with additional formatting or
-   status updates from Netsuke itself.
-
-In the initial implementation a small helper wraps `Command::new` to forward the
-`-j` and `-C` flags and any explicit build targets. Standard output and error
-are piped and written back to Netsuke's own streams so users see Ninja's
-messages in order. A non-zero exit status or failure to spawn the process is
-reported as an `io::Error` for the CLI to surface.
+   Netsuke to capture the real-time output from Ninja, which is then streamed
+   to the user's console, potentially with additional formatting or status
+   updates from Netsuke itself.
+
+Standard output and error are piped and written back to Netsuke's own streams
+so users see Ninja's messages in order. A non-zero exit status or failure to
+spawn the process is reported as an `io::Error` for the CLI to surface.
+
+The `ninja_subprocess` span and its spawn and exit events carry
+`env_override_count` and `path_overridden`, derived from the prepared
+`Command`, so an environment-caused failure is diagnosable from the logs alone.
+Neither field names a variable or discloses a value, because overrides may
+carry secrets; `CommandEnv`'s own `Debug` output honours the same contract.
+Whether an override *is* `PATH` follows the target's naming rules — exact on
+Unix, where `Path` is a different variable, and case-insensitive on Windows,
+where it is not — and `CommandEnv` replaces same-key overrides by the same
+rule, so its view of the environment always matches the child's.
+
+The developers' guide documents the module layout and the `PATH` composition
+helper under "Module: `runner::process::command_env`".
 
 ### 6.2 The Criticality of Shell Escaping
 

docs/test-isolation-with-ninja-env.md

@@ -20,6 +20,15 @@ runner::run_with_ninja_program(&cli, output_prefs, &ninja_path)?;
 
 Keep the returned temporary directory alive until the runner finishes.
 
+To control the environment of the spawned Ninja process itself, pass a
+`CommandEnv` through `runner::run_ninja_with` or `runner::run_ninja_tool_with`.
+A `PATH` injected this way affects the commands Ninja launches, not which
+Ninja program runs — provided the program is an absolute or otherwise
+resolved path. Selection happens first, via `NETSUKE_NINJA` or an explicitly
+injected programme path, and the resolved program is passed to `Command`
+as given; a bare relative name such as `ninja` would still be looked up in
+the child's `PATH` on Unix, so callers pass resolved paths.
+
 ## End-to-end tests
 
 End-to-end tests may set `NETSUKE_NINJA` or `PATH` on the spawned command.