Repository navigation
fix(cli): resolve the base of sub-N: before subtracting - #11796
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change adds shared resolution for ChangesSub-version resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant resolve_sub_base
participant ABackend
participant version_sub
CLI->>resolve_sub_base: Resolve sub-N base
resolve_sub_base->>ABackend: Resolve latest version
resolve_sub_base->>version_sub: Apply subtraction
version_sub-->>resolve_sub_base: Return version or error
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/toolset/tool_version.rs`:
- Around line 710-717: Update resolve_sub_base around the existing base match so
aliases resolving to the literal "latest" are resolved through
latest_version_with_refresh before calling tool_request::version_sub, while
preserving the current direct-"latest" branch. Add regression coverage for both
sub-1:latest and a sub-1 request using an alias mapped to latest.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 408d1ebe-0a85-452f-9496-5cff4d45e1e4
📒 Files selected for processing (7)
e2e/cli/test_lateste2e/cli/test_ls_remotesrc/cli/latest.rssrc/cli/ls_remote.rssrc/toolset/mod.rssrc/toolset/tool_request.rssrc/toolset/tool_version.rs
5221b06 to
296c7c0
Compare
`sub-N:<base>` accepts `latest` and aliases as its base, but `version_sub()`
can only subtract from a concrete version. `ToolVersion::resolve_sub` resolves
the base first, which is why `mise use`/`mise install` handle `sub-2:lts`; two
other call sites did not.
`mise ls-remote node@sub-2:lts` passed the raw string through and panicked:
called `Option::unwrap()` on a `None` value
src/toolset/tool_request.rs:592
`mise latest node@sub-2:lts` had no `Sub` arm at all and bailed with
"invalid version" for every `sub-N:` spec, numeric base included.
Extracts the base resolution into `resolve_sub_base()` and routes all three
callers through it, and makes `version_sub()` return a `Result` so an
unresolved base can no longer abort the process.
296c7c0 to
21b87bc
Compare
Greptile SummaryThe PR centralizes resolution of
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The shared resolver consistently converts aliases and Important Files Changed
Reviews (1): Last reviewed commit: "fix(cli): resolve the base of `sub-N:` b..." | Re-trigger Greptile |
Problem
sub-N:<base>acceptslatestand aliases as its base —docs/configuration.mddocumentsnode sub-2:ltsandpython sub-0.1:latest. Butversion_sub()can only subtract from a concreteversion, so the base has to be resolved first.
ToolVersion::resolve_subdoes that, which is whymise useandmise installhandle it. Two other call sites do not.Measured on v2026.8.3 linux-x64:
node@sub-2:ltsnode@sub-1:24mise ls-remotemise latestinvalid versioninvalid versionmise use --dry-run22.23.2mise install --dry-run22.23.2[tools] node = ["lts", "sub-2:lts"]24.19.0and22.23.2Version::new("lts")parses fine, but the resulting chunk is alphanumeric, soChunk::single_digit()returnsNoneand theunwrap()aborts. A numeric base does not reach thatline, which is why
sub-1:24works andsub-2:ltsdoes not.src/cli/ls_remote.rspassedorig_versiontoversion_sub()verbatim.src/cli/latest.rshad noToolRequest::Subarm; everysub-N:spec fell into_ => bail!("invalid version: …").Change
resolve_sub_base()(src/toolset/tool_version.rs) holds the resolution that was inline inToolVersion::resolve_sub—latestvia the backend, anything else viaConfig::resolve_alias—and then subtracts.
resolve_sub,ls-remoteandlatestall go through it, so the three cannotdrift again.
version_sub()returnsResult<String>. The fourunwrap()s become errors naming the offendingvalue, so an unresolved base is a message rather than an abort. Subtraction and underflow
behaviour are unchanged; the existing tests only gained
.unwrap().ToolRequest::local_resolve'sSubarm treats a version it cannot subtract from as "no matchinginstall", which is what the surrounding lookup already means.
resolve_sub'slatest+offlineearly return is untouched — the comment there explains whyversion_submust not run in that case.Tests
test_version_sub_rejects_non_numeric_baseinsrc/toolset/tool_request.rs. Without the fixthis test panics rather than fails, which is the point of it.
e2e/cli/test_ls_remotealready covered a numeric base (dummy@sub-1:2); an alias base is addednext to it via
mise alias set dummy lts 2, expecting the same output.e2e/cli/test_latestgains both forms.dummypublishes1.0.0,1.1.0,2.0.0(
test/data/plugins/dummy/bin/list-all), sosub-1:2selects prefix1and the latest of thatis
1.1.0.Why draft
version_subreturningResult. The alternative is a total function returning the inputunchanged for a non-numeric base — smaller diff, but then
ls-remotefilters on a nonsenseprefix and prints nothing at all. An error seemed clearly better; flagging it since it is a
public signature.
resolve_sub_baselives. It sits intool_version.rsbecause it needs the backend andconfig. Next to
version_subintool_request.rswould keep the pair together at the cost ofmore dependencies there.
ToolRequest::Prefixhas the same shape of gap and is not touched here —latestbails onprefix:20, andls-remotesilently drops the filter and lists everything. Happy to fold it inif wanted.
Summary by CodeRabbit
New Features
latestinsub-N:selectors forlatestandls-remote.Bug Fixes
latest-based version subtraction scenarios.