Make PATHEXT normalisation testable on any host (#485) by leynos · Pull Request #503 · leynos/netsuke

leynos · 2026-08-01T19:53:24Z

Summary

parse_pathext and its DEFAULT_PATHEXT fallback sat behind #[cfg(windows)]. Every rule they implement — lowercasing, inserting missing leading dots, trimming, de-duplicating, preserving declaration order, and falling back to the built-in list when the value yields nothing — went unverified on the platform where the suite actually runs.

Clippy never linted the function either. On first exposure it produced two findings (map(..).unwrap_or_else(..) and a redundant closure), both fixed here — which is a fair indication of what else was going unchecked.

Approach

Ungate parse_pathext, DEFAULT_PATHEXT, and the IndexSet import behind #[cfg(any(windows, test))], matching the pattern used in #486. Runtime behaviour is unchanged: only the Windows snapshot consults them, and on a non-test Unix build they are still compiled out.

Coverage

Ten cases, none mutating anything:

unset PATHEXT yields the default list
valueless input falls back to the default — "", ";;;", and " ; ; ". This is the one that matters: were a blank PATHEXT to yield an empty extension list, Windows would treat nothing as executable and which would report every command missing
extensions are lowercased; missing leading dots inserted; surrounding whitespace trimmed
duplicates collapse case-insensitively and after dot insertion, so COM, .com, and .COM become one entry
declaration order is preserved, so author-declared precedence survives
entries normalising to nothing are skipped

Removed

The two #[cfg(windows)] tests driving this through VarGuard::set("PATHEXT", ..). They mutated the process environment, which the AGENTS.md mandate forbids; they never ran on the Unix CI host; and their coverage is subsumed by the direct cases. No VarGuard use remains under src/.

Deferred

The wider EnvSnapshot::capture override plumbing. capture already takes a path_override used by a dozen call sites; adding a third positional Option<&OsStr> is churn better done alongside the broader which seam work, and would draw a CodeScene argument-count finding for no present benefit. The untested normalisation was the substantive gap, and it is closed.

Verification

All gates pass: check-fmt, lint, typecheck, test (1198 nextest), markdownlint, nixie. CodeScene delta: no issues.

Closes #485.
Refs #496.

🤖 Generated with Claude Code

Summary by Sourcery

Make PATHEXT normalisation logic testable on non-Windows hosts while keeping runtime behaviour unchanged.

New Features:

Add a dedicated PATHEXT normalisation test module that exercises parse_pathext directly on all test hosts.

Bug Fixes:

Ensure valueless PATHEXT inputs fall back to the default executable extension list instead of yielding an empty set.

Enhancements:

Expose parse_pathext and DEFAULT_PATHEXT under cfg(any(windows, test)) so their behaviour is compiled and linted in test builds.
Simplify parse_pathext’s fallback handling and deduplication implementation without changing its observable behaviour.

Tests:

Replace Windows-only EnvSnapshot-based PATHEXT tests with host-agnostic unit tests that avoid mutating process environment.
Remove remaining VarGuard-based PATHEXT tests that were gated behind cfg(windows) and not exercised on Unix CI.

coderabbitai · 2026-08-01T19:53:33Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0d579ce4-a2a5-4dbd-955b-b191a8de4853

📥 Commits

Reviewing files that changed from the base of the PR and between 525a0b6 and ab3db61.

📒 Files selected for processing (6)

docs/developers-guide.md
docs/users-guide.md
proptest-regressions/stdlib/which/pathext_tests.txt
src/stdlib/which/env.rs
src/stdlib/which/mod.rs
src/stdlib/which/pathext_tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/whitaker (auto-detected)
leynos/shared-actions (auto-detected)

Summary

Compile PATHEXT parsing on Windows and during tests.
Add Unix-runnable unit and property-based tests for fallback behaviour, normalization, filtering, ordering, deduplication, idempotence, and invariants.
Remove environment-mutating PATHEXT tests.
Fix Clippy findings in parse_pathext.
Document the PATHEXT contract, fallback order, normalization rules, and test-only behaviour.
Add Proptest regression seeds.

Scope

Keep the existing environment-reader seam from issue #485 unchanged.
Test PATHEXT parsing directly without mutating the process environment.

Walkthrough

Enable direct Unix-runnable PATHEXT parser tests, add regression seeds, and document environment capture, parsing rules, fallback behaviour, and Windows executable resolution.

Changes

PATHEXT contract

Layer / File(s)	Summary
Enable and validate PATHEXT parsing `src/stdlib/which/env.rs`, `src/stdlib/which/mod.rs`, `src/stdlib/which/pathext_tests.rs`, `proptest-regressions/...`	Compile the parser during tests. Validate fallback, normalisation, filtering, ordering, de-duplication, idempotence, and saved regression cases.
Document environment and extension behaviour `docs/developers-guide.md`, `docs/users-guide.md`	Document injected environment capture, parser ownership, PATHEXT rules, and Windows extension resolution.

Possibly related PRs

leynos/netsuke#478: Both changes modify src/stdlib/which and related executable path fixtures.
leynos/netsuke#501: Both changes use the injectable environment-reader design.
leynos/netsuke#530: This change extends related src/stdlib/which/env.rs work with PATHEXT parsing tests and documentation.

Suggested labels: Issue

Suggested reviewers: codescene-access

Poem

Trim each extension.
Restore defaults when needed.
Test order and casing.
Document the Windows path.
Let which resolve cleanly.

🚥 Pre-merge checks | ✅ 19 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Testing (Compile-Time / Ui)	⚠️ Warning	The PR adds compile-time cfg behaviour (`#[cfg(any(windows, test))]`) but adds no trybuild or equivalent compile-time test; the new tests only exercise runtime parsing.	Add a trybuild or equivalent UI suite that verifies the relevant Windows/test and non-test Unix compilation paths, then retain the focused parser tests.

✅ Passed checks (19 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes host-independent PATHEXT normalisation testing and includes the linked issue reference (`#485`).
Description check	✅ Passed	The description clearly explains the PATHEXT testing changes, fallback fix, scope, verification, and documentation updates.
Linked Issues check	✅ Passed	The PR delivers the amended `#485` scope: host-independent PATHEXT coverage, normalisation fixes, fallback tests, and documentation.
Out of Scope Changes check	✅ Passed	The code, tests, regression seeds, and documentation remain within the amended `#485` scope.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall)	✅ Passed	Accept this check: direct tests assert fallback, normalisation, trimming, dot insertion, case-insensitive de-duplication, ordering, and unusable-entry filtering; property tests cover invariants and...
User-Facing Documentation	✅ Passed	Documented the Windows `PATHEXT` behaviour in `docs/users-guide.md`, including fallback rules, ordered defaults, matching, and explicit-extension handling.
Developer Documentation	✅ Passed	docs/developers-guide.md documents the EnvSnapshot seam, permitted callers, cfg gating, PATHEXT contract, fallback rules, and tests; no pending roadmap or locale documentation was found.
Module-Level Documentation	✅ Passed	The changed Rust modules have module-level `//!` documentation: `env`, `which`, `pathext_tests`, and its `properties` submodule state purpose, use, and component relationships.
Testing (Unit And Behavioural)	✅ Passed	Cover None, empty and unusable values, normalisation, trimming, deduplication, order, and invariants with fixed and property tests; existing Windows lookup tests cover PATHEXT resolution.
Testing (Property / Proof)	✅ Passed	Accept the check: the #[cfg(test)] suite uses proptest with bounded generated inputs and substantive properties for normalization, uniqueness, fallback, idempotence, and order.
Unit Architecture	✅ Passed	Keep the pure PATHEXT parser separate: tests call parse_pathext directly, while EnvSnapshot reads through the injected Env boundary and returns explicit Result errors.
Domain Architecture	✅ Passed	Keep the boundary: `parse_pathext` is pure normalisation in `which::env`, while environment access uses injected `Env`; the PR adds no domain-model or transport/storage coupling.
Observability	✅ Passed	The commit adds tests/docs and cfg/refactoring only; PATHEXT fallback logic was already present, and existing which tracing plus bounded outcome metrics remain in place.
Security And Privacy	✅ Passed	Pass the security and privacy check: the patch adds no credentials, sensitive data, execution, deserialization, permission, or authentication logic; runtime PATHEXT behaviour is otherwise unchanged.
Performance And Resource Use	✅ Passed	The parser keeps one linear split loop and bounded per-entry allocation; the diff preserves its algorithm, and property tests cap generated segments at 8 or 6 entries.
Concurrency And State	✅ Passed	Accept this check: keep PATHEXT parsing local and pure; the patch adds no shared mutable or async state, and tests verify ordering without environment mutation.
Architectural Complexity And Maintainability	✅ Passed	The patch adds only a local PATHEXT helper and test module; it reuses existing IndexSet, rstest and proptest, adds no dependencies, traits, layers, global state or parallel abstraction.
Rust Compiler Lint Integrity	✅ Passed	The Rust diff adds no lint suppressions or clone calls; new helpers are narrowly cfg-gated and directly referenced by Windows code and the #[cfg(test)] module.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch issue-485-inject-env-seam-into-which-pathext

_{Comment @coderabbitai help to get the list of available commands.}

sourcery-ai · 2026-08-01T19:53:43Z

Reviewer's Guide

Make PATHEXT normalization logic testable on all hosts by ungating the parser/default list behind cfg(any(windows, test)), refining its implementation, and replacing Windows-only EnvSnapshot-based tests with direct, host-independent unit tests that cover normalization, de-duplication, ordering, and fallback behavior.

Sequence diagram for PATHEXT tests using parse_pathext on all hosts

sequenceDiagram
  actor TestRunner
  participant pathext_tests
  participant parse_pathext
  participant DEFAULT_PATHEXT

  TestRunner->>pathext_tests: run
  pathext_tests->>parse_pathext: parse_pathext(raw)
  alt raw is None
    parse_pathext->>DEFAULT_PATHEXT: join segments with semicolons for source
  else raw is Some
    parse_pathext->>parse_pathext: value.to_string_lossy().into_owned()
  end
  parse_pathext->>parse_pathext: normalise segments and deduplicate
  alt dedup is empty
    parse_pathext->>DEFAULT_PATHEXT: iter().copied().map(String::from).collect()
  end
  parse_pathext-->>pathext_tests: Vec<String> extensions
  pathext_tests-->>TestRunner: assertions pass

Flow diagram for PATHEXT normalisation in parse_pathext

flowchart TD
  A["raw Option<&OsStr>"] --> B{raw is Some}
  B -- Yes --> C["source = raw.to_string_lossy().into_owned()"]
  B -- No --> D["source = DEFAULT_PATHEXT.join(';')"]
  C --> E["split source by ';'"]
  D --> E
  E --> F["trim segment; skip if empty"]
  F --> G["ensure leading '.'; lowercase"]
  G --> H["insert into IndexSet dedup"]
  H --> I{dedup is empty}
  I -- Yes --> J["return DEFAULT_PATHEXT.iter().copied().map(String::from).collect()"]
  I -- No --> K["return dedup.into_iter().collect()"]

File-Level Changes

Change	Details	Files
Make PATHEXT parsing and default extension list available under cfg(any(windows, test)) and slightly refactor the parser implementation.	Ungate IndexSet import, DEFAULT_PATHEXT constant, and parse_pathext function from cfg(windows) to cfg(any(windows, test)) so they compile on test builds regardless of host. Document DEFAULT_PATHEXT and parse_pathext with comments explaining Windows executable extension behavior and why the logic is ungated for tests. Refactor parse_pathext to use map_or_else for Option handling, clarifying the fallback to DEFAULT_PATHEXT when raw is None. Change the empty-dedup fallback to build a Vec using DEFAULT_PATHEXT.iter().copied().map(String::from) for slightly more idiomatic string conversion.	`src/stdlib/which/env.rs`
Replace Windows-only EnvSnapshot/VarGuard-based PATHEXT tests with direct unit tests that drive parse_pathext and run on all hosts.	Remove two cfg(windows) tests that mutate PATHEXT via VarGuard and assert behavior via EnvSnapshot::pathext, eliminating VarGuard usage from src/. Introduce a new test module pathext_tests.rs that imports DEFAULT_PATHEXT and parse_pathext and asserts core normalization rules: unset PATHEXT uses default list, valueless inputs fall back, case normalization, dot insertion, whitespace trimming, de-duplication, order preservation, and skipping empty-normalized entries. Wire the new pathext_tests module into which::mod via a cfg(test) submodule so tests run under the main which crate test harness.	`src/stdlib/which/lookup/tests.rs` `src/stdlib/which/mod.rs` `src/stdlib/which/pathext_tests.rs`

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#485	Inject an Env seam into EnvSnapshot::capture so that PATH and PATHEXT are read via &impl mockable::Env instead of ambient std::env::var_os calls.	❌	The PR explicitly defers the wider EnvSnapshot::capture override plumbing. It does not change capture’s signature to accept &impl mockable::Env, nor does it reroute PATH/PATHEXT reads through a mockable environment interface.
#485	Eliminate direct std::env::var_os usage in src/stdlib/which/env.rs and remove EnvLock/PathGuard usage from the which-related test suites listed in the issue.	❌	While the PR removes VarGuard-based PATHEXT tests and notes that no VarGuard use remains under src/, it does not modify src/stdlib/which/env.rs to remove std::env::var_os calls for PATH/PATHEXT, nor does it address EnvLock/PathGuard usage across all the specified test suites.
#485	Make PATHEXT parsing behavior directly testable (including Windows normalization rules) from the Unix CI host with dedicated tests.	✅

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

Addresses two review findings on #503. Sourcery: the doc comments claimed the items were "deliberately free of `cfg` gating" while carrying `#[cfg(any(windows, test))]`. The wording now matches the attribute and records why that pair is the right one: `#[cfg(windows)]` alone hides the logic from the CI host, while no gate at all leaves it dead in a Unix release build, which `-D warnings` rejects. Sourcery also observed that asserting the parse equals `DEFAULT_PATHEXT` is tautological — emptying or mangling the constant would keep that assertion true. Their suggested replacement hard-codes a four-entry list, which does not match the eleven-entry constant, so rather than inline a duplicate the new test asserts the properties consumers rely on: non-empty, every entry lowercase and dot-prefixed, and the four extensions callers actually expect. Adds the usage example and the developers' guide entry required by AGENTS.md. Refs #485, #496. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

buzzybee-df12 · 2026-08-02T11:27:10Z

@coderabbitai review

coderabbitai · 2026-08-02T11:27:15Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/stdlib/which/env.rs`:
- Line 132: Update the documentation comment for the PATHEXT normalization logic
to use “Normalize” instead of “Normalise”; change prose only and leave the
existing “normalised” identifier unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9910bae3-2ef9-4dcd-a0a2-4de2823a9b83

📥 Commits

Reviewing files that changed from the base of the PR and between 3f84545 and 8502694.

📒 Files selected for processing (10)

docs/adr-006-adopt-polonius-nightly-toolchain.md
docs/developers-guide.md
docs/netsuke-design.md
docs/polonius.md
docs/snapshot-testing-in-netsuke-using-insta.md
docs/users-guide.md
src/stdlib/which/env.rs
src/stdlib/which/lookup/tests.rs
src/stdlib/which/mod.rs
src/stdlib/which/pathext_tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/shared-actions (auto-detected)

💤 Files with no reviewable changes (1)

src/stdlib/which/lookup/tests.rs

Addresses two review findings on #503. Sourcery: the doc comments claimed the items were "deliberately free of `cfg` gating" while carrying `#[cfg(any(windows, test))]`. The wording now matches the attribute and records why that pair is the right one: `#[cfg(windows)]` alone hides the logic from the CI host, while no gate at all leaves it dead in a Unix release build, which `-D warnings` rejects. Sourcery also observed that asserting the parse equals `DEFAULT_PATHEXT` is tautological — emptying or mangling the constant would keep that assertion true. Their suggested replacement hard-codes a four-entry list, which does not match the eleven-entry constant, so rather than inline a duplicate the new test asserts the properties consumers rely on: non-empty, every entry lowercase and dot-prefixed, and the four extensions callers actually expect. Adds the usage example and the developers' guide entry required by AGENTS.md. Refs #485, #496. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

buzzybee-df12 · 2026-08-04T04:05:01Z

@coderabbitai review

coderabbitai · 2026-08-04T04:05:07Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

The `normalised` local identifier is left alone: the repository convention governs prose, and renaming a binding is not a spelling fix. Addresses a CodeRabbit finding on #503.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/users-guide.md`:
- Around line 400-406: Update the Windows PATHEXT paragraph in the users guide
to avoid implying that every effective PATHEXT includes .exe: state that
cargo.exe is found with the default list, and refer to the canonical
DEFAULT_PATHEXT definition instead of “usual script suffixes.”

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5da0d109-2f70-483c-99f3-d1ea73372433

📥 Commits

Reviewing files that changed from the base of the PR and between f230114 and 39a5308.

📒 Files selected for processing (8)

docs/developers-guide.md
docs/users-guide.md
proptest-regressions/stdlib/which/pathext_tests.txt
src/stdlib/which/cache.rs
src/stdlib/which/env.rs
src/stdlib/which/lookup/tests.rs
src/stdlib/which/mod.rs
src/stdlib/which/pathext_tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/whitaker (auto-detected)
leynos/shared-actions (auto-detected)

The note claimed `which('cargo')` finds `cargo.exe`, which holds only when `.exe` is among the effective entries — a custom `PATHEXT` may legitimately omit it. It also said "the usual script suffixes", which tells a reader nothing they can check. Both are now exact: the example is qualified, and the fallback list is spelled out in order, since order is what `which` tries. Addresses a CodeRabbit finding on #503.

pandalump · 2026-08-05T13:36:03Z

@coderabbitai review

coderabbitai · 2026-08-05T13:36:11Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/users-guide.md`:
- Around line 405-406: Update the PATHEXT fallback wording in the documentation
to state that fallback occurs only when PATHEXT is unset or contains exclusively
empty or whitespace entries; do not describe other non-empty values as unusable.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4595d654-906d-488c-bd33-8c8ca61c18d8

📥 Commits

Reviewing files that changed from the base of the PR and between 39a5308 and 6a2c26c.

📒 Files selected for processing (1)

docs/users-guide.md

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/whitaker (auto-detected)
leynos/shared-actions (auto-detected)

buzzybee-df12 · 2026-08-05T22:49:09Z

@coderabbitai review

coderabbitai · 2026-08-05T22:49:14Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/users-guide.md`:
- Around line 405-407: Update the PATHEXT behavior description near
parse_pathext to state that non-empty values are normalized by trimming entries,
ignoring empties, lowercasing, adding a leading dot, and de-duplicating while
preserving order, before use; retain the existing fallback description for unset
or entirely empty values.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 42a79131-5592-4a46-863b-58e3a95c4505

📥 Commits

Reviewing files that changed from the base of the PR and between 39a5308 and d38be7d.

📒 Files selected for processing (1)

docs/users-guide.md

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/whitaker (auto-detected)
leynos/shared-actions (auto-detected)

Rebuild PR #503's branch on origin/main, which absorbed the injectable environment capture this branch pioneered — EnvSnapshot::capture now delegates to capture_with_env over mockable::Env, with a Windows capture_with_pathext override — so the old EnvReader closure seam, the WhichResolver reader plumbing, process_env_reader, and the reader-based rewrites of cache.rs and lookup/tests.rs are dropped as absorbed. What remains novel, and is ported: - Widen parse_pathext and DEFAULT_PATHEXT from #[cfg(windows)] to #[cfg(any(windows, test))], expose parse_pathext pub(super), and give the fallback a single construction site, so PATHEXT normalization is compiled and lintable on the Unix CI host instead of only on Windows. - Port src/stdlib/which/pathext_tests.rs: fixed rstest cases for the normalization rules plus the properties proptest module (normalized entries, case-insensitive uniqueness, idempotence, fallback, and first-occurrence order) with its deliberately small mixed-case alphabet, together with the proptest-regressions seed file. - Document the effective PATHEXT matching, the built-in fallback list in order, and the exact fallback condition in docs/users-guide.md. - Document the which environment capture seam as it now exists (the mockable::Env provider, DefaultEnv binding, and PATHEXT override) and the full parse_pathext normalization contract in docs/developers-guide.md, next to the environment lookup seam material. Main's capture_with_pathext lookup tests are kept: they cover the Windows snapshot path, while the ported suite verifies the parser rules from the host that gates merges. Closes #485. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

codescene-access

No quality gates enabled for this code.

pandalump · 2026-08-06T08:18:24Z

@coderabbitai review

coderabbitai · 2026-08-06T08:18:30Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-08-06T08:18:34Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai

♻️ Duplicate comments (1)

docs/users-guide.md (1)
406-412: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the normalization statement.

Replace “used as given”. parse_pathext trims entries, ignores empty entries,
lowercases ASCII text, inserts missing leading dots, and de-duplicates entries
without changing first-occurrence order.

Triage: [type:docstyle]
Proposed correction
-Any other value is used as given, however unusual. The built-in list, in
-order:
+Any other value is normalized before use. Entries are trimmed, empty entries
+are ignored, ASCII letters are lowercased, missing leading dots are added, and
+duplicates are removed while first-occurrence order remains. The built-in list,
+in order:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/users-guide.md` around lines 406 - 412, Update the PATHEXT documentation
around parse_pathext to replace “used as given” with the actual normalization
behavior: trim entries, discard empty values, lowercase ASCII text, add missing
leading dots, and de-duplicate while preserving first-occurrence order. Keep the
fallback description and built-in list unchanged.
Sources: Coding guidelines, Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@docs/users-guide.md`:
- Around line 406-412: Update the PATHEXT documentation around parse_pathext to
replace “used as given” with the actual normalization behavior: trim entries,
discard empty values, lowercase ASCII text, add missing leading dots, and
de-duplicate while preserving first-occurrence order. Keep the fallback
description and built-in list unchanged.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2b8dae92-158a-4850-901b-d1ee2beca457

📥 Commits

Reviewing files that changed from the base of the PR and between 8ac4103 and 056a52f.

📒 Files selected for processing (6)

docs/developers-guide.md
docs/users-guide.md
proptest-regressions/stdlib/which/pathext_tests.txt
src/stdlib/which/env.rs
src/stdlib/which/mod.rs
src/stdlib/which/pathext_tests.rs

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

leynos/rstest-bdd (auto-detected)
leynos/ortho-config (auto-detected)
leynos/whitaker (auto-detected)
leynos/shared-actions (auto-detected)

leynos · 2026-08-06T08:44:03Z

This round's two warnings, verified against the tree:

Linked Issues — the injected seam Inject an Env seam into which PATH and PATHEXT capture #485 required now exists on main: EnvSnapshot::capture_with_env takes &impl mockable::Env (plus capture_with_pathext for the Windows override), DefaultEnv is the production adapter, and both PATH and PATHEXT read through the provider — it arrived through the parallel migration rather than this PR, which is why this branch no longer carries it. Inject an Env seam into which PATH and PATHEXT capture #485's body now records that amendment; this PR delivers the remaining scope (the Unix-runnable normalization coverage and documentation).
Testing (Compile-Time / Ui) — declined as disproportionate, per the precedent set on Stop the mock environment mutating the real process environment (#490) #497: the #[cfg(any(windows, test))] widening is enforced by the workspace build itself (cargo check --all-targets compiles both the gated-in and gated-out views across the suite), and a trybuild/UI case asserting cfg outcomes would couple the suite to unstable diagnostic wording while proving nothing the gates do not already prove. trybuild additionally cannot compile this crate while the tree is Polonius-only (docs/polonius.md).

leynos · 2026-08-06T10:03:33Z

On the three failed pre-merge rows:

Linked Issues (Inject an Env seam into which PATH and PATHEXT capture #485): the injected environment seam that issue Inject an Env seam into which PATH and PATHEXT capture #485 originally demanded landed on main via the parallel migration (EnvSnapshot::capture_with_env and the reader plumbing), and issue Inject an Env seam into which PATH and PATHEXT capture #485 has been amended to record that. This PR delivers the remaining scope: exposing parse_pathext/DEFAULT_PATHEXT for direct coverage and pinning the normalization contract. There is no var_os access left to remove in the seam sense — capture goes through the injected provider.
Testing (Compile-Time / UI): an absence-style compile-fail fixture proving cfg(any(windows, test)) exclusion on non-test Unix would have to assert on resolution-error diagnostics, which this repository has declined per precedent (see PR Stop the mock environment mutating the real process environment (#490) #497's documented rationale: trybuild strips RUSTFLAGS in this Polonius-only tree — Allow enabling -Zui-testing=yes dtolnay/trybuild#315/Document cycle detector state ownership #333 — and absence tests keyed to unstable diagnostics are brittle). The positive side of the cfg contract is exercised where it is meaningful: the unit suite compiles the items under cfg(test) on every platform, and Windows CI compiles the windows arm.
Developer Documentation (inconclusive): validated directly — docs/developers-guide.md has a dedicated "PATHEXT normalization" section covering parse_pathext behaviour (split on ;, trim, lowercase, leading-dot, dedup), DEFAULT_PATHEXT as the single fallback source, and the pins in src/stdlib/which/pathext_tests.rs.

Rebuild PR #503's branch on origin/main, which absorbed the injectable environment capture this branch pioneered — EnvSnapshot::capture now delegates to capture_with_env over mockable::Env, with a Windows capture_with_pathext override — so the old EnvReader closure seam, the WhichResolver reader plumbing, process_env_reader, and the reader-based rewrites of cache.rs and lookup/tests.rs are dropped as absorbed. What remains novel, and is ported: - Widen parse_pathext and DEFAULT_PATHEXT from #[cfg(windows)] to #[cfg(any(windows, test))], expose parse_pathext pub(super), and give the fallback a single construction site, so PATHEXT normalization is compiled and lintable on the Unix CI host instead of only on Windows. - Port src/stdlib/which/pathext_tests.rs: fixed rstest cases for the normalization rules plus the properties proptest module (normalized entries, case-insensitive uniqueness, idempotence, fallback, and first-occurrence order) with its deliberately small mixed-case alphabet, together with the proptest-regressions seed file. - Document the effective PATHEXT matching, the built-in fallback list in order, and the exact fallback condition in docs/users-guide.md. - Document the which environment capture seam as it now exists (the mockable::Env provider, DefaultEnv binding, and PATHEXT override) and the full parse_pathext normalization contract in docs/developers-guide.md, next to the environment lookup seam material. Main's capture_with_pathext lookup tests are kept: they cover the Windows snapshot path, while the ported suite verifies the parser rules from the host that gates merges. Closes #485. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

pandalump · 2026-08-06T11:33:27Z

@coderabbitai review

coderabbitai · 2026-08-06T11:33:34Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-08-06T11:33:38Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

This comment was marked as outdated.

Sign in to view

leynos marked this pull request as ready for review August 2, 2026 07:14

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

coderabbitai Bot approved these changes Aug 2, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

This was referenced Aug 2, 2026

Enforce the environment mandate with clippy disallowed-methods #504

Closed

Enforce the environment mandate with clippy disallowed-methods (#504) #505

Merged

coderabbitai Bot added the Issue label Aug 2, 2026

coderabbitai Bot requested changes Aug 2, 2026

View reviewed changes

Comment thread src/stdlib/which/env.rs Outdated

This was referenced Aug 2, 2026

Meta: mandate injected environments and retire EnvLock #496

Open

Normalize Markdown to the mdtablefix 0.5.0 standard #512

Merged

leynos force-pushed the issue-485-inject-env-seam-into-which-pathext branch from 8502694 to 15442a7 Compare August 4, 2026 01:26

This comment was marked as outdated.

Sign in to view

leynos mentioned this pull request Aug 4, 2026

Add a Windows CI job covering lint, compile, and test #518

Open

5 tasks

coderabbitai Bot approved these changes Aug 4, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

coderabbitai Bot requested changes Aug 5, 2026

View reviewed changes

Comment thread docs/users-guide.md Outdated

This comment was marked as outdated.

Sign in to view

leynos mentioned this pull request Aug 5, 2026

Give the workspace fallback switch an injectable env seam (#487) #498

Open

coderabbitai Bot requested changes Aug 5, 2026

View reviewed changes

Comment thread docs/users-guide.md Outdated

This comment was marked as outdated.

Sign in to view

coderabbitai Bot requested changes Aug 5, 2026

View reviewed changes

Comment thread docs/users-guide.md

leynos force-pushed the issue-485-inject-env-seam-into-which-pathext branch from d38be7d to 056a52f Compare August 6, 2026 01:55

This comment was marked as outdated.

Sign in to view

codescene-access Bot approved these changes Aug 6, 2026

View reviewed changes

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

leynos mentioned this pull request Aug 6, 2026

Inject an Env seam into which PATH and PATHEXT capture #485

Open

leynos force-pushed the issue-485-inject-env-seam-into-which-pathext branch from 056a52f to ab3db61 Compare August 6, 2026 11:23

codescene-access Bot approved these changes Aug 6, 2026

View reviewed changes

Conversation

leynos commented Aug 1, 2026 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Approach

Coverage

Removed

Deferred

Verification

Summary by Sourcery

Uh oh!

coderabbitai Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Summary

Scope

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

sourcery-ai Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Sequence diagram for PATHEXT tests using parse_pathext on all hosts

Flow diagram for PATHEXT normalisation in parse_pathext

File-Level Changes

Assessment against linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

buzzybee-df12 commented Aug 2, 2026

Uh oh!

coderabbitai Bot commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

buzzybee-df12 commented Aug 4, 2026

Uh oh!

coderabbitai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

pandalump commented Aug 5, 2026

Uh oh!

coderabbitai Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

leynos commented Aug 1, 2026 •

edited by sourcery-ai Bot

Loading

coderabbitai Bot commented Aug 1, 2026 •

edited

Loading

sourcery-ai Bot commented Aug 1, 2026 •

edited

Loading

coderabbitai Bot commented Aug 2, 2026 •

edited

Loading

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading

coderabbitai Bot commented Aug 5, 2026 •

edited

Loading

coderabbitai Bot commented Aug 5, 2026 •

edited

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading