Skip to content

fix(vfox): use selected install path for env keys#9907

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:fix/vfox-system-bin-paths
May 17, 2026
Merged

fix(vfox): use selected install path for env keys#9907
jdx merged 2 commits into
jdx:mainfrom
risu729:fix/vfox-system-bin-paths

Conversation

@risu729

@risu729 risu729 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pass the resolved ToolVersion install path into traditional vfox EnvKeys hooks
  • key/cache vfox exec env by install path so shared/system installs do not reuse user-path cache entries
  • add e2e coverage for a fake system-installed vfox poetry tool

Fixes part of #9857.

Testing

  • cargo fmt --check
  • git diff --check
  • CARGO_TARGET_DIR=/tmp/mise-m7-vfox-test /home/risu/.rustup/toolchains/1.95.0-x86_64-unknown-linux-gnu/bin/cargo test -p vfox test_env_keys_for_install_dir
  • CARGO_TARGET_DIR=/tmp/mise-m7-e2e-target mise run test:e2e e2e/backend/test_vfox_system_bin_paths

@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 refactors the vfox backend to support explicit installation directories when retrieving environment keys. Key changes include the introduction of env_keys_for_install_dir in the vfox crate, updating the VfoxBackend to incorporate the installation path into its cache key and cache freshness checks, and adding an end-to-end test for system binary paths. I have no feedback to provide as there were no review comments to evaluate.

@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes vfox env key resolution for system-installed tools by threading the resolved tv.install_path() through to the plugin's EnvKeys hook, and keys the exec-env cache on both options and install path so that system and user installs no longer share cache entries.

  • crates/vfox/src/vfox.rs: New env_keys_for_install_dir public method accepts an explicit install dir; both public variants delegate to a shared private env_keys_for_sdk_install_dir helper. A unit test verifies the custom path is forwarded correctly.
  • src/backend/vfox.rs: _exec_env now captures tv.install_path() upfront, includes it in the opts_hash used for both the in-memory cache key and the on-disk cache file name, and replaces the coarse installs_path freshness watch with the specific install_path.
  • e2e/backend/test_vfox_system_bin_paths: New e2e script sets up a fake system-installed poetry plugin and asserts mise bin-paths returns the system install dir rather than the user install dir.

Confidence Score: 5/5

Safe to merge — changes are narrowly scoped to the vfox env-key path and cache logic, with no regressions on the existing code path.

The fix is well-targeted: the root cause (hardcoded install-dir construction in the hook call and the cache hash) is corrected at both layers, the existing env_keys public API is preserved unchanged by delegating to the new shared helper, and the e2e test validates the system-install scenario end-to-end.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/vfox.rs Uses the resolved install_path from tv.install_path() for both the env-key computation and the cache key/freshness file, ensuring system-installed tools get their own cache entries and correct path passed to the plugin hook.
crates/vfox/src/vfox.rs Adds env_keys_for_install_dir public API and a shared private helper, refactoring env_keys to delegate into the same code path; a unit test validates custom install-dir resolution.
e2e/backend/test_vfox_system_bin_paths New e2e test that sets up a fake system-installed vfox poetry tool and asserts mise bin-paths reports the system install path rather than the user install path.

Reviews (2): Last reviewed commit: "test(vfox): use platform separator in en..." | Re-trigger Greptile

Comment thread e2e/backend/test_vfox_system_bin_paths
@risu729 risu729 marked this pull request as ready for review May 16, 2026 09:35
@jdx jdx merged commit f8c1a65 into jdx:main May 17, 2026
33 checks passed
@risu729 risu729 deleted the fix/vfox-system-bin-paths branch May 17, 2026 15:32
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