Repository navigation
fix(link): reject selector-style link requests - #11218
Conversation
📝 WalkthroughWalkthrough
ChangesLink validation and alias handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Link_run
participant Backend
participant Filesystem
User->>Link_run: request tool link
Link_run->>Link_run: validate concrete version or ref
Link_run->>Backend: resolve backend alias
Link_run->>Filesystem: create expected symlink
Link_run->>Backend: reset backend state
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryUpdates
Confidence Score: 5/5The PR appears safe to merge. No blocking failures remain within the scope of this follow-up review. Important Files Changed
Reviews (4): Last reviewed commit: "fix(link): reject selector-style link re..." | Re-trigger Greptile |
c06bc9e to
5b6979f
Compare
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/cli/link.rs`:
- Around line 36-46: Update the Version/Ref match in the link flow to reject
unqualified Version requests such as latest, LTS, or channel names before
constructing ToolVersion, while preserving support for truly explicit versions
and refs. Use the existing ToolRequest version metadata or validation helpers to
distinguish qualified versions; keep the current bail behavior for unsupported
requests and add separate Ref handling only if unqualified references are
intentionally supported.
🪄 Autofix (Beta)
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: 89cc7c49-692e-49b7-ab6c-ef405fc5c808
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomle2e/backend/test_aqua_failed_install_cleanupe2e/cli/test_linksrc/backend/mod.rssrc/cli/link.rssrc/file.rssrc/toolset/install_state.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- e2e/backend/test_aqua_failed_install_cleanup
- Cargo.toml
- src/file.rs
- e2e/cli/test_link
- src/backend/mod.rs
|
This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
5b6979f to
6e96c57
Compare
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 `@e2e/cli/test_link`:
- Around line 60-66: Extend the rejected-request cases in the CLI link tests to
verify each corresponding install path is absent and not a symlink after the
command fails. Add checks for prefix, path, sub-tool, system, and tiny@latest
requests, matching the existing zig@master absence assertion and using each
request’s expected install location.
🪄 Autofix (Beta)
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: 33547531-eb72-4abe-a7e0-a968eb6fcef3
📒 Files selected for processing (1)
e2e/cli/test_link
6e96c57 to
fa0457e
Compare
This breaks
mise link node@latestor similar. It didn't work well before this PR, but if you prefer to keep them, I'll create another PR to fix it somehow (idk if we can).Summary
mise linkonly for concrete version identities and explicit refslatest, configured version aliases, and backend-declared rolling channels before filesystem mutationWhy
A link creates a named managed install. Resolution selectors such as prefixes, paths, sub-tools,
system,latest, aliases, and rolling channels do not identify a stable install pathname; accepting them can create links that later commands cannot address consistently.Refs remain supported through their normalized ref pathname, and arbitrary literal labels such as
brewremain supported when they are not aliases or backend-declared moving channels.Breaking changes
mise linkpreviously accepted request-style selectors and moving names even though the resulting link did not reliably round-trip through later resolution. These commands now fail before creating or replacing a filesystem link:For example,
latestis also the name of mise's managed runtime symlink. Previously, forcing a link at that name could replace the pointer and then break installed-version resolution:This also prevents mise from updating its normal
latestpointer when newer concrete versions are installed.Use a concrete version, an explicit ref, or an unconfigured stable label instead:
Existing links are not removed by this change; it only rejects new
mise linkinvocations that use non-concrete requests.Testing
/usr/bin/mise run test:e2e e2e/cli/test_link/usr/bin/mise run lint-fixhk check src/cli/link.rs e2e/cli/test_linkSummary by CodeRabbit
mise linknow rejects non-concrete selectors and unsupported version/ref requests with a clear constraint error.latestthat can’t resolve to a fixed target.