fix(ls): sort sources deterministically in --all-sources output#8037
fix(ls): sort sources deterministically in --all-sources output#8037
Conversation
The collect_sources function built a Vec<SourceEntry> in iteration order of tracked config files, which is non-deterministic. This caused the e2e test_ls_all_sources test to flake depending on which config file was iterated first. Sort the entries vector for each tool so sources appear in a stable, deterministic order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @jdx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the output of Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Sorts source entries to make mise ls --all-sources output deterministic, preventing flakes caused by non-deterministic config iteration order.
Changes:
- Derive ordering traits for
SourceEntry - Sort each
Vec<SourceEntry>incollect_sources()before returning
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type SourcesMap = BTreeMap<(String, String), Vec<SourceEntry>>; | ||
|
|
||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] |
There was a problem hiding this comment.
Deriving Ord/PartialOrd on SourceEntry couples the sort semantics to field order and to ToolSource’s Ord implementation (which may not reflect the intended UX order). Consider removing these derives and instead sorting with an explicit comparator/key at the call site (e.g., by a stable display string / path / priority fields), so the ordering remains intentional and easier to change safely.
| #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] | |
| #[derive(Clone, Debug, PartialEq, Eq)] |
| for entries in sources_map.values_mut() { | ||
| entries.sort(); | ||
| } |
There was a problem hiding this comment.
collect_sources() now always sorts SourceEntry vectors, which can be an observable behavior change for any other caller that relied on the previous insertion/collection order (e.g., if that order conveyed precedence). If deterministic ordering is only required for mise ls --all-sources rendering, consider moving the sort closer to the output formatting layer or introducing a dedicated helper (e.g., collect_sources_sorted) so other call sites keep their original semantics.
There was a problem hiding this comment.
Code Review
This pull request addresses non-deterministic output in mise ls --all-sources by sorting the collected sources. The changes are correct and effectively fix the reported issue. I've added one suggestion to refactor the collect_sources function for improved performance and idiomatic Rust.
| for entries in sources_map.values_mut() { | ||
| entries.sort(); | ||
| } |
There was a problem hiding this comment.
While this correctly sorts the entries, the collection logic before this loop is inefficient. It uses a contains check inside a loop (O(n) for each item), resulting in roughly O(n^2) complexity for building each vector of sources.
A more efficient and idiomatic approach would be to use a BTreeSet to collect unique, sorted entries directly. This would have a complexity closer to O(n log n) and would also make the code cleaner by removing both the contains check and this explicit sorting loop.
Consider refactoring the collect_sources function like this:
use std::collections::BTreeSet;
fn collect_sources(ts: &Toolset) -> SourcesMap {
let mut sources_map: BTreeMap<(String, String), BTreeSet<SourceEntry>> = BTreeMap::new();
for (ba, tvl) in ts.versions.iter() {
for tv in &tvl.versions {
let key = (ba.short.to_string(), tv.tv_pathname());
let entry = SourceEntry {
source: tv.request.source().clone(),
requested: tv.request.version(),
};
sources_map.entry(key).or_default().insert(entry);
}
}
sources_map
.into_iter()
.map(|(k, v)| (k, v.into_iter().collect()))
.collect()
}This would require adding use std::collections::BTreeSet; at the top of the file.
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.5 x -- echo |
17.1 ± 0.9 | 15.4 | 19.7 | 1.00 |
mise x -- echo |
17.7 ± 1.3 | 15.6 | 20.9 | 1.04 ± 0.09 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.5 env |
16.9 ± 1.0 | 14.9 | 19.7 | 1.00 |
mise env |
17.2 ± 1.1 | 14.9 | 20.5 | 1.02 ± 0.09 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.5 hook-env |
17.2 ± 1.1 | 15.1 | 20.4 | 1.00 |
mise hook-env |
18.3 ± 1.1 | 15.4 | 21.3 | 1.06 ± 0.10 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.5 ls |
16.6 ± 1.0 | 14.2 | 19.7 | 1.00 |
mise ls |
17.4 ± 1.3 | 14.7 | 20.3 | 1.05 ± 0.10 |
xtasks/test/perf
| Command | mise-2026.2.5 | mise | Variance |
|---|---|---|---|
| install (cached) | 88ms | 86ms | +2% |
| ls (cached) | 56ms | 58ms | -3% |
| bin-paths (cached) | 58ms | 59ms | -1% |
| task-ls (cached) | 490ms | 493ms | +0% |
### 🚀 Features - **(env)** add shell-style variable expansion in env values by @jdx in [#8029](#8029) - **(list)** add --all-sources flag to list command by @TylerHillery in [#8019](#8019) ### 🐛 Bug Fixes - **(gem)** Windows support for gem backend by @my1e5 in [#8031](#8031) - **(gem)** revert gem.rs script newline change by @my1e5 in [#8034](#8034) - **(lock)** write tools to lockfile matching their source config by @jdx in [#8012](#8012) - **(ls)** sort sources deterministically in --all-sources output by @jdx in [#8037](#8037) - **(task)** auto-install tools from mise.toml for file tasks by @jdx in [#8030](#8030) ### 📚 Documentation - fix wrong positions of `mise run` flags by @muzimuzhi in [#8036](#8036) ### 📦️ Dependency Updates - update ghcr.io/jdx/mise:copr docker digest to 3e00d7d by @renovate[bot] in [#8023](#8023) - update ghcr.io/jdx/mise:alpine docker digest to 0ced1b3 by @renovate[bot] in [#8022](#8022) ### 📦 Registry - add tirith ([github:sheeki03/tirith](https://github.com/sheeki03/tirith)) by @sheeki03 in [#8024](#8024) - add mas by @TyceHerrman in [#8032](#8032) ### Security - **(deps)** update time crate to 0.3.47 to fix RUSTSEC-2026-0009 by @jdx in [#8026](#8026) ### New Contributors - @sheeki03 made their first contribution in [#8024](#8024) - @TylerHillery made their first contribution in [#8019](#8019) ## 📦 Aqua Registry Updates #### New Packages (1) - [`kubernetes-sigs/kubectl-validate`](https://github.com/kubernetes-sigs/kubectl-validate) #### Updated Packages (6) - [`flux-iac/tofu-controller/tfctl`](https://github.com/flux-iac/tofu-controller/tfctl) - [`gogs/gogs`](https://github.com/gogs/gogs) - [`j178/prek`](https://github.com/j178/prek) - [`syncthing/syncthing`](https://github.com/syncthing/syncthing) - [`tuist/tuist`](https://github.com/tuist/tuist) - [`yaml/yamlscript`](https://github.com/yaml/yamlscript)
…8037) ## Summary - Sort `SourceEntry` vectors in `collect_sources()` to ensure deterministic ordering of sources in `mise ls --all-sources` output - The iteration order of tracked config files is non-deterministic, causing the `test_ls_all_sources` e2e test to flake when config file paths happen to be iterated in a different order ## Test plan - [x] `cargo check` passes - [x] `mise run lint-fix` passes - [ ] CI `test_ls_all_sources` should pass consistently 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, localized change to ordering of CLI output only; main risk is minor changes to source display order or any downstream tests that asserted the previous incidental ordering. > > **Overview** > Ensures `mise ls --all-sources` output is deterministic by sorting the collected source entries per tool/version. > > `SourceEntry` now derives ordering traits and `collect_sources()` sorts each `(tool, version)` source list after deduping, reducing flaky output/tests caused by non-deterministic config file iteration order. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 512e4e5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
### 🚀 Features - **(env)** add shell-style variable expansion in env values by @jdx in [jdx#8029](jdx#8029) - **(list)** add --all-sources flag to list command by @TylerHillery in [jdx#8019](jdx#8019) ### 🐛 Bug Fixes - **(gem)** Windows support for gem backend by @my1e5 in [jdx#8031](jdx#8031) - **(gem)** revert gem.rs script newline change by @my1e5 in [jdx#8034](jdx#8034) - **(lock)** write tools to lockfile matching their source config by @jdx in [jdx#8012](jdx#8012) - **(ls)** sort sources deterministically in --all-sources output by @jdx in [jdx#8037](jdx#8037) - **(task)** auto-install tools from mise.toml for file tasks by @jdx in [jdx#8030](jdx#8030) ### 📚 Documentation - fix wrong positions of `mise run` flags by @muzimuzhi in [jdx#8036](jdx#8036) ### 📦️ Dependency Updates - update ghcr.io/jdx/mise:copr docker digest to 3e00d7d by @renovate[bot] in [jdx#8023](jdx#8023) - update ghcr.io/jdx/mise:alpine docker digest to 0ced1b3 by @renovate[bot] in [jdx#8022](jdx#8022) ### 📦 Registry - add tirith ([github:sheeki03/tirith](https://github.com/sheeki03/tirith)) by @sheeki03 in [jdx#8024](jdx#8024) - add mas by @TyceHerrman in [jdx#8032](jdx#8032) ### Security - **(deps)** update time crate to 0.3.47 to fix RUSTSEC-2026-0009 by @jdx in [jdx#8026](jdx#8026) ### New Contributors - @sheeki03 made their first contribution in [jdx#8024](jdx#8024) - @TylerHillery made their first contribution in [jdx#8019](jdx#8019) ## 📦 Aqua Registry Updates #### New Packages (1) - [`kubernetes-sigs/kubectl-validate`](https://github.com/kubernetes-sigs/kubectl-validate) #### Updated Packages (6) - [`flux-iac/tofu-controller/tfctl`](https://github.com/flux-iac/tofu-controller/tfctl) - [`gogs/gogs`](https://github.com/gogs/gogs) - [`j178/prek`](https://github.com/j178/prek) - [`syncthing/syncthing`](https://github.com/syncthing/syncthing) - [`tuist/tuist`](https://github.com/tuist/tuist) - [`yaml/yamlscript`](https://github.com/yaml/yamlscript)
Summary
SourceEntryvectors incollect_sources()to ensure deterministic ordering of sources inmise ls --all-sourcesoutputtest_ls_all_sourcese2e test to flake when config file paths happen to be iterated in a different orderTest plan
cargo checkpassesmise run lint-fixpassestest_ls_all_sourcesshould pass consistently🤖 Generated with Claude Code
Note
Low Risk
Small, localized change to ordering of CLI output only; main risk is minor changes to source display order or any downstream tests that asserted the previous incidental ordering.
Overview
Ensures
mise ls --all-sourcesoutput is deterministic by sorting the collected source entries per tool/version.SourceEntrynow derives ordering traits andcollect_sources()sorts each(tool, version)source list after deduping, reducing flaky output/tests caused by non-deterministic config file iteration order.Written by Cursor Bugbot for commit 512e4e5. This will update automatically on new commits. Configure here.