Skip to content

fix: respect disable_backends for explicitly declared backend-prefixed tools #8791

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.

Sign up for GitHub

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

Closed
palootcenas-outreach wants to merge 2 commits into from
Closed

fix: respect disable_backends for explicitly declared backend-prefixed tools #8791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0937b3
fix: respect disable_backends for explicitly declared backend-prefixe…
palootcenas-outreach Mar 26, 2026
1bebe35
Merge branch 'main' into fix/disable-backends-explicit-tools
palootcenas-outreach Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions e2e/backend/test_disable_backends
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,18 @@ mise uninstall age

MISE_DISABLE_BACKENDS=aqua mise install age
ls "$MISE_DATA_DIR/plugins/age"

# Test that disable_backends also works for tools with explicit backend
# prefixes in config files (not just short-name registry tools).
# Previously, get() in mod.rs would bypass the disable_backends filter
# and re-create the backend on demand for explicitly declared tools.
cat >.mise.toml <<'EOF'
[tools]
"aqua:FiloSottile/age" = "latest"
EOF

# With aqua backend disabled, the explicit aqua: tool should be skipped
# entirely — mise should not attempt to resolve or install it.
MISE_DISABLE_BACKENDS=aqua mise install
assert_fail "mise ls --installed aqua:FiloSottile/age 2>/dev/null | grep age"
rm .mise.toml
11 changes: 11 additions & 0 deletions src/backend/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,17 @@ pub fn get(ba: &BackendArg) -> Option<ABackend> {
let tools_ = tools.as_ref().unwrap();
if let Some(backend) = tools_.get(&ba.short) {
Some(backend.clone())
} else if Settings::get()
.disable_backends
.contains(&ba.backend_type().to_string())
{
None
} else if !tool_enabled(
&Settings::get().enable_tools(),
&Settings::get().disable_tools(),
&ba.short,
) {
None
Comment on lines +223 to +233

@greptile-apps greptile-apps Bot Mar 26, 2026

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.

P2 Multiple Settings::get() calls while holding the lock

Settings::get() is called three times in this block — once for disable_backends, once for enable_tools(), and once for disable_tools(). Each call acquires the BASE_SETTINGS read-lock and clones the Arc. Capturing it once avoids redundant reference-count churn and makes the code easier to follow (a single consistent snapshot of settings for the whole check):

Suggested change
} else if Settings::get()
.disable_backends
.contains(&ba.backend_type().to_string())
{
None
} else if !tool_enabled(
&Settings::get().enable_tools(),
&Settings::get().disable_tools(),
&ba.short,
) {
None
} else if {
let settings = Settings::get();
settings
.disable_backends
.contains(&ba.backend_type().to_string())
|| !tool_enabled(
&settings.enable_tools(),
&settings.disable_tools(),
&ba.short,
)
} {
None

This is a minor style point only — the current code is functionally correct.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

} else if let Some(backend) = arg_to_backend(ba.clone()) {
let mut tools_ = tools_.deref().clone();
tools_.insert(ba.short.clone(), backend.clone());
Expand Down
Loading