Skip to content

feat(doctor): report shadowed shims - #10919

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:codex-20260710-224125-0a1d18
Jul 12, 2026
Merged

jdx merged 2 commits into
jdx:mainfrom
risu729:codex-20260710-224125-0a1d18

Conversation

@risu729

@risu729 risu729 commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • warn when a command provided by an active mise shim resolves to another executable first
  • preserve intentional PATH overrides by checking concrete command collisions instead of requiring shims to be first
  • share the desired-shim set from the existing doctor analysis so tools are not scanned twice

Root cause

mise doctor only checked whether the shim directory appeared anywhere in PATH. Its existing PATH-order diagnostic applies to activated shells, so shim-only setups could report shims_on_path: yes and No problems found even when a system executable shadowed a mise shim.

This does not change PATH precedence. Entries before the shim directory still intentionally win; doctor now explains the concrete consequence.

Maintainer note

This is an optional diagnostic improvement rather than a runtime fix. If warning about intentional PATH overrides does not fit the intended scope of mise doctor, feel free to close this PR.

Context: #5395

Validation

  • mise run format
  • cargo check --all-features
  • mise run test:e2e cli/test_doctor_shim_shadowing
  • mise run test:e2e cli/test_doctor

Summary by CodeRabbit

  • New Features
    • mise doctor now warns when mise shims are shadowed by earlier executables in your PATH.
    • Reports the conflicting command and where it resolves, with guidance to move the mise shims directory earlier.
    • Shadowing diagnostics are included in both text and JSON doctor output.
  • Bug Fixes
    • Improved shim analysis and diagnostics for missing, extra, and shadowed shims across platforms.
  • Tests
    • Added an end-to-end test covering shim shadowing and verification when PATH ordering is corrected.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

mise doctor now preserves desired shim names, detects when PATH resolves them to earlier executables, reports collisions, and validates both shadowed and correctly ordered PATH configurations with an end-to-end test.

Changes

Shim shadowing diagnostics

Layer / File(s) Summary
Preserve desired shim data
src/shims.rs
ShimDiffs now returns missing, extra, and desired shim sets; reshim consumes the structured result.
Pass desired shims through doctor
src/cli/doctor/mod.rs
Doctor captures desired shims from analysis in JSON and shared configuration flows.
Detect and test PATH collisions
src/cli/doctor/mod.rs, e2e/cli/test_doctor_shim_shadowing
Doctor resolves shim commands against PATH, reports shadowing executables, normalizes Windows names, and tests both PATH orderings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit found shims in a PATH so wide,
With shadowy binaries hopping beside.
Doctor took note, then pointed the way:
“Put mise first, and shadows away!”
The test hops green through the night.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: mise doctor now reports shadowed shims.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps

greptile-apps Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves mise doctor to detect when a mise-managed shim is shadowed by an earlier entry in PATH, reporting the specific conflicting executable rather than just checking whether the shim directory appears somewhere in PATH.

  • src/shims.rs: get_shim_diffs now returns a ShimDiffs struct that exposes the full desired shim set alongside missing/extra, avoiding a redundant toolset scan in doctor.
  • src/cli/doctor/mod.rs: analyze_shims returns the desired-shim set; new check_shim_shadowing iterates over it, skips shims that don't exist on disk or are already winning command resolution, and emits a warning listing each concrete shadowed command and the executable that shadows it.
  • e2e/cli/test_doctor_shim_shadowing: End-to-end test covering both the shadowed and non-shadowed PATH orderings.

Confidence Score: 5/5

Safe to merge — the new shadowing check is purely diagnostic (warnings only, no PATH mutations), gated behind is_activated() and shims_on_path(), and the refactor of get_shim_diffs into a struct is backward-compatible with the one other call site in reshim.

All three changes are additive and low-risk: the struct refactor is mechanical, the new check_shim_shadowing method correctly deduplicates via checked_commands, filters missing shims, and only emits a warning rather than an error so false positives do not break workflows. The e2e test covers both positive and negative cases.

No files require special attention.

Important Files Changed

Filename Overview
src/shims.rs Refactors get_shim_diffs to return a ShimDiffs struct instead of a bare tuple, exposing the full desired shims set so doctor can reuse it without a second toolset scan
src/cli/doctor/mod.rs Adds check_shim_shadowing to report concrete PATH collisions in shim-only setups; analyze_shims now returns the desired-shim set to avoid scanning the toolset twice
e2e/cli/test_doctor_shim_shadowing New e2e test that verifies the shadowing warning fires when a shadow binary precedes the shim directory in PATH, and is absent when shims win

Reviews (2): Last reviewed commit: "fix(doctor): deduplicate shim checks" | Re-trigger Greptile

Comment thread src/cli/doctor/mod.rs Resolved
Comment thread src/cli/doctor/mod.rs Resolved
@risu729 risu729 changed the title fix(doctor): report shadowed shims feat(doctor): report shadowed shims Jul 10, 2026
@risu729
risu729 marked this pull request as ready for review July 10, 2026 13:48
@jdx
jdx merged commit c6b2588 into jdx:main Jul 12, 2026
33 checks passed
@risu729
risu729 deleted the codex-20260710-224125-0a1d18 branch July 12, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants