Skip to content

Remove duplicate lstk install detection - #455

Closed
gtsiolis wants to merge 3 commits into
localstack:mainfrom
gtsiolis:agent/fix-asdf-install-warning
Closed

Remove duplicate lstk install detection#455
gtsiolis wants to merge 3 commits into
localstack:mainfrom
gtsiolis:agent/fix-asdf-install-warning

Conversation

@gtsiolis

@gtsiolis gtsiolis commented Aug 13, 2026

Copy link
Copy Markdown
Member

Motivation

Multiple lstk binaries on PATH are increasingly legitimate: users may install the CLI through package or version managers, and tools such as the VS Code extension are expected to bundle their own copy. Duplicate-install detection cannot reliably distinguish those intentional copies from stale installations, which produces false positives and verbose warnings without useful remediation.

Linear decision record: DEVX-1015.

Solution

  • Stop scanning PATH from both lstk update and the startup update notification.
  • Remove the duplicate-install scanner and its Unix and Windows executable probes.
  • Remove the dedicated plain, JSON, and TUI warning event and rendering.
  • Remove the associated unit and integration tests.
  • Keep detection of the active executable install method, which is still required to choose the Homebrew, npm, or binary updater.

Docs

Docs impact

No external documentation changes are needed. The internal architecture guidance no longer advertises duplicate-install detection.

Review

Human review advised: this removes existing user-facing warning behavior from both the update and startup paths. The implementation is a focused deletion following reviewer feedback.

Validated with:

  • make build
  • make test (1,621 tests passed)
  • make test-integration RUN=TestUpdateBinaryMockGitHubChecksumMismatch
  • Pre-commit secret scan
  • git diff --check

Co-Authored-By: Claude noreply@anthropic.com

Closes DEVX-1015

@gtsiolis gtsiolis added semver: patch docs: skip Pull request does not require documentation changes labels Aug 13, 2026
@gtsiolis

Copy link
Copy Markdown
Member Author

Second attempt to improve #408.

@skyrpex I'm curious what you think sice you added this.

@gtsiolis

Copy link
Copy Markdown
Member Author

Added this because I kept seeing this warning with breaking lines on my terminal.
Screenshot 2026-08-13 at 12 59 26 copy

@gtsiolis gtsiolis self-assigned this Aug 13, 2026

skyrpex commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

I remember somebody (maybe Peter?) suggesting to remove the whole dupe detection altogether, which I'd be in favor of. Should we then just remove detection?

@gtsiolis
gtsiolis force-pushed the agent/fix-asdf-install-warning branch from 221a966 to b00f6f5 Compare August 13, 2026 12:57
@gtsiolis gtsiolis changed the title Ignore active asdf shims in install warnings Remove duplicate lstk install detection Aug 13, 2026

Copy link
Copy Markdown
Member Author

I'm also in favor of removing this low-impact check.

Re-purposed PR in b00f6f5. Could you take another look?

@skyrpex skyrpex left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, only deletions :party:

@gtsiolis
gtsiolis marked this pull request as ready for review August 13, 2026 14:24
@gtsiolis
gtsiolis requested review from a team and peter-smith-phd as code owners August 13, 2026 14:24
@peter-smith-phd

Copy link
Copy Markdown
Contributor

@gtsiolis, what's the Linear issue associated with this PR? I think we should have that issue in backlog status while we discuss the solution, then when the issue moves to Todo, we can be confident that we've agreed on the correct solution. Thanks.

@peter-smith-phd peter-smith-phd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a valid Linear issue number in Todo state.

gtsiolis and others added 3 commits August 13, 2026 21:47
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@gtsiolis
gtsiolis force-pushed the agent/fix-asdf-install-warning branch 2 times, most recently from 5cf5f0a to 9d6fbde Compare August 13, 2026 18:50
@gtsiolis

Copy link
Copy Markdown
Member Author

Thanks, @peter-smith-phd, this was already tracked in DEVX-1015.

I鈥檝e updated the issue with additional context and linked it from the PR.

Please add any remaining concerns or risks to the issue or here. Otherwise, I think we can merge this to resolve the user-facing friction and keep the feedback loop short, since this is a low-risk removal that doesn鈥檛 affect core CLI functionality, and we can iterate if needed.

@gtsiolis

Copy link
Copy Markdown
Member Author

Superseded by #460, which carries the identical scoped change from a branch in localstack/lstk so the integration workflow can access repository secrets. The original discussion remains here for context.

@gtsiolis gtsiolis closed this Aug 13, 2026
@gtsiolis
gtsiolis deleted the agent/fix-asdf-install-warning branch August 13, 2026 18:59
@peter-smith-phd

Copy link
Copy Markdown
Contributor

and we can iterate if needed.

I'd prefer not to "iterate" too much. We have enough experience in this company to know what the correct behaviour should be, and as long as we talk about issues upfront, we won't find ourselves adding features, then removing them later. Enterprise customers expect stability 馃槃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: skip Pull request does not require documentation changes semver: patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants