Skip to content

fix(aqua): skip registry lookup for linked versions in list_bin_paths#8801

Merged
jdx merged 4 commits intojdx:mainfrom
nikobockerman:fix/aqua-linked-version-panic
Mar 31, 2026
Merged

fix(aqua): skip registry lookup for linked versions in list_bin_paths#8801
jdx merged 4 commits intojdx:mainfrom
nikobockerman:fix/aqua-linked-version-panic

Conversation

@nikobockerman
Copy link
Copy Markdown
Contributor

Ref: #8772

Summary

  • Fix StripPrefixError panic when running mise doctor, mise reshim, or any command that calls list_bin_paths on an aqua-backed tool with a linked version (created via mise link)
  • Detect linked versions (absolute-path symlinks) early in list_bin_paths and return install_path/bin directly, skipping the aqua registry lookup entirely
  • Harden strip_prefix from .unwrap() to .filter_map(...ok()...) as a safety net

Root cause

  1. Linked version names (e.g. "brew", "mylink") are passed to the aqua registry as version strings
  2. Versioning::new("brew") from the versions crate parses them as valid General versions
  3. Semver constraints like <= 0.2.13 incorrectly match because alphabetic versions sort before numeric ones
  4. This selects a wrong version_override with no asset/format fields
  5. An empty asset renders {{.AssetWithoutExt}}/sccache as "/sccache" (absolute path)
  6. PathBuf::join("/sccache") replaces the entire install path, so strip_prefix panics

🤖 Generated with Claude Code

nikobockerman and others added 2 commits March 27, 2026 22:42
Aqua's list_bin_paths panicked with StripPrefixError when processing
linked versions (created via `mise link`, e.g. symlinks to brew installs).

The versions crate parsed non-version strings like "brew" as valid
General versions, causing wrong semver constraint matches in the aqua
registry. This produced empty asset templates, leading to absolute paths
that couldn't be strip_prefix'd against the install path.

Fix by detecting linked versions (absolute-path symlinks) early and
returning install_path/bin directly, skipping the registry lookup
entirely. Also harden strip_prefix from unwrap to filter_map as a
safety net.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document the root cause in the aqua-registry crate:
- AquaFile::src produces absolute path "/name" when asset is empty
- version_override matches wrong semver constraint for non-version strings
  like "brew" because the versions crate parses them as valid General versions

Co-Authored-By: Claude Opus 4.6 <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 panic in the aqua backend that occurs when processing linked versions created via mise link. The changes introduce logic to skip the aqua registry lookup for symlinked versions, opting for a standard bin path heuristic instead. It also replaces an unsafe unwrap() with a filter_map when stripping path prefixes to improve robustness. A review comment suggests removing the is_absolute() check on symlink targets to ensure that relative symlinks and version aliases are also correctly handled without falling back to the registry lookup.

Comment on lines +486 to +489
if target.is_absolute() {
let bin = install_path.join("bin");
return Ok(if bin.is_dir() { vec![bin] } else { vec![install_path] });
}
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.

high

The check if target.is_absolute() makes this fix incomplete. While it handles linked versions created with absolute paths, it fails to cover linked versions created with relative paths (e.g., mise link mytool@mylink ../local-build). In that case, target.is_absolute() would be false, causing the code to fall through to the aqua registry lookup with the link name, which would trigger the original panic.

Furthermore, this check prevents correct handling of version aliases (e.g., latest pointing to a specific version), which are often relative symlinks.

To make the fix more robust and handle all symlinked versions (both linked versions and aliases), I suggest removing the is_absolute() check. Any symlink at the install_path can be treated as a version that doesn't need an aqua registry lookup for its bin paths.

