Skip to content

fix(env): drop stale install paths during reactivation#10162

Merged
jdx merged 1 commit into
jdx:mainfrom
risu729:codex-20260531-173330-1ad98e
May 31, 2026
Merged

fix(env): drop stale install paths during reactivation#10162
jdx merged 1 commit into
jdx:mainfrom
risu729:codex-20260531-173330-1ad98e

Conversation

@risu729

@risu729 risu729 commented May 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Reproduction

The new e2e/env/test_zsh_reactivate_drops_stale_install_path test sets global node = "24", enables package.json idiomatic versions, creates a project with devEngines.runtime.version = "22.x", activates zsh, then simulates a parent/editor environment that prepends the inactive global Node install path before spawning a non-interactive zsh child.

Before this change, the test fails with:

v24.16.0
.../installs/node/24/bin
.../installs/node/22.17.1/bin

After this change, reactivation keeps node@22.17.1 first.

Discussion context: #10095 (reply in thread)

Tests

  • mise run build
  • CARGO_TARGET_DIR=/home/risu/.cache/cargo-target/mise-409845c58888dcc6 e2e/run_test env/test_zsh_reactivate_drops_stale_install_path
  • CARGO_TARGET_DIR=/home/risu/.cache/cargo-target/mise-409845c58888dcc6 e2e/run_test env/test_path_reorder_after_activate
  • CARGO_TARGET_DIR=/home/risu/.cache/cargo-target/mise-409845c58888dcc6 e2e/run_test env/test_path_interleaved_regression
  • CARGO_TARGET_DIR=/home/risu/.cache/cargo-target/mise-409845c58888dcc6 e2e/run_test cli/test_activate_multiple_zsh
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where stale tool installation paths could remain in the PATH after shell reactivation, potentially causing the wrong tool version to be executed.
  • Tests

    • Added regression test to verify PATH is correctly ordered after shell reactivation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a fix to prevent stale mise-managed install paths from being preserved as user prefixes in the PATH environment variable, which could otherwise shadow the active tool version. It adds a helper function is_mise_install_path to identify paths within the mise installs directory (including canonicalized paths) and skip them during PATH restoration. An end-to-end test test_zsh_reactivate_drops_stale_install_path is also added to verify this behavior in zsh. There are no review comments, so no feedback is provided.

@greptile-apps

greptile-apps Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes a PATH-poisoning bug where a stale mise install path inherited from a parent shell (e.g. from an editor extension) was preserved as a user prefix ahead of the active project version during hook-env reactivation. The fix classifies any PATH entry under dirs::INSTALLS as mise-managed—even when the previous session diff did not record it—and drops it from the pre/post_user user sections, letting the correct active tool path win.

  • src/cli/hook_env.rs: Adds is_mise_install_path(path: &Path) -> bool that checks against dirs::INSTALLS both as a raw prefix and (for symlinked data dirs like macOS /tmp) via canonicalize_cached; calls it in the path-classification loop inside the non-aggressive activation branch.
  • e2e/env/test_zsh_reactivate_drops_stale_install_path: New regression test that stubs node 22 and 24 shims, prepends the inactive global install dir to PATH in a parent shell, then asserts the non-interactive child reactivation resolves node to 22 first.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the non-aggressive activation branch, correctly handles symlinked data directories via a canonicalization fallback, and is covered by a targeted regression test.

The is_mise_install_path helper correctly handles both direct and symlinked paths under INSTALLS, the canonicalization cache prevents repeated filesystem hits, and the fix is gated to the same !activate_aggressive branch where the stale-path problem can occur. The e2e test faithfully reproduces the reported scenario. No correctness issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/cli/hook_env.rs Adds is_mise_install_path helper and uses it to drop stale installs-dir entries during reactivation; logic is correct, canonicalization fallback handles symlinked MISE_DATA_DIR (e.g. macOS /tmp → /private/tmp), and the check is correctly scoped to the non-aggressive activation branch.
e2e/env/test_zsh_reactivate_drops_stale_install_path New zsh e2e regression test that stubs two node versions, activates mise in a parent shell, prepends the inactive global install path, then verifies the child reactivation keeps node 22 first. Test structure and assertions are correct.

Reviews (3): Last reviewed commit: "fix(env): drop stale install paths durin..." | Re-trigger Greptile

