Conversation
Greptile SummaryThis PR fixes a cache poisoning bug where Confidence Score: 5/5Safe to merge — the fix correctly addresses cache poisoning with no new logic errors or regressions introduced. Both previously-raised concerns (bail! breaking unresolved_latest_version fallback, and in-memory OnceCell remaining poisoned) are addressed. The No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Backend as Backend::list_remote_versions_with_info
participant Mutex as TokioMutex<CacheManager>
participant OnceCell as OnceCell (memory)
participant Disk as Disk cache
Caller->>Backend: list_remote_versions_with_info()
Backend->>Mutex: lock()
Backend->>OnceCell: get_or_try_init_async()
OnceCell-->>Backend: (uninitialized — fetch)
Backend->>Backend: _list_remote_versions() → []
Backend->>Disk: write(empty Vec)
Backend->>OnceCell: initialize(empty Vec)
OnceCell-->>Backend: &[]
Backend->>Backend: .clone() → owned []
alt versions.is_empty()
Backend->>Disk: remove cache file
Backend->>OnceCell: *cell = Default::default() (reset)
end
Backend->>Mutex: unlock()
Backend-->>Caller: Ok([]) — caller sees empty this run
Note over Caller,Disk: Next call in same process re-fetches (OnceCell reset)
Caller->>Backend: list_remote_versions_with_info() [retry]
Backend->>Mutex: lock()
Backend->>OnceCell: get_or_try_init_async()
OnceCell-->>Backend: (uninitialized again — re-fetch)
Backend->>Backend: _list_remote_versions() → [v1, v2, ...]
Backend->>Disk: write(versions)
Backend->>OnceCell: initialize(versions)
Backend->>Mutex: unlock()
Backend-->>Caller: Ok([v1, v2, ...])
Reviews (11): Last reviewed commit: "fix(backend): Don't cache empty version ..." | Re-trigger Greptile |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the backend version retrieval logic to return an error instead of a warning when no versions are found, specifically for non-HTTP backends. The review feedback points out that S3 backends should likely be excluded from this strict check as well, consistent with how they are handled elsewhere in the codebase, to avoid unexpected errors when version lists are missing.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
c22
force-pushed
the
main
branch
2 times, most recently
from
8dc00fb to
10c05c4
Compare
c22
changed the title
c22
force-pushed
the
main
branch
3 times, most recently
from
8b0aab6 to
1da830c
Compare
|
The e2e failures appear to be related to GH rate limits. |
c22
force-pushed
the
main
branch
4 times, most recently
from
a6c352f to
c6bf536
Compare
|
With the newer, simpler approach it should be noted that This PR ended up being a focused change, so I am not changing the logging for It's probably better to refactor this into trait-based logic in the future if desirable (which is what my first attempt did but ended up getting out of hand). |
c22
force-pushed
the
main
branch
2 times, most recently
from
92fa5b6 to
7b4b25e
Compare
When _list_remote_versions returns no versions (e.g. invalid Go module path), the empty list was cached as a positive result via get_or_try_init_async. This poisons the cache for up to 1 hour, both on disk and in the in-memory OnceCell. Clear both the disk cache file and the in-memory cells after fetching if the version list is empty so subsequent calls re-fetch instead of serving a stale empty result.
…ends (#9548) ## Summary Backends that opt into installing an unresolved `latest` selector (today only `pipx` for git-backed requests) legitimately return an empty remote version list when their source has no discoverable versions — e.g. a GitHub repo with no releases (only tags). The hard-error path was already addressed in #9401 by falling back to the unresolved selector. What's still left is noise: every command that touches such a tool emits 3–12 repeated `mise WARN No versions found for …` warnings. The repetition comes from #9444 (don't cache empty version lists) — every separate call site that asks for remote versions in the same process re-fetches and re-warns. Underlying GitHub API calls are still deduplicated by `RELEASE_CACHE`, so this is purely log noise, not extra HTTP traffic. This change suppresses the warning when the backend opts into `unresolved_latest_version()`. For those backends an empty list is the expected steady state, not an error worth flagging. Other backends still warn as before, so the warning continues to catch real problems (typo'd repos, configs pointing at the wrong place). Refs #9381. ## Reproduction \`\`\`toml # ~/.config/mise/conf.d/tools.toml [tools] "pipx:a13xp0p0v/kernel-hardening-checker" = "latest" \`\`\` `a13xp0p0v/kernel-hardening-checker` has 0 GitHub releases (14 tags). Before this PR: \`\`\` $ mise current mise WARN No versions found for pipx:a13xp0p0v/kernel-hardening-checker mise WARN No versions found for pipx:a13xp0p0v/kernel-hardening-checker mise WARN No versions found for pipx:a13xp0p0v/kernel-hardening-checker pipx:a13xp0p0v/kernel-hardening-checker latest \`\`\` After: \`\`\` $ mise current pipx:a13xp0p0v/kernel-hardening-checker latest \`\`\` `mise ls`, `mise outdated`, and `mise install` are similarly quiet now. ## Test plan - [x] Manual repro with the discussion's tool — all warnings gone for `mise ls`, `mise current`, `mise outdated`, `mise install` - [x] `mise run lint` - [x] `cargo build` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: only changes warning emission logic when a backend opts into `unresolved_latest_version()`, with no effect on version fetching or installation behavior. > > **Overview** > Suppresses repeated `No versions found` warnings when `list_remote_versions_with_info` returns an empty list for backends that support installing an unresolved `latest` selector (via `unresolved_latest_version()`). Other backends still warn on empty remote version lists as before. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9ca20e5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
2 participants
When _list_remote_versions returns no versions (e.g. invalid Go module path), the empty list was cached as a positive result via
get_or_try_init_async.
This poisons the cache for up to 1 hour, both on disk and in the in-memory OnceCell.
Clear both the disk cache file and the in-memory cells after fetching if the version list is empty so subsequent calls re-fetch instead of serving a stale empty result.
This is an alternative approach to my earlier attempt at fixing this because the previous approach ended up getting a bit complicated (required backend specific changes).