Suggested change
if target.is_absolute() {
let bin = install_path.join("bin");
return Ok(if bin.is_dir() { vec![bin] } else { vec![install_path] });
}
// Any symlink at the install path is considered a linked/aliased version.
// We can use the simple bin path heuristic and skip aqua registry lookup.
let bin = install_path.join("bin");
return Ok(if bin.is_dir() { vec![bin] } else { vec![install_path] });

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR fixes a StripPrefixError panic that occurred when running mise doctor, mise reshim, or any command triggering list_bin_paths on an aqua-backed tool whose version was created via mise link. The fix adds an early-return guard in list_bin_paths that detects linked versions (identified as installs whose install_path is a symlink pointing to an absolute path) and returns the bin paths directly, skipping the aqua registry lookup entirely. A secondary hardening changes the .unwrap() on strip_prefix to a filter_map as a defensive safety net.

  • Root cause confirmed: mise link always calls absolutize() on the user-supplied path, so linked-version install paths are always absolute-target symlinks — the target.is_absolute() guard reliably distinguishes them from regular aqua installs.
  • Symlink semantics are correct: install_path.join(\"bin\").is_dir() follows the symlink to check the real directory structure, so the early-return yields the right paths.
  • Two unit tests in types.rs document the underlying root-cause behaviour (empty asset producing /sccache, non-version strings matching semver constraints), which is valuable for future maintainers.
  • E2e regression test (test_aqua_linked_version) installs sccache, creates a linked version, and asserts mise reshim succeeds without panicking — covers the reported crash scenario end-to-end.

Confidence Score: 5/5

Safe to merge — targeted fix for a confirmed panic with no behavioural regression for normal aqua installs

The guard condition is provably correct (mise link always produces absolute-target symlinks via absolutize()), the fix is well-scoped to list_bin_paths, all pre-existing paths are unaffected, and a concrete e2e regression test covers the crash scenario. No P0/P1 findings remain.

No files require special attention

Important Files Changed

Filename Overview
src/backend/aqua.rs Adds early-exit for linked versions (absolute-path symlinks) in list_bin_paths, bypassing aqua registry lookup; also hardens strip_prefix from unwrap to filter_map
e2e/backend/test_aqua_linked_version New e2e regression test that installs sccache, creates a linked version via mise link, and asserts mise reshim no longer panics
crates/aqua-registry/src/types.rs Adds two unit tests that document the root-cause behavior: empty asset producing an absolute path and non-version strings unexpectedly matching semver constraints

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[list_bin_paths called] --> B{symlink_bins?}
    B -- yes --> C[return .mise-bins path]
    B -- no --> D[resolve_symlink install_path]
    D --> E{Ok + Some + is_absolute?}
    E -- yes --> F{bin dir exists?}
    F -- yes --> G[return install_path/bin]
    F -- no --> H[return install_path]
    E -- no --> I[cache lookup / registry query]
    I --> J[pkg.version_override + srcs]
    J --> K[filter_map strip_prefix]
    K --> L[return relative paths, mounted]
Loading

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

.unique()
.filter(|p| p.exists())
.map(|p| p.strip_prefix(&install_path).unwrap().to_path_buf())
.filter_map(|p| p.strip_prefix(&install_path).ok().map(|p| p.to_path_buf()))
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 Silent path-drop has no diagnostic log

The filter_map is a sound safety net, but when strip_prefix fails for a path it silently removes that bin path from the result — the caller has no visibility into why a tool's binary is missing. Adding a debug! (or at minimum a trace!) would make future debugging much easier without changing behavior.

Suggested change
.filter_map(|p| p.strip_prefix(&install_path).ok().map(|p| p.to_path_buf()))
.filter_map(|p| {
p.strip_prefix(&install_path)
.map(|rel| rel.to_path_buf())
.map_err(|e| {
debug!("strip_prefix failed for {}: {e}", p.display());
e
})
.ok()
})

@jdx jdx merged commit 4101066 into jdx:main Mar 31, 2026
32 of 34 checks passed
mise-en-dev added a commit that referenced this pull request Apr 1, 2026
### 🚀 Features

- add azd (Azure Developer CLI) to registry by @rajeshkamal5050 in
[#8828](#8828)

### 🐛 Bug Fixes

- **(aqua)** skip registry lookup for linked versions in list_bin_paths
by @nikobockerman in [#8801](#8801)
- **(rust)** handle rustup check exit code 100 as non-error by @shalk in
[#8832](#8832)
- **(task)** resolve bare aliases in monorepo with config_roots by
@nkakouros in [#8819](#8819)
- show usage help when long_about is defined w/o args/flags by
@nkakouros in [#8824](#8824)

### 📚 Documentation

- fix serif font in sidebar and increase heading sizes by @jdx in
[#8831](#8831)
- fix #vscode link in ide integration page by @jedymatt in
[#8833](#8833)
- fix nested Markdown code fences by @muzimuzhi in
[#8835](#8835)

### New Contributors

- @shalk made their first contribution in
[#8832](#8832)
- @jedymatt made their first contribution in
[#8833](#8833)
- @nikobockerman made their first contribution in
[#8801](#8801)
- @rajeshkamal5050 made their first contribution in
[#8828](#8828)

## 📦 Aqua Registry Updates

#### New Packages (2)

- [`gastownhall/beads`](https://github.com/gastownhall/beads)
- [`getdbt.com/dbt-fusion`](https://github.com/getdbt.com/dbt-fusion)

#### Updated Packages (2)

- [`Azure/azure-dev`](https://github.com/Azure/azure-dev)
- [`magefile/mage`](https://github.com/magefile/mage)
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