Comment thread src/cli/hook_env.rs Outdated
@risu729 risu729 force-pushed the codex-20260531-173330-1ad98e branch from 0d4eb22 to 1059761 Compare May 31, 2026 08:50
@risu729 risu729 force-pushed the codex-20260531-173330-1ad98e branch from 1059761 to a62adfc Compare May 31, 2026 09:20
@risu729 risu729 marked this pull request as ready for review May 31, 2026 15:23
@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 684cc422-a46a-49dd-96b2-a6bb09108353

📥 Commits

Reviewing files that changed from the base of the PR and between a0dd654 and a62adfc.

📒 Files selected for processing (2)
  • e2e/env/test_zsh_reactivate_drops_stale_install_path
  • src/cli/hook_env.rs

📝 Walkthrough

Walkthrough

This PR fixes a bug where stale mise install paths persist in PATH during shell reactivation. The implementation filters mise install-paths during the non-aggressive PATH recomputation phase, and a new end-to-end test validates that reactivation correctly prioritizes project-specified tool versions over stale global paths.

Changes

Stale install path filtering and regression test

Layer / File(s) Summary
PATH filtering implementation
src/cli/hook_env.rs
Updated imports to include dirs and Path. Modified PATH-splitting logic to skip entries under the mise installs directory when activate_aggressive is disabled. Added is_mise_install_path helper to detect paths under installs directory using fast-path and canonicalization fallback.
End-to-end regression test for stale path scenario
e2e/env/test_zsh_reactivate_drops_stale_install_path
New zsh test script that configures mise offline, creates two Node versions (22.17.1 and 24), writes config and package.json, then verifies that mise activate zsh correctly selects Node 22 in the parent shell, maintains selection after a stale Node 24 path is injected into PATH, and preserves correct PATH ordering in a child shell reactivation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A stale path once lingered where it shouldn't dwell,
But now mise filters out that shell's false spell,
The installs are checked, the ordering clear,
Tests verify old paths disappear,
Fresh activations spring from depths below! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and concisely summarizes the main change: fixing the environment hook to drop stale install paths during reactivation, which is the core objective of the PR.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jdx jdx merged commit 5bfaf73 into jdx:main May 31, 2026
33 checks passed
@risu729 risu729 deleted the codex-20260531-173330-1ad98e branch May 31, 2026 15:40
JamBalaya56562 added a commit to JamBalaya56562/mise that referenced this pull request Jun 14, 2026
`mise x tool@X` / `run` / `env` correctly remap the direct command, but a stale
mise-managed install dir left earlier on PATH (e.g. from a frozen env snapshot of
a previously activated shell) stayed ahead of the bin dir mise injects, so PATH
lookups inside the process tree (subshells, env shebangs, package-manager workers)
still resolved the other version -- a split-brain where `node --version` says 22
while the tests actually run on 24.

Filter mise-managed install dirs out of the inherited PATH when Toolset composes
the child env, so the requested version wins for the whole process tree. Mirrors
the hook-env reactivation fix (jdx#10162): share its is_mise_install_path helper
(moved to path_env) and extend it to also cover shared/system install dirs
(MISE_SHARED_INSTALL_DIRS + the system installs dir) that find_in_shared_installs
resolves into.

Addresses discussion jdx#10345.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JamBalaya56562 added a commit to JamBalaya56562/mise that referenced this pull request Jun 14, 2026
`mise x tool@X` / `run` / `env` correctly remap the direct command, but a stale
mise-managed install dir left earlier on PATH (e.g. from a frozen env snapshot of
a previously activated shell) stayed ahead of the bin dir mise injects, so PATH
lookups inside the process tree (subshells, env shebangs, package-manager workers)
still resolved the other version -- a split-brain where `node --version` says 22
while the tests actually run on 24.

Filter mise-managed install dirs out of the inherited PATH when Toolset composes
the child env, so the requested version wins for the whole process tree. Mirrors
the hook-env reactivation fix (jdx#10162): share its is_mise_install_path helper
(moved to path_env) and extend it to also cover shared/system install dirs
(MISE_SHARED_INSTALL_DIRS + the system installs dir) that find_in_shared_installs
resolves into.

Addresses discussion jdx#10345.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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