Make PATHEXT normalisation testable on any host (#485)

docs/developers-guide.md

@@ -2327,6 +2327,86 @@ platform-path form through `resolve_ninja_program`, which itself calls the
 UTF-8 resolver and converts its result, so no production path constructs a
 platform `PathBuf` independently of `resolve_ninja_program_utf8_with`.
 
+#### `which` environment capture
+
+`EnvSnapshot::capture` (`stdlib::which::env`) reads `PATH` on every
+platform, and `PATHEXT` on Windows only, through an injected
+`mockable::Env` provider rather than straight from the process:
+
+- `capture` is the production entry point. It delegates to `capture_with_env`
+  with `mockable::DefaultEnv`, so it is the single site that binds the
+  resolver's lookups to the live process environment.
+- `capture_with_env` takes `&impl mockable::Env`, so tests drive the whole
+  capture with a `MockEnv` without mutating process-global state.
+- An optional `path_override` parameter shadows `PATH` while leaving `PATHEXT`
+  to the provider. `capture_with_pathext` additionally shadows `PATHEXT`; it is
+  defined on every platform so the resolver has one capture entry point, and
+  the override is accepted and discarded off Windows, where nothing consults
+  the extension list.
+- `capture_common` owns the shared working-directory and `PATH` handling, so
+  the platform-specific `capture_impl` variants differ only in how they obtain
+  `PATHEXT`.
+
+Keep the ambient read at that boundary. Adding a `std::env` call elsewhere in
+`env.rs` would put it back where no test can reach it, and the module is where
+the clippy `disallowed-methods` gate would then fire.
+
+Both overrides reach the snapshot from configuration rather than from the
+process: `StdlibConfig::with_path_override` and
+`StdlibConfig::with_pathext_override` are copied into `WhichConfig`, which
+`WhichResolver::new` consumes whole — the resolver takes the configuration
+rather than its fields so a new environment seam does not lengthen the
+signature again. Pinning both is what lets a behavioural test drive `which`
+and `command_available` over a temporary directory with a chosen extension
+list; see `tests/stdlib_which_pathext_tests.rs`, which is gated to Windows
+because `PATHEXT` governs resolution only there.
+
+That gating has a cost worth stating: CI runs `make test` on `ubuntu-latest`
+only, so a `#[cfg(windows)]` test does not gate a merge. Keep host-independent
+rules — normalization, the fallback — in the `#[cfg(any(windows, test))]` unit
+tests that the Linux suite executes, and reserve the Windows-gated suite for
+behaviour that genuinely cannot run elsewhere.
+
+#### `PATHEXT` normalization
+
+`stdlib::which::env::parse_pathext` turns a raw `PATHEXT` value into lowercase,
+dot-prefixed extensions. It is pure string handling, consulted only by the
+Windows snapshot, and compiled under `#[cfg(any(windows, test))]`.
+
+Ownership and permitted call sites:
+
+- Owned by `stdlib::which::env` and `pub(super)`. The Windows
+  `EnvSnapshot::capture_impl` is its only production caller.
+- `DEFAULT_PATHEXT` is the single source of the built-in fallback and shares
+  the same gating.
+
+Composition rules:
+
+- Gate platform-only pure logic `#[cfg(any(windows, test))]` rather than
+  `#[cfg(windows)]`. The latter hides it from the CI host, so its rules go
+  unverified *and* unlinted. Compiling it unconditionally would instead leave
+  it dead in a Unix release build, which `-D warnings` rejects.
+- A value yielding no usable extension falls back to the built-in list. An
+  empty result would mean Windows treats nothing as executable, so `which`
+  would report every command missing.
+
+The full normalization contract, which the property tests in
+`src/stdlib/which/pathext_tests.rs` pin:
+
+- **Split on `;`.** That is the `PATHEXT` separator on Windows, and unlike
+  `PATH` it is not the platform path-list separator, so `split_paths` is the
+  wrong tool here.
+- **Trim whitespace** from each segment, then discard the segment if nothing
+  remains. `".COM; .EXE"` and `".COM;.EXE"` are the same list.
+- **Lowercase, then dot-prefix.** Comparison is case-insensitive, and a
+  segment written without its dot (`COM`) means the same extension as `.com`.
+- **First occurrence wins.** De-duplication is by the *normalized* form, so
+  `.EXE;.exe` yields one entry, positioned where the first appeared. Order is
+  significant: it is the order `which` tries extensions in.
+- **Fall back when nothing usable remains**, including for an absent value —
+  `parse_pathext(None)` and `parse_pathext(Some(";  ;"))` both yield
+  `DEFAULT_PATHEXT`.
+
 ### Configuration discovery module layout
 
 `src/cli/discovery.rs` attaches several small `#[path = "..."]` modules that

docs/netsuke-design.md

@@ -1244,15 +1244,36 @@ Semantics honour platform conventions while enforcing predictable behaviour:
 - Canonicalization happens after discovery and only when requested so that
   manifests can balance reproducibility against host-specific absolute paths.
 
+The resolver reads `PATH` and `PATHEXT` through an injected `mockable::Env`
+provider rather than straight from the process. `EnvSnapshot::capture` is the
+production entry point and binds `mockable::DefaultEnv`; `capture_with_env`
+takes the provider explicitly so tests drive a whole capture with a `MockEnv`;
+and `capture_with_pathext` additionally shadows `PATHEXT`. That last helper is
+defined on every platform — off Windows the override is accepted and discarded,
+because nothing there consults the extension list — so the resolver keeps a
+single capture entry point instead of forking on the target.
+
+Both overrides arrive as configuration rather than as ambient state.
+`StdlibConfig::with_path_override` and `StdlibConfig::with_pathext_override`
+are copied into `WhichConfig`, which `WhichResolver::new` consumes whole; the
+resolver takes the configuration rather than its individual fields so that
+adding a further environment seam does not lengthen the constructor again.
+Pinning both is what allows a behavioural test to drive `which` and
+`command_available` over a temporary directory with a chosen extension list
+without mutating the process environment.
+
 The resolver keeps a small LRU cache keyed by the command, a fingerprint of
-`PATH`/`PATHEXT`, the working directory, and the cache-relevant options (`all`,
-`canonical`, `cwd_mode`). Entries are validated once at insertion; cache reads
-no longer re-probe executability, keeping the hot path lean. Because `fresh`
-only controls bypass behaviour, it is stripped from the cache key so fresh
-lookups still repopulate the cache for subsequent calls. The fingerprint means
-environment changes invalidate keys without cloning large strings, and the
-helper remains pure because all inputs still derive from the manifest or
-process environment. Callers can request a bypass with `fresh=true` when they
+`PATH`/`PATHEXT`, the working directory, the captured `NETSUKE_WHICH_WORKSPACE`
+state, and the cache-relevant options (`all`, `canonical`, `cwd_mode`).
+Including the workspace switch keeps a fallback hit cached while the search was
+enabled from answering a resolution made with it disabled. Entries are
+validated once at insertion; cache reads no longer re-probe executability,
+keeping the hot path lean. Because `fresh` only controls bypass behaviour, it is
+stripped from the cache key so fresh lookups still repopulate the cache for
+subsequent calls. The fingerprint means environment changes invalidate keys
+without cloning large strings, and the helper remains pure because all inputs
+still derive from the manifest, the stdlib configuration, or the captured
+environment. Callers can request a bypass with `fresh=true` when they
 need to observe recent toolchain changes during a long session.
 
 Cache capacity defaults to 64 entries, covering typical PATH sizes without
@@ -1330,8 +1351,8 @@ sequenceDiagram
     participant "SearchWorkspace" as "search_workspace()"
 
     "Caller"->>"WhichResolver": "resolve(command, options)"
-    "WhichResolver"->>"EnvSnapshot": "capture(cwd_override)"
-    "EnvSnapshot"-->>"WhichResolver": "EnvSnapshot { cwd, raw_path }"
+    "WhichResolver"->>"EnvSnapshot": "capture_with_pathext(cwd_override, path_override, pathext_override)"
+    "EnvSnapshot"-->>"WhichResolver": "EnvSnapshot { cwd, raw_path, raw_pathext }"
     "WhichResolver"->>"Lookup": "lookup(env, command, options)"
     "Lookup"->>"Lookup": "search PATH directories for matches"
     alt "matches found"
@@ -1380,6 +1401,8 @@ classDiagram
         +workspace_root_path() -> OptionalPath
         +workspace_skip_dirs() -> StringList
         +which_cache_capacity() -> NonZeroUsize
+        +with_path_override(path: OsString) -> StdlibConfig
+        +with_pathext_override(pathext: OsString) -> StdlibConfig
     }
 
     class Environment {
@@ -1393,15 +1416,30 @@ classDiagram
     class WhichResolver {
         -cache: LruCache
         -cwd_override: OptionalPath
+        -path_override: OptionalOsString
+        -pathext_override: OptionalOsString
         -workspace_skips: WorkspaceSkipList
-        +new(cwd_override: OptionalPath, skips: WorkspaceSkipList, cache_capacity: NonZeroUsize) -> Result
+        +new(config: WhichConfig) -> WhichResolver
         +resolve(command: String, options: WhichOptions) -> Result
     }
 
     class EnvSnapshot {
         +cwd: Utf8PathBuf
         +raw_path: OptionalString
-        +capture(cwd_override: OptionalPath) -> Result
+        +raw_pathext: OptionalOsString
+        +capture(cwd: OptionalPath, path: OptionalOsStr) -> Result
+        +capture_with_env(cwd: OptionalPath, path: OptionalOsStr, env: Env) -> Result
+        +capture_with_pathext(cwd: OptionalPath, path: OptionalOsStr, pathext: OptionalOsStr) -> Result
+    }
+
+    class Env {
+        <<interface>>
+        +os_string(key: String) -> OptionalOsString
+        +raw(key: String) -> Result
+    }
+
+    class DefaultEnv {
+        +os_string(key: String) -> OptionalOsString
     }
 
     class WhichOptions {
@@ -1412,7 +1450,11 @@ classDiagram
     }
 
     class WhichConfig {
-        +new(cwd_override: OptionalPath, skips: WorkspaceSkipList, cache_capacity: NonZeroUsize) -> WhichConfig
+        +cwd_override: OptionalPath
+        +path_override: OptionalOsString
+        +pathext_override: OptionalOsString
+        +new(cwd: OptionalPath, path: OptionalOsString, skips: WorkspaceSkipList, capacity: NonZeroUsize) -> WhichConfig
+        +with_pathext_override(pathext: OptionalOsString) -> WhichConfig
     }
 
     class WorkspaceSkipList {
@@ -1428,9 +1470,12 @@ classDiagram
 
     Environment --> StdlibConfig : uses
     Environment --> WhichModule : calls register
+    StdlibConfig --> WhichConfig : copies PATH and PATHEXT overrides
     StdlibConfig --> WhichModule : provides workspace_root_path, skip dirs, cache capacity
-    WhichModule --> WhichResolver : constructs via new(cwd_override, skips, cache_capacity)
-    WhichResolver --> EnvSnapshot : calls capture(cwd_override)
+    WhichModule --> WhichResolver : constructs via new(config)
+    WhichResolver --> EnvSnapshot : calls capture_with_pathext(cwd, path, pathext)
+    EnvSnapshot --> Env : reads PATH and PATHEXT through the provider
+    DefaultEnv ..|> Env : production adapter bound by capture
     WhichResolver --> WhichOptions : reads lookup options
     WhichResolver --> WorkspaceSkipList : reads traversal filters
     WhichOptions --> CwdMode : uses cwd_mode
@@ -1465,14 +1510,17 @@ sequenceDiagram
     participant WhichResolver
     participant Cache
     participant EnvSnapshot
+    participant Env as "mockable::Env (DefaultEnv in production)"
     participant Lookup
     participant Workspace
 
     Caller->>WhichResolver: resolve(command, options)
     activate WhichResolver
 
-    WhichResolver->>EnvSnapshot: capture(cwd_override)
+    WhichResolver->>EnvSnapshot: capture_with_pathext(cwd_override, path_override, pathext_override)
     activate EnvSnapshot
+    EnvSnapshot->>Env: os_string("PATH"), os_string("PATHEXT")
+    Env-->>EnvSnapshot: values (overrides shadow the provider)
     EnvSnapshot-->>WhichResolver: env snapshot
     deactivate EnvSnapshot
 

docs/users-guide.md

@@ -406,6 +406,24 @@ defaults:
 `which(name, **kwargs)` returns an executable path and fails when the command
 is absent. The same helper is also available as a filter.
 
+On Windows, a name without an extension is matched against the effective
+`PATHEXT`, the same list the shell uses — so `which('cargo')` finds
+`cargo.exe` provided `.exe` is among those entries. A custom `PATHEXT` may
+legitimately omit it, in which case it is not a candidate.
+
+`PATHEXT` falls back to the built-in list only when it is unset or when no
+entry survives normalization — that is, every entry is empty or whitespace.
+Any other value is used as given, however unusual. The built-in list, in
+order:
+
+`.com`, `.exe`, `.bat`, `.cmd`, `.vbs`, `.vbe`, `.js`, `.jse`, `.wsf`,
+`.wsh`, `.msc`
+
+The fallback exists because an empty effective list would match nothing and
+report every command missing. Entries are matched case-insensitively and
+tried in the order the list gives them. A name that already carries an
+extension is used as written.
+
 `command_available(name, **kwargs)` returns a boolean and is better for
 complementary branches:
 

proptest-regressions/stdlib/which/pathext_tests.txt

@@ -0,0 +1,9 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 5575495465cd729dcdfb4a8097c53fc11ed0abcc6bd6cc9bb553ed4ffaf65149 # shrinks to stems = ["a"]
+cc 115b011e0b401ec9fd6308a474e89994205af1eccea6f5b567a3e6d999aa2fa4 # shrinks to raw = "A"
+cc 54be5d538c9ce0e4e4dc6596fc4ec236b4f66e7f9facf5bbd2b653493cca7137 # shrinks to raw = "EXE;exe"