-
-
Notifications
You must be signed in to change notification settings - Fork 14
WSL refactor to handle multiple distros and use JB WSL API #397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @sh41, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses several critical test failures that were blocking development. The changes primarily focus on improving cross-platform compatibility for path handling, enhancing the robustness and coverage of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
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 effectively addresses several test suite failures, unblocking further development. The fixes for cross-platform path handling, TOML syntax in tests, and test assertions are all correct and well-implemented. The refactoring of MiseServiceTest to use parameterized tests is a significant improvement, making the test suite more comprehensive and easier to maintain. I have one minor suggestion to improve the code style in PathUtils.kt. Overall, this is a high-quality contribution.
modules/core/src/main/kotlin/com/github/l34130/mise/core/util/PathUtils.kt
Outdated
Show resolved
Hide resolved
|
@gemini-code-assist review |
There was a problem hiding this comment.
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 effectively addresses several test suite failures, unblocking further development. The fixes are well-implemented and cover cross-platform path handling, incorrect test assertions, and test setup issues. The refactoring of MiseServiceTest to use parameterized tests is a significant improvement, making the test suite more robust and easier to maintain. I've added a couple of suggestions to further improve code clarity and adhere to Kotlin best practices in the test files. Overall, this is an excellent contribution.
modules/core/src/test/kotlin/com/github/l34130/mise/core/MiseServiceTest.kt
Outdated
Show resolved
Hide resolved
modules/core/src/test/kotlin/com/github/l34130/mise/core/model/MiseTomlTableTaskTest.kt
Show resolved
Hide resolved
… syntax and adjust `createConfigurationContext` method
The test was failing after commit 215434f because the source paths now include the `/src/` prefix from IntelliJ's light test fixture. The breaking commit added path normalization via collapsePath() which correctly strips the project base directory, but in test mode the VFS root is `/src/` while the project base is empty, leaving `/src/` as the path prefix. Updated test expectations to match the actual behavior with the test fixture's virtual file system structure.
Refactored `getRelativePath` to normalize input paths to system-independent format and convert the result back to system-dependent format. Fixes the `MiseServiceTest`.
Auto-detection was incorrectly selecting WSL mise for Windows C:\ drive projects, causing commands to execute in wrong context. This occurred because auto-detection ran globally without project-specific context. Removed WSL discovery to force explicit configuration or PATH resolution, which correctly determines mise based on working directory context.
MiseCommandLine needs project context to determine correct mise executable based on project location (Windows C:\ vs WSL UNC paths). Without this, all operations used global settings and couldn't distinguish between Windows and WSL project contexts. Added required project parameter to enable context-aware executable resolution in subsequent commits.
Different projects may need different mise executables (e.g., projects in containers, specific WSL distributions). Application-level setting forced all projects to share same executable. Added per-project executable path with priority over application setting, and removed annoying "executable not found" notification that appeared even when mise was correctly available in PATH.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an excellent pull request that significantly improves the WSL integration and overall robustness of the plugin. The refactoring to delegate WSL execution to the IntelliJ Platform's GeneralCommandLine and Eel framework is a major step forward, removing a lot of fragile, manual command construction. The introduction of project-level settings for the executable path, with a clear fallback mechanism, is also a great enhancement. The test suite fixes and improvements, especially the parameterization of MiseServiceTest, add a lot of value.
I've found one issue in the new getMiseVersion implementation that needs to be addressed. Otherwise, the changes are solid and well-executed.
modules/core/src/main/kotlin/com/github/l34130/mise/core/command/MiseCommandLine.kt
Outdated
Show resolved
Hide resolved
134130
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sh41 Thank you for your effort 🥺
| Not installed? Visit the <a href='https://mise.jdx.dev/installing-mise.html'>mise installation</a></br> | ||
| For WSL: <code>wsl.exe -d DistroName mise</code> or <code>\\wsl.localhost\DistroName\usr\bin\mise</code></br> | ||
| Optional: Override mise executable path for all projects.</br> | ||
| Leave empty to auto-detect 'mise' from PATH or common locations (recommended).</br> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attempted to explicitly specify the executable file path.
This is because other IDEA settings automatically detect tools and add them to the dropdown list.
For a unified user experience, automatically detecting tools and adding them to the text field seems preferable. Therefore, I added a notification at MiseProjectSettings.kt:L40. (If it doesn't work, it's a bug.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @134130, thanks for the great plugin and thanks for the feedback! I want to explain why I changed this and why persisting the executable path creates problems, especially with WSL.
The WSL Multi-Distribution Problem
Application-level auto-detection causes issues with WSL:
Scenario A: Which distro wins?
- First project is in
Ubuntu-24.04→ auto-detect saves that Ubuntu path - Open second project in
LinuxMint-21→ still uses Ubuntu's mise ❌ - This is the bug we're fixing
Scenario B: Windows vs WSL
- User has
miseon Windows PATH - Opens WSL project first → saves WSL path globally
- All Windows projects now try to use WSL
mise❌
Scenario C: Multiple distros
- User has Ubuntu, Debian, and LinuxMint distros, each with different
miseversions - Application setting picks one, forces it on all projects
- Need project overrides for every WSL project
Why PATH-First Works
The current approach:
- Windows projects: Get Windows
misefrom PATH automatically - WSL projects: Each distro provides its own
misefrom PATH automatically - 90% case: Zero configuration needed
- 10% case: Settings available for custom installs
About the UX Concern
You're right that other settings like Node.js/ESLint do project-aware auto-detection. But I think there's a difference:
ESLint detection:
- Check
./node_modules/.bin/eslint→ global npm → done - Deterministic, project-scoped search
- Found path is correct for that project
Mise + WSL detection:
- Application-level runs at startup (no project context yet)
- Finds first mise (say, in Ubuntu)
- User opens project in LinuxMint later
- Wrong mise gets used - Ubuntu's instead of LinuxMint's
The problem: application-level detection happens before we know what projects/contexts the user needs. ESLint is inherently project-scoped; application settings are inherently global, but WSL needs per-distro context.
I think that mise is more like git in it's usage. Without any configuration by the user, for a project root on Windows you see this:
for a project root on WSL, you see this:
What Proper Auto-Detection Would Need
To match other IDEA plugins:
- No application-level detection - no project context yet
- Per-project detection when project opens:
- Windows project → Windows mise
- Ubuntu WSL project → Ubuntu's mise
- LinuxMint WSL project → LinuxMint's mise
- Store at project level - like Python interpreter
IntelliJ's GeneralCommandLine + Eel already does this automatically via PATH resolution. When you run a command with a WSL working directory, it automatically resolves executables from that distro's default user's PATH.
Principle of Least Surprise
If a user has mise in their PATH, moves it, and updates PATH accordingly, they expect things to keep working. Persisting an absolute path breaks this - the plugin will look in the old location and fail. Users who maintain their PATH properly shouldn't need to also update IDE settings. I know moving a mise installation is probably a rare occurrence, but especially on Windows I've done it myself a few times switching between scoop, chocolatey & winget.
Suggested changes
What if we get the UX benefits without the drawbacks? Here are some options, do you think any or all of these would address your concern?
- Show detected path as hint in settings - greyed out placeholder like "(will use: /usr/local/bin/mise from PATH)" like
git. - Only persist if user explicitly saves - don't auto-populate, like
git - Have application/project level options -
githas the tick box, we currently have the two different fields. Could change to thegitmodel, or keep what we've got.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason I only noticed your new commits after I posted the above comment. I see you've added a checkbox-based project override. I like this approach, similar to the git model - it makes the override explicit and opt-in.
However, I noticed the fallback chain may need adjustment - currently when the checkbox is unchecked, it appears to skip the application setting entirely and go straight to PATH. Is this intentional?
I think that application level setting makes sense for the non-WSL scenarios, i.e. the user is on macOS or Linux or Windows without WSL (although that would be hard to detect and could change). When WSL is involved I think that application level has potential to break things. Maybe there should be two different configuration UIs, one for macOS/Linux and one for Windows?
Let me know your thoughts 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your detailed description! It looks a great deal of thought.
currently when the checkbox is unchecked, it appears to skip the application setting entirely and go straight to PATH. Is this intentional?
I intended: When the checkbox is unchecked, it uses the application setting, and if it is blank, it will uses the PATH. Is the code wrong?
What if we get the UX benefits without the drawbacks? Here are some options, do you think any or all of these would address your concern?
- Show detected path as hint in settings - greyed out placeholder like "(will use: /usr/local/bin/mise from PATH)" like git.
- Only persist if user explicitly saves - don't auto-populate, like git
- Have application/project level options - git has the tick box, we currently have the two different fields. Could change to the git model, or keep what we've got.
I've never knew that Git UX has exists. I love it! ❤️
I just had afraid of the using executable from PATH implicitly can makes surprise, associated with "Principle of Least Surprise"; yeah, I know the mise is just a command line tool which not having really breaking changes like JVM or NodeJS.
I think we can take all the three options! The first one needs to be implemented, but the second and third one looks already figured. 😄
|
I've been using this version for a while today and noticed that there are a lot of duplicated calls to What it does: Here's an extract from the log showing the cache hits, then I edit the I will look at the executable path stuff next. |
Previous implementation manually built WSL commands using distribution detection and path translation, which was fragile and didn't align with IntelliJ's WSL integration model. IntelliJ's GeneralCommandLine with Eel automatically handles WSL context when workDir is a UNC path (\wsl.localhost\...), eliminating need for manual command construction and distribution resolution.
Removed tests for WSL discovery functions that no longer exist, converted to LightPlatformTestCase for proper IDE testing support, and renamed test methods to follow JUnit convention (test prefix).
Mise version differs between Windows and WSL installations. Static version check used wrong executable (always Windows mise, even for WSL projects) causing incorrect flag usage (--env vs --profile). Changed to instance method that uses same context-aware executable resolution as actual commands, with per-executable version caching.
Trust and tracked config commands executed without project context, causing Windows mise to run for WSL projects. When opening projects with untrusted mise.toml, multiple parallel operations all failed and showed separate notifications (4+), flooding the user. Fixed by passing project context and configEnvironment to these commands, and implementing per-file notification debouncing so only one notification appears regardless of how many operations fail on the same config file.
executablePath can contain spaces (e.g., 'wsl.exe -d Ubuntu mise'), but was being passed as single argument causing command failure. Split on spaces to match behavior in runRawCommandLine().
…tion Ensures all code uses a single, reliable method to resolve project paths with proper fallback to user home when project dir is unavailable. Eliminates inconsistent direct basePath usage across the codebase.
Extracts reusable stampede protection pattern into a generic utility, enabling reuse in other caches.
|
Hi @134130, I've done some more refactoring. This is the Mise Executable area on Windows:
And this is on Windows with a WSL project: The project home is essential to determining the correct WSL Distro to run in, so I added a helper I've also refactored out the caching logic to make it reusable and implemented caching of the executable path so that it is absolutely consistent in across every usage location and when displayed in the UI. Given the size of this change, I think this needs some real world testing before it is considered done. If you have a canary channel that would be a good place to put it, or there's a built version here that people can install and test with.
I don't have macOs available to test this on, but will be at work and using it on a Windows -> multiple WSL setup from Monday next week. Thanks again for the great plugin & apologies for the huge PR! @gemini-code-assist review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an impressive and comprehensive refactoring of the WSL integration and command execution logic. The changes to make components project-aware, introduce a proper executable manager, and add a robust caching layer are excellent architectural improvements. By removing the global WSL auto-detection and delegating WSL execution to the IntelliJ Eel framework, you've addressed the core issue of handling multiple distributions correctly. The updated settings UI is also a significant improvement in user experience. I've found a couple of minor areas for improvement in the caching logic, but overall this is a very high-quality pull request.
| is CacheMetadata.ExecutableDependent -> { | ||
| // Check if this file matches the stored executable path in metadata | ||
| file.path == metadata.executablePath || file.path.contains(metadata.executablePath) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for invalidating ExecutableDependent cache entries could be more robust. The file.path.contains(metadata.executablePath) check is a bit too broad and may not work correctly for command-string style paths like "wsl.exe -d Ubuntu mise".
For example, if metadata.executablePath is "wsl.exe -d Ubuntu mise", and a change event comes for wsl.exe (e.g., file.path is C:/Windows/System32/wsl.exe), the condition file.path.contains(metadata.executablePath) will be false, and the cache won't be invalidated.
A more precise approach would be to check if the file path ends with the executable part of the command. This would correctly handle both simple paths (/usr/bin/mise) and command strings (wsl.exe ...).
| is CacheMetadata.ExecutableDependent -> { | |
| // Check if this file matches the stored executable path in metadata | |
| file.path == metadata.executablePath || file.path.contains(metadata.executablePath) | |
| } | |
| is CacheMetadata.ExecutableDependent -> { | |
| // Check if this file matches the stored executable path in metadata | |
| val execName = metadata.executablePath.substringBefore(" ") | |
| file.path == metadata.executablePath || file.path.endsWith(execName) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Eel/IJEnt executor handles all of the wsl interaction, all we do is pass in the executable. I don't think that the wsl.exe case should ever really arise. The executable path should just be a straight path to a single file.
modules/core/src/main/kotlin/com/github/l34130/mise/core/setting/MiseExecutableManager.kt
Show resolved
Hide resolved
|
@sh41 Thanks again for the great work! I don't usually use the canary channel, but since downgrading is an option, I'll consider it if we have volunteers. Let me know when it's stable on WSL—I'll handle the macOS testing on my end. |
Establishes single source of truth for mise executable path with clear priority order: project settings > app settings > auto-detection. Eliminates scattered path resolution logic and inconsistent behavior. Includes WSL support with proper Unix-to-Windows path conversion.
Replaces ambiguous dual override system (app-level + project-level fields) with single path field and "project-only" checkbox, inspired by the Version Control>Git UX pattern. Shows auto-detected path as placeholder for better discoverability.
…hout Completes migration to centralized path resolution, removing all direct executable path logic and basePath usage from consumers. Simplifies call sites by removing null-handling and fallback logic that is now handled by the utilities.
|
I made it so that the tests that rely on WSL only run on Windows platforms. I also changed the way that the executable is queried in WSL. New build here: mise-5.13.0-multi-wsl.zip @gemini-code-assist review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a significant and well-executed refactoring of WSL handling and executable path management. The changes make command execution project-aware, which is a crucial fix for handling multiple WSL distributions correctly. The introduction of a centralized MiseExecutableManager, a sophisticated caching layer with MiseCommandCache, and the move to leverage IntelliJ's native WSL support via UNC paths are all excellent architectural improvements. The updated settings UI is also much more intuitive.
The test suite has been substantially improved, becoming more robust and comprehensive. Overall, this pull request greatly enhances the plugin's correctness, performance, and maintainability. I have one suggestion regarding the new caching logic to make it more robust.
| } | ||
| is CacheMetadata.ExecutableDependent -> { | ||
| // Check if this file matches the stored executable path in metadata | ||
| file.path == metadata.executablePath || file.path.contains(metadata.executablePath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache invalidation logic for executable changes appears to have a potential flaw that could lead to excessive cache invalidations. The condition file.path.contains(metadata.executablePath) is problematic.
For instance, if metadata.executablePath is mise (when resolved from PATH), this condition will be true for any file path containing the substring "mise", such as .../mise.toml or a directory named mise-project. This would cause unrelated file changes to incorrectly invalidate the cache.
A more specific check is needed. Given the refactoring towards using full paths (including UNC paths for WSL), a simple equality check would be safer and cover the primary use case, preventing these false positives.
| file.path == metadata.executablePath || file.path.contains(metadata.executablePath) | |
| file.path == metadata.executablePath |



Fixes test suite failures and resolves WSL distribution execution issues.
Part 1: Test Suite Fixes
Fixes multiple test failures caused by path handling issues on Windows and incorrect test assertions, unblocking work on WSL refactoring.
PathUtils.getRelativePath()- Fix cross-platform path handlingFile.separatorCharis backslashMiseServiceTest- Update for VFS structure and add environment testing/src/prefix from test VFSmise.test.tomlonly loads whenMISE_ENV=test)MiseTomlTableTaskTest- Fix dependency assertion[0]→[0][0]MiseTomlTaskRunConfigurationProducerTest- Fix test setup//→#createConfigurationContext()to wrap element inPsiLocationPart 2: WSL Distribution Fix
Fixes #395 - Plugin was executing mise commands in wrong WSL distribution when project was in one distribution but application settings referenced another.
Root cause: Auto-detection populated application-level settings at startup, applying same WSL mise path to all projects regardless of their actual location.
Solution: Remove auto-detection, default to PATH resolution which automatically finds correct mise for each context (Windows native mise for C:\ projects, WSL mise for UNC path projects). Added project-level override for special cases.
Changes
Remove WSL auto-detection from application settings
Make MiseCommandLine project-aware
Add project-level executable path setting
Delegate WSL execution to IntelliJ Eel framework
Make getMiseVersion() context-aware
Fix trust command and notification spam