Skip to content

fix(llm): use correct binary name for VS Code Insiders detection#5109

Merged
yrobla merged 1 commit intomainfrom
llm/strengthen-tool-detection-binary-check
Apr 29, 2026
Merged

fix(llm): use correct binary name for VS Code Insiders detection#5109
yrobla merged 1 commit intomainfrom
llm/strengthen-tool-detection-binary-check

Conversation

@yrobla
Copy link
Copy Markdown
Contributor

@yrobla yrobla commented Apr 29, 2026

Summary

DetectedLLMGatewayClients() uses exec.LookPath(cfg.LLMBinaryName) to verify a tool is actually installed before reporting it as detected. The vscode-insider entry had LLMBinaryName: "code" — the stable VS Code binary name — instead of "code-insiders", which is the actual CLI shim shipped by the Insiders build.

On a machine with only VS Code Insiders installed (no stable VS Code), LookPath("code") returns not-found, so thv llm setup silently skips the Insiders integration even though the tool is present.

  • Fixes LLMBinaryName for vscode-insider from "code" to "code-insiders"
  • Adds TestRealClientConfigs_LLMBinaryNames as a regression guard that asserts the binary name for every LLM-gateway-capable tool in supportedClientIntegrations

Fixes #5089

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test) — TestRealClientConfigs_LLMBinaryNames passes and would have caught the bug
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Does this introduce a user-facing change?

Yes. thv llm setup now correctly detects VS Code Insiders on machines where only the Insiders build is installed.

Special notes for reviewers

The regression test checks the binary name for all five tools that have LLMBinaryName set. If a future entry gets the wrong name, the test will catch it immediately rather than requiring manual verification on a machine with that specific tool installed.

VS Code Insiders ships its CLI shim as `code-insiders`, not `code`.
With `LLMBinaryName: "code"`, DetectedLLMGatewayClients() would fail
to detect vscode-insider on machines that only have the Insiders build
installed (no stable VS Code), causing `thv llm setup` to silently
skip it.

Adds TestRealClientConfigs_LLMBinaryNames as a regression guard that
asserts the binary name for every LLM-gateway-capable tool in
supportedClientIntegrations.

Fixes #5089

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@yrobla yrobla force-pushed the llm/strengthen-tool-detection-binary-check branch from 9944d06 to 3b1345d Compare April 29, 2026 08:42
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Apr 29, 2026
@yrobla yrobla requested a review from Copilot April 29, 2026 08:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes VS Code Insiders LLM gateway detection by correcting the CLI shim name used for exec.LookPath, and adds a regression test to guard LLMBinaryName values for real client configs.

Changes:

  • Update VS Code Insiders LLMBinaryName from code to code-insiders so detection works when only Insiders is installed.
  • Add a unit test asserting expected LLMBinaryName values for LLM-gateway-capable client configs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/client/config.go Corrects the VS Code Insiders binary name used during LLM gateway client detection.
pkg/client/llm_gateway_test.go Adds a regression test validating real supportedClientIntegrations LLMBinaryName values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/client/llm_gateway_test.go
Comment thread pkg/client/llm_gateway_test.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.03%. Comparing base (9c66743) to head (3b1345d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5109      +/-   ##
==========================================
- Coverage   67.08%   67.03%   -0.05%     
==========================================
  Files         595      595              
  Lines       60000    60000              
==========================================
- Hits        40250    40222      -28     
- Misses      16691    16720      +29     
+ Partials     3059     3058       -1     

☔ View full report in Codecov by Sentry.
📢 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.

@yrobla yrobla merged commit 239ea90 into main Apr 29, 2026
50 checks passed
@yrobla yrobla deleted the llm/strengthen-tool-detection-binary-check branch April 29, 2026 08:57
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.

llm: VSCode Insiders binary detection uses wrong shim name

4 participants