Repository navigation
refactor(lock): skip aqua prefix ambiguity pass without override prefixes - #14232
Conversation
Avoid cloning the package per candidate when no version override sets a version_prefix, split the ambiguity unit test from the known-tag test, tighten the error text, and simplify the e2e ambiguity assertion. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4G6X1ZQAGAFT759VTJYFF4T
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughAqua lockfile resolution now checks version-prefix ambiguity only when a version override defines a prefix and no exact release tag is resolved. Unit and end-to-end tests cover the resolution behavior. ChangesAqua lockfile resolution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is identified. Run the e2e test as part of normal validation. 🚥 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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 134c2c9. Configure here.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4G7FSS9W3EFPFAH6WAM7ESE
# Conflicts: # e2e/lockfile/test_lockfile_aqua_version_prefix_override # src/backend/aqua.rs
<!-- entire-trail-link-start --> https://entire.io/gh/jdx/mise/trails/613 <!-- entire-trail-link-end --> ### 🚀 Features - **(upgrade)** add tool_update.global_auto to update every global tool by @jdx in [#14237](#14237) - reuse downloads the server confirms are unchanged by @jdx in [#14222](#14222) ### 🐛 Bug Fixes - **(backend)** stop list() and get() panicking while the tool map reloads by @jdx in [#14240](#14240) - **(bootstrap)** stop reporting freshly cloned repos as dirty with core.autocrlf by @jdx in [#14225](#14225) - **(cargo)** lock the same rust homes the rust plugin resolves by @wislertt in [#14219](#14219) - **(doctor)** show dotfiles repo and origin when history is disabled by @jdx in [#14244](#14244) - **(dotfiles)** replace a junction target when a symlink source changes by @jdx in [#14226](#14226) - **(env)** keep backslashes literal in single-quoted dotenv values by @jdx in [#14228](#14228) - **(env)** join adjacent quoted and unquoted dotenv value parts again by @jdx in [#14231](#14231) - **(history)** describe tracked variant changes by @oppegard in [#14194](#14194) - **(lock)** apply aqua version prefixes before selecting overrides by @nettlesh in [#14218](#14218) - **(rust)** treat rustup profile aliases like their full names by @JamBalaya56562 in [#14220](#14220) - **(schema)** accept task references in a task array by @JamBalaya56562 in [#14229](#14229) - **(self-update)** keep showing the auto-update hint until it is disabled by @jdx in [#14233](#14233) ### 🚜 Refactor - **(lock)** skip aqua prefix ambiguity pass without override prefixes by @jdx in [#14232](#14232) ### 📚 Documentation - explain auto_update's release delay and prefer mise settings KEY=VALUE by @jdx in [#14235](#14235) ### New Contributors - @wislertt made their first contribution in [#14219](#14219) - @oppegard made their first contribution in [#14194](#14194)
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|

https://entire.io/gh/jdx/mise/trails/615
Follow-up to #14218, stacked on it (base is a copy of that PR's head, since its branch does not allow maintainer edits). Retarget to
mainonce #14218 merges.mise lockno longer clones the aqua package once per version candidate for packages whose version overrides have noversion_prefix, which is nearly every package. In that case every candidate resolves to the root prefix, so there is nothing to disambiguate and the ambiguity pass is skipped.Also:
mise lockdirectly throughassert_containsinstead ofecho '$output'with|| trueBehavior is otherwise unchanged. Validated with the 123 aqua backend unit tests and
test_lockfile_aqua_version_prefix_override.🤖 Generated with Claude Code
AI-assisted — Tool: Claude Code; model: anthropic/claude-sonnet-5-5; version: unavailable.
Note
Low Risk
Targeted lock-path optimization and clearer errors; ambiguity rejection is unchanged for packages that actually use override prefixes.
Overview
lock_package_for_versionnow runs the multi-prefix ambiguity check only when there is no release tag and at least oneversion_overridedefines aversion_prefix. Packages without override prefixes skip the per-candidatepkg.clone()pass because every candidate shares the same root prefix—behavior stays the same, with less work on the common path.The ambiguous-prefix error text now says "no exact release tag matched" (better for HTTP packages with no tags). Unit coverage splits the “known tag resolves” case from reject ambiguous prefixes without a tag; the lockfile e2e asserts the ambiguity message by running
mise lockdirectly and notes that an unresolvable platform is reported whilemise lockstill exits 0.Reviewed by Cursor Bugbot for commit 28adc57. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit