Skip to content

fix(client): detect Amp extension clients via amp.mcpServers key#5659

Open
syf2211 wants to merge 2 commits into
stacklok:mainfrom
syf2211:fix/amp-client-installed-detection
Open

fix(client): detect Amp extension clients via amp.mcpServers key#5659
syf2211 wants to merge 2 commits into
stacklok:mainfrom
syf2211:fix/amp-client-installed-detection

Conversation

@syf2211

@syf2211 syf2211 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix false-positive "installed" status for Amp editor extensions (amp-vscode, amp-cursor, amp-windsurf, amp-vscode-insider) in thv client status.

Motivation

These Amp clients share the host editor's config directory (Code/User, Cursor/User, Windsurf/User). Checking only for directory presence reports Amp as installed whenever VS Code, Cursor, or Windsurf is installed, even without the Amp extension.

Fixes #2174

Changes

  • Add optional InstallSettingsKey to clientAppConfig
  • Set InstallSettingsKey: "amp.mcpServers" for Amp extension clients (not amp-cli, which has its own directory)
  • Update IsClientInstalled to require the settings key when configured, using hujson + gjson with dot escaping for dotted JSON keys
  • Add regression tests for Amp-on-Cursor detection

Tests

go test ./pkg/client/... -count=1

All tests pass.

Notes

  • amp-cli is unchanged (dedicated ~/.config/amp directory)
  • Uses the same JSONC parsing path as other client config readers
  • A stale amp.mcpServers: null key after uninstall could still report installed; acceptable edge case per maintainer guidance

Amp editor extensions (amp-vscode, amp-cursor, amp-windsurf) share the
host editor's config directory, so checking only for directory presence
incorrectly reports them as installed when VS Code, Cursor, or Windsurf
is present without the Amp extension.

Require the amp.mcpServers key in settings.json for these clients.
Fixes stacklok#2174
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.35%. Comparing base (87bcb74) to head (38db8d4).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/client/discovery.go 89.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5659      +/-   ##
==========================================
+ Coverage   70.33%   70.35%   +0.01%     
==========================================
  Files         649      651       +2     
  Lines       66185    66304     +119     
==========================================
+ Hits        46554    46647      +93     
- Misses      16277    16295      +18     
- Partials     3354     3362       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ToolHive always shows Amp clients as installed

2 participants