Skip to content

fix(github): use alias-specific options when tool_alias has its own config#8892

Merged
jdx merged 2 commits intomainfrom
fix/tool-alias-asset-pattern
Apr 4, 2026
Merged

fix(github): use alias-specific options when tool_alias has its own config#8892
jdx merged 2 commits intomainfrom
fix/tool-alias-asset-pattern

Conversation

@jdx
Copy link
Copy Markdown
Owner

@jdx jdx commented Apr 4, 2026

Summary

  • Fixed regression where tool_alias with per-alias asset_pattern (or other options) would use the original tool's options instead of the alias-specific ones
  • Changed get_tool_opts() to prefer the alias-specific config entry when it has options set, falling back to the resolved name match for CLI-created requests without options (preserves the treesize/gitlab fix from fix(gitlab): resolve tool options from config for aliased tools #8084)
  • Added e2e test verifying both the original tool and its alias install with their respective asset patterns

Fixes #8847

Test plan

  • cargo test — all 563 unit tests pass
  • mise run test:e2e test_github_tool_alias_asset_pattern — new e2e test passes
  • All lint hooks pass

🤖 Generated with Claude Code


Note

Medium Risk
Changes tool option resolution for aliased backends, which can affect how tools are downloaded/installed (e.g., asset_pattern) for some configurations; scope is localized but impacts installation behavior.

Overview
Fixes a regression where a tool_alias could incorrectly inherit the original tool’s option set (notably asset_pattern) instead of using the alias-specific config.

Config::get_tool_opts() now considers both the alias short-name match and the resolved full-name match and selects the entry that actually has options, preferring the alias-specific entry when present while preserving the fallback for resolved-name configs.

Adds an e2e regression test that configures a GitHub tool and its alias with different asset_pattern values and verifies both install using their respective assets.

Reviewed by Cursor Bugbot for commit 7f9ce83. Bugbot is set up for automated code reviews on this repo. Configure here.

…onfig

When both a tool and its alias are configured with different options (e.g.,
different asset_pattern), get_tool_opts() now prefers the alias-specific
entry when it has options set, instead of always returning the resolved
tool's options.

Fixes #8847

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 addresses a shadowing issue where tool alias options were ignored in favor of resolved tool names. The logic now prioritizes matches with defined options, favoring specific aliases when available. An end-to-end test has been added to verify that asset_pattern is correctly applied for aliased tools. Review feedback suggests optimizing the tool request lookup from O(N) to O(1) and refining the logic that detects whether options are present to include the os field.

Comment on lines +354 to 382
let short_match = trs.iter().find(|tr| tr.0.short == backend_arg.short);
// Also try matching by resolved full name for aliased tools.
// e.g., ba.short="treesize" resolves to full="gitlab:FBibonne/treesize"
// while the config entry has short="gitlab-f-bibonne-treesize" with api_url set.
// We check the resolved name first because the direct short match might find
// a CLI-created tool request without options.
let full = backend_arg.full();
let resolved_ba = BackendArg::new(full, None);
let tool_request = trs
.iter()
.find(|tr| tr.0.short == resolved_ba.short)
.or_else(|| trs.iter().find(|tr| tr.0.short == backend_arg.short));
let resolved_match = trs.iter().find(|tr| tr.0.short == resolved_ba.short);

let has_opts = |tr: &(&Arc<BackendArg>, &Vec<crate::toolset::ToolRequest>, _)| -> bool {
tr.1.first()
.map_or(false, |req| !req.options().opts.is_empty())
};
// Prefer whichever match has options set. When both have options,
// prefer the short (alias-specific) match since it's more specific.
// Fall back to the resolved match for cases where a CLI-created
// request without options shadows the config entry (treesize case).
let tool_request = match (short_match, resolved_match) {
(Some(s), Some(r)) => {
if has_opts(&s) {
Some(s)
} else {
Some(r)
}
}
(Some(s), None) => Some(s),
(None, Some(r)) => Some(r),
(None, None) => None,
};
Ok(tool_request.and_then(|tr| tr.1.first().map(|req| req.options())))
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.

medium

The current implementation uses trs.iter().find() which performs a linear search O(N) for each lookup. Since ToolRequestSet uses an IndexMap internally and BackendArg implements Hash and Eq based on its short name, you can use .get() for O(1) lookups.

Additionally, the has_opts closure only checks the opts field, ignoring install_env and os. It's better to use the is_empty() method on ToolVersionOptions and also check the os field to ensure all possible tool options are considered.

        let short_reqs = trs.tools.get(backend_arg);
        let full = backend_arg.full();
        let resolved_ba = BackendArg::new(full, None);
        let resolved_reqs = trs.tools.get(&resolved_ba);

        let has_opts = |reqs: &Vec<crate::toolset::ToolRequest>| -> bool {
            reqs.first().map_or(false, |req| {
                let opts = req.options();
                !opts.is_empty() || opts.os.is_some()
            })
        };
        // Prefer whichever match has options set. When both have options,
        // prefer the short (alias-specific) match since it's more specific.
        // Fall back to the resolved match for cases where a CLI-created
        // request without options shadows the config entry (treesize case).
        let reqs = match (short_reqs, resolved_reqs) {
            (Some(s), Some(r)) => {
                if has_opts(s) {
                    Some(s)
                } else {
                    Some(r)
                }
            }
            (Some(s), None) => Some(s),
            (None, Some(r)) => Some(r),
            (None, None) => None,
        };
        Ok(reqs.and_then(|v| v.first().map(|req| req.options())))

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 4, 2026

Greptile Summary

Fixes a regression in Config::get_tool_opts() where a tool_alias with its own asset_pattern (or other per-alias options) would incorrectly inherit the original tool's options instead of its own. The new logic checks both the short (alias) match and the resolved full-name match, preferring the alias-specific entry when it carries options while preserving the existing fallback for the treesize/gitlab case from #8084.

Confidence Score: 5/5

Safe to merge — the fix is targeted, well-reasoned, and backed by a regression test; only a minor completeness gap in the has_opts check remains.

All remaining findings are P2: the has_opts closure omits install_env/os from its check, but the affected scenario (alias with only those fields and no opts) is extremely unlikely in practice. The core regression is correctly fixed and covered by the new e2e test.

src/config/mod.rs — minor: has_opts should use is_empty() instead of opts.is_empty() for full coverage of ToolVersionOptions fields.

Important Files Changed

Filename Overview
src/config/mod.rs Fixes alias option resolution in get_tool_opts() by preferring the alias-specific short match when it has options; has_opts only checks opts field and misses install_env/os fields of ToolVersionOptions.
e2e/backend/test_github_tool_alias_asset_pattern New regression test covering alias-specific asset_pattern; verifies the alias installs with the correct directory layout, but lacks a symmetric assertion for the original tool's install path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[get_tool_opts called with backend_arg] --> B[Find short_match by backend_arg.short]
    A --> C[Resolve full name → find resolved_match]
    B --> D{Both matches found?}
    C --> D
    D -- Yes --> E{short_match has opts?}
    E -- Yes --> F[Use short_match alias-specific config]
    E -- No --> G[Use resolved_match treesize/gitlab fallback]
    D -- Only short_match --> F
    D -- Only resolved_match --> G
    D -- Neither --> H[Return None]
Loading

Fix All in Claude Code

Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile

Comment on lines +26 to +28
assert_contains "mise x -- hello-world" "hello world"
# The aliased tool should have installed using hello-world-2.0.0.tar.gz
assert "test -d ~/.local/share/mise/installs/hw2/1.0.0/hello-world-2.0.0"
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 Missing assertion for original tool's install directory

The test only verifies the hw2 alias installed with the correct directory layout (hello-world-2.0.0). A symmetric assertion for github:jdx/mise-test-fixtures would catch any future regression where the fix inadvertently swaps which set of options each tool receives.

Suggested change
assert_contains "mise x -- hello-world" "hello world"
# The aliased tool should have installed using hello-world-2.0.0.tar.gz
assert "test -d ~/.local/share/mise/installs/hw2/1.0.0/hello-world-2.0.0"
assert_contains "mise x -- hello-world" "hello world"
# The aliased tool should have installed using hello-world-2.0.0.tar.gz
assert "test -d ~/.local/share/mise/installs/hw2/1.0.0/hello-world-2.0.0"
# The original tool should have installed using hello-world-1.0.0.tar.gz
assert "test -d ~/.local/share/mise/installs/github-jdx-mise-test-fixtures/1.0.0/hello-world-1.0.0"

Fix in Claude Code

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.3 x -- echo 23.1 ± 0.8 21.9 33.9 1.00
mise x -- echo 23.7 ± 0.7 22.4 30.5 1.03 ± 0.05

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.3 env 22.4 ± 0.7 21.2 27.8 1.00
mise env 23.2 ± 0.9 21.7 26.4 1.04 ± 0.05

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.3 hook-env 23.3 ± 0.6 22.0 27.4 1.00
mise hook-env 24.0 ± 0.7 22.5 27.6 1.03 ± 0.04

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.3 ls 20.5 ± 0.6 19.2 22.0 1.00
mise ls 21.1 ± 0.6 19.8 23.2 1.03 ± 0.04

xtasks/test/perf

Command mise-2026.4.3 mise Variance
install (cached) 151ms 152ms +0%
ls (cached) 79ms 80ms -1%
bin-paths (cached) 85ms 84ms +1%
task-ls (cached) 844ms 805ms +4%

@jdx jdx merged commit e9afca6 into main Apr 4, 2026
34 checks passed
@jdx jdx deleted the fix/tool-alias-asset-pattern branch April 4, 2026 14:59
jdx pushed a commit that referenced this pull request Apr 5, 2026
### 🚀 Features

- **(ci)** auto-convert external PRs to draft mode by @jdx in
[#8896](#8896)
- **(deps)** add `depends` field for user-specified tool dependencies by
@cprecioso in [#8776](#8776)
- **(dotnet)** support runtime-only installs by @fragon10 in
[#8524](#8524)
- **(npm)** apply install_before to transitive dependencies by @risu729
in [#8851](#8851)
- **(task)** allow passing arguments to task dependencies via
{{usage.*}} templates by @jdx in
[#8893](#8893)
- add options field to BackendListVersionsCtx by @esteve in
[#8875](#8875)

### 🐛 Bug Fixes

- **(backend)** filter PEP 440 .dev versions in fuzzy version matching
by @richardthe3rd in [#8849](#8849)
- **(ci)** update COPR BuildRequires rust version to match MSRV 1.88 by
@jdx in [#8911](#8911)
- **(ci)** add Ruby build dependencies to e2e Docker image by @jdx in
[#8910](#8910)
- **(ci)** add missing build dependencies to e2e Docker image by @jdx in
[#8912](#8912)
- **(ci)** add missing build dependencies to e2e Docker image by @jdx in
[#8914](#8914)
- **(ci)** use Node 24 LTS for corepack e2e test by @jdx in
[#8915](#8915)
- **(ci)** add libxml2 and pkg-config to e2e Docker image by @jdx in
[#8917](#8917)
- **(ci)** add libxml2-dev to e2e image and disable Swift SPM tests by
@jdx in [#8918](#8918)
- **(docs)** use sans-serif font for badges by @jdx in
[#8887](#8887)
- **(env)** parse --env=VALUE and -E=VALUE flag forms correctly by @jdx
in [#8889](#8889)
- **(exec)** use i64::from() for seccomp syscall numbers to survive
autofix by @jdx in [#8882](#8882)
- **(github)** preserve tool options like filter_bins when version
specified via CLI by @jdx in
[#8888](#8888)
- **(github)** use alias-specific options when tool_alias has its own
config by @jdx in [#8892](#8892)
- **(install)** add locked_verify_provenance setting and detect github
attestations at lock time by @jdx in
[#8901](#8901)
- **(lock)** prune stale version entries during filtered `mise lock
<tool>` runs by @altendky in
[#8599](#8599)
- **(python)** use lockfile URL for precompiled installs by @hehaoqian
in [#8750](#8750)
- **(release)** verify all build targets succeed before releasing by
@jdx in [#8886](#8886)
- **(ruby)** support build revisions for precompiled binaries in
mise.lock by @jdx in [#8900](#8900)
- **(swift)** fall back to Ubuntu 24.04 for unsupported Ubuntu versions
by @jdx in [#8916](#8916)
- **(zsh)** avoid duplicate trust warning after cd by @timothysparg in
[#8898](#8898)
- update flake.lock and add fix for rust-bindgen to default.nix by
@esteve in [#8874](#8874)
- when direnv diff is empty, do not try to parse it by @yaleman in
[#8857](#8857)
- skip trust check for plain .tool-versions in task list by @dportalesr
in [#8876](#8876)

### 🚜 Refactor

- **(go)** rename go_* settings to go.* namespace by @jdbruijn in
[#8598](#8598)

### 📚 Documentation

- **(tasks)** clarify task_config.includes behavior by @risu729 in
[#8905](#8905)

### 🧪 Testing

- **(ci)** run e2e tests inside Docker containers by @jdx in
[#8899](#8899)

### 📦️ Dependency Updates

- bump ubi from 0.8 to 0.9 by @jdx in
[#8906](#8906)
- bump zip from 3 to 8 by @jdx in
[#8908](#8908)
- update lockfile deps (hold back rattler) by @jdx in
[#8909](#8909)
- update bun.lock by @jdx in
[#8913](#8913)

### 📦 Registry

- add turso
([github:tursodatabase/turso-cli](https://github.com/tursodatabase/turso-cli))
by @kenn in [#8884](#8884)
- remove carp test by @jdx in
[#8894](#8894)

### Chore

- **(ci)** add workflow to warn PRs modifying vendored aqua-registry by
@jdx in [#8897](#8897)
- **(ci)** use github.token for draft conversion in auto-draft workflow
by @jdx in [#8903](#8903)
- remove deprecated settings older than 12 months by @jdx in
[#8904](#8904)

### New Contributors

- @dportalesr made their first contribution in
[#8876](#8876)
- @timothysparg made their first contribution in
[#8898](#8898)
- @hehaoqian made their first contribution in
[#8750](#8750)
- @jdbruijn made their first contribution in
[#8598](#8598)
- @cprecioso made their first contribution in
[#8776](#8776)
- @yaleman made their first contribution in
[#8857](#8857)
- @kenn made their first contribution in
[#8884](#8884)
- @fragon10 made their first contribution in
[#8524](#8524)

## 📦 Aqua Registry Updates

#### New Packages (6)

- [`ahkohd/oyo`](https://github.com/ahkohd/oyo)
- [`bellicose100xp/jiq`](https://github.com/bellicose100xp/jiq)
- [`kurama/dealve-tui`](https://github.com/kurama/dealve-tui)
- [`micahkepe/jsongrep`](https://github.com/micahkepe/jsongrep)
- [`textfuel/lazyjira`](https://github.com/textfuel/lazyjira)
- [`ubugeeei/vize`](https://github.com/ubugeeei/vize)

#### Updated Packages (1)

- [`sigstore/cosign`](https://github.com/sigstore/cosign)
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.

1 participant