Fix multiple-installs false positive for asdf shims#408
Draft
gtsiolis wants to merge 1 commit into
Draft
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
Contributor
|
@gtsiolis thanks, didn't know about this. I thought we were fine deduping symlinks 😄 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
lstk warns about "multiple installs" whenever an asdf-managed npm install is also reachable through its asdf shim on PATH, even though both paths point at the same install — a false positive that fires on every run for asdf users.
Changes
internal/update/installs_unix.go) by parsing the# asdf-plugin: <name> <version>comment asdf writes into each shim, before dedup runs inFindInstalls(internal/update/installs.go)executableAliason Windows (installs_windows.go), where asdf shims don't applyTestFindInstallsDeduplicatesAsdfShimAliasand integration testTestUpdateCheckDoesNotWarnOnAsdfShimAliasReview
Human review advised — found and fixed today with no prior Slack/Linear discussion before this PR, even though it's small, self-contained, and fully tested (per the review-pr skill's scope checklist: undiscussed work defaults to review regardless of size).
Tests
New unit + integration tests reproduce the false positive and confirm it's resolved. Also verified directly against a real asdf + npm install on this machine: today's code reports 2 installs for this PATH combination, this branch reports 1, correctly classified as npm.
Closes DEVX-1015
Co-Authored-By: Claude noreply@anthropic.com